[ovs-dev] [PATCH v6 4/5] ovn-northd: Delete the references to gateway_chasss in SB DB

nusiddiq at redhat.com nusiddiq at redhat.com
Wed Mar 27 18:56:50 UTC 2019


From: Numan Siddique <nusiddiq at redhat.com>

Previous patch in the series added the support in ovn-controller
to use ha_chassis_group table in SB DB to support HA chassis
and establishing BFD tunnels instead of the gateway_chassis table.
There is no need for ovn-northd to create any gateway_chassis
rows in SB DB. This patch does that and deletes the code
which is not required anymore.

This patch also now supports 'ha_chassis_group' to be associated
with a distributed logical router port and ignores 'gateway_chassis'
and 'redirect-chassis' if set along with 'ha_chassis_group'.

Signed-off-by: Numan Siddique <nusiddiq at redhat.com>
Acked-by: Han Zhou <hzhou8 at ebay.com>
---
 NEWS                    |   1 +
 ovn/northd/ovn-northd.c | 331 ++++++++++++++++++++--------------------
 tests/ovn-northd.at     | 232 ++++++++++++++++++++++++----
 tests/ovn.at            | 175 +++++++++++++++++++++
 4 files changed, 540 insertions(+), 199 deletions(-)

diff --git a/NEWS b/NEWS
index 1e4744dbd..22c7895c8 100644
--- a/NEWS
+++ b/NEWS
@@ -24,6 +24,7 @@ Post-v2.11.0
        protocol extension.
    - OVN:
      * Select IPAM mac_prefix in a random manner if not provided by the user
