ospfd: rename the "graceful-restart helper-only" command

Considering that both the GR helper mode and restarting mode can be
enabled at the same time, the "graceful-restart helper-only" command
can be a bit misleading since it implies that only the helper mode
is enabled. Rename the command to "graceful-restart helper enable"
to clarify what the command does.

Start a deprecation cycle of one year before removing the original
command

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>

diff --git a/doc/user/ospfd.rst b/doc/user/ospfd.rst
index e8ca3947276cc9c0460e1246cb4ef14169e4e5e7..0122e2ac75948c189598fde2487f2065c0dbb205 100644 (file)
--- a/doc/user/ospfd.rst
+++ b/doc/user/ospfd.rst
@@ -722,7 +722,7 @@ Graceful Restart
    To perform a graceful shutdown, the "graceful-restart prepare ip ospf"
    EXEC-level command needs to be issued before restarting the ospfd daemon.
 
-.. clicmd:: graceful-restart helper-only [A.B.C.D]
+.. clicmd:: graceful-restart helper enable [A.B.C.D]
 
 
    Configure Graceful Restart (RFC 3623) helper support.

diff --git a/ospfd/ospf_vty.c b/ospfd/ospf_vty.c
index 3ae9707f5f95c46e31d455ec2007cfea0ca1c12b..4109ada64a198b1a28f81e1ed8613a8a30ce6ffe 100644 (file)
--- a/ospfd/ospf_vty.c
+++ b/ospfd/ospf_vty.c
@@ -9894,24 +9894,17 @@ DEFUN (no_ospf_proactive_arp,
 
 /* Graceful Restart HELPER Commands */
 DEFPY(ospf_gr_helper_enable, ospf_gr_helper_enable_cmd,
-      "graceful-restart helper-only [A.B.C.D]",
+      "graceful-restart helper enable [A.B.C.D$address]",
       "OSPF Graceful Restart\n"
+      "OSPF GR Helper\n"
       "Enable Helper support\n"
-      "Advertising router id\n")
+      "Advertising Router-ID\n")
 {
        VTY_DECLVAR_INSTANCE_CONTEXT(ospf, ospf);
-       struct in_addr addr;
-       int ret;
-
-       if (argc == 3) {
-               ret = inet_aton(argv[2]->arg, &addr);
-               if (!ret) {
-                       vty_out(vty,
-                               "Please specify the valid routerid address.\n");
-                       return CMD_WARNING_CONFIG_FAILED;
-               }
 
-               ospf_gr_helper_support_set_per_routerid(ospf, &addr, OSPF_GR_TRUE);
+       if (address_str) {
+               ospf_gr_helper_support_set_per_routerid(ospf, &address,
+                                                       OSPF_GR_TRUE);
                return CMD_SUCCESS;
        }
 
@@ -9922,33 +9915,68 @@ DEFPY(ospf_gr_helper_enable, ospf_gr_helper_enable_cmd,
 
 DEFPY(no_ospf_gr_helper_enable,
       no_ospf_gr_helper_enable_cmd,
-      "no graceful-restart helper-only [A.B.C.D]",
+      "no graceful-restart helper enable [A.B.C.D$address]",
       NO_STR
       "OSPF Graceful Restart\n"
-      "Disable Helper support\n"
+      "OSPF GR Helper\n"
+      "Enable Helper support\n"
+      "Advertising Router-ID\n")
+{
+       VTY_DECLVAR_INSTANCE_CONTEXT(ospf, ospf);
+
+       if (address_str) {
+               ospf_gr_helper_support_set_per_routerid(ospf, &address,
+                                                       OSPF_GR_FALSE);
+               return CMD_SUCCESS;
+       }
+
+       ospf_gr_helper_support_set(ospf, OSPF_GR_FALSE);
+       return CMD_SUCCESS;
+}
+
+#if CONFDATE > 20220921
+CPP_NOTICE(
+       "Time to remove the deprecated \"[no] graceful-restart helper-only\" commands")
+#endif
+
+DEFPY_HIDDEN(ospf_gr_helper_only, ospf_gr_helper_only_cmd,
+      "graceful-restart helper-only [A.B.C.D]",
+      "OSPF Graceful Restart\n"
+      "Enable Helper support\n"
       "Advertising router id\n")
 {
        VTY_DECLVAR_INSTANCE_CONTEXT(ospf, ospf);
        struct in_addr addr;
        int ret;
 
-       if (argc == 4) {
-               ret = inet_aton(argv[3]->arg, &addr);
+       vty_out(vty,
+               "%% This command is deprecated. Please, use `graceful-restart helper enable` instead.\n");
+
+       if (argc == 3) {
+               ret = inet_aton(argv[2]->arg, &addr);
                if (!ret) {
                        vty_out(vty,
                                "Please specify the valid routerid address.\n");
                        return CMD_WARNING_CONFIG_FAILED;
                }
 
-               ospf_gr_helper_support_set_per_routerid(ospf, &addr,
-                                                       OSPF_GR_FALSE);
+               ospf_gr_helper_support_set_per_routerid(ospf, &addr, OSPF_GR_TRUE);
                return CMD_SUCCESS;
        }
 
-       ospf_gr_helper_support_set(ospf, OSPF_GR_FALSE);
+       ospf_gr_helper_support_set(ospf, OSPF_GR_TRUE);
+
        return CMD_SUCCESS;
 }
 
+ALIAS_HIDDEN(no_ospf_gr_helper_enable,
+      no_ospf_gr_helper_only_cmd,
+      "no graceful-restart helper-only [A.B.C.D]",
+      NO_STR
+      "OSPF Graceful Restart\n"
+      "Disable Helper support\n"
+      "Advertising router id\n")
+
 DEFPY(ospf_gr_helper_enable_lsacheck,
       ospf_gr_helper_enable_lsacheck_cmd,
       "graceful-restart helper strict-lsa-checking",
@@ -12259,7 +12287,7 @@ static int ospf_cfg_write_helper_dis_rtr_walkcb(struct hash_bucket *bucket,
        struct advRtr *rtr = bucket->data;
        struct vty *vty = (struct vty *)arg;
 
-       vty_out(vty, " graceful-restart helper-only %pI4\n",
+       vty_out(vty, " graceful-restart helper enable %pI4\n",
                &rtr->advRtrAddr);
        return HASHWALK_CONTINUE;
 }
@@ -12279,7 +12307,7 @@ static void config_write_ospf_gr(struct vty *vty, struct ospf *ospf)
 static int config_write_ospf_gr_helper(struct vty *vty, struct ospf *ospf)
 {
        if (ospf->is_helper_supported)
-               vty_out(vty, " graceful-restart helper-only\n");
+               vty_out(vty, " graceful-restart helper enable\n");
 
        if (!ospf->strict_lsa_check)
                vty_out(vty,
@@ -12742,6 +12770,8 @@ static void ospf_vty_zebra_init(void)
        /*Ospf garcefull restart helper configurations */
        install_element(OSPF_NODE, &ospf_gr_helper_enable_cmd);
        install_element(OSPF_NODE, &no_ospf_gr_helper_enable_cmd);
+       install_element(OSPF_NODE, &ospf_gr_helper_only_cmd);
+       install_element(OSPF_NODE, &no_ospf_gr_helper_only_cmd);
        install_element(OSPF_NODE, &ospf_gr_helper_enable_lsacheck_cmd);
        install_element(OSPF_NODE, &no_ospf_gr_helper_enable_lsacheck_cmd);
        install_element(OSPF_NODE, &ospf_gr_helper_supported_grace_time_cmd);

diff --git a/tests/topotests/lib/ospf.py b/tests/topotests/lib/ospf.py
index c21cbf0dd87b18b620898b53fd67bdb9f51b21a1..c425e121af18cde2de103316851c70e185a4d8fb 100644 (file)
--- a/tests/topotests/lib/ospf.py
+++ b/tests/topotests/lib/ospf.py
@@ -329,14 +329,14 @@ def __create_ospf_global(tgen, input_dict, router, build, load_config, ospf):
                 cmd = "no {}".format(cmd)
             config_data.append(cmd)
 
-        if "helper-only" in gr_data and not gr_data["helper-only"]:
-            cmd = "graceful-restart helper-only"
+        if "helper enable" in gr_data and not gr_data["helper enable"]:
+            cmd = "graceful-restart helper enable"
             if gr_data.setdefault("delete", False):
                 cmd = "no {}".format(cmd)
             config_data.append(cmd)
-        elif "helper-only" in gr_data and type(gr_data["helper-only"]) is list:
-            for rtrs in gr_data["helper-only"]:
-                cmd = "graceful-restart helper-only {}".format(rtrs)
+        elif "helper enable" in gr_data and type(gr_data["helper enable"]) is list:
+            for rtrs in gr_data["helper enable"]:
+                cmd = "graceful-restart helper enable {}".format(rtrs)
                 if gr_data.setdefault("delete", False):
                     cmd = "no {}".format(cmd)
                 config_data.append(cmd)

diff --git a/tests/topotests/ospf_gr_helper/test_ospf_gr_helper.py b/tests/topotests/ospf_gr_helper/test_ospf_gr_helper.py
index 4137712468ebc95a5f1e8354dd13053ebb7ec040..2c7c6df37ef5f46de48e350f7e4f04c7efd2d949 100644 (file)
--- a/tests/topotests/ospf_gr_helper/test_ospf_gr_helper.py
+++ b/tests/topotests/ospf_gr_helper/test_ospf_gr_helper.py
@@ -226,7 +226,7 @@ def test_ospf_gr_helper_tc1_p0(request):
 
     step("Configure graceful restart in the DUT")
     ospf_gr_r0 = {
-        "r0": {"ospf": {"graceful-restart": {"helper-only": [], "opaque": True}}}
+        "r0": {"ospf": {"graceful-restart": {"helper enable": [], "opaque": True}}}
     }
     result = create_router_ospf(tgen, topo, ospf_gr_r0)
     assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result)
@@ -243,7 +243,7 @@ def test_ospf_gr_helper_tc1_p0(request):
     assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result)
 
     ospf_gr_r1 = {
-        "r1": {"ospf": {"graceful-restart": {"helper-only": [], "opaque": True}}}
+        "r1": {"ospf": {"graceful-restart": {"helper enable": [], "opaque": True}}}
     }
     result = create_router_ospf(tgen, topo, ospf_gr_r1)
     assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result)
@@ -267,7 +267,7 @@ def test_ospf_gr_helper_tc1_p0(request):
     ospf_gr_r0 = {
         "r0": {
             "ospf": {
-                "graceful-restart": {"helper-only": [], "opaque": True, "delete": True}
+                "graceful-restart": {"helper enable": [], "opaque": True, "delete": True}
             }
         }
     }
@@ -282,7 +282,7 @@ def test_ospf_gr_helper_tc1_p0(request):
     step("Configure gr helper using the router id")
     ospf_gr_r0 = {
         "r0": {
-            "ospf": {"graceful-restart": {"helper-only": ["1.1.1.1"], "opaque": True}}
+            "ospf": {"graceful-restart": {"helper enable": ["1.1.1.1"], "opaque": True}}
         }
     }
     result = create_router_ospf(tgen, topo, ospf_gr_r0)
@@ -307,7 +307,7 @@ def test_ospf_gr_helper_tc1_p0(request):
         "r0": {
             "ospf": {
                 "graceful-restart": {
-                    "helper-only": ["1.1.1.1"],
+                    "helper enable": ["1.1.1.1"],
                     "opaque": True,
                     "delete": True,
                 }
@@ -356,13 +356,13 @@ def test_ospf_gr_helper_tc2_p0(request):
         ospf_covergence is True
     ), "OSPF is not after reset config \n Error:" " {}".format(ospf_covergence)
     ospf_gr_r0 = {
-        "r0": {"ospf": {"graceful-restart": {"helper-only": [], "opaque": True}}}
+        "r0": {"ospf": {"graceful-restart": {"helper enable": [], "opaque": True}}}
     }
     result = create_router_ospf(tgen, topo, ospf_gr_r0)
     assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result)
 
     ospf_gr_r1 = {
-        "r1": {"ospf": {"graceful-restart": {"helper-only": [], "opaque": True}}}
+        "r1": {"ospf": {"graceful-restart": {"helper enable": [], "opaque": True}}}
     }
     result = create_router_ospf(tgen, topo, ospf_gr_r1)
     assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result)
@@ -450,13 +450,13 @@ def test_ospf_gr_helper_tc3_p1(request):
     assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result)
 
     ospf_gr_r0 = {
-        "r0": {"ospf": {"graceful-restart": {"helper-only": [], "opaque": True}}}
+        "r0": {"ospf": {"graceful-restart": {"helper enable": [], "opaque": True}}}
     }
     result = create_router_ospf(tgen, topo, ospf_gr_r0)
     assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result)
 
     ospf_gr_r1 = {
-        "r1": {"ospf": {"graceful-restart": {"helper-only": [], "opaque": True}}}
+        "r1": {"ospf": {"graceful-restart": {"helper enable": [], "opaque": True}}}
     }
     result = create_router_ospf(tgen, topo, ospf_gr_r1)
     assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result)
@@ -544,13 +544,13 @@ def test_ospf_gr_helper_tc4_p1(request):
     assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result)
 
     ospf_gr_r0 = {
-        "r0": {"ospf": {"graceful-restart": {"helper-only": [], "opaque": True}}}
+        "r0": {"ospf": {"graceful-restart": {"helper enable": [], "opaque": True}}}
     }
     result = create_router_ospf(tgen, topo, ospf_gr_r0)
     assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result)
 
     ospf_gr_r1 = {
-        "r1": {"ospf": {"graceful-restart": {"helper-only": [], "opaque": True}}}
+        "r1": {"ospf": {"graceful-restart": {"helper enable": [], "opaque": True}}}
     }
     result = create_router_ospf(tgen, topo, ospf_gr_r1)
     assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result)
