diff mbox series

[ovs-dev] ovn-sbctl: Fix removal of Chassis_Private record on chassis-del.

Message ID 20221214190205.126684-1-frode.nordahl@canonical.com
State Superseded
Headers show
Series [ovs-dev] ovn-sbctl: Fix removal of Chassis_Private record on chassis-del. | expand

Checks

Context Check Description
ovsrobot/apply-robot warning apply and check: warning
ovsrobot/github-robot-_Build_and_Test fail github build: failed
ovsrobot/github-robot-_ovn-kubernetes success github build: passed

Commit Message

Frode Nordahl Dec. 14, 2022, 7:02 p.m. UTC
After commit 4adc10f58127e, the ovn-controller will populate both
Chassis and Chassis_Private tables on registration.

The `ovn-sbctl chassis-del` command exists so that an administrator
can manually remove a chassis record when needed.

Before this patch any existing Chassis_Private record would be left
behind, and this may cause problems for other automation or CMS
software.

Fixes: 4adc10f58127e ("Avoid nb_cfg update notification flooding")
Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com>
---
 tests/ovn-sbctl.at    | 22 +++++++++++++++++
 utilities/ovn-sbctl.c | 57 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+)

Comments

0-day Robot Dec. 14, 2022, 7:19 p.m. UTC | #1
Bleep bloop.  Greetings Frode Nordahl, 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.


checkpatch:
ERROR: "Fixes" tag is malformed.
Use the following format:
  git log -1 --pretty=format:"Fixes: %h (\"%s\")" --abbrev=12 COMMIT_REF

15: Fixes: 4adc10f58127e ("Avoid nb_cfg update notification flooding")

Lines checked: 174, Warnings: 0, Errors: 1


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

Thanks,
0-day Robot
diff mbox series

Patch

diff --git a/tests/ovn-sbctl.at b/tests/ovn-sbctl.at
index 1d1eee23e..19ac55c80 100644
--- a/tests/ovn-sbctl.at
+++ b/tests/ovn-sbctl.at
@@ -93,6 +93,28 @@  AT_CHECK([ovn-sbctl -f csv -d bare --no-headings --columns ip,type list encap |
 1.2.3.5,vxlan
 ])
 
+AT_CHECK([ovn-sbctl chassis-add ch2 geneve 2.3.4.5])
+ch2_uuid=$(ovn-sbctl -d bare --no-headings --columns _uuid find chassis name=ch2)
+ovn-sbctl create Chassis_Private name=ch2 chassis=$ch2_uuid
+check_row_count Chassis_Private 1
+
+AT_CHECK([ovn-sbctl -f csv -d bare --no-headings --columns ip,type list encap | sort],
+         [0], [dnl
+1.2.3.5,geneve
+1.2.3.5,stt
+1.2.3.5,vxlan
+2.3.4.5,geneve
+])
+
+AT_CHECK([ovn-sbctl chassis-del ch2])
+AT_CHECK([ovn-sbctl -f csv -d bare --no-headings --columns ip,type list encap | sort],
+         [0], [dnl
+1.2.3.5,geneve
+1.2.3.5,stt
+1.2.3.5,vxlan
+])
+check_row_count Chassis_Private 0
+
 as ovn-sb
 OVS_APP_EXIT_AND_WAIT([ovsdb-server])
 as
diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c
index 4d7e7d670..761377cdf 100644
--- a/utilities/ovn-sbctl.c
+++ b/utilities/ovn-sbctl.c
@@ -164,6 +164,8 @@  struct sbctl_context {
     bool cache_valid;
     /* Maps from chassis name to struct sbctl_chassis. */
     struct shash chassis;
+    /* Maps from chassis name to struct sbctl_chassis_private. */
+    struct shash chassis_private;
     /* Maps from lport name to struct sbctl_port_binding. */
     struct shash port_bindings;
 };
@@ -179,6 +181,10 @@  struct sbctl_chassis {
     const struct sbrec_chassis *ch_cfg;
 };
 
+struct sbctl_chassis_private {
+    const struct sbrec_chassis_private *ch_priv;
+};
+
 struct sbctl_port_binding {
     const struct sbrec_port_binding *bd_cfg;
 };
@@ -207,11 +213,13 @@  sbctl_context_get(struct ctl_context *ctx)
     }
 
     const struct sbrec_chassis *chassis_rec;
+    const struct sbrec_chassis_private *chassis_private_rec;
     const struct sbrec_port_binding *port_binding_rec;
     struct sset chassis, port_bindings;
 
     sbctl_ctx->cache_valid = true;
     shash_init(&sbctl_ctx->chassis);
+    shash_init(&sbctl_ctx->chassis_private);
     shash_init(&sbctl_ctx->port_bindings);
     sset_init(&chassis);
     SBREC_CHASSIS_FOR_EACH(chassis_rec, ctx->idl) {
@@ -229,6 +237,25 @@  sbctl_context_get(struct ctl_context *ctx)
     }
     sset_destroy(&chassis);
 
+    sset_init(&chassis);
+    SBREC_CHASSIS_PRIVATE_FOR_EACH (chassis_private_rec, ctx->idl) {
+        struct sbctl_chassis_private *ch_priv;
+
+        if (!sset_add(&chassis, chassis_private_rec->name)) {
+            VLOG_WARN("database contains duplicate private record for "
+                      "chassis named (%s)",
+                      chassis_rec->name);
+            continue;
+        }
+
+        ch_priv = xmalloc(sizeof *ch_priv);
+        ch_priv->ch_priv = chassis_private_rec;
+        shash_add(&sbctl_ctx->chassis_private,
+                  chassis_private_rec->name,
+                  ch_priv);
+    }
+    sset_destroy(&chassis);
+
     sset_init(&port_bindings);
     SBREC_PORT_BINDING_FOR_EACH(port_binding_rec, ctx->idl) {
         struct sbctl_port_binding *bd;
@@ -280,6 +307,22 @@  find_chassis(struct ctl_context *ctx, const char *name, bool must_exist)
     return sbctl_ch;
 }
 
+static struct sbctl_chassis_private *
+find_chassis_private(struct ctl_context *ctx,
+                     const char *name,
+                     bool must_exist)
+{
+    struct sbctl_context *sbctl_ctx = sbctl_context_get(ctx);
+    struct sbctl_chassis_private *sbctl_ch_priv = shash_find_data(
+        &sbctl_ctx->chassis_private, name);
+
+    if (must_exist && !sbctl_ch_priv) {
+        ctl_error(ctx, "no private record for chassis named %s", name);
+    }
+
+    return sbctl_ch_priv;
+}
+
 static struct sbctl_port_binding *
 find_port_binding(struct ctl_context *ctx, const char *name, bool must_exist)
 {
@@ -299,6 +342,8 @@  pre_get_info(struct ctl_context *ctx)
     ovsdb_idl_add_column(ctx->idl, &sbrec_chassis_col_name);
     ovsdb_idl_add_column(ctx->idl, &sbrec_chassis_col_encaps);
 
+    ovsdb_idl_add_column(ctx->idl, &sbrec_chassis_private_col_name);
+
     ovsdb_idl_add_column(ctx->idl, &sbrec_encap_col_type);
     ovsdb_idl_add_column(ctx->idl, &sbrec_encap_col_ip);
 
@@ -432,6 +477,18 @@  cmd_chassis_del(struct ctl_context *ctx)
             for (i = 0; i < sbctl_ch->ch_cfg->n_encaps; i++) {
                 sbrec_encap_delete(sbctl_ch->ch_cfg->encaps[i]);
             }
+
+            struct sbctl_chassis_private *sbctl_ch_priv;
+            sbctl_ch_priv = find_chassis_private(ctx, ctx->argv[1], false);
+            if (sbctl_ch_priv) {
+                if (sbctl_ch_priv->ch_priv) {
+                    sbrec_chassis_private_delete(sbctl_ch_priv->ch_priv);
+                }
+                shash_find_and_delete(&sbctl_ctx->chassis_private,
+                                      ctx->argv[1]);
+                free(sbctl_ch_priv);
+            }
+
             sbrec_chassis_delete(sbctl_ch->ch_cfg);
         }
         shash_find_and_delete(&sbctl_ctx->chassis, ctx->argv[1]);