From 9f83ace943bc4938a0bfc75239ecdac8600e5b56 Mon Sep 17 00:00:00 2001
From: jingwenxie <jingwenxie@microsoft.com>
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