@@ -606,13 +606,13 @@ def test_ospf_gr_helper_tc7_p1(request):
         ospf_covergence is True
     ), "OSPF is not after reset config \n Error:" " {}".format(ospf_covergence)
     ospf_gr_r0 = {
-        "r0": {"ospf": {"graceful-restart": {"helper-only": [], "opaque": True}}}
+        "r0": {"ospf": {"graceful-restart": {"helper enable": [], "opaque": True}}}
     }
     result = create_router_ospf(tgen, topo, ospf_gr_r0)
     assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result)
 
     ospf_gr_r1 = {
-        "r1": {"ospf": {"graceful-restart": {"helper-only": [], "opaque": True}}}
+        "r1": {"ospf": {"graceful-restart": {"helper enable": [], "opaque": True}}}
     }
     result = create_router_ospf(tgen, topo, ospf_gr_r1)
     assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result)
@@ -666,13 +666,13 @@ def test_ospf_gr_helper_tc8_p1(request):
         ospf_covergence is True
     ), "OSPF is not after reset config \n Error:" " {}".format(ospf_covergence)
     ospf_gr_r0 = {
-        "r0": {"ospf": {"graceful-restart": {"helper-only": [], "opaque": True}}}
+        "r0": {"ospf": {"graceful-restart": {"helper enable": [], "opaque": True}}}
     }
     result = create_router_ospf(tgen, topo, ospf_gr_r0)
     assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result)
 
     ospf_gr_r1 = {
-        "r1": {"ospf": {"graceful-restart": {"helper-only": [], "opaque": True}}}
+        "r1": {"ospf": {"graceful-restart": {"helper enable": [], "opaque": True}}}
     }
     result = create_router_ospf(tgen, topo, ospf_gr_r1)
     assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result)

