On 10/04/2018 06:08 PM, Han Zhou wrote:


On Wed, Oct 3, 2018 at 2:14 PM Mark Michelson <mmich...@redhat.com <mailto:mmich...@redhat.com>> wrote:
 >
> Signed-off-by: Mark Michelson <mmich...@redhat.com <mailto:mmich...@redhat.com>>
 > ---
 >  ovn/utilities/ovn-nbctl.8.xml | 22 +++++++++++++
>  ovn/utilities/ovn-nbctl.c     | 72 +++++++++++++++++++++++++++++++++++++++++++ >  tests/ovn.at <http://ovn.at>                  | 37 ++++++++++++++++++++++
 >  3 files changed, 131 insertions(+)
 >
> diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
 > index 5e18fa7c0..14cc02f53 100644
 > --- a/ovn/utilities/ovn-nbctl.8.xml
 > +++ b/ovn/utilities/ovn-nbctl.8.xml
 > @@ -881,6 +881,28 @@
 >        </dd>
 >      </dl>
 >
 > +    <h1>Port Group commands</h1>
 > +
 > +    <dl>
 > +      <dt><code>pg-add</code> <var>group</var> [<var>port</var>]...</dt>
 > +      <dd>
> +        Creates a new port group in the <code>Port_Group</code> table named > +        <code>group</code> with optional <code>ports</code> added to the group.
 > +      </dd>
 > +
> +      <dt><code>pg-set-ports</code> <var>group</var> <var>port</var>...</dt>
 > +      <dd>
> +        Sets <code>ports</code> on the port group named <code>group</code>. It
 > +        is an error if <code>group</code> does not exist.
 > +      </dd>
 > +
 > +      <dt><code>pg-del</code> <var>group</var></dt>
 > +      <dd>
 > +        Deletes port group <code>group</code>. It is an error if
 > +        <code>group</code> does not exist.
 > +      </dd>
 > +    </dl>
 > +
 >      <h1>Database Commands</h1>
>      <p>These commands query and modify the contents of <code>ovsdb</code> tables. >      They are a slight abstraction of the <code>ovsdb</code> interface and
 > diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
 > index eabd30308..b586db502 100644
 > --- a/ovn/utilities/ovn-nbctl.c
 > +++ b/ovn/utilities/ovn-nbctl.c
 > @@ -4655,6 +4655,73 @@ cmd_set_ssl(struct ctl_context *ctx)
 >      nbrec_nb_global_set_ssl(nb_global, ssl);
 >  }
 >
 > +static char *
