generic_config_updater: Filename changed & VLAN validator added (#1919)

#### What I did
Change filename per review comments from last PR.
Removed print statement from change_applier
Added vlan validator & test code for the same

Author: renukamanavalan

generic_config_updater/change_applier.py

@@ -9,29 +9,27 @@
 from .gu_common import genericUpdaterLogging
 
 SCRIPT_DIR = os.path.dirname(os.path.realpath(__file__))
-UPDATER_CONF_FILE = f"{SCRIPT_DIR}/generic_updater_config.conf.json"
+UPDATER_CONF_FILE = f"{SCRIPT_DIR}/generic_config_updater.conf.json"
 logger = genericUpdaterLogging.get_logger(title="Change Applier")
 
 print_to_console = False
-print_to_stdout = False
 
-def set_print_options(to_console=False, to_stdout=False):
-    global print_to_console, print_to_stdout
+def set_verbose(verbose=False):
+    global print_to_console, logger
 
-    print_to_console = to_console
-    print_to_stdout = to_stdout
+    print_to_console = verbose
+    if verbose:
+        logger.set_min_log_priority_debug()
+    else:
+        logger.set_min_log_priority_notice()
 
 
 def log_debug(m):
-    logger.log_debug(m, print_to_console)
-    if print_to_stdout:
-        print(m)
+    logger.log(logger.LOG_PRIORITY_DEBUG, m, print_to_console)
 
 
 def log_error(m):
-    logger.log_error(m, print_to_console)
-    if print_to_stdout:
-        print(m)
+    logger.log(logger.LOG_PRIORITY_ERROR, m, print_to_console)
 
 
 def get_config_db():

generic_config_updater/generic_updater_config.conf.json renamed to generic_config_updater/generic_config_updater.conf.json

@@ -39,6 +39,9 @@
         },
         "DHCP_SERVER": {
             "services_to_validate": [ "dhcp-relay" ]
+        },
+        "VLAN": {
+            "services_to_validate": [ "vlan-service" ]
         }
     },
     "services": {
@@ -49,10 +52,13 @@
             "validate_commands": [ ]
         },
         "rsyslog": {
-            "validate_commands": [ "generic_config_updater.services_validator.ryslog_validator" ]
+            "validate_commands": [ "generic_config_updater.services_validator.rsyslog_validator" ]
         },
         "dhcp-relay": {
             "validate_commands": [ "generic_config_updater.services_validator.dhcp_validator" ]
+        },
+        "vlan-service": {
+            "validate_commands": [ "generic_config_updater.services_validator.vlan_validator" ]
         }
     }
 }

generic_config_updater/services_validator.py

@@ -3,15 +3,41 @@
 
 logger = genericUpdaterLogging.get_logger(title="Service Validator")
 
+print_to_console = False
+
+def set_verbose(verbose=False):
+    global print_to_console, logger
+
+    print_to_console = verbose
+    if verbose:
+        logger.set_min_log_priority_debug()
+    else:
+        logger.set_min_log_priority_notice()
+
+
 def _service_restart(svc_name):
-    os.system(f"systemctl restart {svc_name}")
-    logger.log_notice(f"Restarted {svc_name}")
+    rc = os.system(f"systemctl restart {svc_name}")
+    logger.log(logger.LOG_PRIORITY_NOTICE,
+            f"Restarted {svc_name}", print_to_console)
+    return rc == 0
 
 
-def ryslog_validator(old_config, upd_config, keys):
-    _service_restart("rsyslog-config")
+def rsyslog_validator(old_config, upd_config, keys):
+    return _service_restart("rsyslog-config")
 
 
 def dhcp_validator(old_config, upd_config, keys):
-    _service_restart("dhcp_relay")
+    return _service_restart("dhcp_relay")
+
+
+def vlan_validator(old_config, upd_config, keys):
+    old_vlan = old_config.get("VLAN", {})
+    upd_vlan = upd_config.get("VLAN", {})
+
+    for key in set(old_vlan.keys()).union(set(upd_vlan.keys())):
+        if (old_vlan.get(key, {}).get("dhcp_servers", []) != 
+                upd_vlan.get(key, {}).get("dhcp_servers", [])):
+            return _service_restart("dhcp_relay")
+    # No update to DHCP servers.
+    return True
 

