-
Notifications
You must be signed in to change notification settings - Fork 22
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
Fix parser for ovn-nbctl show command output #147
Conversation
@hzhou8 out of curiosity, do you need ovn-scale-test to work with openvswitch <= 2.6.0? |
@@ -186,6 +186,8 @@ def acl_del(self, lswitch): | |||
params = [lswitch] | |||
self.run("acl-del", args=params) | |||
|
|||
# TODO: Don't try to process free-form output from `ovn-nbctl show`. | |||
# Ask for machine readable output, in JSON or CSV, instead. |
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.
Good point. Ideally we can use OVSDB python lib directly, but it might be a little bit overkill. For this ovsclients_impl I think the show command has nothing TODO here. I'd suggest to remove the TODO and create an issue for using OVSDB python lib.
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.
Agree, let me drop it and open an issue. For now using ovn-nbctl
is sufficient for generating the workload but likely it will become a bottleneck as we increase the load. OpenStack Neutron plugin for OVN has started out using ovn-nbctl
but then had to switch to ovsdbapp lib because the overhead of spawning a process and receiving whole database contents for each operation, like a adding a port, was too costly.
@jsitnicki it seems not much value to support <= 2.6 since OVN was immature back at that time. |
Since openvswitch v2.6.0 output format of `ovn-nbctl show` has changed. Any occurrences of "lswitch" have been replaced with "switch", and occurrences of "lport" have replaced with just "port". Adapt to the changes and drop support for OVN < 2.6.0.
00d8153
to
2c27f31
Compare
@hzhou8 I've addressed your comments, can you please give it another look? |
Opened issue #153. |
LGTM. Thanks @jsitnicki ! |
Since openvswitch v2.6.0 output format of
ovn-nbctl show
haschanged. Any occurrences of "lswitch" have been replaced with "switch",
and occurrences of "lport" have replaced with just "port". Adapt to
the changes.
Ideally we shouldn't be doing output scraping if output format is
free-form but instead ask for machine readable output like JSON or
CSV.