> +set_ports_on_pg(struct ctl_context *ctx, const struct nbrec_port_group *pg,
 > +                char **new_ports, size_t num_new_ports)
 > +{
 > +    struct nbrec_logical_switch_port **lports;
 > +    lports = xmalloc(sizeof *lports * num_new_ports);
 > +
 > +    size_t i;
 > +    char *error = NULL;
 > +    for (i = 0; i < num_new_ports; i++) {
 > +        const struct nbrec_logical_switch_port *lsp;
 > +        error = lsp_by_name_or_uuid(ctx, new_ports[i], true, &lsp);
 > +        if (error) {
 > +            goto out;
 > +        }
 > +        lports[i] = (struct nbrec_logical_switch_port *) lsp;
 > +    }
 > +
 > +    nbrec_port_group_set_ports(pg, lports, num_new_ports);
 > +
 > +out:
 > +    free(lports);
 > +    return error;
 > +}
 > +
 > +static void
 > +cmd_pg_add(struct ctl_context *ctx)
 > +{
 > +    const struct nbrec_port_group *pg;
 > +
 > +    pg = nbrec_port_group_insert(ctx->txn);
 > +    nbrec_port_group_set_name(pg, ctx->argv[1]);
 > +    if (ctx->argc > 2) {
> +        ctx->error = set_ports_on_pg(ctx, pg, ctx->argv + 2, ctx->argc - 2);
 > +    }
 > +}
 > +
 > +static void
 > +cmd_pg_set_ports(struct ctl_context *ctx)
 > +{
 > +    const struct nbrec_port_group *pg;
 > +
 > +    char *error;
 > +    error = pg_by_name_or_uuid(ctx, ctx->argv[1], true, &pg);
 > +    if (error) {
 > +        ctx->error = error;
 > +        return;
 > +    }
 > +
 > +    ctx->error = set_ports_on_pg(ctx, pg, ctx->argv + 2, ctx->argc - 2);
 > +}
 > +
 > +static void
 > +cmd_pg_del(struct ctl_context *ctx)
 > +{
 > +    const struct nbrec_port_group *pg;
 > +
 > +    char *error;
 > +    error = pg_by_name_or_uuid(ctx, ctx->argv[1], true, &pg);
 > +    if (error) {
 > +        ctx->error = error;
 > +        return;
 > +    }
 > +
 > +    nbrec_port_group_delete(pg);
 > +}
 > +
 >  static const struct ctl_table_class tables[NBREC_N_TABLES] = {
 >      [NBREC_TABLE_DHCP_OPTIONS].row_ids
 >      = {{&nbrec_logical_switch_port_col_name, NULL,
> @@ -5123,6 +5190,11 @@ static const struct ctl_command_syntax nbctl_commands[] = {
 >          "PRIVATE-KEY CERTIFICATE CA-CERT [SSL-PROTOS [SSL-CIPHERS]]",
 >          pre_cmd_set_ssl, cmd_set_ssl, NULL, "--bootstrap", RW},
 >
 > +    /* Port Group Commands */
 > +    {"pg-add", 1, INT_MAX, "", NULL, cmd_pg_add, NULL, "", RW },
> +    {"pg-set-ports", 2, INT_MAX, "", NULL, cmd_pg_set_ports, NULL, "", RW },
 > +    {"pg-del", 1, 1, "", NULL, cmd_pg_del, NULL, "", RW },
 > +
 >      {NULL, 0, 0, NULL, NULL, NULL, NULL, "", RO},
 >  };
 >
 > diff --git a/tests/ovn.at <http://ovn.at> b/tests/ovn.at <http://ovn.at>
 > index 769e09f81..81dff3da2 100644
 > --- a/tests/ovn.at <http://ovn.at>
 > +++ b/tests/ovn.at <http://ovn.at>
> @@ -11253,3 +11253,40 @@ AT_CHECK([ovn-nbctl lsp-set-addresses sw2-p1 "00:00:00:00:00:04 192.168.0.3"]) >  AT_CHECK([ovn-nbctl lsp-set-addresses sw2-p1 "00:00:00:00:00:04 aef0::1"])
 >
 >  AT_CLEANUP
 > +
 > +AT_SETUP([ovn -- ovn-nbctl port group commands])
 > +ovn_start
 > +
 > +ovn-nbctl pg-add pg1
 > +AT_CHECK([ovn-nbctl --bare --columns=name list port_group pg1], [0],
 > +[pg1
 > +])
 > +
 > +ovn-nbctl pg-del pg1
 > +AT_CHECK([ovn-nbctl list port_group], [0], [])
 > +
 > +ovn-nbctl ls-add sw1
 > +ovn-nbctl lsp-add sw1 sw1-p1
> +SW1P1=$(ovn-nbctl --bare --columns=_uuid list logical_switch_port sw1-p1)
 > +ovn-nbctl lsp-add sw1 sw1-p2
> +SW1P2=$(ovn-nbctl --bare --columns=_uuid list logical_switch_port sw1-p2)
 > +
 > +ovn-nbctl list logical_switch_port
 > +
 > +ovn-nbctl pg-add pg1 sw1-p1
 > +AT_CHECK([ovn-nbctl --bare --columns=name list port_group pg1], [0],
 > +[pg1
 > +])
> +AT_CHECK_UNQUOTED([ovn-nbctl --bare --columns=ports list port_group pg1], [0],
 > +[[$SW1P1
 > +]])
 > +
 > +ovn-nbctl pg-set-ports pg1 sw1-p2
> +AT_CHECK_UNQUOTED([ovn-nbctl --bare --columns=ports list port_group pg1], [0],
 > +[[$SW1P2
 > +]])
 > +
 > +ovn-nbctl pg-del pg1
 > +AT_CHECK([ovn-nbctl list port_group], [0], [])
 > +
 > +AT_CLEANUP
 > --
 > 2.14.4
 >
 > _______________________________________________
 > dev mailing list
 > d...@openvswitch.org <mailto:d...@openvswitch.org>
 > https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Thanks Mark for improving the commands for port-groups. It looks good to me, except that the nbctl related tests may be better to stay in ovn-nbctl.at <http://ovn-nbctl.at>.

Han

Thanks for the feedback Han. Interestingly, when I move the tests from ovn.at to ovn-nbctl.at, the tests no longer pass. When running in non-daemon mode, for whatever reason, trying to get the port group's ports from the database results in an empty return. I can't reproduce this in the sandbox, so I'll need to dive in and see what's going wrong.

When run in daemon mode, interestingly, it appears that there are a couple of legitimate bugs I'm running into. First, there's an extra blank line at the beginning before the output of the database output. Second, it appears the --bare flag is ignored. I can reproduce both of these in the sandbox.

So I think this is going to result in another patch from me to fix the daemon mode bugs mentioned above.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to