setup.py

@@ -62,7 +62,7 @@
         'sonic_cli_gen',
     ],
     package_data={
-        'generic_config_updater': ['generic_updater_config.conf.json'],
+        'generic_config_updater': ['generic_config_updater.conf.json'],
         'show': ['aliases.ini'],
         'sonic_installer': ['aliases.ini'],
         'tests': ['acl_input/*',

tests/generic_config_updater/change_applier_test.py

@@ -7,6 +7,7 @@
 from unittest.mock import patch
 
 import generic_config_updater.change_applier
+import generic_config_updater.services_validator
 import generic_config_updater.gu_common
 
 SCRIPT_DIR = os.path.dirname(os.path.realpath(__file__))
@@ -227,7 +228,8 @@ def test_change_apply(self, mock_set, mock_db, mock_os_sys):
         json_changes = copy.deepcopy(read_data["json_changes"])
 
         generic_config_updater.change_applier.UPDATER_CONF_FILE = CONF_FILE
-        generic_config_updater.change_applier.set_print_options(to_stdout=True)
+        generic_config_updater.change_applier.set_verbose(True)
+        generic_config_updater.services_validator.set_verbose(True)
 
         applier = generic_config_updater.change_applier.ChangeApplier()
         debug_print("invoked applier")

tests/generic_config_updater/service_validator_test.py

@@ -0,0 +1,71 @@
+import copy
+import json
+import jsondiff
+import os
+import unittest
+from collections import defaultdict
+from unittest.mock import patch
+
+from generic_config_updater.services_validator import vlan_validator
+import generic_config_updater.gu_common
+
+
+# Mimics os.system call
+#
+os_system_expected_cmd = ""
+msg = ""
+
+def os_system_cfggen(cmd):
+    assert cmd == os_system_expected_cmd, msg
+    return 0
+
+
+test_data = [
+        { "old": {}, "upd": {}, "cmd": "" },
+        {
+            "old": { "VLAN": { "XXX": { "dhcp_servers": [ "10.10.10.10" ] } } },
+            "upd": { "VLAN": { "XXX": { "dhcp_servers": [ "10.10.10.10" ] } } },
+            "cmd": ""
+        },
+        {
+            "old": { "VLAN": { "XXX": { "dhcp_servers": [ "10.10.10.10" ] } } },
+            "upd": { "VLAN": { "XXX": { "dhcp_servers": [ "10.10.10.11" ] } } },
+            "cmd": "systemctl restart dhcp_relay"
+        },
+        {
+            "old": { "VLAN": { "XXX": { "dhcp_servers": [ "10.10.10.10" ] } } },
+            "upd": { "VLAN": {
+                "XXX": { "dhcp_servers": [ "10.10.10.10" ] },
+                "YYY": { "dhcp_servers": [ ] } } },
+            "cmd": ""
+        },
+        {
+            "old": { "VLAN": { "XXX": { "dhcp_servers": [ "10.10.10.10" ] } } },
+            "upd": { "VLAN": {
+                "XXX": { "dhcp_servers": [ "10.10.10.10" ] },
+                "YYY": { "dhcp_servers": [ "10.12.12.1" ] } } },
+            "cmd": "systemctl restart dhcp_relay"
+        },
+        {
+            "old": { "VLAN": { "XXX": { "dhcp_servers": [ "10.10.10.10" ] } } },
+            "upd": {},
+            "cmd": "systemctl restart dhcp_relay"
+        }
+    ]
+
+class TestServiceValidator(unittest.TestCase):
+
+    @patch("generic_config_updater.change_applier.os.system")
+    def test_change_apply(self, mock_os_sys):
+        global os_system_expected_cmd
+
+        mock_os_sys.side_effect = os_system_cfggen
+
+        i = 0
+        for entry in test_data:
+            os_system_expected_cmd = entry["cmd"]
+            msg = "case failed: {}".format(str(entry))
+
+            vlan_validator(entry["old"], entry["upd"], None)
+
+