[config vlan] Stop, reset-failed, then start dhcp_relay service (soni…

…c-net#1269)

**- What I did**

When adding or deleting an DHCP relay destination address to a VLAN, previously, the service was simply restarted. However, we have set a start limit on services in SONiC, such that if a service is restarted X times within Y time, the service will enter a failed state and cannot be started until the failed state is cleared (currently 3 times within 20 minutes). However, if someone attempts to perform more than 3 add or delete operations within 20 minutes, it would trigger this failure condition. This change prevents this form occurring.

**- How I did it**

Rather than simply calling `systemctl restart dhcp_relay` after adding or deleting a DHCP relay destination IP address, we now call:

```
systemctl stop dhcp_relay
systemctl reset-failed dhcp_relay
systemctl start dhcp_relay
```

config/vlan.py

@@ -201,7 +201,9 @@ def add_vlan_dhcp_relay_destination(db, vid, dhcp_relay_destination_ip):
     click.echo("Added DHCP relay destination address {} to {}".format(dhcp_relay_destination_ip, vlan_name))
     try:
         click.echo("Restarting DHCP relay service...")
-        clicommon.run_command("systemctl restart dhcp_relay", display_cmd=False)
+        clicommon.run_command("systemctl stop dhcp_relay", display_cmd=False)
+        clicommon.run_command("systemctl reset-failed dhcp_relay", display_cmd=False)
+        clicommon.run_command("systemctl start dhcp_relay", display_cmd=False)
     except SystemExit as e:
         ctx.fail("Restart service dhcp_relay failed with error {}".format(e))
 
@@ -235,6 +237,8 @@ def del_vlan_dhcp_relay_destination(db, vid, dhcp_relay_destination_ip):
     click.echo("Removed DHCP relay destination address {} from {}".format(dhcp_relay_destination_ip, vlan_name))
     try:
         click.echo("Restarting DHCP relay service...")
-        clicommon.run_command("systemctl restart dhcp_relay", display_cmd=False)
+        clicommon.run_command("systemctl stop dhcp_relay", display_cmd=False)
+        clicommon.run_command("systemctl reset-failed dhcp_relay", display_cmd=False)
+        clicommon.run_command("systemctl start dhcp_relay", display_cmd=False)
     except SystemExit as e:
         ctx.fail("Restart service dhcp_relay failed with error {}".format(e))

doc/Command-Reference.md

@@ -2158,7 +2158,6 @@ This command is used to delete a configured DHCP Relay Destination IP address fr
   admin@sonic:~$ sudo config vlan dhcp_relay del 1000 7.7.7.7
   Removed DHCP relay destination address 7.7.7.7 from Vlan1000
   Restarting DHCP relay service...
-  Running command: systemctl restart dhcp_relay
   ```
 
 Go Back To [Beginning of the document](#) or [Beginning of this section](#dhcp-relay)

tests/vlan_test.py

@@ -435,72 +435,84 @@ def test_config_add_del_vlan_and_vlan_member_in_alias_mode(self):
 
     def test_config_vlan_add_dhcp_relay_with_nonexist_vlanid(self):
         runner = CliRunner()
-        result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["add"],
-                ["1001", "192.0.0.100"])
-        print(result.exit_code)
-        print(result.output)
-        # traceback.print_tb(result.exc_info[2])
-        assert result.exit_code != 0
-        assert "Error: Vlan1001 doesn't exist" in result.output
+
+        with mock.patch('utilities_common.cli.run_command') as mock_run_command:
+            result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["add"],
+                                   ["1001", "192.0.0.100"])
+            print(result.exit_code)
+            print(result.output)
+            # traceback.print_tb(result.exc_info[2])
+            assert result.exit_code != 0
+            assert "Error: Vlan1001 doesn't exist" in result.output
+            assert mock_run_command.call_count == 0
 
     def test_config_vlan_add_dhcp_relay_with_invalid_vlanid(self):
         runner = CliRunner()
-        result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["add"],
-                ["4096", "192.0.0.100"])
-        print(result.exit_code)
-        print(result.output)
-        # traceback.print_tb(result.exc_info[2])
-        assert result.exit_code != 0
-        assert "Error: Vlan4096 doesn't exist" in result.output
+
+        with mock.patch('utilities_common.cli.run_command') as mock_run_command:
+            result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["add"],
+                                   ["4096", "192.0.0.100"])
+            print(result.exit_code)
+            print(result.output)
+            # traceback.print_tb(result.exc_info[2])
+            assert result.exit_code != 0
+            assert "Error: Vlan4096 doesn't exist" in result.output
+            assert mock_run_command.call_count == 0
 
     def test_config_vlan_add_dhcp_relay_with_invalid_ip(self):
         runner = CliRunner()
-        result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["add"],
-                ["1000", "192.0.0.1000"])
-        print(result.exit_code)
-        print(result.output)
-        # traceback.print_tb(result.exc_info[2])
-        assert result.exit_code != 0
-        assert "Error: 192.0.0.1000 is invalid IP address" in result.output
+
+        with mock.patch('utilities_common.cli.run_command') as mock_run_command:
+            result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["add"],
+                                   ["1000", "192.0.0.1000"])
+            print(result.exit_code)
+            print(result.output)
+            # traceback.print_tb(result.exc_info[2])
+            assert result.exit_code != 0
+            assert "Error: 192.0.0.1000 is invalid IP address" in result.output
+            assert mock_run_command.call_count == 0
 
     def test_config_vlan_add_dhcp_relay_with_exist_ip(self):
         runner = CliRunner()
-        result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["add"],
-                ["1000", "192.0.0.1"])
-        print(result.exit_code)
-        print(result.output)
-        # traceback.print_tb(result.exc_info[2])
-        assert result.exit_code == 0
-        assert "192.0.0.1 is already a DHCP relay destination for Vlan1000" in result.output
+
+        with mock.patch('utilities_common.cli.run_command') as mock_run_command:
+            result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["add"],
+                                   ["1000", "192.0.0.1"])
+            print(result.exit_code)
+            print(result.output)
+            # traceback.print_tb(result.exc_info[2])
+            assert result.exit_code == 0
+            assert "192.0.0.1 is already a DHCP relay destination for Vlan1000" in result.output
+            assert mock_run_command.call_count == 0
 
     def test_config_vlan_add_del_dhcp_relay_dest(self):
         runner = CliRunner()
         db = Db()
 
         # add new relay dest
-        with mock.patch("utilities_common.cli.run_command", mock.MagicMock()) as mock_run_command:
+        with mock.patch("utilities_common.cli.run_command") as mock_run_command:
             result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["add"],
-                    ["1000", "192.0.0.100"], obj=db)
+                                   ["1000", "192.0.0.100"], obj=db)
             print(result.exit_code)
             print(result.output)
             assert result.exit_code == 0
             assert result.output == config_vlan_add_dhcp_relay_output
-            assert mock_run_command.call_count == 1
+            assert mock_run_command.call_count == 3
 
         # show output
         result = runner.invoke(show.cli.commands["vlan"].commands["brief"], [], obj=db)
         print(result.output)
         assert result.output == show_vlan_brief_output_with_new_dhcp_relay_address
 
         # del relay dest
-        with mock.patch("utilities_common.cli.run_command", mock.MagicMock()) as mock_run_command:
+        with mock.patch("utilities_common.cli.run_command") as mock_run_command:
             result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["del"],
-                    ["1000", "192.0.0.100"], obj=db)
+                                   ["1000", "192.0.0.100"], obj=db)
             print(result.exit_code)
             print(result.output)
             assert result.exit_code == 0
             assert result.output == config_vlan_del_dhcp_relay_output
-            assert mock_run_command.call_count == 1
+            assert mock_run_command.call_count == 3
 
         # show output
         result = runner.invoke(show.cli.commands["vlan"].commands["brief"], [], obj=db)
@@ -509,23 +521,29 @@ def test_config_vlan_add_del_dhcp_relay_dest(self):
 
     def test_config_vlan_remove_nonexist_dhcp_relay_dest(self):
         runner = CliRunner()
-        result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["del"],
-                ["1000", "192.0.0.100"])
-        print(result.exit_code)
-        print(result.output)
-        # traceback.print_tb(result.exc_info[2])
-        assert result.exit_code != 0
-        assert "Error: 192.0.0.100 is not a DHCP relay destination for Vlan1000" in result.output
+
+        with mock.patch('utilities_common.cli.run_command') as mock_run_command:
+            result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["del"],
+                                   ["1000", "192.0.0.100"])
+            print(result.exit_code)
+            print(result.output)
+            # traceback.print_tb(result.exc_info[2])
+            assert result.exit_code != 0
+            assert "Error: 192.0.0.100 is not a DHCP relay destination for Vlan1000" in result.output
+            assert mock_run_command.call_count == 0
 
     def test_config_vlan_remove_dhcp_relay_dest_with_nonexist_vlanid(self):
         runner = CliRunner()
-        result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["del"],
-                ["1001", "192.0.0.1"])
-        print(result.exit_code)
-        print(result.output)
-        # traceback.print_tb(result.exc_info[2])
-        assert result.exit_code != 0
-        assert "Error: Vlan1001 doesn't exist" in result.output
+
+        with mock.patch('utilities_common.cli.run_command') as mock_run_command:
+            result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["del"],
+                                   ["1001", "192.0.0.1"])
+            print(result.exit_code)
+            print(result.output)
+            # traceback.print_tb(result.exc_info[2])
+            assert result.exit_code != 0
+            assert "Error: Vlan1001 doesn't exist" in result.output
+            assert mock_run_command.call_count == 0
 
     def test_config_vlan_proxy_arp_with_nonexist_vlan_intf_table(self):
         modes = ["enabled", "disabled"]