ovn-nbctl: Fix lrp-set-gateway-chassis.

Currently this command assumes that if the gateway_chassis record with expected name exists it is set to the logical port, so once the record is found it not set to the lrp again. However, this assumption is not always true. An example is that when combined with a lrp-del and then lrp-add commands before lrp-set-gateway-chassis in the same transaction, the gateway_chassis record will be found but it is not set to the lrp. This is causing the gateway-chassis setting flapping in ovn-kubernetes' cluster router logical ports. This patch makes sure the gateway_chassis record (existed or newly created) is set to the lrp's gateway-chassis column. Signed-off-by: Han Zhou <[email protected]> Acked-by: Numan Siddique <[email protected]> Acked-by: Mark Michelson <[email protected]>
zhaiyj · Nov 11, 2021 · e6bcb88 · e6bcb88
1 parent d659b65
commit e6bcb88
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 11 deletions.
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
@@ -1389,6 +1389,12 @@ AT_CHECK([ovn-nbctl lrp-set-gateway-chassis lrp0 chassis1 10])
 AT_CHECK([ovn-nbctl lrp-get-gateway-chassis lrp0], [0], [dnl
 lrp0-chassis1    10
 ])
+
+AT_CHECK([ovn-nbctl lrp-del lrp0 -- lrp-add lr0 lrp0 00:00:00:01:02:03 192.168.1.1/24 -- lrp-set-gateway-chassis lrp0 chassis1 10])
+AT_CHECK([ovn-nbctl lrp-get-gateway-chassis lrp0], [0], [dnl
+lrp0-chassis1    10
+])
+
 AT_CHECK([ovn-nbctl lrp-set-gateway-chassis lrp0 chassis1 20])
 
 AT_CHECK([ovn-nbctl lrp-get-gateway-chassis lrp0], [0], [dnl

diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
@@ -5100,24 +5100,21 @@ nbctl_lrp_set_gateway_chassis(struct ctl_context *ctx)
     }
 
     gc_name = xasprintf("%s-%s", lrp_name, chassis_name);
-    const struct nbrec_gateway_chassis *existing_gc;
-    error = gc_by_name_or_uuid(ctx, gc_name, false, &existing_gc);
+    const struct nbrec_gateway_chassis *gc;
+    error = gc_by_name_or_uuid(ctx, gc_name, false, &gc);
     if (error) {
         ctx->error = error;
         free(gc_name);
         return;
     }
-    if (existing_gc) {
-        nbrec_gateway_chassis_set_priority(existing_gc, priority);
-        free(gc_name);
-        return;
+
+    if (!gc) {
+        /* Create the logical gateway chassis. */
+        gc = nbrec_gateway_chassis_insert(ctx->txn);
+        nbrec_gateway_chassis_set_name(gc, gc_name);
+        nbrec_gateway_chassis_set_chassis_name(gc, chassis_name);
     }
 
-    /* Create the logical gateway chassis. */
-    struct nbrec_gateway_chassis *gc
-        = nbrec_gateway_chassis_insert(ctx->txn);
-    nbrec_gateway_chassis_set_name(gc, gc_name);
-    nbrec_gateway_chassis_set_chassis_name(gc, chassis_name);
     nbrec_gateway_chassis_set_priority(gc, priority);
 
     /* Insert the logical gateway chassis into the logical router port. */