vyos.ifconfig: T7135: only restart DHCPv6 client if needed

Previously the DHCPv6 client was restarted on any change to the interface,
including changes only to the interface description. Re-use pattern from IPv4
DHCP to only restart the DHCP client if necessary.

3 files changed, 38 insertions, 7 deletions

diff --git a/python/vyos/configdict.py b/python/vyos/configdict.py
index a6594871e..78b98a3eb 100644
--- a/python/vyos/configdict.py
+++ b/python/vyos/configdict.py
@@ -491,6 +491,8 @@ def get_interface_dict(config, base, ifname='', recursive_defaults=True, with_pk
     # Check if any DHCP options changed which require a client restat
     dhcp = is_node_changed(config, base + [ifname, 'dhcp-options'])
     if dhcp: dict.update({'dhcp_options_changed' : {}})
+    dhcpv6 = is_node_changed(config, base + [ifname, 'dhcpv6-options'])
+    if dhcpv6: dict.update({'dhcpv6_options_changed' : {}})
 
     # Some interfaces come with a source_interface which must also not be part
     # of any other bond or bridge interface as it is exclusivly assigned as the
@@ -539,6 +541,8 @@ def get_interface_dict(config, base, ifname='', recursive_defaults=True, with_pk
         # Check if any DHCP options changed which require a client restat
         dhcp = is_node_changed(config, base + [ifname, 'vif', vif, 'dhcp-options'])
         if dhcp: dict['vif'][vif].update({'dhcp_options_changed' : {}})
+        dhcpv6 = is_node_changed(config, base + [ifname, 'vif', vif, 'dhcpv6-options'])
+        if dhcpv6: dict['vif'][vif].update({'dhcpv6_options_changed' : {}})
 
     for vif_s, vif_s_config in dict.get('vif_s', {}).items():
         # Add subinterface name to dictionary
@@ -565,6 +569,8 @@ def get_interface_dict(config, base, ifname='', recursive_defaults=True, with_pk
         # Check if any DHCP options changed which require a client restat
         dhcp = is_node_changed(config, base + [ifname, 'vif-s', vif_s, 'dhcp-options'])
         if dhcp: dict['vif_s'][vif_s].update({'dhcp_options_changed' : {}})
+        dhcpv6 = is_node_changed(config, base + [ifname, 'vif-s', vif_s, 'dhcpv6-options'])
+        if dhcpv6: dict['vif_s'][vif_s].update({'dhcpv6_options_changed' : {}})
 
         for vif_c, vif_c_config in vif_s_config.get('vif_c', {}).items():
             # Add subinterface name to dictionary
@@ -593,6 +599,8 @@ def get_interface_dict(config, base, ifname='', recursive_defaults=True, with_pk
             # Check if any DHCP options changed which require a client restat
             dhcp = is_node_changed(config, base + [ifname, 'vif-s', vif_s, 'vif-c', vif_c, 'dhcp-options'])
             if dhcp: dict['vif_s'][vif_s]['vif_c'][vif_c].update({'dhcp_options_changed' : {}})
+            dhcpv6 = is_node_changed(config, base + [ifname, 'vif-s', vif_s, 'vif-c', vif_c, 'dhcpv6-options'])
+            if dhcpv6: dict['vif_s'][vif_s]['vif_c'][vif_c].update({'dhcpv6_options_changed' : {}})
 
     # Check vif, vif-s/vif-c VLAN interfaces for removal
     dict = get_removed_vlans(config, base + [ifname], dict)
diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py
index 91ee09d90..85f2d3484 100644
--- a/python/vyos/ifconfig/interface.py
+++ b/python/vyos/ifconfig/interface.py
@@ -1222,7 +1222,7 @@ class Interface(Control):
         if addr == 'dhcp':
             self.set_dhcp(True, vrf_changed=vrf_changed)
         elif addr == 'dhcpv6':
-            self.set_dhcpv6(True)
+            self.set_dhcpv6(True, vrf_changed=vrf_changed)
         elif not is_intf_addr_assigned(self.ifname, addr, netns=netns):
             netns_cmd  = f'ip netns exec {netns}' if netns else ''
             tmp = f'{netns_cmd} ip addr add {addr} dev {self.ifname}'
@@ -1430,7 +1430,7 @@ class Interface(Control):
 
         return None
 
-    def set_dhcpv6(self, enable):
+    def set_dhcpv6(self, enable: bool, vrf_changed: bool=False):
         """
         Enable/Disable DHCPv6 client on a given interface.
         """
@@ -1459,7 +1459,10 @@ class Interface(Control):
 
             # We must ignore any return codes. This is required to enable
             # DHCPv6-PD for interfaces which are yet not up and running.
-            return self._popen(f'systemctl restart {systemd_service}')
+            if (vrf_changed or
+                ('dhcpv6_options_changed' in self.config) or
+                (not is_systemd_service_active(systemd_service))):
+                return self._popen(f'systemctl restart {systemd_service}')
         else:
             if is_systemd_service_active(systemd_service):
                 self._cmd(f'systemctl stop {systemd_service}')
@@ -1676,10 +1679,6 @@ class Interface(Control):
                 else:
                     self.del_addr(addr)
 
-        # start DHCPv6 client when only PD was configured
-        if dhcpv6pd:
-            self.set_dhcpv6(True)
-
         # XXX: Bind interface to given VRF or unbind it if vrf is not set. Unbinding
         # will call 'ip link set dev eth0 nomaster' which will also drop the
         # interface out of any bridge or bond - thus this is checked before.
@@ -1698,6 +1697,10 @@ class Interface(Control):
         else:
             vrf_changed = self.set_vrf(config.get('vrf', ''))
 
+        # start DHCPv6 client when only PD was configured
+        if dhcpv6pd:
+            self.set_dhcpv6(True, vrf_changed=vrf_changed)
+
         # Add this section after vrf T4331
         for addr in new_addr:
             self.add_addr(addr, vrf_changed=vrf_changed)
diff --git a/smoketest/scripts/cli/base_interfaces_test.py b/smoketest/scripts/cli/base_interfaces_test.py
index 85888a448..78c807d59 100644
--- a/smoketest/scripts/cli/base_interfaces_test.py
+++ b/smoketest/scripts/cli/base_interfaces_test.py
@@ -366,6 +366,26 @@ class BasicInterfaceTest:
                 vrf_pids = cmd(f'ip vrf pids {vrf_name}')
                 self.assertIn(str(tmp), vrf_pids)
 
+            # T7135: remove interface from VRF instance and move DHCP client
+            # back to default VRF. This must restart the DHCP client process
+            for interface in self._interfaces:
+                self.cli_delete(self._base_path + [interface, 'vrf'])
+
+            self.cli_commit()
+
+            # Validate interface state
+            for interface in self._interfaces:
+                tmp = get_interface_vrf(interface)
+                self.assertEqual(tmp, 'default')
+
+                # Check if dhclient process runs
+                tmp = process_named_running(dhcp6c_process_name, cmdline=interface, timeout=10)
+                self.assertTrue(tmp)
+                # .. inside the appropriate VRF instance
+                vrf_pids = cmd(f'ip vrf pids {vrf_name}')
+                self.assertNotIn(str(tmp), vrf_pids)
+
+
             self.cli_delete(['vrf', 'name', vrf_name])
 
         def test_move_interface_between_vrf_instances(self):