From 337ae8b2518d606a7b1dccac3ba017c3cac756cb Mon Sep 17 00:00:00 2001 From: Mingjun Zhang <54682183+mingjunzhang2019@users.noreply.github.com> Date: Fri, 28 Jul 2023 16:01:12 -0700 Subject: [PATCH] Add diff and check modes support for NTP module (#281) * Add diff and check modes support for NTP module * Add fragment file * Address review comments * Address review comment * TACACS diff and check modes * Address review comments * Fix sanity errors * Address review comment --- ...ybook-check-diff-modes-for-ntp-module.yaml | 2 + .../network/sonic/config/ntp/ntp.py | 75 +++++++++++------- .../config/radius_server/radius_server.py | 2 + .../config/tacacs_server/tacacs_server.py | 20 +++++ .../sonic/utils/formatted_diff_utils.py | 76 +++++++++++++++++-- 5 files changed, 141 insertions(+), 34 deletions(-) create mode 100644 changelogs/fragments/281-playbook-check-diff-modes-for-ntp-module.yaml diff --git a/changelogs/fragments/281-playbook-check-diff-modes-for-ntp-module.yaml b/changelogs/fragments/281-playbook-check-diff-modes-for-ntp-module.yaml new file mode 100644 index 000000000..360f4297e --- /dev/null +++ b/changelogs/fragments/281-playbook-check-diff-modes-for-ntp-module.yaml @@ -0,0 +1,2 @@ +major_changes: + - ntp_and_tacacs - Playbook check and diff modes support for sonic_ntp and sonic_tacacs_server modules (https://github.com/ansible-collections/dellemc.enterprise_sonic/pull/281). diff --git a/plugins/module_utils/network/sonic/config/ntp/ntp.py b/plugins/module_utils/network/sonic/config/ntp/ntp.py index 7660679ef..41dbe4698 100644 --- a/plugins/module_utils/network/sonic/config/ntp/ntp.py +++ b/plugins/module_utils/network/sonic/config/ntp/ntp.py @@ -28,10 +28,16 @@ from ansible_collections.dellemc.enterprise_sonic.plugins.module_utils.network.sonic.utils.utils import ( get_diff, get_replaced_config, - send_requests, update_states, normalize_interface_name_list ) +from ansible_collections.dellemc.enterprise_sonic.plugins.module_utils.network.sonic.utils.formatted_diff_utils import ( + __DELETE_CONFIG_IF_NO_SUBCONFIG, + __DELETE_LEAFS_OR_CONFIG_IF_NO_NON_KEY_LEAF, + get_new_config, + get_formatted_config_diff +) + from ansible.module_utils.connection import ConnectionError PATCH = 'PATCH' @@ -41,6 +47,11 @@ {"servers": {"address": ""}}, {"ntp_keys": {"key_id": ""}} ] +TEST_KEYS_formatted_diff = [ + {'__delete_op_default': {'__delete_op': __DELETE_LEAFS_OR_CONFIG_IF_NO_NON_KEY_LEAF}}, + {"servers": {"address": "", '__delete_op': __DELETE_CONFIG_IF_NO_SUBCONFIG}}, + {"ntp_keys": {"key_id": "", '__delete_op': __DELETE_CONFIG_IF_NO_SUBCONFIG}} +] class Ntp(ConfigBase): @@ -103,6 +114,17 @@ def execute_module(self): if result['changed']: result['after'] = changed_ntp_facts + new_config = changed_ntp_facts + if self._module.check_mode: + result.pop('after', None) + new_config = get_new_config(commands, existing_ntp_facts, + TEST_KEYS_formatted_diff) + result['after(generated)'] = new_config + + if self._module._diff: + result['config_diff'] = get_formatted_config_diff(existing_ntp_facts, + new_config) + result['warnings'] = warnings return result @@ -220,32 +242,31 @@ def _state_replaced(self, want, have): :returns: the commands necessary to migrate the current configuration to the desired configuration """ + commands = [] + requests = [] replaced_config = get_replaced_config(want, have, TEST_KEYS) + add_commands = [] if replaced_config: self.sort_lists_in_config(replaced_config) self.sort_lists_in_config(have) delete_all = (replaced_config == have) - requests = self.get_delete_requests(replaced_config, delete_all) - send_requests(self._module, requests) + del_requests = self.get_delete_requests(replaced_config, delete_all) + requests.extend(del_requests) + commands.extend(update_states(replaced_config, "deleted")) - commands = want + add_commands = want else: diff = get_diff(want, have, TEST_KEYS) - commands = diff - - requests = [] + add_commands = diff - if commands: - self.preprocess_merge_commands(commands, want) - requests = self.get_merge_requests(commands, have) + if add_commands: + self.preprocess_merge_commands(add_commands, want) + add_requests = self.get_merge_requests(add_commands, have) - if len(requests) > 0: - commands = update_states(commands, "replaced") - else: - commands = [] - else: - commands = [] + if len(add_requests) > 0: + requests.extend(add_requests) + commands.extend(update_states(add_commands, "replaced")) return commands, requests @@ -262,23 +283,23 @@ def _state_overridden(self, want, have): self.sort_lists_in_config(want) self.sort_lists_in_config(have) + commands = [] + requests = [] + if have and have != want: delete_all = True - requests = self.get_delete_requests(have, delete_all) - send_requests(self._module, requests) + del_requests = self.get_delete_requests(have, delete_all) + requests.extend(del_requests) + commands.extend(update_states(have, "deleted")) have = [] - commands = [] - requests = [] - if not have and want: - commands = want - requests = self.get_merge_requests(commands, have) + add_commands = want + add_requests = self.get_merge_requests(add_commands, have) - if len(requests) > 0: - commands = update_states(commands, "overridden") - else: - commands = [] + if len(add_requests) > 0: + requests.extend(add_requests) + commands.extend(update_states(add_commands, "overridden")) return commands, requests diff --git a/plugins/module_utils/network/sonic/config/radius_server/radius_server.py b/plugins/module_utils/network/sonic/config/radius_server/radius_server.py index 4dca5548c..a2f22d8d2 100644 --- a/plugins/module_utils/network/sonic/config/radius_server/radius_server.py +++ b/plugins/module_utils/network/sonic/config/radius_server/radius_server.py @@ -32,6 +32,7 @@ ) from ansible_collections.dellemc.enterprise_sonic.plugins.module_utils.network.sonic.utils.formatted_diff_utils import ( __DELETE_CONFIG_IF_NO_SUBCONFIG, + __DELETE_LEAFS_OR_CONFIG_IF_NO_NON_KEY_LEAF, get_new_config, get_formatted_config_diff ) @@ -42,6 +43,7 @@ {'host': {'name': ''}}, ] TEST_KEYS_formatted_diff = [ + {'__delete_op_default': {'__delete_op': __DELETE_LEAFS_OR_CONFIG_IF_NO_NON_KEY_LEAF}}, {'host': {'name': '', '__delete_op': __DELETE_CONFIG_IF_NO_SUBCONFIG}}, ] diff --git a/plugins/module_utils/network/sonic/config/tacacs_server/tacacs_server.py b/plugins/module_utils/network/sonic/config/tacacs_server/tacacs_server.py index 6bc5dc395..7a813b067 100644 --- a/plugins/module_utils/network/sonic/config/tacacs_server/tacacs_server.py +++ b/plugins/module_utils/network/sonic/config/tacacs_server/tacacs_server.py @@ -30,12 +30,22 @@ get_replaced_config, get_normalize_interface_name, ) +from ansible_collections.dellemc.enterprise_sonic.plugins.module_utils.network.sonic.utils.formatted_diff_utils import ( + __DELETE_CONFIG_IF_NO_SUBCONFIG, + __DELETE_LEAFS_OR_CONFIG_IF_NO_NON_KEY_LEAF, + get_new_config, + get_formatted_config_diff +) PATCH = 'patch' DELETE = 'delete' TEST_KEYS = [ {'host': {'name': ''}}, ] +TEST_KEYS_formatted_diff = [ + {'__delete_op_default': {'__delete_op': __DELETE_LEAFS_OR_CONFIG_IF_NO_NON_KEY_LEAF}}, + {'host': {'name': '', '__delete_op': __DELETE_CONFIG_IF_NO_SUBCONFIG}}, +] class Tacacs_server(ConfigBase): @@ -92,6 +102,16 @@ def execute_module(self): if result['changed']: result['after'] = changed_tacacs_server_facts + new_config = changed_tacacs_server_facts + if self._module.check_mode: + result.pop('after', None) + new_config = get_new_config(commands, existing_tacacs_server_facts, + TEST_KEYS_formatted_diff) + result['after(generated)'] = new_config + + if self._module._diff: + result['config_diff'] = get_formatted_config_diff(existing_tacacs_server_facts, + new_config) result['warnings'] = warnings return result diff --git a/plugins/module_utils/network/sonic/utils/formatted_diff_utils.py b/plugins/module_utils/network/sonic/utils/formatted_diff_utils.py index c01f6c6f5..cf02613be 100644 --- a/plugins/module_utils/network/sonic/utils/formatted_diff_utils.py +++ b/plugins/module_utils/network/sonic/utils/formatted_diff_utils.py @@ -36,11 +36,21 @@ def get_key_sets(dict_conf): # +""" +Delete entire configuration. +""" + + def __DELETE_CONFIG(key_set, command, exist_conf): new_conf = [] return True, new_conf +""" +Delete entire configuration if there is no sub-configuration. +""" + + def __DELETE_CONFIG_IF_NO_SUBCONFIG(key_set, command, exist_conf): nu, dict_list_cmd_key_set = get_key_sets(command) if len(dict_list_cmd_key_set) == 0: @@ -51,6 +61,11 @@ def __DELETE_CONFIG_IF_NO_SUBCONFIG(key_set, command, exist_conf): return False, new_conf +""" +Delete sub-configuration and leaf configuration, if any. +""" + + def __DELETE_SUBCONFIG_AND_LEAFS(key_set, command, exist_conf): new_conf = exist_conf @@ -68,6 +83,11 @@ def __DELETE_SUBCONFIG_AND_LEAFS(key_set, command, exist_conf): return True, new_conf +""" +Delete sub-configuration only, if any. +""" + + def __DELETE_SUBCONFIG_ONLY(key_set, command, exist_conf): new_conf = exist_conf nu, dict_list_cmd_key_set = get_key_sets(command) @@ -78,7 +98,13 @@ def __DELETE_SUBCONFIG_ONLY(key_set, command, exist_conf): return True, new_conf -def __DELETE_OP_DEFAULT(key_set, command, exist_conf): +""" +Delete configuration if there is no non-key leaf, and +delete non-key leaf configuration, if any. +""" + + +def __DELETE_LEAFS_OR_CONFIG_IF_NO_NON_KEY_LEAF(key_set, command, exist_conf): new_conf = exist_conf trival_cmd_key_set, dict_list_cmd_key_set = get_key_sets(command) @@ -94,14 +120,44 @@ def __DELETE_OP_DEFAULT(key_set, command, exist_conf): return False, new_conf +""" +This is default deletion operation. +Delete configuration if there is no non-key leaf, and +delete non-key leaf configuration, if any, if the values of non-key leaf are +equal between command and existing configuration. +""" + + +def __DELETE_OP_DEFAULT(key_set, command, exist_conf): + new_conf = exist_conf + trival_cmd_key_set, dict_list_cmd_key_set = get_key_sets(command) + + if (len(key_set) == len(trival_cmd_key_set)) and \ + (len(dict_list_cmd_key_set) == 0): + new_conf = [] + return True, new_conf + + trival_cmd_key_not_key_set = trival_cmd_key_set.difference(key_set) + for key in trival_cmd_key_not_key_set: + command_val = command.get(key, None) + new_conf_val = new_conf.get(key, None) + if command_val == new_conf_val: + new_conf.pop(key, None) + + return False, new_conf + + def get_test_key_set_and_delete_op(key, test_keys): tst_keys = deepcopy(test_keys) del_op = __DELETE_OP_DEFAULT t_key_set = set() + if not test_keys: + return del_op, t_key_set + if not key: + key = '__delete_op_default' t_keys = next((t_key_item[key] for t_key_item in tst_keys if key in t_key_item), None) if t_keys: - del_op = t_keys.get('__delete_op', __DELETE_OP_DEFAULT) - t_keys.pop('__delete_op', None) + del_op = t_keys.pop('__delete_op', __DELETE_OP_DEFAULT) t_key_set = set(t_keys.keys()) return del_op, t_key_set @@ -109,6 +165,9 @@ def get_test_key_set_and_delete_op(key, test_keys): def get_new_config(commands, exist_conf, test_keys=None): + if not commands: + return exist_conf + cmds = deepcopy(commands) n_conf = list() @@ -138,8 +197,6 @@ def derive_config_from_merged_cmd(command, exist_conf, test_keys=None): if not command: return exist_conf - if not exist_conf: - return command if isinstance(command, list) and isinstance(exist_conf, list): nu, new_conf_dict = derive_config_from_merged_cmd_dict({"config": command}, @@ -269,7 +326,7 @@ def derive_config_from_merged_cmd_dict(command, exist_conf, test_keys=None, key_ def derive_config_from_deleted_cmd(command, exist_conf, test_keys=None): if not command or not exist_conf: - return [] + return exist_conf if isinstance(command, list) and isinstance(exist_conf, list): nu, new_conf_dict = derive_config_from_deleted_cmd_dict({"config": command}, @@ -277,8 +334,11 @@ def derive_config_from_deleted_cmd(command, exist_conf, test_keys=None): test_keys) new_conf = new_conf_dict.get("config", []) elif isinstance(command, dict) and isinstance(exist_conf, dict): + delete_op_dft, key_set = get_test_key_set_and_delete_op('__delete_op_default', + test_keys) nu, new_conf = derive_config_from_deleted_cmd_dict(command, exist_conf, - test_keys) + test_keys, key_set, + delete_op_dft) elif isinstance(command, dict) and isinstance(exist_conf, list): nu, new_conf_dict = derive_config_from_deleted_cmd_dict({"config": [command]}, {"config": exist_conf}, @@ -373,6 +433,8 @@ def derive_config_from_deleted_cmd_dict(command, exist_conf, test_keys=None, key delete_set = e_set.difference(c_set) if delete_set: new_conf[key] = list(delete_set) + else: + new_conf[key] = [] elif new_conf_list: new_conf[key].extend(new_conf_list)