diff --git a/tests/topotests/ospf_gr_topo1/rt1/ospfd.conf b/tests/topotests/ospf_gr_topo1/rt1/ospfd.conf
index 9c04b74d35e3d9b9172dfb159acfe2048ff5534a..9590a7cadf3a2337ff595902c63c53a198d85a90 100644 (file)
--- a/tests/topotests/ospf_gr_topo1/rt1/ospfd.conf
+++ b/tests/topotests/ospf_gr_topo1/rt1/ospfd.conf
@@ -28,5 +28,5 @@ router ospf
  capability opaque
  redistribute connected
  graceful-restart grace-period 120
- graceful-restart helper-only
+ graceful-restart helper enable
 !

diff --git a/tests/topotests/ospf_gr_topo1/rt2/ospfd.conf b/tests/topotests/ospf_gr_topo1/rt2/ospfd.conf
index 922db8c8cc3c64880b60d7e44a328ce1306631a3..4f60d37b184b995d7acd5e029608b5399c635f97 100644 (file)
--- a/tests/topotests/ospf_gr_topo1/rt2/ospfd.conf
+++ b/tests/topotests/ospf_gr_topo1/rt2/ospfd.conf
@@ -33,5 +33,5 @@ router ospf
  router-id 2.2.2.2
  capability opaque
  graceful-restart grace-period 120
