diff --git a/generic_config_updater/change_applier.py b/generic_config_updater/change_applier.py
index 3786e1a3722d..63ca98049c0c 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 0951e31be0f3..417da035cd86 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 01bad16d8556..b059677c59cf 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 55df84c855e1..63944b2571cf 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 dcfc5a41cc71..2f51771d3384 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)
+