From b31c4c29078e834869ab0178e6bdea2799d667d9 Mon Sep 17 00:00:00 2001 From: Oleksandr Ivantsiv Date: Fri, 21 Jun 2024 09:38:15 -0700 Subject: [PATCH] [dhcp_server] Fix the issue with "kea-dhcp4.conf" file generation for Smart Switch. (#19200) The configuration generated from the template for the Smart Switch contained incorrect data in the "subnet4:id" field. For regular cases, the subnet ID is deduced from the VLAN name. For the Smart Switch, there is always one subnet, and the ID is set to 0. --- .../dhcp_utilities/dhcpservd/dhcp_cfggen.py | 8 ++-- .../dhcp_utilities/dhcpservd/dhcp_lease.py | 16 ++++++- .../tests/test_data/kea-dhcp4.conf.j2 | 1 + .../test_data/kea-lease_smart_switch.csv | 5 ++ .../tests/test_dhcp_cfggen.py | 1 + .../tests/test_dhcp_lease.py | 46 ++++++++++--------- .../tests/test_dhcpservd.py | 5 +- .../tests/test_smart_switch.py | 35 ++++++++++++++ 8 files changed, 90 insertions(+), 27 deletions(-) create mode 100644 src/sonic-dhcp-utilities/tests/test_data/kea-lease_smart_switch.csv diff --git a/src/sonic-dhcp-utilities/dhcp_utilities/dhcpservd/dhcp_cfggen.py b/src/sonic-dhcp-utilities/dhcp_utilities/dhcpservd/dhcp_cfggen.py index eb7ae34d852f..c41e61d5f9f8 100755 --- a/src/sonic-dhcp-utilities/dhcp_utilities/dhcpservd/dhcp_cfggen.py +++ b/src/sonic-dhcp-utilities/dhcp_utilities/dhcpservd/dhcp_cfggen.py @@ -16,6 +16,7 @@ VLAN_MEMBER = "VLAN_MEMBER" DPUS = "DPUS" MID_PLANE_BRIDGE = "MID_PLANE_BRIDGE" +MID_PLANE_BRIDGE_SUBNET_ID = 10000 PORT_MODE_CHECKER = ["DhcpServerTableCfgChangeEventChecker", "DhcpPortTableEventChecker", "DhcpRangeTableEventChecker", "DhcpOptionTableEventChecker", "VlanTableEventChecker", "VlanIntfTableEventChecker", "VlanMemberTableEventChecker"] @@ -90,7 +91,7 @@ def generate(self): port_ips, used_ranges = self._parse_port(port_ipv4, dhcp_interfaces, dhcp_members, ranges) customized_options = self._parse_customized_options(customized_options_ipv4) render_obj, enabled_dhcp_interfaces, used_options, subscribe_table = \ - self._construct_obj_for_template(dhcp_server_ipv4, port_ips, hostname, customized_options) + self._construct_obj_for_template(dhcp_server_ipv4, port_ips, hostname, customized_options, smart_switch) if smart_switch: subscribe_table |= set(SMART_SWITCH_CHECKER) @@ -175,7 +176,7 @@ def _parse_port_map_alias(self): for pc_name in pc_table.keys(): self.port_alias_map[pc_name] = pc_name - def _construct_obj_for_template(self, dhcp_server_ipv4, port_ips, hostname, customized_options): + def _construct_obj_for_template(self, dhcp_server_ipv4, port_ips, hostname, customized_options, smart_switch=False): subnets = [] client_classes = [] enabled_dhcp_interfaces = set() @@ -223,8 +224,9 @@ def _construct_obj_for_template(self, dhcp_server_ipv4, port_ips, hostname, cust "condition": "substring(relay4[1].hex, -{}, {}) == '{}'".format(class_len, class_len, client_class) }) + subnet_obj = { - "id": dhcp_interface_name.replace("Vlan", ""), + "id": MID_PLANE_BRIDGE_SUBNET_ID if smart_switch else dhcp_interface_name.replace("Vlan", ""), "subnet": str(ipaddress.ip_network(dhcp_interface_ip, strict=False)), "pools": pools, "gateway": dhcp_config["gateway"], diff --git a/src/sonic-dhcp-utilities/dhcp_utilities/dhcpservd/dhcp_lease.py b/src/sonic-dhcp-utilities/dhcp_utilities/dhcpservd/dhcp_lease.py index 6dda767287ca..e2e95fb7b532 100644 --- a/src/sonic-dhcp-utilities/dhcp_utilities/dhcpservd/dhcp_lease.py +++ b/src/sonic-dhcp-utilities/dhcp_utilities/dhcpservd/dhcp_lease.py @@ -5,6 +5,7 @@ from abc import abstractmethod from collections import deque from datetime import datetime +from dhcp_utilities.common.utils import is_smart_switch DHCP_SERVER_IPV4_LEASE = "DHCP_SERVER_IPV4_LEASE" KEA_LEASE_FILE_PATH = "/tmp/kea-lease.csv" @@ -29,6 +30,13 @@ def __init__(self, db_connector, lease_update_interval=DEFAULE_LEASE_UPDATE_INTE self.lease_update_interval = lease_update_interval self.last_update_time = None self.lock = threading.Lock() + device_metadata = self.db_connector.get_config_db_table("DEVICE_METADATA") + self.is_smart_switch = is_smart_switch(device_metadata) + + if self.is_smart_switch: + mid_plane_table = self.db_connector.get_config_db_table("MID_PLANE_BRIDGE") + if "GLOBAL" in mid_plane_table and mid_plane_table["GLOBAL"]: + self.midplane_bridge_name = mid_plane_table["GLOBAL"]["bridge"] @abstractmethod def _read(self): @@ -97,6 +105,12 @@ def register(self): """ signal.signal(signal.SIGUSR1, self._update_lease) + def _lease_key(self, subnet_id, mac_address): + if self.is_smart_switch: + return f"{self.midplane_bridge_name}|{mac_address}" + else: + return f"Vlan{subnet_id}|{mac_address}" + def _read(self): # Read lease file generated by kea-dhcp4 try: @@ -120,7 +134,7 @@ def _read(self): lease_end = splits[4] subnet_id = splits[5] - new_key = "{}|{}".format("Vlan" + subnet_id, mac_address) + new_key = self._lease_key(subnet_id, mac_address) if new_key in new_lease: continue new_lease[new_key] = { diff --git a/src/sonic-dhcp-utilities/tests/test_data/kea-dhcp4.conf.j2 b/src/sonic-dhcp-utilities/tests/test_data/kea-dhcp4.conf.j2 index 082cbca14dd5..f5f7a9b054f0 100644 --- a/src/sonic-dhcp-utilities/tests/test_data/kea-dhcp4.conf.j2 +++ b/src/sonic-dhcp-utilities/tests/test_data/kea-dhcp4.conf.j2 @@ -42,6 +42,7 @@ {%- if add_subnet_preceding_comma.flag -%},{%- endif -%} {%- set _dummy = add_subnet_preceding_comma.update({'flag': True}) %} { + "id": {{ subnet_info["id"] }}, "subnet": "{{ subnet_info["subnet"] }}", "pools": [ {%- set add_pool_preceding_comma = { 'flag': False } %} diff --git a/src/sonic-dhcp-utilities/tests/test_data/kea-lease_smart_switch.csv b/src/sonic-dhcp-utilities/tests/test_data/kea-lease_smart_switch.csv new file mode 100644 index 000000000000..ea7d4ed2cbc4 --- /dev/null +++ b/src/sonic-dhcp-utilities/tests/test_data/kea-lease_smart_switch.csv @@ -0,0 +1,5 @@ +address,hwaddr,client_id,valid_lifetime,expire,subnet_id,fqdn_fwd,fqdn_rev,hostname,state,user_context +169.254.200.4,aa:bb:cc:dd:ff:04,,900,1718053209,10000,0,0,sonic,0, +169.254.200.1,aa:bb:cc:dd:ff:01,,900,1718053209,10000,0,0,sonic,0, +169.254.200.3,aa:bb:cc:dd:ff:03,,900,1718053210,10000,0,0,sonic,0, +169.254.200.2,aa:bb:cc:dd:ff:02,,900,1718053210,10000,0,0,sonic,0, \ No newline at end of file diff --git a/src/sonic-dhcp-utilities/tests/test_dhcp_cfggen.py b/src/sonic-dhcp-utilities/tests/test_dhcp_cfggen.py index 723731839649..3548e39c1435 100644 --- a/src/sonic-dhcp-utilities/tests/test_dhcp_cfggen.py +++ b/src/sonic-dhcp-utilities/tests/test_dhcp_cfggen.py @@ -42,6 +42,7 @@ }, "subnet4": [ { + "id": 1000, "subnet": "192.168.0.0/21", "pools": [ { diff --git a/src/sonic-dhcp-utilities/tests/test_dhcp_lease.py b/src/sonic-dhcp-utilities/tests/test_dhcp_lease.py index a831b2ad5a11..04009d2ee0f5 100644 --- a/src/sonic-dhcp-utilities/tests/test_dhcp_lease.py +++ b/src/sonic-dhcp-utilities/tests/test_dhcp_lease.py @@ -3,6 +3,7 @@ from freezegun import freeze_time from swsscommon import swsscommon from unittest.mock import patch, call, MagicMock +from common_utils import mock_get_config_db_table expected_lease = { "Vlan1000|10:70:fd:b6:13:00": { @@ -34,20 +35,22 @@ def test_read_kea_lease_with_file_not_found(mock_swsscommon_dbconnector_init): - db_connector = DhcpDbConnector() - kea_lease_handler = KeaDhcp4LeaseHandler(db_connector) - try: - kea_lease_handler._read() - except FileNotFoundError: - pass + with patch.object(DhcpDbConnector, "get_config_db_table", side_effect=mock_get_config_db_table): + db_connector = DhcpDbConnector() + kea_lease_handler = KeaDhcp4LeaseHandler(db_connector) + try: + kea_lease_handler._read() + except FileNotFoundError: + pass def test_read_kea_lease(mock_swsscommon_dbconnector_init): - db_connector = DhcpDbConnector() - kea_lease_handler = KeaDhcp4LeaseHandler(db_connector, lease_file="tests/test_data/kea-lease.csv") - # Verify whether lease information read is as expected - lease = kea_lease_handler._read() - assert lease == expected_lease + with patch.object(DhcpDbConnector, "get_config_db_table", side_effect=mock_get_config_db_table): + db_connector = DhcpDbConnector() + kea_lease_handler = KeaDhcp4LeaseHandler(db_connector, lease_file="tests/test_data/kea-lease.csv") + # Verify whether lease information read is as expected + lease = kea_lease_handler._read() + assert lease == expected_lease # Cannot mock built-in/extension type function(datetime.datetime.timestamp), need to free time @@ -88,13 +91,14 @@ def test_update_kea_lease(mock_swsscommon_dbconnector_init, mock_swsscommon_tabl def test_no_implement(mock_swsscommon_dbconnector_init): - db_connector = DhcpDbConnector() - lease_handler = LeaseHanlder(db_connector) - try: - lease_handler._read() - except NotImplementedError: - pass - try: - lease_handler.register() - except NotImplementedError: - pass + with patch.object(DhcpDbConnector, "get_config_db_table", side_effect=mock_get_config_db_table): + db_connector = DhcpDbConnector() + lease_handler = LeaseHanlder(db_connector) + try: + lease_handler._read() + except NotImplementedError: + pass + try: + lease_handler.register() + except NotImplementedError: + pass diff --git a/src/sonic-dhcp-utilities/tests/test_dhcpservd.py b/src/sonic-dhcp-utilities/tests/test_dhcpservd.py index f11ffdbba0b5..4c8175b7e0bc 100644 --- a/src/sonic-dhcp-utilities/tests/test_dhcpservd.py +++ b/src/sonic-dhcp-utilities/tests/test_dhcpservd.py @@ -4,7 +4,7 @@ import signal import sys import time -from common_utils import MockProc +from common_utils import MockProc, mock_get_config_db_table from dhcp_utilities.common.utils import DhcpDbConnector from dhcp_utilities.common.dhcp_db_monitor import DhcpServdDbMonitor from dhcp_utilities.dhcpservd.dhcp_cfggen import DhcpServCfgGenerator @@ -110,7 +110,8 @@ def test_update_dhcp_server_ip(mock_swsscommon_dbconnector_init, mock_parse_port def test_start(mock_swsscommon_dbconnector_init, mock_parse_port_map_alias, mock_get_render_template): with patch.object(DhcpServd, "dump_dhcp4_config") as mock_dump, \ patch.object(DhcpServd, "_update_dhcp_server_ip") as mock_update_dhcp_server_ip, \ - patch.object(DhcpServdDbMonitor, "enable_checkers"): + patch.object(DhcpServdDbMonitor, "enable_checkers"), \ + patch.object(DhcpDbConnector, "get_config_db_table", side_effect=mock_get_config_db_table): dhcp_db_connector = DhcpDbConnector() dhcp_cfg_generator = DhcpServCfgGenerator(dhcp_db_connector, "/usr/local/lib/kea/hooks/libdhcp_run_script.so") dhcpservd = DhcpServd(dhcp_cfg_generator, dhcp_db_connector, MagicMock()) diff --git a/src/sonic-dhcp-utilities/tests/test_smart_switch.py b/src/sonic-dhcp-utilities/tests/test_smart_switch.py index 0347d5df2f6d..a5c6be2fd273 100644 --- a/src/sonic-dhcp-utilities/tests/test_smart_switch.py +++ b/src/sonic-dhcp-utilities/tests/test_smart_switch.py @@ -1,6 +1,7 @@ import json import pytest from common_utils import MockConfigDb, dhcprelayd_refresh_dhcrelay_test, dhcprelayd_proceed_with_check_res_test +from dhcp_utilities.dhcpservd.dhcp_lease import KeaDhcp4LeaseHandler from dhcp_utilities.dhcprelayd.dhcprelayd import DHCP_SERVER_CHECKER, MID_PLANE_CHECKER from dhcp_utilities.dhcpservd.dhcp_cfggen import DhcpServCfgGenerator from dhcp_utilities.common.utils import DhcpDbConnector @@ -35,6 +36,7 @@ }, "subnet4": [ { + "id": 10000, "subnet": "169.254.200.0/24", "pools": [ { @@ -103,6 +105,30 @@ } +expected_lease = { + 'bridge_midplane|aa:bb:cc:dd:ff:01': { + 'ip': '169.254.200.1', + 'lease_end': '1718053209', + 'lease_start': '1718052309' + }, + 'bridge_midplane|aa:bb:cc:dd:ff:02': { + 'ip': '169.254.200.2', + 'lease_end': '1718053210', + 'lease_start': '1718052310' + }, + 'bridge_midplane|aa:bb:cc:dd:ff:03': { + 'ip': '169.254.200.3', + 'lease_end': '1718053210', + 'lease_start': '1718052310' + }, + 'bridge_midplane|aa:bb:cc:dd:ff:04': { + 'ip': '169.254.200.4', + 'lease_end': '1718053209', + 'lease_start': '1718052309' + } +} + + def test_dhcprelayd_refresh_dhcrelay(mock_swsscommon_dbconnector_init): expected_checkers = set(["MidPlaneTableEventChecker"]) dhcprelayd_refresh_dhcrelay_test(expected_checkers, True, mock_get_config_db_table) @@ -140,3 +166,12 @@ def test_dhcp_dhcp_cfggen_generate(mock_swsscommon_dbconnector_init, mock_parse_ def mock_get_config_db_table(table_name): mock_config_db = MockConfigDb(MOCK_CONFIG_DB_PATH_SMART_SWITCH) return mock_config_db.get_config_db_table(table_name) + + +def test_read_kea_lease(mock_swsscommon_dbconnector_init): + with patch.object(DhcpDbConnector, "get_config_db_table", side_effect=mock_get_config_db_table): + db_connector = DhcpDbConnector() + kea_lease_handler = KeaDhcp4LeaseHandler(db_connector, lease_file="tests/test_data/kea-lease_smart_switch.csv") + # Verify whether lease information read is as expected + lease = kea_lease_handler._read() + assert lease == expected_lease