Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[generic-config-updater] Add caclrule validator #2103

Merged
merged 5 commits into from
Mar 21, 2022
Merged

[generic-config-updater] Add caclrule validator #2103

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0998989
add caclrule validator
wen587 Mar 15, 2022
2c979f3
only sleep for cacl rule
wen587 Mar 15, 2022
13f88c1
fix unit test
wen587 Mar 16, 2022
e75de41
rename to caclmgrd service validator; optimize loop
wen587 Mar 16, 2022
7c1d162
replace os sleep with builtin time.sleep
wen587 Mar 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion generic_config_updater/change_applier.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
6 changes: 6 additions & 0 deletions generic_config_updater/generic_config_updater.conf.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@
},
"VLAN": {
"services_to_validate": [ "vlan-service" ]
},
"ACL_RULE": {
"services_to_validate": [ "caclrule-service" ]
wen587 marked this conversation as resolved.
Show resolved Hide resolved
}
},
"services": {
Expand All @@ -59,6 +62,9 @@
},
"vlan-service": {
"validate_commands": [ "generic_config_updater.services_validator.vlan_validator" ]
},
"caclrule-service": {
"validate_commands": [ "generic_config_updater.services_validator.caclrule_validator" ]
}
}
}
Expand Down
32 changes: 32 additions & 0 deletions generic_config_updater/services_validator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,35 @@ def vlan_validator(old_config, upd_config, keys):
# No update to DHCP servers.
return True

def caclrule_validator(old_config, upd_config, keys):
old_acltable = old_config.get("ACL_TABLE", {})
upd_acltable = upd_config.get("ACL_TABLE", {})

try:
old_cacltable = [table for table in old_acltable
wen587 marked this conversation as resolved.
Show resolved Hide resolved
if old_acltable.get(table)["type"] == "CTRLPLANE"]
upd_cacltable = [table for table in upd_acltable
if upd_acltable.get(table)["type"] == "CTRLPLANE"]
except KeyError:
logger.log(logger.LOG_PRIORITY_ERROR,
"Field 'type' is required in ACL_TABLE",
print_to_console)
return False

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]

for key in set(old_caclrule).union(set(upd_caclrule)):
if (old_aclrule.get(key, {}) != upd_aclrule.get(key, {})):
wen587 marked this conversation as resolved.
Show resolved Hide resolved
# caclmgrd will update in 0.5 sec when configuration stops,
# we sleep 1 sec to make sure it does update.
rc = os.system("sleep 1s")
Copy link
Contributor

@qiluo-msft qiluo-msft Mar 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

os.system("sleep 1s")

python has time.sleep(). #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to check the test cmds https://github.com/Azure/sonic-utilities/blob/13f88c1aad4c6a5293fef978b7affb95335d1f04/tests/generic_config_updater/service_validator_test.py#L106 also avoid conflict use with
https://github.com/Azure/sonic-utilities/blob/13f88c1aad4c6a5293fef978b7affb95335d1f04/generic_config_updater/services_validator.py#L37

return rc == 0
# No update to ACL_RULE.
return True

3 changes: 3 additions & 0 deletions tests/generic_config_updater/change_applier_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down
82 changes: 80 additions & 2 deletions tests/generic_config_updater/service_validator_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, caclrule_validator
import generic_config_updater.gu_common


Expand Down Expand Up @@ -60,6 +60,77 @@ def mock_os_system_call(cmd):
}
]

test_caclrule = [
{ "old": {}, "upd": {}, "cmd": "" },
{
"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" } }
},
"cmd": ""
},
{
"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" }
}
},
"cmd": ""
},
{
"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" } }
},
"cmd": "sleep 1s"
},
{
"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" }
}
},
"cmd": "sleep 1s"
},
{
"old": {
"ACL_TABLE": { "XXX": { "type": "CTRLPLANE" } },
"ACL_RULE": { "XXX|RULE_1": { "SRC_IP": "10.10.10.10/16" } }
},
"upd": {},
"cmd": "sleep 1s"
},
]

test_rsyslog_fail = [
# Fail the calls, to get the entire fail path calls invoked
#
Expand All @@ -80,7 +151,6 @@ def test_change_apply(self, mock_os_sys):

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 })
Expand All @@ -89,6 +159,14 @@ def test_change_apply(self, mock_os_sys):
vlan_validator(entry["old"], entry["upd"], None)


for entry in test_caclrule:
if entry["cmd"]:
os_system_calls.append({"cmd": entry["cmd"], "rc": 0 })
msg = "case failed: {}".format(str(entry))

caclrule_validator(entry["old"], entry["upd"], None)


# Test failure case
#
os_system_calls = test_rsyslog_fail
Expand Down