[dhcp_relay]: sonic dhcp relay agent for IPv4 (sonic-net#4004)

Currently SONiC uses the 'isc-dhcp-relay' package to allow DHCP relay functionality on IPv4 networks.
With this PR we are adding sonic dhcp relay agent for IPv4 as described in this HLD(sonic-net/SONiC#1938).

What I did
Added dependency checks in SONiC config CLI to prevent deletion of interfaces, VRFs, and VLANs that are in use by DHCPv4 relay configurations.
Added/updated unit tests.
How I did it
Implemented a new function check_dhcpv4_relay_dependencies to check if an interface or VRF is referenced in the DHCPV4_RELAY table.
Called this function before allowing deletion of portchannels, loopbacks, VRFs, and VLANs.
Modified the VLAN deletion logic to block removal if the VLAN is present in the DHCPV4_RELAY table.
Extended the test suite to verify that deletion is blocked when dependencies exist and allowed after cleanup.

How to verify it
Run the updated unit tests: they now include cases where deletion of interfaces, VRFs, or VLANs in use by DHCPv4 relay is attempted and should fail.
Manually test by configuring DHCPv4 relay on an interface/VRF/VLAN, then attempt to delete it using the CLI; the command should fail with an appropriate error message.
After removing the DHCPv4 relay configuration, deletion should succeed.

Author: cshivashgit

config/main.py

@@ -1625,6 +1625,28 @@ def config_file_yang_validation(filename):
     return True
 
 
+def check_dhcpv4_relay_dependencies(db, object_name, object_type):
+    """Checks if to be deleted interface/VRF is used in DHCPV4_RELAY table."""
+    # Check if has_sonic_dhcpv4_relay flag is enabled
+    feature_table = db.get_table("DEVICE_METADATA")
+    dhcp_relay_feature = feature_table.get("localhost", {})
+    if dhcp_relay_feature.get("has_sonic_dhcpv4_relay") != "True":
+        return
+
+    for relay_vlan, data in db.get_table('DHCPV4_RELAY').items():
+        if object_type == 'interface':
+            source_intf = data.get('source_interface')
+            if source_intf == object_name:
+                raise ValueError(f"Interface '{object_name}' is in use by {relay_vlan}")
+
+        elif object_type == 'vrf':
+            server_vrf = data.get('server_vrf')
+            if server_vrf == object_name:
+                raise ValueError(f"VRF '{object_name}' is in use for dhcp_relay configurations for {relay_vlan}")
+        else:
+            raise ValueError("Unsupported object_type: {}".format(object_type))
+
+
 # This is our main entrypoint - the main 'config' command
 @click.group(cls=clicommon.AbbreviationGroup, context_settings=CONTEXT_SETTINGS)
 @click.pass_context
@@ -2803,6 +2825,12 @@ def remove_portchannel(ctx, portchannel_name):
         if len([(k, v) for k, v in db.get_table('PORTCHANNEL_MEMBER') if k == portchannel_name]) != 0: # TODO: MISSING CONSTRAINT IN YANG MODEL
             ctx.fail("Error: Portchannel {} contains members. Remove members before deleting Portchannel!".format(portchannel_name))
 
+        # Dont proceed if the port channel is used in dhcpv4_relay
+        try:
+            check_dhcpv4_relay_dependencies(db, portchannel_name, 'interface')
+        except ValueError as e:
+            ctx.fail(str(e))
+
     try:
         db.set_entry('PORTCHANNEL', portchannel_name, None)
     except JsonPatchConflict:
@@ -7371,6 +7399,12 @@ def del_vrf(ctx, vrf_name):
         syslog_vrf = syslog_data.get("vrf")
         if syslog_vrf == syslog_vrf_dev:
             ctx.fail("Failed to remove VRF device: {} is in use by SYSLOG_SERVER|{}".format(syslog_vrf, syslog_entry))
+    # Dont proceed if the vrf is used in dhcpv4_relay
+    try:
+        check_dhcpv4_relay_dependencies(config_db, vrf_name, 'vrf')
+    except ValueError as e:
+        ctx.fail(str(e))
+
     if not is_vrf_exists(config_db, vrf_name):
         ctx.fail("VRF {} does not exist!".format(vrf_name))
     elif (vrf_name == 'mgmt' or vrf_name == 'management'):
@@ -8549,6 +8583,12 @@ def del_loopback(ctx, loopback_name):
         if loopback_name not in lo_intfs:
             ctx.fail("{} does not exist".format(loopback_name))
 
+    # Dont proceed if the loopback is used in dhcpv4_relay
+    try:
+        check_dhcpv4_relay_dependencies(config_db, loopback_name, 'interface')
+    except ValueError as e:
+        ctx.fail(str(e))
+
     ips = [ k[1] for k in lo_config_db if type(k) == tuple and k[0] == loopback_name ]
     for ip in ips:
         config_db.set_entry('LOOPBACK_INTERFACE', (loopback_name, ip), None)

config/vlan.py

@@ -184,6 +184,8 @@ def del_vlan(db, vid, multiple, no_restart_dhcp_relay):
                              "First remove vxlan mapping '{}' assigned to VLAN".format(
                               vid, '|'.join(vxmap_key)))
 
+            if vlan in db.cfgdb.get_table('DHCPV4_RELAY'):
+                ctx.fail(f"{vlan} cannot be removed as it is being used in DHCPV4_RELAY table.")
             # set dhcpv4_relay table
             set_dhcp_relay_table('VLAN', config_db, vlan, None)
 

scripts/db_migrator.py

@@ -902,6 +902,40 @@ def migrate_aaa(self):
             if keys:
                 self.configDB.delete(self.configDB.CONFIG_DB, authorization_key)
 
+    def migrate_dhcp_servers_to_dhcpv4_relay(self):
+        try:
+            vlan_table = self.configDB.get_table("VLAN")
+        except Exception as e:
+            log.log_error(f"Failed to read VLAN table: {str(e)}")
+            return
+
+        for vlan_key, vlan_data in vlan_table.items():
+            if "dhcp_servers" not in vlan_data:
+                continue
+            try:
+                dhcp_servers = vlan_data.get("dhcp_servers")
+                relay_data = self.configDB.get_entry("DHCPV4_RELAY", vlan_key) or {}
+                if "dhcpv4_servers" not in relay_data:
+                    relay_data["dhcpv4_servers"] = dhcp_servers
+                    self.configDB.set_entry("DHCPV4_RELAY", vlan_key, relay_data)
+                    migrated_entry = self.configDB.get_entry("DHCPV4_RELAY", vlan_key)
+                    if migrated_entry.get("dhcpv4_servers") == dhcp_servers:
+                        log.log_notice(f"Migrated DHCP servers for {vlan_key} to DHCPV4_RELAY table")
+                    else:
+                        log.log_error(f"Verification failed for {vlan_key}: Migration did not persist correctly")
+                        continue
+                else:
+                    log.log_notice(f"Skipping migration for {vlan_key}: dhcpv4_servers already present in DHCPV4_RELAY")
+                updated_vlan_data = vlan_data.copy()
+                del updated_vlan_data["dhcp_servers"]
+                self.configDB.set_entry("VLAN", vlan_key, updated_vlan_data)
+
+                log.log_notice(f"Migrated DHCP servers for {vlan_key} to DHCPV4_RELAY table")
+
+            except Exception as e:
+                log.log_error(f"Failed to migrate DHCP servers for {vlan_key}: {str(e)}")
+
+
     def version_unknown(self):
         """
         version_unknown tracks all SONiC versions that doesn't have a version
@@ -1231,12 +1265,23 @@ def version_4_0_3(self):
         self.set_version('version_202305_01')
         return 'version_202305_01'
 
+    def check_has_sonic_dhcpv4_relay_flag(self):
+        device_metadata_table = self.configDB.get_table("DEVICE_METADATA")
+        dhcp_relay_feature = device_metadata_table.get("localhost", {})
+        if dhcp_relay_feature.get("has_sonic_dhcpv4_relay") == "True":
+            return True
+        return False
+
     def version_202305_01(self):
         """
         Version 202305_01.
         This is current last erversion for 202305 branch
         """
         log.log_info('Handling version_202305_01')
+        if self.check_has_sonic_dhcpv4_relay_flag():
+            log.log_info("Triggering migrate_dhcp_servers_to_dhcpv4_relay()")
+            self.migrate_dhcp_servers_to_dhcpv4_relay()
+
         self.set_version('version_202311_01')
         return 'version_202311_01'
 
@@ -1250,6 +1295,10 @@ def version_202311_01(self):
         self.migrate_dns_nameserver()
 
         self.migrate_sflow_table()
+        if self.check_has_sonic_dhcpv4_relay_flag():
+            log.log_info("Triggering migrate_dhcp_servers_to_dhcpv4_relay()")
+            self.migrate_dhcp_servers_to_dhcpv4_relay()
+
         self.set_version('version_202311_02')
         return 'version_202311_02'
 
@@ -1260,6 +1309,9 @@ def version_202311_02(self):
         log.log_info('Handling version_202311_02')
         # Update GNMI table
         self.migrate_gnmi()
+        if self.check_has_sonic_dhcpv4_relay_flag():
+            log.log_info("Triggering migrate_dhcp_servers_to_dhcpv4_relay()")
+            self.migrate_dhcp_servers_to_dhcpv4_relay()
 
         self.set_version('version_202311_03')
         return 'version_202311_03'
@@ -1270,6 +1322,10 @@ def version_202311_03(self):
         This is current last erversion for 202311 branch
         """
         log.log_info('Handling version_202311_03')
+        if self.check_has_sonic_dhcpv4_relay_flag():
+            log.log_info("Triggering migrate_dhcp_servers_to_dhcpv4_relay()")
+            self.migrate_dhcp_servers_to_dhcpv4_relay()
+
         self.set_version('version_202405_01')
         return 'version_202405_01'
 
@@ -1278,6 +1334,10 @@ def version_202405_01(self):
         Version 202405_01.
         """
         log.log_info('Handling version_202405_01')
+        if self.check_has_sonic_dhcpv4_relay_flag():
+            log.log_info("Triggering migrate_dhcp_servers_to_dhcpv4_relay()")
+            self.migrate_dhcp_servers_to_dhcpv4_relay()
+
         self.set_version('version_202405_02')
         return 'version_202405_02'
 
@@ -1286,6 +1346,10 @@ def version_202405_02(self):
         Version 202405_02.
         """
         log.log_info('Handling version_202405_02')
+        if self.check_has_sonic_dhcpv4_relay_flag():
+            log.log_info("Triggering migrate_dhcp_servers_to_dhcpv4_relay()")
+            self.migrate_dhcp_servers_to_dhcpv4_relay()
+
         self.migrate_ipinip_tunnel()
         self.set_version('version_202411_01')
         return 'version_202411_01'
@@ -1295,6 +1359,10 @@ def version_202411_01(self):
         Version 202411_01.
         """
         log.log_info('Handling version_202411_01')
+        if self.check_has_sonic_dhcpv4_relay_flag():
+            log.log_info("Triggering migrate_dhcp_servers_to_dhcpv4_relay()")
+            self.migrate_dhcp_servers_to_dhcpv4_relay()
+
         self.set_version('version_202411_02')
         return 'version_202411_02'
 
@@ -1303,6 +1371,10 @@ def version_202411_02(self):
         Version 202411_02.
         """
         log.log_info('Handling version_202411_02')
+        if self.check_has_sonic_dhcpv4_relay_flag():
+            log.log_info("Triggering migrate_dhcp_servers_to_dhcpv4_relay()")
+            self.migrate_dhcp_servers_to_dhcpv4_relay()
+
         self.set_version('version_202505_01')
         return 'version_202505_01'
 
@@ -1312,6 +1384,10 @@ def version_202505_01(self):
         master branch until 202505 branch is created.
         """
         log.log_info('Handling version_202505_01')
+        if self.check_has_sonic_dhcpv4_relay_flag():
+            log.log_info("Triggering migrate_dhcp_servers_to_dhcpv4_relay()")
+            self.migrate_dhcp_servers_to_dhcpv4_relay()
+
         self.migrate_flex_counter_delay_status_removal()
         return None
 

tests/config_test.py

@@ -3227,6 +3227,34 @@ def test_del_nonexistent_loopback_adhoc_validation(self):
         assert result.exit_code != 0
         assert "Loopbax1 is invalid, name should have prefix 'Loopback' and suffix '<0-999>'" in result.output
 
+    def test_del_loopback_with_dhcpv4_relay_entry(self):
+        config.ADHOC_VALIDATION = True
+        runner = CliRunner()
+        db = Db()
+        obj = {'db': db.cfgdb}
+
+        result = runner.invoke(config.config.commands["loopback"].commands["add"], ["Loopback1"], obj=obj)
+        print(result.exit_code)
+        print(result.output)
+        assert result.exit_code == 0
+        assert db.cfgdb.get_entry("LOOPBACK_INTERFACE", "Loopback1") == {}
+
+        # Enable has_sonic_dhcpv4_relay flag
+        db.cfgdb.set_entry("DEVICE_METADATA", "localhost", {"has_sonic_dhcpv4_relay": "True"})
+
+        db.cfgdb.set_entry("DHCPV4_RELAY", "Vlan100", {
+            "dhcpv4_servers": ["192.0.2.100"],
+            "source_interface": "Loopback1"
+        })
+
+        result = runner.invoke(config.config.commands["loopback"].commands["del"], ["Loopback1"], obj=obj)
+        print(result.exit_code)
+        print(result.output)
+        assert result.exit_code != 0
+        assert "Error: Interface 'Loopback1' is in use by Vlan100" in result.output
+
+        db.cfgdb.set_entry("DHCPV4_RELAY", "Vlan200", None)
+
     @patch("config.validated_config_db_connector.ValidatedConfigDBConnector.validated_set_entry", mock.Mock(return_value=True))
     @patch("validated_config_db_connector.device_info.is_yang_config_validation_enabled", mock.Mock(return_value=True))
     def test_add_loopback_yang_validation(self):