-
Notifications
You must be signed in to change notification settings - Fork 366
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
Add a flag for antctl to print OVS table names only #5895
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should reference issue #5810
@@ -194,6 +203,20 @@ func HandleFunc(aq agentquerier.AgentQuerier) http.HandlerFunc { | |||
namespace := r.URL.Query().Get("namespace") | |||
table := r.URL.Query().Get("table") | |||
groups := r.URL.Query().Get("groups") | |||
tableNamesOnly := r.URL.Query().Get("table-names") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
intuitively, maybe a subresource (/ovsflows/tablenames
) would make more sense to me
but that may be a better change for not much value
so maybe /ovsflows?table-names-only
instead? There is no need for a value IMO (=true
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I was thinking table-names
is shorter to type. I will remove the true
value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, In current antctl command framework, it doesn't support boolean type of flags. All flags' arguments will be saved into argMap
which is a type of "map[string]string", so the "true" value is replaced by an empty string.
ad6f1ea
to
6d8a095
Compare
} | ||
} | ||
|
||
var _ common.TableOutput = new(Response) | ||
|
||
func (r Response) GetTableHeader() []string { | ||
return []string{"FLOW"} | ||
return []string{""} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "FLOW" doesn't provide extra info and it's strange to have this header when printing the table name only, so removed this table header.
@@ -194,6 +203,20 @@ func HandleFunc(aq agentquerier.AgentQuerier) http.HandlerFunc { | |||
namespace := r.URL.Query().Get("namespace") | |||
table := r.URL.Query().Get("table") | |||
groups := r.URL.Query().Get("groups") | |||
tableNamesOnly := r.URL.Query().Get("table-names") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I was thinking table-names
is shorter to type. I will remove the true
value.
6d8a095
to
fb1d992
Compare
t := obj.(binding.Table) | ||
t := obj.(*Table).ofTable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks like a bug? Was this function (GetTableList
) not used by anything else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think it was a bug, this function is only used in antctl get ovsflows
, it probably never worked before.
pkg/antctl/antctl.go
Outdated
{ | ||
name: "table-names-only", | ||
usage: "Print all Antrea OVS flow table names only", | ||
shorthand: "b", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we don't need the shorthand for this flag? I don't see a connection between --table-names-only
and -b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed
Signed-off-by: Lan Luo <luola@vmware.com>
fb1d992
to
91d080c
Compare
/test-all |
/test-conformance |
1 similar comment
/test-conformance |
Hi @antoninbas could you help to check and merge this one. The failed tests were caused by the testbed instead of code. I rerun them and they are passed. |
Add a new flag to print OVS table names only. Fixes #5810