From 9f83ace943bc4938a0bfc75239ecdac8600e5b56 Mon Sep 17 00:00:00 2001 From: jingwenxie Date: Thu, 9 Mar 2023 09:12:19 +0800 Subject: [PATCH] [GCU] Add vlanintf-validator (#2697) What I did Fix the bug of GCU vlan interface modification. It should call ip neigh flush dev after removing interface ip. The fix is basically following config CLI's tradition. How I did it Add vlanintf service validator to check if extra step of ip neigh flush is needed. How to verify it GCU E2E test in dualtor testbed. --- .../gcu_services_validator.conf.json | 6 +++ generic_config_updater/services_validator.py | 21 ++++++++ .../service_validator_test.py | 51 ++++++++++++++++++- 3 files changed, 77 insertions(+), 1 deletion(-) diff --git a/generic_config_updater/gcu_services_validator.conf.json b/generic_config_updater/gcu_services_validator.conf.json index 907b5a6863..852b587286 100644 --- a/generic_config_updater/gcu_services_validator.conf.json +++ b/generic_config_updater/gcu_services_validator.conf.json @@ -48,6 +48,9 @@ }, "NTP_SERVER": { "services_to_validate": [ "ntp-service" ] + }, + "VLAN_INTERFACE": { + "services_to_validate": [ "vlanintf-service" ] } }, "services": { @@ -71,6 +74,9 @@ }, "ntp-service": { "validate_commands": [ "generic_config_updater.services_validator.ntp_validator" ] + }, + "vlanintf-service": { + "validate_commands": [ "generic_config_updater.services_validator.vlanintf_validator" ] } } } diff --git a/generic_config_updater/services_validator.py b/generic_config_updater/services_validator.py index 44a9e095eb..5d8c1f0d51 100644 --- a/generic_config_updater/services_validator.py +++ b/generic_config_updater/services_validator.py @@ -101,3 +101,24 @@ def caclmgrd_validator(old_config, upd_config, keys): def ntp_validator(old_config, upd_config, keys): return _service_restart("ntp-config") + +def vlanintf_validator(old_config, upd_config, keys): + old_vlan_intf = old_config.get("VLAN_INTERFACE", {}) + upd_vlan_intf = upd_config.get("VLAN_INTERFACE", {}) + + # Get the tuple with format (iface, iface_ip) then check deleted tuple + # Example: + # old_keys = [("Vlan1000", "192.168.0.1")] + # upd_keys = [("Vlan1000", "192.168.0.2")] + old_keys = [ tuple(key.split("|")) + for key in old_vlan_intf if len(key.split("|")) == 2 ] + upd_keys = [ tuple(key.split("|")) + for key in upd_vlan_intf if len(key.split("|")) == 2 ] + + deleted_keys = list(set(old_keys) - set(upd_keys)) + for key in deleted_keys: + iface, iface_ip = key + rc = os.system(f"ip neigh flush dev {iface} {iface_ip}") + if not rc: + return False + return True diff --git a/tests/generic_config_updater/service_validator_test.py b/tests/generic_config_updater/service_validator_test.py index 2f51771d33..f14a3ad7b0 100644 --- a/tests/generic_config_updater/service_validator_test.py +++ b/tests/generic_config_updater/service_validator_test.py @@ -6,7 +6,7 @@ from collections import defaultdict from unittest.mock import patch -from generic_config_updater.services_validator import vlan_validator, rsyslog_validator, caclmgrd_validator +from generic_config_updater.services_validator import vlan_validator, rsyslog_validator, caclmgrd_validator, vlanintf_validator import generic_config_updater.gu_common @@ -152,6 +152,46 @@ def mock_time_sleep_call(sleep_time): { "cmd": "systemctl restart rsyslog", "rc": 1 }, # restart again; fails ] +test_vlanintf_data = [ + { "old": {}, "upd": {}, "cmd": "" }, + { + "old": { "VLAN_INTERFACE": { + "Vlan1000": {}, + "Vlan1000|192.168.0.1/21": {} } }, + "upd": { "VLAN_INTERFACE": { + "Vlan1000": {}, + "Vlan1000|192.168.0.1/21": {} } }, + "cmd": "" + }, + { + "old": { "VLAN_INTERFACE": { + "Vlan1000": {}, + "Vlan1000|192.168.0.1/21": {} } }, + "upd": { "VLAN_INTERFACE": { + "Vlan1000": {}, + "Vlan1000|192.168.0.2/21": {} } }, + "cmd": "ip neigh flush dev Vlan1000 192.168.0.1/21" + }, + { + "old": { "VLAN_INTERFACE": { + "Vlan1000": {}, + "Vlan1000|192.168.0.1/21": {} } }, + "upd": { "VLAN_INTERFACE": { + "Vlan1000": {}, + "Vlan1000|192.168.0.1/21": {}, + "Vlan1000|192.168.0.2/21": {} } }, + "cmd": "" + }, + { + "old": { "VLAN_INTERFACE": { + "Vlan1000": {}, + "Vlan1000|192.168.0.1/21": {} } }, + "upd": {}, + "cmd": "ip neigh flush dev Vlan1000 192.168.0.1/21" + } + ] + + class TestServiceValidator(unittest.TestCase): @patch("generic_config_updater.change_applier.os.system") @@ -177,6 +217,15 @@ def test_change_apply_os_system(self, mock_os_sys): rc = rsyslog_validator("", "", "") assert not rc, "rsyslog_validator expected to fail" + os_system_calls = [] + os_system_call_index = 0 + for entry in test_vlanintf_data: + if entry["cmd"]: + os_system_calls.append({"cmd": entry["cmd"], "rc": 0 }) + msg = "case failed: {}".format(str(entry)) + + vlanintf_validator(entry["old"], entry["upd"], None) + @patch("generic_config_updater.services_validator.time.sleep") def test_change_apply_time_sleep(self, mock_time_sleep): global time_sleep_calls, time_sleep_call_index