vyos.ifconfig: T5103: force dhclient restart on VRF change

Moving an interface in, out or between VRFs will not re-install the received
default route. This is because the dhclient binary is not restarted in the new
VRF. Dhclient itself will report an error like: "receive_packet failed on
eth0.10: Network is down".

Take the return value of vyos.ifconfig.Interface().set_vrf() into account to
forcefully restart the DHCP client process and optain a proper lease.

python/vyos/ifconfig/interface.py

@@ -595,12 +595,16 @@ def set_vrf(self, vrf: str) -> bool:
         """
         Add/Remove interface from given VRF instance.
 
+        Keyword arguments:
+        vrf: VRF instance name or empty string (default VRF)
+
+        Return True if VRF was changed, False otherwise
+
         Example:
         >>> from vyos.ifconfig import Interface
         >>> Interface('eth0').set_vrf('foo')
         >>> Interface('eth0').set_vrf()
         """
-
         # Don't allow for netns yet
         if 'netns' in self.config:
             return False
@@ -619,7 +623,7 @@ def set_vrf(self, vrf: str) -> bool:
             # Add map element with interface and zone ID
             if vrf_table_id:
                 # delete old table ID from nftables if it has changed, e.g. interface moved to a different VRF
-                if old_vrf_tableid and old_vrf_tableid != int(vrf_table_id):
+                if old_vrf_tableid != vrf_table_id:
                     self._del_interface_from_ct_iface_map()
                 self._add_interface_to_ct_iface_map(vrf_table_id)
         else:
@@ -1181,7 +1185,7 @@ def get_addr(self):
         """
         return self.get_addr_v4() + self.get_addr_v6()
 
-    def add_addr(self, addr):
+    def add_addr(self, addr: str, vrf_changed: bool=False) -> bool:
         """
         Add IP(v6) address to interface. Address is only added if it is not
         already assigned to that interface. Address format must be validated
@@ -1214,15 +1218,14 @@ def add_addr(self, addr):
 
         # add to interface
         if addr == 'dhcp':
-            self.set_dhcp(True)
+            self.set_dhcp(True, vrf_changed=vrf_changed)
         elif addr == 'dhcpv6':
             self.set_dhcpv6(True)
         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}'
             # Add broadcast address for IPv4
             if is_ipv4(addr): tmp += ' brd +'
-
             self._cmd(tmp)
         else:
             return False
@@ -1232,7 +1235,7 @@ def add_addr(self, addr):
 
         return True
 
-    def del_addr(self, addr):
+    def del_addr(self, addr: str, vrf_changed: bool=False) -> bool:
         """
         Delete IP(v6) address from interface. Address is only deleted if it is
         assigned to that interface. Address format must be exactly the same as
@@ -1264,7 +1267,7 @@ def del_addr(self, addr):
 
         # remove from interface
         if addr == 'dhcp':
-            self.set_dhcp(False)
+            self.set_dhcp(False, vrf_changed=False)
         elif addr == 'dhcpv6':
             self.set_dhcpv6(False)
         elif is_intf_addr_assigned(self.ifname, addr, netns=netns):
@@ -1356,7 +1359,7 @@ def add_to_bridge(self, bridge_dict):
                     cmd = f'bridge vlan add dev {ifname} vid {native_vlan_id} pvid untagged master'
                     self._cmd(cmd)
 
-    def set_dhcp(self, enable):
+    def set_dhcp(self, enable, vrf_changed: bool=False):
         """
         Enable/Disable DHCP client on a given interface.
         """
@@ -1396,7 +1399,9 @@ def set_dhcp(self, enable):
             # the old lease is released a new one is acquired (T4203). We will
             # only restart DHCP client if it's option changed, or if it's not
             # running, but it should be running (e.g. on system startup)
-            if 'dhcp_options_changed' in self.config or not is_systemd_service_active(systemd_service):
+            if (vrf_changed or
+                ('dhcp_options_changed' in self.config) or
+                (not is_systemd_service_active(systemd_service))):
                 return self._cmd(f'systemctl restart {systemd_service}')
         else:
             if is_systemd_service_active(systemd_service):
@@ -1676,23 +1681,24 @@ def update(self, config):
         # 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.
+        vrf_changed = False
         if 'is_bond_member' in config:
             bond_if = next(iter(config['is_bond_member']))
             tmp = get_interface_config(config['ifname'])
             if 'master' in tmp and tmp['master'] != bond_if:
-                self.set_vrf('')
+                vrf_changed = self.set_vrf('')
 
         elif 'is_bridge_member' in config:
             bridge_if = next(iter(config['is_bridge_member']))
             tmp = get_interface_config(config['ifname'])
             if 'master' in tmp and tmp['master'] != bridge_if:
-                self.set_vrf('')
+                vrf_changed = self.set_vrf('')
         else:
-            self.set_vrf(config.get('vrf', ''))
+            vrf_changed = self.set_vrf(config.get('vrf', ''))
 
         # Add this section after vrf T4331
         for addr in new_addr:
-            self.add_addr(addr)
+            self.add_addr(addr, vrf_changed=vrf_changed)
 
         # Configure MSS value for IPv4 TCP connections
         tmp = dict_search('ip.adjust_mss', config)

smoketest/scripts/cli/base_interfaces_test.py

@@ -38,6 +38,7 @@
 from vyos.utils.network import is_ipv6_link_local
 from vyos.utils.network import get_nft_vrf_zone_mapping
 from vyos.xml_ref import cli_defined
+from vyos.xml_ref import default_value
 
 dhclient_base_dir = directories['isc_dhclient_dir']
 dhclient_process_name = 'dhclient'
@@ -282,6 +283,9 @@ def test_dhcp_vrf(self):
             if not self._test_dhcp or not self._test_vrf:
                 self.skipTest('not supported')
 
+            cli_default_metric = default_value(self._base_path + [self._interfaces[0],
+                                               'dhcp-options', 'default-route-distance'])
+
             vrf_name = 'purple4'
             self.cli_set(['vrf', 'name', vrf_name, 'table', '65000'])
 
@@ -307,7 +311,28 @@ def test_dhcp_vrf(self):
                 self.assertIn(str(dhclient_pid), vrf_pids)
                 # and the commandline has the appropriate options
                 cmdline = read_file(f'/proc/{dhclient_pid}/cmdline')
-                self.assertIn('-e\x00IF_METRIC=210', cmdline) # 210 is the default value
+                self.assertIn(f'-e\x00IF_METRIC={cli_default_metric}', cmdline)
+
+            # T5103: 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
+                dhclient_pid = process_named_running(dhclient_process_name, cmdline=interface, timeout=10)
+                self.assertTrue(dhclient_pid)
+                # .. inside the appropriate VRF instance
+                vrf_pids = cmd(f'ip vrf pids {vrf_name}')
+                self.assertNotIn(str(dhclient_pid), vrf_pids)
+                # and the commandline has the appropriate options
+                cmdline = read_file(f'/proc/{dhclient_pid}/cmdline')
+                self.assertIn(f'-e\x00IF_METRIC={cli_default_metric}', cmdline)
 
             self.cli_delete(['vrf', 'name', vrf_name])