diff --git a/generic_config_updater/change_applier.py b/generic_config_updater/change_applier.py index 3786e1a372..63ca98049c 100644 --- a/generic_config_updater/change_applier.py +++ b/generic_config_updater/change_applier.py @@ -107,7 +107,7 @@ def _services_validate(self, old_cfg, upd_cfg, keys): for cmd in lst_cmds: ret = self._invoke_cmd(cmd, old_cfg, upd_cfg, keys) - if ret: + if not ret: log_error("service invoked: {} failed with ret={}".format(cmd, ret)) return ret log_debug("service invoked: {}".format(cmd)) diff --git a/generic_config_updater/generic_config_updater.conf.json b/generic_config_updater/generic_config_updater.conf.json index 0951e31be0..417da035cd 100644 --- a/generic_config_updater/generic_config_updater.conf.json +++ b/generic_config_updater/generic_config_updater.conf.json @@ -42,6 +42,9 @@ }, "VLAN": { "services_to_validate": [ "vlan-service" ] + }, + "ACL_RULE": { + "services_to_validate": [ "caclmgrd-service" ] } }, "services": { @@ -59,6 +62,9 @@ }, "vlan-service": { "validate_commands": [ "generic_config_updater.services_validator.vlan_validator" ] + }, + "caclmgrd-service": { + "validate_commands": [ "generic_config_updater.services_validator.caclmgrd_validator" ] } } } diff --git a/generic_config_updater/services_validator.py b/generic_config_updater/services_validator.py index 01bad16d85..b059677c59 100644 --- a/generic_config_updater/services_validator.py +++ b/generic_config_updater/services_validator.py @@ -71,3 +71,30 @@ def vlan_validator(old_config, upd_config, keys): # No update to DHCP servers. return True +def caclmgrd_validator(old_config, upd_config, keys): + old_acltable = old_config.get("ACL_TABLE", {}) + upd_acltable = upd_config.get("ACL_TABLE", {}) + + old_cacltable = [table for table, fields in old_acltable.items() + if fields.get("type", "") == "CTRLPLANE"] + upd_cacltable = [table for table, fields in upd_acltable.items() + if fields.get("type", "") == "CTRLPLANE"] + + old_aclrule = old_config.get("ACL_RULE", {}) + upd_aclrule = upd_config.get("ACL_RULE", {}) + + old_caclrule = [rule for rule in old_aclrule + if rule.split("|")[0] in old_cacltable] + upd_caclrule = [rule for rule in upd_aclrule + if rule.split("|")[0] in upd_cacltable] + + # Only sleep when cacl rule is changed as this will update iptable. + for key in set(old_caclrule).union(set(upd_caclrule)): + if (old_aclrule.get(key, {}) != upd_aclrule.get(key, {})): + # caclmgrd will update in 0.5 sec when configuration stops, + # we sleep 1 sec to make sure it does update. + time.sleep(1) + return True + # No update to ACL_RULE. + return True + diff --git a/tests/generic_config_updater/change_applier_test.py b/tests/generic_config_updater/change_applier_test.py index 55df84c855..63944b2571 100644 --- a/tests/generic_config_updater/change_applier_test.py +++ b/tests/generic_config_updater/change_applier_test.py @@ -158,6 +158,7 @@ def system_health(old_cfg, new_cfg, keys): if svcs != None: assert svc_name in svcs svcs.remove(svc_name) + return True def _validate_keys(keys): @@ -201,11 +202,13 @@ def _validate_svc(svc_name, old_cfg, new_cfg, keys): def acl_validate(old_cfg, new_cfg, keys): debug_print("acl_validate called") _validate_svc("acl_validate", old_cfg, new_cfg, keys) + return True def vlan_validate(old_cfg, new_cfg, keys): debug_print("vlan_validate called") _validate_svc("vlan_validate", old_cfg, new_cfg, keys) + return True class TestChangeApplier(unittest.TestCase): diff --git a/tests/generic_config_updater/service_validator_test.py b/tests/generic_config_updater/service_validator_test.py index dcfc5a41cc..2f51771d33 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 +from generic_config_updater.services_validator import vlan_validator, rsyslog_validator, caclmgrd_validator import generic_config_updater.gu_common @@ -14,6 +14,8 @@ # os_system_calls = [] os_system_call_index = 0 +time_sleep_calls = [] +time_sleep_call_index = 0 msg = "" def mock_os_system_call(cmd): @@ -26,6 +28,15 @@ def mock_os_system_call(cmd): assert cmd == entry["cmd"], msg return entry["rc"] +def mock_time_sleep_call(sleep_time): + global time_sleep_calls, time_sleep_call_index + + assert time_sleep_call_index < len(time_sleep_calls) + entry = time_sleep_calls[time_sleep_call_index] + time_sleep_call_index += 1 + + assert sleep_time == entry["sleep_time"], msg + test_data = [ { "old": {}, "upd": {}, "cmd": "" }, @@ -60,6 +71,77 @@ def mock_os_system_call(cmd): } ] +test_caclrule = [ + { "old": {}, "upd": {}, "sleep_time": 0 }, + { + "old": { + "ACL_TABLE": { "XXX": { "type": "CTRLPLANE" } }, + "ACL_RULE": { "XXX|RULE_1": { "SRC_IP": "10.10.10.10/16" } } + }, + "upd": { + "ACL_TABLE": { "XXX": { "type": "CTRLPLANE" } }, + "ACL_RULE": { "XXX|RULE_1": { "SRC_IP": "10.10.10.10/16" } } + }, + "sleep_time": 0 + }, + { + "old": { + "ACL_TABLE": { + "XXX": { "type": "CTRLPLANE" }, + "YYY": { "type": "L3" } + }, + "ACL_RULE": { + "XXX|RULE_1": { "SRC_IP": "10.10.10.10/16" }, + "YYY|RULE_1": { "SRC_IP": "192.168.1.10/32" } + } + }, + "upd": { + "ACL_TABLE": { + "XXX": { "type": "CTRLPLANE" } + }, + "ACL_RULE": { + "XXX|RULE_1": { "SRC_IP": "10.10.10.10/16" } + } + }, + "sleep_time": 0 + }, + { + "old": { + "ACL_TABLE": { "XXX": { "type": "CTRLPLANE" } }, + "ACL_RULE": { "XXX|RULE_1": { "SRC_IP": "10.10.10.10/16" } } + }, + "upd": { + "ACL_TABLE": { "XXX": { "type": "CTRLPLANE" } }, + "ACL_RULE": { "XXX|RULE_1": { "SRC_IP": "11.11.11.11/16" } } + }, + "sleep_time": 1 + }, + { + "old": { + "ACL_TABLE": { "XXX": { "type": "CTRLPLANE" } }, + "ACL_RULE": { + "XXX|RULE_1": { "SRC_IP": "10.10.10.10/16" } + } + }, + "upd": { + "ACL_TABLE": { "XXX": { "type": "CTRLPLANE" } }, + "ACL_RULE": { + "XXX|RULE_1": { "SRC_IP": "10.10.10.10/16" }, + "XXX|RULE_2": { "SRC_IP": "12.12.12.12/16" } + } + }, + "sleep_time": 1 + }, + { + "old": { + "ACL_TABLE": { "XXX": { "type": "CTRLPLANE" } }, + "ACL_RULE": { "XXX|RULE_1": { "SRC_IP": "10.10.10.10/16" } } + }, + "upd": {}, + "sleep_time": 1 + }, + ] + test_rsyslog_fail = [ # Fail the calls, to get the entire fail path calls invoked # @@ -70,17 +152,14 @@ def mock_os_system_call(cmd): { "cmd": "systemctl restart rsyslog", "rc": 1 }, # restart again; fails ] - 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 + def test_change_apply_os_system(self, mock_os_sys): global os_system_calls, os_system_call_index mock_os_sys.side_effect = mock_os_system_call - i = 0 for entry in test_data: if entry["cmd"]: os_system_calls.append({"cmd": entry["cmd"], "rc": 0 }) @@ -89,6 +168,7 @@ def test_change_apply(self, mock_os_sys): vlan_validator(entry["old"], entry["upd"], None) + # Test failure case # os_system_calls = test_rsyslog_fail @@ -97,3 +177,16 @@ def test_change_apply(self, mock_os_sys): rc = rsyslog_validator("", "", "") assert not rc, "rsyslog_validator expected to fail" + @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 + + mock_time_sleep.side_effect = mock_time_sleep_call + + for entry in test_caclrule: + if entry["sleep_time"]: + time_sleep_calls.append({"sleep_time": entry["sleep_time"]}) + msg = "case failed: {}".format(str(entry)) + + caclmgrd_validator(entry["old"], entry["upd"], None) +