On 29 August 2016 at 20:12, nickcooper-zhangtonghao <
nickcooper-zhangtong...@opencloud.tech> wrote:
> Add a name column for the load balancer. This must be unique among
> all load balancers. This name has no special meaning or purpose
> other than to provide convenience for human interaction with the
> ovn-nb database. This patch provides the command line to
> create load-balancer and add the unit tests.
>
I think you missed one of my points in the previous review. I want to be
able to create the load balancer independently.
i..e I don't want to wait till I add it to a switch. This helps in
creating a load-balancer and adding it to multiple switches. Also, I have
code sent out for review to add the load-balancer to the gateway router
too. So the commands should work with that too.
So with your patch, can I create a load balancer independently and add it
to multiple switches?
>
> Signed-off-by: nickcooper-zhangtonghao <nickcooper-zhangtonghao@openc
> loud.tech>
> ---
> ovn/ovn-nb.ovsschema | 5 +-
> ovn/ovn-nb.xml | 6 ++
> ovn/utilities/ovn-nbctl.8.xml | 35 +++++++++
> ovn/utilities/ovn-nbctl.c | 163 ++++++++++++++++++++++++++++++
> +++++++++++-
> tests/ovn-nbctl.at | 44 ++++++++++++
> tests/system-ovn.at | 13 ++--
> 6 files changed, 255 insertions(+), 11 deletions(-)
>
> diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
> index 456ae98..87ce15f 100644
> --- a/ovn/ovn-nb.ovsschema
> +++ b/ovn/ovn-nb.ovsschema
> @@ -1,7 +1,7 @@
> {
> "name": "OVN_Northbound",
> "version": "5.3.1",
> - "cksum": "1921908091 9353",
> + "cksum": "360001470 9398",
> "tables": {
> "NB_Global": {
> "columns": {
> @@ -92,6 +92,7 @@
> "isRoot": true},
> "Load_Balancer": {
> "columns": {
> + "name": {"type": "string"},
> "vips": {
> "type": {"key": "string", "value": "string",
> "min": 0, "max": "unlimited"}},
> @@ -102,7 +103,7 @@
> "external_ids": {
> "type": {"key": "string", "value": "string",
> "min": 0, "max": "unlimited"}}},
> - "isRoot": true},
> + "isRoot": false},
> "ACL": {
> "columns": {
> "priority": {"type": {"key": {"type": "integer",
> diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
> index 5719e74..c3666f5 100644
> --- a/ovn/ovn-nb.xml
> +++ b/ovn/ovn-nb.xml
> @@ -660,6 +660,12 @@
> Each row represents one load balancer.
> </p>
>
> + <column name="name">
> + A name for the load balancer. This must be unique among all load
> balancers.
> + This name has no special meaning or purpose other than to provide
> convenience
> + for human interaction with the ovn-nb database.
> + </column>
> +
> <column name="vips">
> <p>
> A map of virtual IPv4 addresses (and an optional port number with
> diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
> index d44f039..fa908b7 100644
> --- a/ovn/utilities/ovn-nbctl.8.xml
> +++ b/ovn/utilities/ovn-nbctl.8.xml
> @@ -102,6 +102,41 @@
> </dd>
> </dl>
>
> + <h1>Logical Switch LB Commands</h1>
> + <dl>
> + <dt>[<code>--may-exist</code>] <code>lb-add</code>
> <var>switch</var> <var>lb</var> <var>vip</var> <var>ips</var>
> [<var>protocol</var>]</dt>
> + <dd>
> + Adds the specified LB to <var>switch</var>. We should assign
> <var>lb</var>
> + a virtual IPv4 address (and an optional port number with : as a
> separator)
> + and the corresponding endpoint IPv4 addresses (and optional port
> numbers
> + with : as separators) separated by commas. The optional argument
> <var>protocol</var>
> + must be either <code>tcp</code> or <code>udp</code>. This
> argument is useful
> + when a port number is provided as part of the <var>vip</var>. If
> the <var>protocol</var>
> + is unspecified and a port number is provided as part of
> <var>vip</var>,
> + OVN assumes the <var>protocol</var> to be <code>tcp</code>. It
> is an error if a load-balancer
> + named <var>lb</var> already exists, unless
> <code>--may-exist</code> is specified.
> + The following example adds a load-balancer with
> <var>protocol</var> <code>udp</code>:
> + <p>
> + <code>lb-add ls0 lb0 30.0.0.10:80 192.168.10.10:80,
> 192.168.10.20:80,192.168.10.30:80 udp</code>
> + </p>
> + </dd>
> +
> + <dt>[<code>--if-exists</code>] <code>lb-del</code>
> <var>switch</var> [<var>lb</var>]</dt>
> + <dd>
> + Deletes LBs from <var>switch</var>. If only
> + <var>switch</var> is supplied, all the LBs from the logical
> + switch are deleted. If <var>lb</var> is also specified,
> + then the <var>lb</var> will be deleted only from the logical
> switch.
> + It is an error if <var>lb</var> does not exist, unless
> <code>--if-exists</code>
> + is specified.
> + </dd>
> +
> + <dt><code>lb-list</code> <var>switch</var></dt>
> + <dd>
> + Lists the LBs on <var>switch</var>.
> + </dd>
> + </dl>
> +
> <h1>Logical Switch Port Commands</h1>
> <dl>
> <dt>[<code>--may-exist</code>] <code>lsp-add</code>
> <var>switch</var> <var>port</var></dt>
> diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
> index d6d64ea..fc02918 100644
> --- a/ovn/utilities/ovn-nbctl.c
> +++ b/ovn/utilities/ovn-nbctl.c
> @@ -331,6 +331,13 @@ ACL commands:\n\
> remove ACLs from SWITCH\n\
> acl-list SWITCH print ACLs for SWITCH\n\
> \n\
> +LB commands:\n\
> + lb-add SWITCH LB VIP[:PORT] IP[:PORT]... [PROTOCOL]\n\
> + add a load-balancer to SWITCH\n\
> + lb-del SWITCH [LB]\n\
> + remove load-balancers from SWITCH\n\
> + lb-list SWITCH [LB] print load-balancers for SWITCH\n\
> +\n\
> Logical switch port commands:\n\
> lsp-add SWITCH PORT add logical port PORT on SWITCH\n\
> lsp-add SWITCH PORT PARENT TAG\n\
> @@ -1315,7 +1322,154 @@ nbctl_acl_del(struct ctl_context *ctx)
> }
> }
> }
> -
> +
> +static void
> +nbctl_lb_add(struct ctl_context *ctx)
> +{
> + const struct nbrec_logical_switch *ls;
> + const char *lb_name = ctx->argv[2];
> + const char *lb_vip = ctx->argv[3];
> + const char *lb_ips = ctx->argv[4];
> +
> + ls = ls_by_name_or_uuid(ctx, ctx->argv[1], true);
> +
> + const char *lb_proto;
> + if (ctx->argc == 5) {
> + /* default protocol.*/
> + lb_proto = "tcp";
> + } else {
> + lb_proto = ctx->argv[5];
> + /* Validate protocol. */
> + if (strcmp(lb_proto, "tcp") && strcmp(lb_proto, "udp")) {
> + ctl_fatal("%s: protocol must be one of \"tcp\", \"udp\".",
> lb_proto);
> + }
> + }
> +
> + bool may_exist = shash_find(&ctx->options, "--may-exist") != NULL;
> + for (int i = 0; i < ls->n_load_balancer; i++) {
> + const struct nbrec_load_balancer *lb
> + = ls->load_balancer[i];
> +
> + if (strcmp(lb->name, lb_name)) {
> + continue;
> + }
> +
> + if (!may_exist) {
> + ctl_fatal("%s: a load balancer with this name already
> exists", lb_name);
> + }
> +
> + nbrec_logical_switch_verify_load_balancer(ls);
> + nbrec_load_balancer_verify_protocol(lb);
> + nbrec_load_balancer_verify_vips(lb);
> + nbrec_load_balancer_set_protocol(lb, lb_proto);
> +
> + struct smap vips = SMAP_INITIALIZER(&vips);
> + smap_add(&vips, lb_vip, lb_ips);
> + nbrec_load_balancer_set_vips(lb, &vips);
> + smap_destroy(&vips);
> +
> + return;
> + }
> +
> + /* Create the load balancer. */
> + struct nbrec_load_balancer *lb = nbrec_load_balancer_insert(ctx
> ->txn);
> + nbrec_load_balancer_set_name(lb, lb_name);
> + nbrec_load_balancer_set_protocol(lb, lb_proto);
> +
> + struct smap vips = SMAP_INITIALIZER(&vips);
> + smap_add(&vips, lb_vip, lb_ips);
> + nbrec_load_balancer_set_vips(lb, &vips);
> + smap_destroy(&vips);
> +
> + /* Insert the load balancer into the logical switch. */
> + nbrec_logical_switch_verify_acls(ls);
> + struct nbrec_load_balancer **new_lbs
> + = xmalloc(sizeof *new_lbs * (ls->n_load_balancer + 1));
> +
> + memcpy(new_lbs, ls->load_balancer, sizeof *new_lbs *
> ls->n_load_balancer);
> + new_lbs[ls->n_load_balancer] = lb;
> + nbrec_logical_switch_set_load_balancer(ls, new_lbs,
> ls->n_load_balancer + 1);
> + free(new_lbs);
> +}
> +
> +static void
> +nbctl_lb_del(struct ctl_context *ctx)
> +{
> + const char *lb_name = ctx->argv[2];
> + const struct nbrec_logical_switch *ls;
> + ls = ls_by_name_or_uuid(ctx, ctx->argv[1], true);
> +
> + if (ctx->argc == 2) {
> + /* If load-balancer is not specified, delete
> + * all load-balancers. */
> + nbrec_logical_switch_verify_load_balancer(ls);
> + nbrec_logical_switch_set_load_balancer(ls, NULL, 0);
> + return;
> + }
> +
> + for (size_t i = 0; i < ls->n_load_balancer; i++) {
> + const struct nbrec_load_balancer *lb
> + = ls->load_balancer[i];
> +
> + if (strcmp(lb->name, lb_name)) {
> + continue;
> + }
> +
> + /* Remove the matching rule. */
> + struct nbrec_load_balancer **new_lbs
> + = xmemdup(ls->load_balancer, sizeof *new_lbs *
> ls->n_load_balancer);
> + new_lbs[i] = ls->load_balancer[ls->n_load_balancer - 1];
> + nbrec_logical_switch_verify_load_balancer(ls);
> + nbrec_logical_switch_set_load_balancer(ls, new_lbs,
> + ls->n_load_balancer - 1);
> + free(new_lbs);
> + return;
> + }
> +
> + bool must_exist = !shash_find(&ctx->options, "--if-exists");
> + if (must_exist) {
> + ctl_fatal("load balancer %s is not part of any logical switch.",
> + lb_name);
> + }
> +}
> +
> +static void
> +nbctl_lb_list(struct ctl_context *ctx)
> +{
> + const struct nbrec_logical_switch *ls;
> +
> + ls = ls_by_name_or_uuid(ctx, ctx->argv[1], true);
> +
> + struct smap lbs = SMAP_INITIALIZER(&lbs);
> + for (int i = 0; i < ls->n_load_balancer; i++) {
> + const struct nbrec_load_balancer *lb
> + = ls->load_balancer[i];
> + if (ctx->argc == 3 && strcmp(lb->name, ctx->argv[2])) {
> + continue;
> + }
> +
> + const struct smap_node **nodes = smap_sort(&lb->vips);
> + if (nodes) {
> + const struct smap_node *node = nodes[0];
> + smap_add_format(&lbs, lb->name, "%-10.8s %-5s %-20s %s",
> + lb->name, lb->protocol, node->key, node->value);
> + free(nodes);
> + }
> + }
> +
> + const struct smap_node **nodes = smap_sort(&lbs);
> + ds_put_format(&ctx->output,
> + "Logical Switch: %s\n"
> + "%-10.8s %-5s %-20s IPs\n",
> + ls->name, "LB", "PROTO", "VIP");
> + for (size_t i = 0; i < smap_count(&lbs); i++) {
> + const struct smap_node *node = nodes[i];
> + ds_put_format(&ctx->output, "%s\n", node->value);
> + }
> + smap_destroy(&lbs);
> + free(nodes);
> +}
> +
> static void
> nbctl_lr_add(struct ctl_context *ctx)
> {
> @@ -2420,6 +2574,13 @@ static const struct ctl_command_syntax
> nbctl_commands[] = {
> nbctl_acl_del, NULL, "", RW },
> { "acl-list", 1, 1, "SWITCH", NULL, nbctl_acl_list, NULL, "", RO },
>
> + /* load balancer commands. */
> + { "lb-add", 4, 5, "SWITCH LB VIP[:PORT] IP[:PORT]... [PROTOCOL]",
> NULL,
> + nbctl_lb_add, NULL, "--may-exist", RW },
> + { "lb-del", 1, 2, "SWITCH [LB]", NULL,
> + nbctl_lb_del, NULL, "--if-exists", RW },
> + { "lb-list", 1, 2, "SWITCH [LB]", NULL, nbctl_lb_list, NULL, "", RO },
> +
> /* logical switch port commands. */
> { "lsp-add", 2, 4, "SWITCH PORT [PARENT] [TAG]", NULL, nbctl_lsp_add,
> NULL, "--may-exist", RW },
> diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
> index 5357ced..3ea246e 100644
> --- a/tests/ovn-nbctl.at
> +++ b/tests/ovn-nbctl.at
> @@ -239,6 +239,50 @@ AT_CLEANUP
>
> dnl ---------------------------------------------------------------------
>
> +AT_SETUP([ovn-nbctl - LBs])
> +OVN_NBCTL_TEST_START
> +
> +AT_CHECK([ovn-nbctl ls-add ls0])
> +AT_CHECK([ovn-nbctl lb-add ls0 lb0 30.0.0.10:80 192.168.10.10:80,
> 192.168.10.20:80])
> +AT_CHECK([ovn-nbctl lb-add ls0 lb1 30.0.0.10:80 192.168.10.10:80,
> 192.168.10.20:80 tcp])
> +AT_CHECK([ovn-nbctl lb-add ls0 lb2 30.0.0.10:80 192.168.10.10:80,
> 192.168.10.20:80 udp])
> +
> +AT_CHECK([ovn-nbctl lb-list ls0], [0], [dnl
> +Logical Switch: ls0
> +LB PROTO VIP IPs
> +lb0 tcp 30.0.0.10:80 192.168.10.10:80,192.168.10.20:80
> +lb1 tcp 30.0.0.10:80 192.168.10.10:80,192.168.10.20:80
> +lb2 udp 30.0.0.10:80 192.168.10.10:80,192.168.10.20:80
> +])
> +
> +dnl List a load-balancer.
> +AT_CHECK([ovn-nbctl lb-list ls0 lb0], [0], [dnl
> +Logical Switch: ls0
> +LB PROTO VIP IPs
> +lb0 tcp 30.0.0.10:80 192.168.10.10:80,192.168.10.20:80
> +])
> +
> +dnl Delete a load-balancer.
> +AT_CHECK([ovn-nbctl lb-del ls0 lb0])
> +AT_CHECK([ovn-nbctl lb-list ls0], [0], [dnl
> +Logical Switch: ls0
> +LB PROTO VIP IPs
> +lb1 tcp 30.0.0.10:80 192.168.10.10:80,192.168.10.20:80
> +lb2 udp 30.0.0.10:80 192.168.10.10:80,192.168.10.20:80
> +])
> +
> +dnl Delete all load-balancers.
> +AT_CHECK([ovn-nbctl lb-del ls0])
> +AT_CHECK([ovn-nbctl lb-list ls0], [0], [dnl
> +Logical Switch: ls0
> +LB PROTO VIP IPs
> +])
> +
> +OVN_NBCTL_TEST_STOP
> +AT_CLEANUP
> +
> +dnl ---------------------------------------------------------------------
> +
> AT_SETUP([ovn-nbctl - basic logical router commands])
> OVN_NBCTL_TEST_START
>
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index e267384..109be85 100755
> --- a/tests/system-ovn.at
> +++ b/tests/system-ovn.at
> @@ -348,15 +348,13 @@ ovn-nbctl lsp-add bar bar3 \
> -- lsp-set-addresses bar3 "f0:00:0f:01:02:05 172.16.1.4"
>
> # Config OVN load-balancer with a VIP.
> -uuid=`ovn-nbctl create load_balancer vips:30.0.0.1="172.16.1.2,172.
> 16.1.3,172.16.1.4"`
> -ovn-nbctl set logical_switch foo load_balancer=$uuid
> +ovn-nbctl lb-add foo lb0 30.0.0.1 172.16.1.2,172.16.1.3,172.16.1.4
>
> # Create another load-balancer with another VIP.
> -uuid=`ovn-nbctl create load_balancer vips:30.0.0.3="172.16.1.2,172.
> 16.1.3,172.16.1.4"`
> -ovn-nbctl add logical_switch foo load_balancer $uuid
> +ovn-nbctl lb-add foo lb1 30.0.0.3 172.16.1.2,172.16.1.3,172.16.1.4
>
> # Config OVN load-balancer with another VIP (this time with ports).
> -ovn-nbctl set load_balancer $uuid vips:'"30.0.0.2:8000"'='"172.16.1.2:80,
> 172.16.1.3:80,172.16.1.4:80"'
> +ovn-nbctl lb-add foo lb2 30.0.0.2:8000 172.16.1.2:80,172.16.1.3:80,17
> 2.16.1.4:80
>
> # Wait for ovn-controller to catch up.
> OVS_WAIT_UNTIL([ovs-ofctl -O OpenFlow13 dump-groups br-int | grep ct\(])
> @@ -468,11 +466,10 @@ for i in `seq 1 4`; do
> done
>
> # Config OVN load-balancer with a VIP.
> -uuid=`ovn-nbctl create load_balancer vips:30.0.0.1="192.168.1.3,192
> .168.1.4,192.168.1.5"`
> -ovn-nbctl set logical_switch foo load_balancer=$uuid
> +ovn-nbctl lb-add foo lb0 30.0.0.1 192.168.1.3,192.168.1.4,192.168.1.5
>
> # Config OVN load-balancer with another VIP (this time with ports).
> -ovn-nbctl set load_balancer $uuid vips:'"30.0.0.2:8000"'='"192.168.1.3:80
> ,192.168.1.4:80,192.168.1.5:80"'
> +ovn-nbctl lb-add foo lb1 30.0.0.2:8000 192.168.1.3:80,192.168.1.4:80,
> 192.168.1.5:80
>
> # Wait for ovn-controller to catch up.
> OVS_WAIT_UNTIL([ovs-ofctl -O OpenFlow13 dump-groups br-int | grep ct\(])
> --
> 1.8.3.1
>
>
>
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
