Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OVN is not a supported type(map[string]interface {} #2281

Open
lebauce opened this issue Nov 12, 2020 · 9 comments
Open

OVN is not a supported type(map[string]interface {} #2281

lebauce opened this issue Nov 12, 2020 · 9 comments

Comments

@lebauce
Copy link
Member

lebauce commented Nov 12, 2020

No description provided.

@amorenoz
Copy link
Contributor

This issue may be the reason why I'm failing to create a MetadataIndexer on an OVN field.
Well, it's being created but no events get generated and all the calls to GetNode() return nil

@amorenoz
Copy link
Contributor

@lebauce , I think this bug also causes the gremlin API to be unable to filter based on OVN Metadata.
I have done the following experiment:

  • Modified aclMetadata to add the Priority Metadata Field in the root Metadata object, so it reads:
func (p *Probe) aclMetadata(acl *goovn.ACL) graph.Metadata {
	return graph.Metadata{
		"Type":     "acl",
		"Name":     acl.UUID,
		"Manager":  "ovn",
		"Priority": int64(acl.Priority),
		"OVN": Metadata{
			ACLMetadata: ACLMetadata{
				Action:    acl.Action,
				Direction: acl.Direction,
				Log:       acl.Log,
				Match:     acl.Match,
				Priority:  int64(acl.Priority),
			},
			ExtID: graph.NormalizeValue(acl.ExternalID).(map[string]interface{}),
		},
	}
}
  • Filtering by the "internal" field fails, while the "root" one works:
$ ./skydive client query "G.V().Has('Type', 'acl').Has('Priority', 1001).Count()"
3
$ ./skydive client query "G.V().Has('Type', 'acl').Has('OVN.Priority', 1001).Count()"
0

@lebauce
Copy link
Member Author

lebauce commented Nov 13, 2020

@amorenoz The ACL metadata was not properly initialized. I just pushed this fix #2282, could you please give it a try ? Thanks !

@amorenoz
Copy link
Contributor

@lebauce Yes. Your patch seems to fix the problem. Many thanks!

@amorenoz
Copy link
Contributor

@lebauce, should your patch allow MetadataIndexing inside the ACLMetadata fields? something like:
exampleIndexer := graph.NewMetadataIndexer(g, p.aclIndexer, nil, "OVN.Direction")

@lebauce
Copy link
Member Author

lebauce commented Nov 22, 2020

That's supposed to work, isn't it ?

@amorenoz
Copy link
Contributor

amorenoz commented Nov 23, 2020

It wasn't in some tests I was running. But maybe my problem was due to something else. Let me write up a small patch to test it and get back to you.

@lebauce
Copy link
Member Author

lebauce commented Nov 24, 2020

I see in your commit, you do (amorenoz@9ae48d1#diff-99dccec1382a2bc49bf3e91c74ac6766c2441bd18d4232ffc46e908e3b674b98R612):

LFMetadata: LFMetadata{
for the indexer to work, you need to do :
LFMetadata: &LFMetadata{

Same for DPBMetadata: DPBMetadata{

@amorenoz
Copy link
Contributor

amorenoz commented Nov 25, 2020

@lebauce you mean the same you did with the ACL metadata?

"OVN": Metadata {

to

"OVN": &Metadata {

I already did that.
I don't see how I could assign an object of type * LFMetadata to a field of type LFMetadata

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants