There are two ways to delete logical router policies with lr-policy-del.
One way is to provide a UUID of a policy to delete. The second is to
provide a priority, match, and/or chain in order to delete any policies
that match.

With the first way, if the UUID is not found, then ovn-nbctl emits an
error message and returns non-zero. This can be mitigated by using the
"--if-exists" flag.

With the second way, ovn-nbctl always returns 0 and gives no indication
about whether it was able to successfully delete policies.

With this change, the second way is now aligned with the first. That is,
if no matching policies are found, then an error message is emitted and
ovn-nbctl returns non-zero. If the "--if-exists" flag is supplied, then
even if no matching policies are found, ovn-nbctl emits no error and
returns 0.

Signed-off-by: Mark Michelson <[email protected]>
---
 NEWS                  |  4 ++++
 tests/ovn-nbctl.at    | 24 ++++++++++++++++++++++++
 utilities/ovn-nbctl.c | 11 ++++++++++-
 3 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index 4176f8d7f..f48fdd68d 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,9 @@
 Post v26.09.0
 --------------
+   - ovn-nbctl lr-policy-del will now return non-zero and emit an error
+     message if a priority, match, or chain is provided as the input, but
+     no router policies are actually deleted. This message is suppressed
+     and ovn-nbctl will return 0 if "--if-exists" is supplied.
    - Learn ARP sender hardware address (arp.sha) and IPv6 Neighbor
      Discovery link-layer addresses (nd.tll, nd.sll) into the FDB
      table in addition to eth.src.  This allows VRRP virtual MACs to
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 51c576f6e..3741d2126 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -3777,3 +3777,27 @@ AT_CHECK([ovn-nbctl lsp-hc-del lport0])
 AT_CHECK([ovn-nbctl lsp-hc-del lport3])
 check_row_count nb:Logical_Switch_Port_Health_Check 0
 ])
+
+OVN_NBCTL_TEST([router_policy_nonexistent_deletion], [Router policy 
nonexistent deletion], [
+check ovn-nbctl lr-add lr0
+
+# Baseline, no policies exist. Try to delete a policy.
+AT_CHECK([ovn-nbctl lr-policy-del lr0 10], [1], [],
+[ovn-nbctl: No matching logical router policies found (match: None, priority 
10, chain None)
+])
+check ovn-nbctl --if-exists lr-policy-del lr0 10
+
+# Now add some policies.
+check ovn-nbctl lr-policy-add lr0 100 "1" allow
+check ovn-nbctl lr-policy-add lr0 200 "ip4" drop
+
+# Try again now that there are actually policies.
+AT_CHECK([ovn-nbctl lr-policy-del lr0 10], [1], [],
+[ovn-nbctl: No matching logical router policies found (match: None, priority 
10, chain None)
+])
+check ovn-nbctl --if-exists lr-policy-del lr0 10
+
+# And let's ensure legitimate deletions work.
+check ovn-nbctl lr-policy-del lr0 100
+check ovn-nbctl lr-policy-del lr0 200
+])
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index f33da5be3..2f78b47ca 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -4922,6 +4922,7 @@ nbctl_lr_policy_del(struct ctl_context *ctx)
 
     int64_t priority = 0;
     struct uuid uuid_from_cmd;
+    bool if_exists = shash_find(&ctx->options, "--if-exists");
 
     /* An uuid is specified, delete the policy with uuid only */
     if (ctx->argc == 3 && uuid_from_string(&uuid_from_cmd, ctx->argv[2])) {
@@ -4934,7 +4935,7 @@ nbctl_lr_policy_del(struct ctl_context *ctx)
             }
         }
 
-        if (!shash_find(&ctx->options, "--if-exists")) {
+        if (!if_exists) {
             ctl_error(ctx, "Logical router policy uuid is not found.");
         }
 
@@ -4951,6 +4952,7 @@ nbctl_lr_policy_del(struct ctl_context *ctx)
 
     char *match = (ctx->argc == 4) ? ctx->argv[3] : NULL;
 
+    bool removed = false;
     for (int i = 0; i < lr->n_policies; i++) {
         struct nbrec_logical_router_policy *policy = lr->policies[i];
 
@@ -4960,8 +4962,15 @@ nbctl_lr_policy_del(struct ctl_context *ctx)
             && (!strcmp(chain, string_ptr(policy->chain)))
             && (!match || !strcmp(match, policy->match))) {
             nbrec_logical_router_update_policies_delvalue(lr, policy);
+            removed = true;
         }
     }
+
+    if (!removed && !if_exists) {
+        ctl_error(ctx, "No matching logical router policies found "
+                  "(match: %s, priority %"PRId64", chain %s)",
+                  match ? match : "None", priority, *chain ? chain : "None");
+    }
 }
 
 struct routing_policy {
-- 
2.55.0

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to