diff mbox series

[ovs-dev,2.10] ovn-nbctl: Don't parse table-formatting options in nbctl_client

Message ID 20190226135150.29736-1-mmichels@redhat.com
State Superseded
Headers show
Series [ovs-dev,2.10] ovn-nbctl: Don't parse table-formatting options in nbctl_client | expand

Commit Message

Mark Michelson Feb. 26, 2019, 1:51 p.m. UTC
This is a backport to 2.10 of a feature already in 2.11. This is
necessary for ovn-kubernetes to function properly when using 2.10. The
original commit message is below.


When ovn-nbctl is running in daemon mode, nbctl_client attempts to parse
table formatting options. The problem is that this then removes the table
formatting options from the array of options passed to the server loop. The
server loop resets the table formatting options to the defaults and then
attempts again to parse table formatting options. Unfortunately, they aren't
present any longer. The result is that tables are always formatted with
the default style.

This patch solves the issue by not parsing the table formatting options
in nbctl_client. Instead, the table formatting options are passed to the
server loop and parsed there instead.

Signed-off-by: Mark Michelson <mmichels@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
---
 ovn/utilities/ovn-nbctl.c |  1 -
 tests/ovn-nbctl.at        | 21 +++++++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

Comments

Mark Michelson Feb. 26, 2019, 2:04 p.m. UTC | #1
Please ignore this patch. There is a problem with it and I'll be sending 
a v2 of it.

On 2/26/19 8:51 AM, Mark Michelson wrote:
> This is a backport to 2.10 of a feature already in 2.11. This is
> necessary for ovn-kubernetes to function properly when using 2.10. The
> original commit message is below.
> 
> 
> When ovn-nbctl is running in daemon mode, nbctl_client attempts to parse
> table formatting options. The problem is that this then removes the table
> formatting options from the array of options passed to the server loop. The
> server loop resets the table formatting options to the defaults and then
> attempts again to parse table formatting options. Unfortunately, they aren't
> present any longer. The result is that tables are always formatted with
> the default style.
> 
> This patch solves the issue by not parsing the table formatting options
> in nbctl_client. Instead, the table formatting options are passed to the
> server loop and parsed there instead.
> 
> Signed-off-by: Mark Michelson <mmichels@redhat.com>
> Signed-off-by: Ben Pfaff <blp@ovn.org>
> ---
>   ovn/utilities/ovn-nbctl.c |  1 -
>   tests/ovn-nbctl.at        | 21 +++++++++++++++++++++
>   2 files changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
> index 09bbcf76a..a78c0008b 100644
> --- a/ovn/utilities/ovn-nbctl.c
> +++ b/ovn/utilities/ovn-nbctl.c
> @@ -5364,7 +5364,6 @@ nbctl_client(const char *socket_name,
>               break;
>   
>           VLOG_OPTION_HANDLERS
> -        TABLE_OPTION_HANDLERS(&table_style)
>   
>           case OPT_LOCAL:
>           default:
> diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
> index a599b1bf7..353eff610 100644
> --- a/tests/ovn-nbctl.at
> +++ b/tests/ovn-nbctl.at
> @@ -1517,3 +1517,24 @@ AT_CHECK([ovn-nbctl create Port_Group name=pg0], [0], [ignore])
>   AT_CHECK([ovn-nbctl get Port_Group pg0 name], [0], [dnl
>   "pg0"
>   ])])
> +
> +OVN_NBCTL_TEST([ovn_nbctl_extra_newlines], [extra newlines], [
> +dnl This test addresses a specific issue seen when running ovn-nbctl in
> +dnl daemon mode. All we have to do is ensure that each time we list database
> +dnl information, there is not an extra newline at the beginning of the output.
> +AT_CHECK([ovn-nbctl ls-add sw1], [0], [ignore])
> +AT_CHECK([ovn-nbctl --columns=name list logical_switch sw1], [0], [dnl
> +name                : "sw1"
> +])
> +AT_CHECK([ovn-nbctl --columns=name list logical_switch sw1], [0], [dnl
> +name                : "sw1"
> +])])
> +
> +OVN_NBCTL_TEST([ovn_nbctl_table_formatting], [table formatting], [
> +dnl This test addresses a specific issue seen when running ovn-nbctl in
> +dnl daemon mode. We need to ensure that table formatting options are honored
> +dnl when listing database information.
> +AT_CHECK([ovn-nbctl ls-add sw1], [0], [ignore])
> +AT_CHECK([ovn-nbctl --bare --columns=name list logical_switch sw1], [0], [dnl
> +sw1
> +])])
>
0-day Robot Feb. 26, 2019, 2:57 p.m. UTC | #2
Bleep bloop.  Greetings Mark Michelson, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


git-am:
Failed to merge in the changes.
Patch failed at 0001 ovn-nbctl: Don't parse table-formatting options in nbctl_client
The copy of the patch that failed is found in:
   /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
When you have resolved this problem, run "git am --resolved".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


Please check this out.  If you feel there has been an error, please email aconole@bytheb.org

Thanks,
0-day Robot
diff mbox series

Patch

diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 09bbcf76a..a78c0008b 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -5364,7 +5364,6 @@  nbctl_client(const char *socket_name,
             break;
 
         VLOG_OPTION_HANDLERS
-        TABLE_OPTION_HANDLERS(&table_style)
 
         case OPT_LOCAL:
         default:
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index a599b1bf7..353eff610 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -1517,3 +1517,24 @@  AT_CHECK([ovn-nbctl create Port_Group name=pg0], [0], [ignore])
 AT_CHECK([ovn-nbctl get Port_Group pg0 name], [0], [dnl
 "pg0"
 ])])
+
+OVN_NBCTL_TEST([ovn_nbctl_extra_newlines], [extra newlines], [
+dnl This test addresses a specific issue seen when running ovn-nbctl in
+dnl daemon mode. All we have to do is ensure that each time we list database
+dnl information, there is not an extra newline at the beginning of the output.
+AT_CHECK([ovn-nbctl ls-add sw1], [0], [ignore])
+AT_CHECK([ovn-nbctl --columns=name list logical_switch sw1], [0], [dnl
+name                : "sw1"
+])
+AT_CHECK([ovn-nbctl --columns=name list logical_switch sw1], [0], [dnl
+name                : "sw1"
+])])
+
+OVN_NBCTL_TEST([ovn_nbctl_table_formatting], [table formatting], [
+dnl This test addresses a specific issue seen when running ovn-nbctl in
+dnl daemon mode. We need to ensure that table formatting options are honored
+dnl when listing database information.
+AT_CHECK([ovn-nbctl ls-add sw1], [0], [ignore])
+AT_CHECK([ovn-nbctl --bare --columns=name list logical_switch sw1], [0], [dnl
+sw1
+])])