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

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).

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
 

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}},
 ]
 

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
 

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,21 +120,54 @@ 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
 
 
 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,16 +326,19 @@ 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},
  {"config": exist_conf},
  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)