- graceful-restart helper-only
+ graceful-restart helper enable
 !

diff --git a/tests/topotests/ospf_gr_topo1/rt3/ospfd.conf b/tests/topotests/ospf_gr_topo1/rt3/ospfd.conf
index 51e48f13da6a12058dba5209848439859f47ac02..870878287d53a9e1c80b5da70e370246b79fa6b7 100644 (file)
--- a/tests/topotests/ospf_gr_topo1/rt3/ospfd.conf
+++ b/tests/topotests/ospf_gr_topo1/rt3/ospfd.conf
@@ -39,5 +39,5 @@ router ospf
  router-id 3.3.3.3
  capability opaque
  graceful-restart grace-period 120
- graceful-restart helper-only
+ graceful-restart helper enable
 !

diff --git a/tests/topotests/ospf_gr_topo1/rt4/ospfd.conf b/tests/topotests/ospf_gr_topo1/rt4/ospfd.conf
index a54f27a1d7f143e5a8eaa7b5e040de769d0cfd84..0aff1faf2cb4063931acf851477e8b499f2a3efa 100644 (file)
--- a/tests/topotests/ospf_gr_topo1/rt4/ospfd.conf
+++ b/tests/topotests/ospf_gr_topo1/rt4/ospfd.conf
@@ -33,5 +33,5 @@ router ospf
  router-id 4.4.4.4
  capability opaque
  graceful-restart grace-period 120