+     * Added the HA chassis group support.
    - New QoS type "linux-netem" on Linux.
 
 v2.11.0 - 19 Feb 2019
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 87199256b..96aec8326 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -1732,7 +1732,8 @@ join_logical_ports(struct northd_context *ctx,
 
                 const char *redirect_chassis = smap_get(&op->nbrp->options,
                                                         "redirect-chassis");
-                if (redirect_chassis || op->nbrp->n_gateway_chassis) {
+                if (op->nbrp->ha_chassis_group || redirect_chassis ||
+                    op->nbrp->n_gateway_chassis) {
                     /* Additional "derived" ovn_port crp represents the
                      * instance of op on the "redirect-chassis". */
                     const char *gw_chassis = smap_get(&op->od->nbr->options,
@@ -1966,112 +1967,54 @@ get_nat_addresses(const struct ovn_port *op, size_t *n)
     return addresses;
 }
 
-static bool
-gateway_chassis_equal(const struct nbrec_gateway_chassis *nb_gwc,
-                      const struct sbrec_chassis *nb_gwc_c,
-                      const struct sbrec_gateway_chassis *sb_gwc)
-{
-    bool equal = !strcmp(nb_gwc->name, sb_gwc->name)
-                 && nb_gwc->priority == sb_gwc->priority
-                 && smap_equal(&nb_gwc->options, &sb_gwc->options)
-                 && smap_equal(&nb_gwc->external_ids, &sb_gwc->external_ids);
-
-    if (!equal) {
-        return false;
-    }
-
-    /* If everything else matched and we were unable to find the SBDB
-     * Chassis entry at this time, assume a match and return true.
-     * This happens when an ovn-controller is restarting and the Chassis
-     * entry is gone away momentarily */
-    return !nb_gwc_c
-           || (sb_gwc->chassis && !strcmp(nb_gwc_c->name,
-                                          sb_gwc->chassis->name));
-}
-
 static bool
 sbpb_gw_chassis_needs_update(
-    struct ovsdb_idl_index *sbrec_chassis_by_name,
-    const struct sbrec_port_binding *port_binding,
+    const struct sbrec_port_binding *pb,
     const struct nbrec_logical_router_port *lrp)
 {
-    if (!lrp || !port_binding) {
+    if (!lrp || !pb) {
         return false;
     }
 
-    if (lrp->n_gateway_chassis && !port_binding->ha_chassis_group) {
+    if (lrp->n_gateway_chassis && !pb->ha_chassis_group) {
         /* If there are gateway chassis in the NB DB, but there is
          * no corresponding HA chassis group in SB DB we need to
          * create the HA chassis group in SB DB for this lrp. */
         return true;
     }
 
-    /* These arrays are used to collect valid Gateway_Chassis and valid
-     * Chassis records from the Logical_Router_Port Gateway_Chassis list,
-     * we ignore the ones we can't match on the SBDB */
-    struct nbrec_gateway_chassis **lrp_gwc = xzalloc(lrp->n_gateway_chassis *
-                                                     sizeof *lrp_gwc);
-    const struct sbrec_chassis **lrp_gwc_c = xzalloc(lrp->n_gateway_chassis *
-                                               sizeof *lrp_gwc_c);
-
-    /* Count the number of gateway chassis chassis names from the logical
-     * router port that we are able to match on the southbound database */
-    int lrp_n_gateway_chassis = 0;
-    int n;
-    for (n = 0; n < lrp->n_gateway_chassis; n++) {
-
-        if (!lrp->gateway_chassis[n]->chassis_name) {
-            continue;
-        }
-
-        const struct sbrec_chassis *chassis =
-            chassis_lookup_by_name(sbrec_chassis_by_name,
-                                   lrp->gateway_chassis[n]->chassis_name);
-
-        lrp_gwc_c[lrp_n_gateway_chassis] = chassis;
-        lrp_gwc[lrp_n_gateway_chassis] = lrp->gateway_chassis[n];
-        lrp_n_gateway_chassis++;
-        if (!chassis) {
-            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
-            VLOG_WARN_RL(
-                &rl, "Chassis name %s referenced in NBDB via Gateway_Chassis "
-                     "on logical router port %s does not exist in SBDB",
-                     lrp->gateway_chassis[n]->chassis_name, lrp->name);
-        }
+    if (strcmp(pb->ha_chassis_group->name, lrp->name)) {
+        /* Name doesn't match. */
+        return true;
     }
 
-    /* Basic check, different amount of Gateway_Chassis means that we
-     * need to update southbound database Port_Binding */
-    if (lrp_n_gateway_chassis != port_binding->n_gateway_chassis) {
-        free(lrp_gwc_c);
-        free(lrp_gwc);
+    if (lrp->n_gateway_chassis != pb->ha_chassis_group->n_ha_chassis) {
         return true;
     }
 
-    for (n = 0; n < lrp_n_gateway_chassis; n++) {
-        int i;
-        /* For each of the valid gw chassis on the lrp, check if there's
-         * a match on the Port_Binding list, we assume order is not
-         * persisted */
-        for (i = 0; i < port_binding->n_gateway_chassis; i++) {
-            if (gateway_chassis_equal(lrp_gwc[n],
-                                      lrp_gwc_c[n],
-                                      port_binding->gateway_chassis[i])) {
-                break; /* we found a match */
+    for (size_t i = 0; i < lrp->n_gateway_chassis; i++) {
+        struct nbrec_gateway_chassis *nbgw_ch = lrp->gateway_chassis[i];
+        bool found = false;
+        for (size_t j = 0; j < pb->ha_chassis_group->n_ha_chassis; j++) {
+            struct sbrec_ha_chassis *sbha_ch =
+                pb->ha_chassis_group->ha_chassis[j];
+            if (sbha_ch->chassis &&
+                !strcmp(nbgw_ch->chassis_name, sbha_ch->chassis->name)) {
+                if (nbgw_ch->priority == sbha_ch->priority) {
+                    found = true;
+                    break;
+                }
+                /* Priority has changed. Return true. */
+                return true;
             }
         }
 
-        /* if no Port_Binding gateway chassis matched for the entry... */
-        if (i == port_binding->n_gateway_chassis) {
-            free(lrp_gwc_c);
-            free(lrp_gwc);
-            return true; /* found no match for this gateway chassis on lrp */
+        if (!found) {
+            return true;
         }
     }
 
-    /* no need for update, all ports matched */
-    free(lrp_gwc_c);
-    free(lrp_gwc);
+    /* No need to update SB DB. Its in sync. */
     return false;
 }
 
@@ -2086,17 +2029,101 @@ create_sb_ha_chassis(struct northd_context *ctx,
     return sb_ha_chassis;
 }
 
+static bool
+chassis_group_list_changed(
+    const struct nbrec_ha_chassis_group *nb_ha_grp,
+    const struct sbrec_ha_chassis_group *sb_ha_grp)
+{
+    if (nb_ha_grp->n_ha_chassis != sb_ha_grp->n_ha_chassis) {
+        return true;
+    }
+
+    struct shash nb_ha_chassis_list = SHASH_INITIALIZER(&nb_ha_chassis_list);
+    for (size_t i = 0; i < nb_ha_grp->n_ha_chassis; i++) {
+        shash_add(&nb_ha_chassis_list,
+                  nb_ha_grp->ha_chassis[i]->chassis_name,
+                  nb_ha_grp->ha_chassis[i]);
+    }
+
+    bool changed = false;
+    const struct sbrec_ha_chassis *sb_ha_chassis;
+    const struct nbrec_ha_chassis *nb_ha_chassis;
+    for (size_t i = 0; i < sb_ha_grp->n_ha_chassis; i++) {
+        sb_ha_chassis = sb_ha_grp->ha_chassis[i];
+        if (!sb_ha_chassis->chassis) {
+            /* This can happen, if ovn-controller exits gracefully in
+             * a chassis, in which case, chassis row for that chassis
+             * would be NULL. */
+            changed = true;
+            break;
+        }
+
+        nb_ha_chassis = shash_find_and_delete(&nb_ha_chassis_list,
+                                              sb_ha_chassis->chassis->name);
+        if (!nb_ha_chassis ||
+            nb_ha_chassis->priority != sb_ha_chassis->priority) {
+            changed = true;
+            break;
+        }
+    }
+
+    struct shash_node *node, *next;
+    SHASH_FOR_EACH_SAFE (node, next, &nb_ha_chassis_list) {
+        shash_delete(&nb_ha_chassis_list, node);
+        changed = true;
+    }
+    shash_destroy(&nb_ha_chassis_list);
+
+    return changed;
+}
+
+static void
+sync_ha_chassis_group_for_sbpb(struct northd_context *ctx,
+                               const struct nbrec_ha_chassis_group *nb_ha_grp,
+                               struct ovsdb_idl_index *sbrec_chassis_by_name,
+                               const struct sbrec_port_binding *pb)
+{
+    bool new_sb_chassis_group = false;
+    const struct sbrec_ha_chassis_group *sb_ha_grp =
+        ha_chassis_group_lookup_by_name(
+            ctx->sbrec_ha_chassis_grp_by_name, nb_ha_grp->name);
+
+    if (!sb_ha_grp) {
+        sb_ha_grp = sbrec_ha_chassis_group_insert(ctx->ovnsb_txn);
+        sbrec_ha_chassis_group_set_name(sb_ha_grp, nb_ha_grp->name);
+        new_sb_chassis_group = true;
+    }
+
+    if (new_sb_chassis_group ||
+        chassis_group_list_changed(nb_ha_grp, sb_ha_grp)) {
+        struct sbrec_ha_chassis **sb_ha_chassis = NULL;
+        size_t n_ha_chassis = nb_ha_grp->n_ha_chassis;
+        sb_ha_chassis = xcalloc(n_ha_chassis, sizeof *sb_ha_chassis);
+        for (size_t i = 0; i < nb_ha_grp->n_ha_chassis; i++) {
+            const struct nbrec_ha_chassis *nb_ha_chassis
+                = nb_ha_grp->ha_chassis[i];
+            const struct sbrec_chassis *chassis =
+                chassis_lookup_by_name(sbrec_chassis_by_name,
+                                       nb_ha_chassis->chassis_name);
+            sb_ha_chassis[i] = sbrec_ha_chassis_insert(ctx->ovnsb_txn);
+            /* It's perfectly ok if the chassis is NULL. This could
+             * happen when ovn-controller exits and removes its row
+             * from the chassis table in OVN SB DB. */
+            sbrec_ha_chassis_set_chassis(sb_ha_chassis[i], chassis);
+            sbrec_ha_chassis_set_priority(sb_ha_chassis[i],
+                                          nb_ha_chassis->priority);
+        }
+        sbrec_ha_chassis_group_set_ha_chassis(sb_ha_grp, sb_ha_chassis,
+                                              n_ha_chassis);
+        free(sb_ha_chassis);
+    }
+
+    sbrec_port_binding_set_ha_chassis_group(pb, sb_ha_grp);
+}
+
 /* This functions translates the gw chassis on the nb database
- * to sb database entries, the only difference is that SB database
- * Gateway_Chassis table references the chassis directly instead
- * of using the name.
- *
- * This function also creates a HA Chassis group in SB DB for
- * the gateway chassis associated to a distributed gateway
- * router port in the NB DB.
- *
- * An upcoming patch will delete the code to create the Gateway chassis
- * in SB DB.*/
+ * to HA chassis group in the sb database entries.
+ */
 static void
 copy_gw_chassis_from_nbrp_to_sbpb(
         struct northd_context *ctx,
@@ -2104,14 +2131,9 @@ copy_gw_chassis_from_nbrp_to_sbpb(
         const struct nbrec_logical_router_port *lrp,
         const struct sbrec_port_binding *port_binding)
 {
-    struct sbrec_gateway_chassis **gw_chassis = NULL;
-    int n_gwc = 0;
-    int n;
 
     /* Make use of the new HA chassis group table to support HA
-     * for the distributed gateway router port. We can delete
-     * the old gateway_chassis code once ovn-controller supports
-     * HA chassis group. */
+     * for the distributed gateway router port. */
     const struct sbrec_ha_chassis_group *sb_ha_chassis_group =
         ha_chassis_group_lookup_by_name(
             ctx->sbrec_ha_chassis_grp_by_name, lrp->name);
@@ -2123,16 +2145,7 @@ copy_gw_chassis_from_nbrp_to_sbpb(
     struct sbrec_ha_chassis **sb_ha_chassis = xcalloc(lrp->n_gateway_chassis,
                                                       sizeof *sb_ha_chassis);
     size_t n_sb_ha_ch = 0;
-    /* XXX: This can be improved. This code will generate a set of new
-     * Gateway_Chassis and push them all in a single transaction, instead
-     * this would be more optimal if we just add/update/remove the rows in
-     * the southbound db that need to change. We don't expect lots of
-     * changes to the Gateway_Chassis table, but if that proves to be wrong
-     * we should optimize this.
-     *
-     * Note: Remove the below code to add gateway_chassis row in OVN
-     * Southbound db once ovn-controller supports HA chassis group. */
-    for (n = 0; n < lrp->n_gateway_chassis; n++) {
+    for (size_t n = 0; n < lrp->n_gateway_chassis; n++) {
         struct nbrec_gateway_chassis *lrp_gwc = lrp->gateway_chassis[n];
         if (!lrp_gwc->chassis_name) {
             continue;
@@ -2142,27 +2155,10 @@ copy_gw_chassis_from_nbrp_to_sbpb(
             chassis_lookup_by_name(sbrec_chassis_by_name,
                                    lrp_gwc->chassis_name);
 
-        gw_chassis = xrealloc(gw_chassis, (n_gwc + 1) * sizeof *gw_chassis);
-
-        /* This code to create gateway_chassis in SB DB needs to be deleted
-         * once ovn-controller supports making use of HA chassis groups. */
-        struct sbrec_gateway_chassis *pb_gwc =
-            sbrec_gateway_chassis_insert(ctx->ovnsb_txn);
-
-        sbrec_gateway_chassis_set_name(pb_gwc, lrp_gwc->name);
-        sbrec_gateway_chassis_set_priority(pb_gwc, lrp_gwc->priority);
-        sbrec_gateway_chassis_set_chassis(pb_gwc, chassis);
-        sbrec_gateway_chassis_set_options(pb_gwc, &lrp_gwc->options);
-        sbrec_gateway_chassis_set_external_ids(pb_gwc, &lrp_gwc->external_ids);
-
-        gw_chassis[n_gwc++] = pb_gwc;
-
         sb_ha_chassis[n_sb_ha_ch] =
             create_sb_ha_chassis(ctx, chassis, lrp_gwc->priority);
         n_sb_ha_ch++;
     }
-    sbrec_port_binding_set_gateway_chassis(port_binding, gw_chassis, n_gwc);
-    free(gw_chassis);
 
     sbrec_ha_chassis_group_set_ha_chassis(sb_ha_chassis_group,
                                           sb_ha_chassis, n_sb_ha_ch);
@@ -2195,18 +2191,37 @@ ovn_port_update_sbrec(struct northd_context *ctx,
         if (op->derived) {
             const char *redirect_chassis = smap_get(&op->nbrp->options,
                                                     "redirect-chassis");
-            if (op->nbrp->n_gateway_chassis && redirect_chassis) {
+            int n_gw_options_set = 0;
+            if (op->nbrp->ha_chassis_group) {
+                n_gw_options_set++;
+            }
+            if (op->nbrp->n_gateway_chassis) {
+                n_gw_options_set++;
+            }
+            if (redirect_chassis) {
+                n_gw_options_set++;
+            }
+            if (n_gw_options_set > 1) {
                 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
                 VLOG_WARN_RL(
-                    &rl, "logical router port %s has both options:"
-                         "redirect-chassis and gateway_chassis populated "
-                         "redirect-chassis will be ignored in favour of "
-                         "gateway chassis", op->nbrp->name);
+                    &rl, "Multiple gatway options set for the logical router "
+                         "port %s. The first preferred option is "
+                         "ha_chassis_group; the second is gateway_chassis; "
+                         "and the last is redirect-chassis.", op->nbrp->name);
             }
 
-            if (op->nbrp->n_gateway_chassis) {
-                if (sbpb_gw_chassis_needs_update(sbrec_chassis_by_name,
-                                                 op->sb, op->nbrp)) {
+            if (op->nbrp->ha_chassis_group) {
+                /* HA Chassis group is set. Ignore 'gateway_chassis'
+                 * column and redirect-chassis option. */
+                sync_ha_chassis_group_for_sbpb(ctx, op->nbrp->ha_chassis_group,
+                                               sbrec_chassis_by_name, op->sb);
+                sset_add(active_ha_chassis_grps,
+                         op->nbrp->ha_chassis_group->name);
+            } else if (op->nbrp->n_gateway_chassis) {
+                /* Legacy gateway_chassis support.
+                 * Create ha_chassis_group for the Northbound gateway_chassis
+                 * associated with the lrp. */
+                if (sbpb_gw_chassis_needs_update(op->sb, op->nbrp)) {
                     copy_gw_chassis_from_nbrp_to_sbpb(ctx,
                                                       sbrec_chassis_by_name,
                                                       op->nbrp, op->sb);
@@ -2216,7 +2231,7 @@ ovn_port_update_sbrec(struct northd_context *ctx,
             } else if (redirect_chassis) {
                 /* Handle ports that had redirect-chassis option attached
                  * to them, and for backwards compatibility convert them
-                 * to a single Gateway_Chassis entry */
+                 * to a single HA Chassis group entry */
                 const struct sbrec_chassis *chassis =
                     chassis_lookup_by_name(sbrec_chassis_by_name,
                                            redirect_chassis);
@@ -2224,36 +2239,7 @@ ovn_port_update_sbrec(struct northd_context *ctx,
                     /* If we found the chassis, and the gw chassis on record
                      * differs from what we expect go ahead and update */
                     char *gwc_name = xasprintf("%s_%s", op->nbrp->name,
-                                               chassis->name);
-                    if (op->sb->n_gateway_chassis != 1
-                        || !op->sb->gateway_chassis[0]->chassis
-                        || strcmp(op->sb->gateway_chassis[0]->chassis->name,
-                                  chassis->name)
-                        || op->sb->gateway_chassis[0]->priority != 0) {
-                        /* This code to create gateway_chassis in SB DB needs
-                         * to be deleted once ovn-controller supports making
-                         * use of HA chassis groups. */
-
-                        /* Construct a single Gateway_Chassis entry on the
-                         * Port_Binding attached to the redirect_chassis
-                         * name */
-                        struct sbrec_gateway_chassis *gw_chassis =
-                            sbrec_gateway_chassis_insert(ctx->ovnsb_txn);
-
-                        /* XXX: Again, here, we could just update an existing
-                         * Gateway_Chassis, instead of creating a new one
-                         * and replacing it */
-                        sbrec_gateway_chassis_set_name(gw_chassis, gwc_name);
-                        sbrec_gateway_chassis_set_priority(gw_chassis, 0);
-                        sbrec_gateway_chassis_set_chassis(gw_chassis, chassis);
-                        sbrec_gateway_chassis_set_external_ids(gw_chassis,
-                                &op->nbrp->external_ids);
-                        sbrec_port_binding_set_gateway_chassis(op->sb,
-                                                               &gw_chassis, 1);
-                    }
-
-                    /* Create HA chassis group in SB DB for the
-                     * redirect-chassis option. */
+                                chassis->name);
                     const struct sbrec_ha_chassis_group *sb_ha_ch_grp;
                     sb_ha_ch_grp = ha_chassis_group_lookup_by_name(
                         ctx->sbrec_ha_chassis_grp_by_name, gwc_name);
@@ -2279,11 +2265,20 @@ ovn_port_update_sbrec(struct northd_context *ctx,
                     VLOG_WARN("chassis name '%s' from redirect from logical "
                               " router port '%s' redirect-chassis not found",
                               redirect_chassis, op->nbrp->name);
-                    if (op->sb->n_gateway_chassis) {
-                        sbrec_port_binding_set_gateway_chassis(op->sb, NULL,
-                                                               0);
+                    if (op->sb->ha_chassis_group) {
+                        sbrec_port_binding_set_ha_chassis_group(op->sb, NULL);
                     }
                 }
+            } else {
+                /* Nothing is set. Clear ha_chassis_group  from pb. */
+                if (op->sb->ha_chassis_group) {
+                    sbrec_port_binding_set_ha_chassis_group(op->sb, NULL);
+                }
+            }
+
+            if (op->sb->n_gateway_chassis) {
+                /* Delete the legacy gateway_chassis from the pb. */
+                sbrec_port_binding_set_gateway_chassis(op->sb, NULL, 0);
             }
             smap_add(&new, "distributed-port", op->nbrp->name);
         } else {
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index b4e8995be..bff0ca48a 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -20,18 +20,22 @@ ovn-nbctl --wait=sb \
     set Logical_Router_Port alice 'gateway_chassis=[@gc0, at gc1]'
 
 nb_gwc1_uuid=`ovn-nbctl --bare --columns _uuid find Gateway_Chassis name="alice_gw1"`
-gwc1_uuid=`ovn-sbctl --bare --columns _uuid find Gateway_Chassis name="alice_gw1"`
-gwc2_uuid=`ovn-sbctl --bare --columns _uuid find Gateway_Chassis name="alice_gw2"`
 
+# With the new ha_chassis_group table added, there should be no rows in
+# gateway_chassis table in SB DB.
+AT_CHECK([ovn-sbctl list gateway_chassis | wc -l], [0], [0
+])
 
-echo "Port_Binding for cr-alice:"
-ovn-sbctl find port_binding logical_port="cr-alice"
-echo "Gateway_Chassis list:"
-ovn-sbctl list Gateway_Chassis
+# There should be one ha_chassis_group with the name "alice"
+ha_chassi_grp_name=`ovn-sbctl --bare --columns name find \
+ha_chassis_group name="alice"`
 
-AT_CHECK([ovn-sbctl --bare --columns gateway_chassis find port_binding logical_port="cr-alice" | grep $gwc1_uuid | wc -l], [0], [1
-])
-AT_CHECK([ovn-sbctl --bare --columns gateway_chassis find port_binding logical_port="cr-alice" | grep $gwc2_uuid | wc -l], [0], [1
+AT_CHECK([test $ha_chassi_grp_name = alice])
+
+ha_chgrp_uuid=`ovn-sbctl --bare --columns _uuid find ha_chassis_group name=alice`
+
+AT_CHECK([ovn-sbctl --bare --columns ha_chassis_group find port_binding \
+logical_port="cr-alice" | grep $ha_chgrp_uuid | wc -l], [0], [1
 ])
 
 # There should be one ha_chassis_group with the name "alice"
@@ -62,15 +66,35 @@ ha_ch_list=`echo $ha_ch_list | sed 's/ //g'`
 AT_CHECK([test "$ha_ch_list" = "$ha_ch"])
 
 # delete the 2nd Gateway_Chassis on NBDB for alice port
+gw_ch=`ovn-sbctl --bare --columns gateway_chassis find port_binding \
+logical_port="cr-alice"`
+AT_CHECK([test "$gw_ch" = ""])
 
-ovn-nbctl --wait=sb set Logical_Router_Port alice gateway_chassis=${nb_gwc1_uuid}
+ha_ch=`ovn-sbctl --bare --columns ha_chassis  find ha_chassis_group`
+ha_ch=`echo $ha_ch | sed 's/ //g'`
+# Trim the spaces.
+echo "ha ch in grp = $ha_ch"
+
+ha_ch_list=''
+for i in `ovn-sbctl --bare --columns _uuid list ha_chassis | sort`
+do
+    ha_ch_list="$ha_ch_list $i"
+done
+
+# Trim the spaces.
+ha_ch_list=`echo $ha_ch_list | sed 's/ //g'`
+
+AT_CHECK([test "$ha_ch_list" = "$ha_ch"])
 
-gwc1_uuid=`ovn-sbctl --bare --columns _uuid find Gateway_Chassis name="alice_gw1"`
+# delete the 2nd Gateway_Chassis on NBDB for alice port
+ovn-nbctl --wait=sb set Logical_Router_Port alice gateway_chassis=${nb_gwc1_uuid}
 
-AT_CHECK([ovn-sbctl --bare --columns gateway_chassis find port_binding logical_port="cr-alice" | grep $gwc1_uuid | wc -l], [0], [1
+# There should be only 1 row in ha_chassis SB DB table.
+AT_CHECK([ovn-sbctl --bare --columns _uuid list ha_chassis | wc -l], [0], [1
 ])
 
-AT_CHECK([ovn-sbctl find Gateway_Chassis name=alice_gw2], [0], [])
+AT_CHECK([ovn-sbctl list gateway_chassis | wc -l], [0], [0
+])
 
 # There should be only 1 row in ha_chassis SB DB table.
 AT_CHECK([ovn-sbctl --bare --columns _uuid list ha_chassis | wc -l], [0], [1
@@ -80,10 +104,15 @@ AT_CHECK([ovn-sbctl --bare --columns _uuid list ha_chassis | wc -l], [0], [1
 
 ovn-nbctl --wait=sb clear Logical_Router_Port alice gateway_chassis
 
-# expect that the Gateway_Chassis doesn't exist anymore
+# expect that the ha_chassis doesn't exist anymore
+AT_CHECK([ovn-sbctl list gateway_chassis | wc -l], [0], [0
+])
 
-AT_CHECK([ovn-sbctl find Gateway_Chassis name=alice_gw1], [0], [])
-AT_CHECK([ovn-sbctl find Gateway_Chassis name=alice_gw2], [0], [])
+AT_CHECK([ovn-sbctl list ha_chassis | wc -l], [0], [0
+])
+
+AT_CHECK([ovn-sbctl list ha_chassis_group | wc -l], [0], [0
+])
 
 # expect that the ha_chassis doesn't exist anymore
 AT_CHECK([ovn-sbctl list ha_chassis | wc -l], [0], [0
@@ -105,19 +134,40 @@ ovn-nbctl --wait=sb lrp-add R1 bob 00:00:02:01:02:03 172.16.1.1/24 \
     -- set Logical_Router_Port bob options:redirect-chassis="gw1"
 
 
-# It should be converted to Gateway_Chassis entries in SBDB, and
+# It should be converted to ha_chassis_group entries in SBDB, and
 # still redirect-chassis is kept for backwards compatibility
 
-gwc1_uuid=`ovn-sbctl --bare --columns _uuid find Gateway_Chassis name="bob_gw1"`
+AT_CHECK([ovn-sbctl list gateway_chassis | wc -l], [0], [0
+])
+
+AT_CHECK([ovn-sbctl --bare --columns _uuid list ha_chassis | wc -l], [0], [1
+])
 
-AT_CHECK([ovn-sbctl --bare --columns gateway_chassis find port_binding logical_port="cr-bob" | grep $gwc1_uuid | wc -l], [0], [1
+AT_CHECK([ovn-sbctl --bare --columns _uuid list ha_chassis_group | wc -l], [0], [1
+])
+
+# There should be one ha_chassis_group with the name "bob_gw1"
+ha_chassi_grp_name=`ovn-sbctl --bare --columns name find \
+ha_chassis_group name="bob_gw1"`
+
+AT_CHECK([test $ha_chassi_grp_name = bob_gw1])
+
+ha_chgrp_uuid=`ovn-sbctl --bare --columns _uuid find ha_chassis_group name=bob_gw1`
+
+AT_CHECK([ovn-sbctl --bare --columns ha_chassis_group find port_binding \
+logical_port="cr-bob" | grep $ha_chgrp_uuid | wc -l], [0], [1
 ])
 
 ovn-nbctl --wait=sb remove Logical_Router_Port bob options redirect-chassis
 
-# expect that the Gateway_Chassis doesn't exist anymore
+# expect that the ha_chassis/ha_chassis_group doesn't exist anymore
 
 AT_CHECK([ovn-sbctl find Gateway_Chassis name=bob_gw1], [0], [])
+AT_CHECK([ovn-sbctl list ha_chassis | wc -l], [0], [0
+])
+
+AT_CHECK([ovn-sbctl list ha_chassis_group | wc -l], [0], [0
+])
 
 AT_CLEANUP
 
@@ -347,8 +397,7 @@ ovn-nbctl --wait=sb ha-chassis-group-add hagrp1
 
 # ovn-northd should not create HA chassis group and HA chassis rows
 # unless the HA chassis group in OVN NB DB is associated to
-# a logical router port. ovn-northd still doesn't support
-# associating a HA chassis group to a logical router port.
+# a logical router port.
 AT_CHECK([ovn-sbctl --bare --columns name find ha_chassis_group name="hagrp1" \
 | wc -l], [0], [0
 ])
@@ -372,25 +421,113 @@ AT_CHECK([ovn-sbctl list ha_chassis | grep chassis | awk '{print $3}' \
 | grep -v '-' | wc -l ], [0], [0
 ])
 
-ovn-nbctl ha-chassis-group-del hagrp1
-OVS_WAIT_UNTIL([test 0 = `ovn-sbctl list ha_chassis_group | wc -l`])
-OVS_WAIT_UNTIL([test 0 = `ovn-sbctl list ha_chassis | wc -l`])
-
-# Create a logical router port and attach Gateway chassis.
-# ovn-northd should create HA chassis group rows in SB DB.
+# Create a logical router port and attach ha chassis group.
 ovn-nbctl lr-add lr0
+ovn-nbctl lrp-add lr0 lr0-public 00:00:20:20:12:13 172.168.0.100/24
+
+hagrp1_uuid=`ovn-nbctl --bare --columns _uuid find ha_chassis_group name=hagrp1`
+ovn-nbctl set logical_router_port lr0-public ha_chassis_group=$hagrp1_uuid
+
+OVS_WAIT_UNTIL([test 1 = `ovn-sbctl --bare --columns name find \
+ha_chassis_group name="hagrp1" | wc -l`])
+
+AT_CHECK([test 3 = `ovn-sbctl list ha_chassis | grep chassis | wc -l`])
+
+# 2 HA chassis should be created with 'chassis' column empty because
+# we have not added hv1 and hv2 chassis to the SB DB.
+AT_CHECK([test 2 = `ovn-sbctl list ha_chassis | grep chassis | awk '{print $3}' \
+| grep -v '-' | wc -l`])
+
+# We should have 1 ha chassis with 'chassis' column set for hv1
+AT_CHECK([test 1 = `ovn-sbctl list ha_chassis | grep chassis | awk '{print $3}' \
+| grep '-' | wc -l`])
+
+# Create another logical router port and associate to the same ha_chasis_group
+ovn-nbctl lr-add lr1
+ovn-nbctl lrp-add lr1 lr1-public 00:00:20:20:12:14 182.168.0.100/24
+
+ovn-nbctl set logical_router_port lr1-public ha_chassis_group=$hagrp1_uuid
+
+# We should still have 1 HA chassis group and 3 HA chassis in SB DB.
+OVS_WAIT_UNTIL([test 1 = `ovn-sbctl --bare --columns name find \
+ha_chassis_group name="hagrp1" | wc -l`])
+
+AT_CHECK([test 3 = `ovn-sbctl list ha_chassis | grep chassis | wc -l`])
+
+# Change the priority of ch1 - ha chassis in NB DB. It should get
+# reflected in SB DB.
+ovn-nbctl --wait=sb ha-chassis-group-add-chassis hagrp1 ch1 100
+
+OVS_WAIT_UNTIL([test 1 = `ovn-sbctl --bare --columns priority find \
+ha_chassis | grep 100 | wc -l`])
+
+# Delete ch1 HA chassis in NB DB.
+ovn-nbctl --wait=sb ha-chassis-group-remove-chassis hagrp1 ch1
+
+OVS_WAIT_UNTIL([test 2 = `ovn-sbctl list ha_chassis | grep chassis | wc -l`])
+
+# Add back the ha chassis
+ovn-nbctl --wait=sb ha-chassis-group-add-chassis hagrp1 ch1 40
+OVS_WAIT_UNTIL([test 3 = `ovn-sbctl list ha_chassis | grep chassis | wc -l`])
+
+# Delete lr0-public. We should still have 1 HA chassis group and
+# 3 HA chassis in SB DB.
+ovn-nbctl --wait=sb lrp-del lr0-public
+
+OVS_WAIT_UNTIL([test 1 = `ovn-sbctl --bare --columns name find \
+ha_chassis_group name="hagrp1" | wc -l`])
+
+AT_CHECK([test 3 = `ovn-sbctl list ha_chassis | grep chassis | wc -l`])
+
+# Delete lr1-public. There should be no HA chassis group in SB DB.
+ovn-nbctl --wait=sb lrp-del lr1-public
+
+OVS_WAIT_UNTIL([test 0 = `ovn-sbctl --bare --columns name find \
+ha_chassis_group name="hagrp1" | wc -l`])
 
+AT_CHECK([test 0 = `ovn-sbctl list ha_chassis | grep chassis | wc -l`])
+
+# Add lr0-public again
 ovn-nbctl lrp-add lr0 lr0-public 00:00:20:20:12:13 172.168.0.100/24
-ovn-nbctl lrp-set-gateway-chassis lr0-public ch1 20
+ovn-nbctl set logical_router_port lr0-public ha_chassis_group=$hagrp1_uuid
+
+OVS_WAIT_UNTIL([test 1 = `ovn-sbctl --bare --columns name find \
+ha_chassis_group name="hagrp1" | wc -l`])
+
+AT_CHECK([test 3 = `ovn-sbctl list ha_chassis | grep chassis | wc -l`])
+
+# Create a Gateway chassis. ovn-northd should ignore this.
+ovn-nbctl lrp-set-gateway-chassis lr0-public ch-1 20
 
-OVS_WAIT_UNTIL([test 1 = `ovn-sbctl --bare --columns name find ha_chassis_group \
-name="lr0-public" | wc -l`])
+# There should be only 1 HA chassis group in SB DB with the
+# name hagrp1.
+OVS_WAIT_UNTIL([test 1 = `ovn-sbctl --bare --columns name find \
+ha_chassis_group | wc -l`])
+
+OVS_WAIT_UNTIL([test 1 = `ovn-sbctl --bare --columns name find \
+ha_chassis_group name="hagrp1" | wc -l`])
+
+AT_CHECK([test 3 = `ovn-sbctl list ha_chassis | grep chassis | wc -l`])
+
+# Now delete HA chassis group. ovn-northd should create HA chassis group
+# with the Gateway chassis name
+ovn-nbctl clear logical_router_port lr0-public ha_chassis_group
+ovn-nbctl ha-chassis-group-del hagrp1
+
+OVS_WAIT_UNTIL([test 0 = `ovn-sbctl --bare --columns name find \
+ha_chassis_group name="hagrp1" | wc -l`])
+
+OVS_WAIT_UNTIL([test 1 = `ovn-sbctl --bare --columns name find \
+ha_chassis_group name="lr0-public" | wc -l`])
 
 OVS_WAIT_UNTIL([test 1 = `ovn-sbctl --bare --columns _uuid \
 find ha_chassis | wc -l`])
 
 ovn-nbctl lrp-set-gateway-chassis lr0-public ch2 10
 
+OVS_WAIT_UNTIL([test 1 = `ovn-sbctl --bare --columns name find \
+ha_chassis_group name="lr0-public" | wc -l`])
+
 ovn-sbctl --bare --columns _uuid find ha_chassis
 OVS_WAIT_UNTIL([test 2 = `ovn-sbctl list ha_chassis | grep chassis | wc -l`])
 
@@ -562,4 +699,37 @@ OVS_WAIT_UNTIL(
      ref_ch_list=`echo $ref_ch_list | sed 's/ //g'`
      test "$comp2_ch_uuid" = "$ref_ch_list"])
 
+# Set redirect-chassis option to lr0-public. It should be ignored.
+ovn-nbctl set logical_router_port lr0-public options:redirect-chassis=ch1
+
+OVS_WAIT_UNTIL([test 1 = `ovn-sbctl --bare --columns name find \
+ha_chassis_group | wc -l`])
+
+OVS_WAIT_UNTIL([test 1 = `ovn-sbctl --bare --columns name find \
+ha_chassis_group name="lr0-public" | wc -l`])
+
+ovn-sbctl --bare --columns _uuid find ha_chassis
+OVS_WAIT_UNTIL([test 2 = `ovn-sbctl list ha_chassis | grep chassis | wc -l`])
+
+# Delete the gateway chassis. HA chassis group should be created in SB DB
+# for the redirect-chassis option.
+ovn-nbctl clear logical_router_port lr0-public gateway_chassis
+
+OVS_WAIT_UNTIL([test 1 = `ovn-sbctl --bare --columns name find \
+ha_chassis_group | wc -l`])
+
+ovn-sbctl list ha_chassis_group
+
+OVS_WAIT_UNTIL([test 1 = `ovn-sbctl --bare --columns name find \
+ha_chassis_group name="lr0-public_ch1" | wc -l`])
+
+ovn-sbctl --bare --columns _uuid find ha_chassis
+OVS_WAIT_UNTIL([test 1 = `ovn-sbctl list ha_chassis | grep chassis | wc -l`])
+
+# Clear the redirect-chassis option.
+ovn-nbctl clear logical_router_port lr0-public options
+
+OVS_WAIT_UNTIL([test 0 = `ovn-sbctl list ha_chassis_group |  wc -l`])
+AT_CHECK([test 0 = `ovn-sbctl list ha_chassis | wc -l`])
+
 AT_CLEANUP
diff --git a/tests/ovn.at b/tests/ovn.at
index 6d3e6002a..17fd5f990 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -10059,6 +10059,181 @@ ovn-nbctl clear Logical_Router_Port outside gateway_chassis
 OVS_WAIT_UNTIL([test 0 = `ovn-sbctl list ha_chassis_group | wc -l`])
 OVS_WAIT_UNTIL([test 0 = `ovn-sbctl list ha_chassis | wc -l`])
 
+ovn-nbctl remove NB_Global . options "bfd-min-rx"
+ovn-nbctl remove NB_Global . options "bfd-min-tx"
+ovn-nbctl remove NB_Global . options "bfd-mult"
+
+# Now test with HA chassis group instead of Gateway chassis in NB DB
+ovn-nbctl --wait=sb ha-chassis-group-add hagrp1
+
+ovn-nbctl list ha_chassis_group
+ovn-nbctl --bare --columns _uuid find ha_chassis_group name=hagrp1
+hagrp1_uuid=`ovn-nbctl --bare --columns _uuid find ha_chassis_group name=hagrp1`
+ovn-nbctl --wait=sb ha-chassis-group-add-chassis hagrp1 gw1 30
+ovn-nbctl --wait=sb ha-chassis-group-add-chassis hagrp1 gw2 20
+
+# ovn-northd should not create HA chassis group and HA chassis rows
+# unless the HA chassis group in OVN NB DB is associated to
+# a logical router port.
+OVS_WAIT_UNTIL([test 0 = `ovn-sbctl list ha_chassis | wc -l`])
+
+# Associate hagrp1 to outside logical router port
+ovn-nbctl set Logical_Router_Port outside ha_chassis_group=$hagrp1_uuid
+
+OVS_WAIT_UNTIL([test 1 = `ovn-sbctl --bare --columns _uuid \
+find ha_chassis_group | wc -l`])
+
+OVS_WAIT_UNTIL([test 2 = `ovn-sbctl list ha_chassis | grep chassis | wc -l`])
+
+OVS_WAIT_UNTIL([as hv1 ovs-ofctl dump-flows br-int table=32 | \
+grep active_backup | grep slaves:$hv1_gw1_ofport,$hv1_gw2_ofport \
+| wc -l], [0], [1
+])
+
+OVS_WAIT_UNTIL([as hv2 ovs-ofctl dump-flows br-int table=32 | \
+grep active_backup | grep slaves:$hv2_gw1_ofport,$hv2_gw2_ofport \
+| wc -l], [0], [1
+])
+
+# make sure that flows for handling the outside router port reside on gw1
+OVS_WAIT_UNTIL([as gw1 ovs-ofctl dump-flows br-int table=24 | \
+grep 00:00:02:01:02:04 | wc -l], [0], [[1
+]])
+OVS_WAIT_UNTIL([as gw2 ovs-ofctl dump-flows br-int table=24 | \
+grep 00:00:02:01:02:04 | wc -l], [0], [[0
+]])
+
+# make sure ARP responder flows for outside router port reside on gw1 too
+OVS_WAIT_UNTIL([as gw1 ovs-ofctl dump-flows br-int table=9 | \
+grep arp_tpa=192.168.0.101 | wc -l], [0], [[1
+]])
+OVS_WAIT_UNTIL([as gw2 ovs-ofctl dump-flows br-int table=9 | grep arp_tpa=192.168.0.101 | wc -l], [0], [[0
+]])
+
+# check that the chassis redirect port has been claimed by the gw1 chassis
+OVS_WAIT_UNTIL([ovn-sbctl --columns chassis --bare find Port_Binding \
+logical_port=cr-outside | grep $gw1_chassis | wc -l], [0],[[1
+]])
+
+# Re add the ovs ports.
+for i in 1 2; do
+    as hv$i
+    ovs-vsctl -- add-port br-int hv$i-vif1 -- \
+        set interface hv$i-vif1 external-ids:iface-id=inside$i \
+        options:tx_pcap=hv$i/vif1-tx.pcap \
+        options:rxq_pcap=hv$i/vif1-rx.pcap \
+        ofport-request=1
+done
+
+hv1_ch_uuid=`ovn-sbctl --bare --columns _uuid find chassis name="hv1"`
+hv2_ch_uuid=`ovn-sbctl --bare --columns _uuid find chassis name="hv2"`
+
+exp_ref_ch_list=''
+for i in `ovn-sbctl --bare --columns _uuid list chassis | sort`
+do
+    if test $i = $hv1_ch_uuid; then
+        exp_ref_ch_list="${exp_ref_ch_list}$i"
+    elif test $i = $hv2_ch_uuid; then
+        exp_ref_ch_list="${exp_ref_ch_list}$i"
+    fi
+done
+
+OVS_WAIT_UNTIL(
+    [ref_ch_list=`ovn-sbctl --bare --columns ref_chassis find ha_chassis_group | sort`
+     # Trim the spaces.
+     ref_ch_list=`echo $ref_ch_list | sed 's/ //g'`
+     test "$exp_ref_ch_list" = "$ref_ch_list"])
+
+# Increase the priority of gw2
+ovn-nbctl --wait=sb ha-chassis-group-add-chassis hagrp1 gw2 40
+
+OVS_WAIT_UNTIL([as hv1 ovs-ofctl dump-flows br-int table=32 | \
+grep active_backup | grep slaves:$hv1_gw2_ofport,$hv1_gw1_ofport \
+| wc -l], [0], [1
+])
+
+OVS_WAIT_UNTIL([as hv2 ovs-ofctl dump-flows br-int table=32 | \
+grep active_backup | grep slaves:$hv2_gw2_ofport,$hv2_gw1_ofport \
+| wc -l], [0], [1
+])
+
+# check that the chassis redirect port has been reclaimed by the gw2 chassis
+OVS_WAIT_UNTIL([ovn-sbctl --columns chassis --bare find Port_Binding \
+logical_port=cr-outside | grep $gw2_chassis | wc -l], [0],[[1
+]])
+
+# check BFD enablement on tunnel ports from gw1 #########
+as gw1
+for chassis in gw2 hv1 hv2; do
+    echo "checking gw1 -> $chassis"
+    AT_CHECK([ovs-vsctl --bare --columns bfd find Interface name=ovn-$chassis-0],[0],
+             [[enable=true
+]])
+done
+
+# check BFD enablement on tunnel ports from gw2 ##########
+as gw2
+for chassis in gw1 hv1 hv2; do
+    echo "checking gw2 -> $chassis"
+    AT_CHECK([ovs-vsctl --bare --columns bfd find Interface name=ovn-$chassis-0],[0],
+             [[enable=true
+]])
+done
+
+# check BFD enablement on tunnel ports from hv1 ###########
+as hv1
+for chassis in gw1 gw2; do
+    echo "checking hv1 -> $chassis"
+    AT_CHECK([ovs-vsctl --bare --columns bfd find Interface name=ovn-$chassis-0],[0],
+             [[enable=true
+]])
+done
+# make sure BFD is not enabled to hv2, we don't need it
+AT_CHECK([ovs-vsctl --bare --columns bfd find Interface name=ovn-hv2-0],[0],
+         [[
+]])
+
+# check BFD enablement on tunnel ports from hv2 ##########
+as hv2
+for chassis in gw1 gw2; do
+    echo "checking hv2 -> $chassis"
+    AT_CHECK([ovs-vsctl --bare --columns bfd find Interface name=ovn-$chassis-0],[0],
+             [[enable=true
+]])
+done
+# make sure BFD is not enabled to hv1, we don't need it
+AT_CHECK([ovs-vsctl --bare --columns bfd find Interface name=ovn-hv1-0],[0],
+         [[
+]])
+
+# make sure that flows for handling the outside router port reside on gw2 now
+OVS_WAIT_UNTIL([as gw2 ovs-ofctl dump-flows br-int table=24 | \
+grep 00:00:02:01:02:04 | wc -l], [0], [[1
+]])
+OVS_WAIT_UNTIL([as gw1 ovs-ofctl dump-flows br-int table=24 | \
+grep 00:00:02:01:02:04 | wc -l], [0], [[0
+]])
+
+# disconnect GW2 from the network, GW1 should take over
+as gw2
+port=${sandbox}_br-phys
+as main ovs-vsctl del-port n1 $port
+
+bfd_dump
+
+# make sure that flows for handling the outside router port reside on gw2 now
+OVS_WAIT_UNTIL([as gw1 ovs-ofctl dump-flows br-int table=24 | \
+grep 00:00:02:01:02:04 | wc -l], [0], [[1
+]])
+OVS_WAIT_UNTIL([as gw2 ovs-ofctl dump-flows br-int table=24 | \
+grep 00:00:02:01:02:04 | wc -l], [0], [[0
+]])
+
+# check that the chassis redirect port has been reclaimed by the gw1 chassis
+OVS_WAIT_UNTIL([ovn-sbctl --columns chassis --bare find Port_Binding \
+logical_port=cr-outside | grep $gw1_chassis | wc -l], [0],[[1
+]])
+
 OVN_CLEANUP([gw1],[gw2],[hv1],[hv2])
 
 AT_CLEANUP
-- 
2.20.1



More information about the dev mailing list