- graceful-restart helper-only
+ graceful-restart helper enable
 !

diff --git a/tests/topotests/ospf_gr_topo1/rt5/ospfd.conf b/tests/topotests/ospf_gr_topo1/rt5/ospfd.conf
index 724af0e97cf58cc56751ac015bd7e62549da7e93..4af89389a53dcec01f18fc10653364900b9434f0 100644 (file)
--- a/tests/topotests/ospf_gr_topo1/rt5/ospfd.conf
+++ b/tests/topotests/ospf_gr_topo1/rt5/ospfd.conf
@@ -27,5 +27,5 @@ router ospf
  router-id 5.5.5.5
  capability opaque
  graceful-restart grace-period 120
- graceful-restart helper-only
+ graceful-restart helper enable
 !

diff --git a/tests/topotests/ospf_gr_topo1/rt6/ospfd.conf b/tests/topotests/ospf_gr_topo1/rt6/ospfd.conf
index 0b9b83bcd2f7fbc225bb499df897d1fbaed3276b..2295a75fe7abe8202b94bfcd2cb2a56c1d80063b 100644 (file)
--- a/tests/topotests/ospf_gr_topo1/rt6/ospfd.conf
+++ b/tests/topotests/ospf_gr_topo1/rt6/ospfd.conf
@@ -34,5 +34,5 @@ router ospf
  capability opaque
  area 3 nssa
  graceful-restart grace-period 120
- graceful-restart helper-only
+ graceful-restart helper enable
 !

diff --git a/tests/topotests/ospf_gr_topo1/rt7/ospfd.conf b/tests/topotests/ospf_gr_topo1/rt7/ospfd.conf
index 49db254410a85decaed7d0285e13ea3ebe73e1c5..8534eda5a7c959ecd43c16e1b78750cc997778e3 100644 (file)
--- a/tests/topotests/ospf_gr_topo1/rt7/ospfd.conf
+++ b/tests/topotests/ospf_gr_topo1/rt7/ospfd.conf
@@ -29,5 +29,5 @@ router ospf
  redistribute connected
  area 3 nssa
  graceful-restart grace-period 120
- graceful-restart helper-only
+ graceful-restart helper enable
 !