diff --git a/changelogs/fragments/301-playbook-check-diff-modes-for-vlans-interfaces.yaml b/changelogs/fragments/301-playbook-check-diff-modes-for-vlans-interfaces.yaml new file mode 100644 index 000000000..6ef1202c9 --- /dev/null +++ b/changelogs/fragments/301-playbook-check-diff-modes-for-vlans-interfaces.yaml @@ -0,0 +1,2 @@ +major_changes: + - vlans_interfaces - Playbook check and diff modes supports for sonic_vlans and sonic_interfaces modules (https://github.com/ansible-collections/dellemc.enterprise_sonic/pull/301). diff --git a/plugins/module_utils/network/sonic/config/interfaces/interfaces.py b/plugins/module_utils/network/sonic/config/interfaces/interfaces.py index 9fcb516e9..58e974431 100644 --- a/plugins/module_utils/network/sonic/config/interfaces/interfaces.py +++ b/plugins/module_utils/network/sonic/config/interfaces/interfaces.py @@ -18,6 +18,14 @@ except ImportError: from urllib.parse import quote +""" +The use of natsort causes sanity error due to it is not available in python version currently used. +When natsort becomes available, the code here and below using it will be applied. +from natsort import ( + natsorted, + ns +) +""" from ansible_collections.ansible.netcommon.plugins.module_utils.network.common.cfg.base import ( ConfigBase, ) @@ -39,10 +47,19 @@ update_states, normalize_interface_name ) +from ansible_collections.dellemc.enterprise_sonic.plugins.module_utils.network.sonic.utils.formatted_diff_utils import ( + __DELETE_CONFIG_IF_NO_SUBCONFIG, + get_new_config, + get_formatted_config_diff +) from ansible.module_utils._text import to_native from ansible.module_utils.connection import ConnectionError import traceback +TEST_KEYS_formatted_diff = [ + {'config': {'name': '', '__delete_op': __DELETE_CONFIG_IF_NO_SUBCONFIG}}, +] + LIB_IMP_ERR = None ERR_MSG = None try: @@ -118,6 +135,22 @@ def execute_module(self): if result['changed']: result['after'] = changed_interfaces_facts + new_config = changed_interfaces_facts + old_config = existing_interfaces_facts + if self._module.check_mode: + result.pop('after', None) + new_config = get_new_config(commands, existing_interfaces_facts, + TEST_KEYS_formatted_diff) + # See the above comment about natsort module + # new_config = natsorted(new_config, key=lambda x: x['name']) + # For time-being, use simple "sort" + new_config.sort(key=lambda x: x['name']) + result['after(generated)'] = new_config + old_config.sort(key=lambda x: x['name']) + + if self._module._diff: + result['config_diff'] = get_formatted_config_diff(old_config, + new_config) result['warnings'] = warnings return result @@ -173,10 +206,19 @@ def _state_replaced(self, want, have, diff): to the desired configuration """ commands = self.filter_comands_to_change(diff, have) - requests = self.get_delete_interface_requests(commands, have) - requests.extend(self.get_modify_interface_requests(commands, have)) + commands_del = self.filter_comands_to_delete(commands, have) + requests = self.get_delete_interface_requests(commands_del, have) + commands_mer = self.filter_comands_to_change(commands, have) + requests.extend(self.get_modify_interface_requests(commands_mer, have)) if commands and len(requests) > 0: - commands = update_states(commands, "replaced") + commands_dlt, commands_rep = self.classify_delete_commands(commands_del) + commands = [] + if commands_dlt: + commands.extend(update_states(commands_dlt, "deleted")) + if commands_rep: + commands.extend(update_states(commands_rep, "replaced")) + if commands_mer: + commands.extend(update_states(commands_mer, "replaced")) else: commands = [] @@ -192,14 +234,18 @@ def _state_overridden(self, want, have, diff): to the desired configuration """ commands = [] - commands_del = self.filter_comands_to_change(want, have) + commands_chg = self.filter_comands_to_change(want, have) + commands_del = self.filter_comands_to_delete(commands_chg, have) requests = self.get_delete_interface_requests(commands_del, have) del_req_count = len(requests) if commands_del and del_req_count > 0: - commands_del = update_states(commands_del, "deleted") - commands.extend(commands_del) + commands_dlt, commands_ovr = self.classify_delete_commands(commands_del) + if commands_dlt: + commands.extend(update_states(commands_dlt, "deleted")) + if commands_ovr: + commands.extend(update_states(commands_ovr, "overridden")) - commands_over = diff + commands_over = self.filter_comands_to_change(diff, have) requests.extend(self.get_modify_interface_requests(commands_over, have)) if commands_over and len(requests) > del_req_count: commands_over = update_states(commands_over, "overridden") @@ -216,7 +262,7 @@ def _state_merged(self, want, have, diff): :returns: the commands necessary to merge the provided into the current configuration """ - commands = diff + commands = self.filter_comands_to_change(diff, have) requests = self.get_modify_interface_requests(commands, have) if commands and len(requests) > 0: commands = update_states(commands, "merged") @@ -241,10 +287,16 @@ def _state_deleted(self, want, have, diff): else: commands = want - requests = self.get_delete_interface_requests(commands, have) + commands_del = self.filter_comands_to_delete(commands, have) + requests = self.get_delete_interface_requests(commands_del, have) - if commands and len(requests) > 0: - commands = update_states(commands, "deleted") + if commands_del and len(requests) > 0: + commands_dlt, commands_mer = self.classify_delete_commands(commands_del) + commands = [] + if commands_dlt: + commands.extend(update_states(commands_dlt, "deleted")) + if commands_mer: + commands.extend(update_states(commands_mer, "merged")) else: commands = [] @@ -255,15 +307,30 @@ def filter_comands_to_delete(self, configs, have): for conf in configs: if self.is_this_delete_required(conf, have): + intf_name = conf['name'] + temp_conf = dict() temp_conf['name'] = conf['name'] - temp_conf['description'] = '' - temp_conf['mtu'] = 9100 - temp_conf['enabled'] = True - temp_conf['speed'] = 'SPEED_DEFAULT' - temp_conf['auto_negotiate'] = False - temp_conf['fec'] = 'FEC_DISABLED' - temp_conf['advertised_speed'] = '' + if intf_name == 'Management0': + temp_conf['description'] = 'Management0' + temp_conf['mtu'] = 1500 + temp_conf['enabled'] = True + temp_conf['speed'] = None + temp_conf['auto_negotiate'] = None + temp_conf['fec'] = None + else: + temp_conf['description'] = '' + temp_conf['mtu'] = 9100 + temp_conf['enabled'] = False + if intf_name.startswith('Eth'): + temp_conf['speed'] = self.retrieve_default_intf_speed(conf['name']) + temp_conf['auto_negotiate'] = False + temp_conf['fec'] = 'FEC_DISABLED' + else: + temp_conf['speed'] = None + temp_conf['auto_negotiate'] = None + temp_conf['fec'] = None + temp_conf['advertised_speed'] = None commands.append(temp_conf) return commands @@ -277,15 +344,11 @@ def filter_comands_to_change(self, configs, have): def get_modify_interface_requests(self, configs, have): self.delete_flag = False - commands = self.filter_comands_to_change(configs, have) - - return self.get_interface_requests(commands, have) + return self.get_interface_requests(configs, have) def get_delete_interface_requests(self, configs, have): self.delete_flag = True - commands = self.filter_comands_to_delete(configs, have) - - return self.get_interface_requests(commands, have) + return self.get_interface_requests(configs, have) def get_interface_requests(self, configs, have): requests = [] @@ -341,17 +404,16 @@ def retrieve_default_intf_speed(self, intf_name): self._module.fail_json(msg=str(exc), code=exc.code) # Read the speed + intf_speed = 'SPEED_DEFAULT' method = GET request = {"path": eth_url, "method": method} try: response = edit_config(self._module, to_request(self._module, request)) - except ConnectionError as exc: - self._module.fail_json(msg=str(exc), code=exc.code) - - intf_speed = 'SPEED_DEFAULT' - if "openconfig-if-ethernet:port-speed" in response[0][1]: - speed_str = response[0][1].get("openconfig-if-ethernet:port-speed", '') - intf_speed = speed_str.split(":", 1)[-1] + if "openconfig-if-ethernet:port-speed" in response[0][1]: + speed_str = response[0][1].get("openconfig-if-ethernet:port-speed", '') + intf_speed = speed_str.split(":", 1)[-1] + except Exception as exc: + pass return intf_speed @@ -362,7 +424,7 @@ def is_this_delete_required(self, conf, have): if intf: if (intf['name'].startswith('Loopback') or not ((intf.get('description') is None or intf.get('description') == '') and - (intf.get('enabled') is None or intf.get('enabled') is True) and + (intf.get('enabled') is None or intf.get('enabled') is False) and (intf.get('mtu') is None or intf.get('mtu') == 9100) and (intf.get('fec') is None or intf.get('fec') == 'FEC_DISABLED') and (intf.get('speed') is None or @@ -468,3 +530,19 @@ def build_create_autoneg_request(self, conf): request = {"path": eth_url, "method": method, "data": payload} return request + + def classify_delete_commands(self, configs): + commands_del = [] + commands_mer = [] + + if not configs: + return commands_del, commands_mer + + for conf in configs: + name = conf["name"] + if name.startswith('Loopback'): + commands_del.append(conf) + else: + commands_mer.append(conf) + + return commands_del, commands_mer diff --git a/plugins/module_utils/network/sonic/config/vlans/vlans.py b/plugins/module_utils/network/sonic/config/vlans/vlans.py index 651652919..58d7449b9 100644 --- a/plugins/module_utils/network/sonic/config/vlans/vlans.py +++ b/plugins/module_utils/network/sonic/config/vlans/vlans.py @@ -35,12 +35,20 @@ to_request, edit_config ) +from ansible_collections.dellemc.enterprise_sonic.plugins.module_utils.network.sonic.utils.formatted_diff_utils import ( + __DELETE_CONFIG_IF_NO_SUBCONFIG, + get_new_config, + get_formatted_config_diff +) from ansible.module_utils.connection import ConnectionError TEST_KEYS = [ {'config': {'vlan_id': ''}}, ] +TEST_KEYS_formatted_diff = [ + {'config': {'vlan_id': '', '__delete_op': __DELETE_CONFIG_IF_NO_SUBCONFIG}}, +] class Vlans(ConfigBase): @@ -98,6 +106,17 @@ def execute_module(self): if result['changed']: result['after'] = changed_vlans_facts + new_config = changed_vlans_facts + if self._module.check_mode: + result.pop('after', None) + new_config = get_new_config(commands, existing_vlans_facts, + TEST_KEYS_formatted_diff) + new_config.sort(key=lambda x: x['vlan_id']) + result['after(generated)'] = new_config + + if self._module._diff: + result['config_diff'] = get_formatted_config_diff(existing_vlans_facts, + new_config) result['warnings'] = warnings return result diff --git a/tests/unit/modules/network/sonic/fixtures/sonic_interfaces.yaml b/tests/unit/modules/network/sonic/fixtures/sonic_interfaces.yaml index ee9d53e80..698ebb671 100644 --- a/tests/unit/modules/network/sonic/fixtures/sonic_interfaces.yaml +++ b/tests/unit/modules/network/sonic/fixtures/sonic_interfaces.yaml @@ -45,33 +45,36 @@ deleted_01: value: openconfig-interfaces:interfaces: interface: - - name: Eth1/1 + - name: 'Eth1/1' config: enabled: false description: 'Test Desc for eth1/1' - mtu: 1600 - - name: Loopback1 + mtu: 8888 + - name: 'Loopback123' config: enabled: false - description: 'Test Desc for eth1/1' + description: 'Test Desc for Loopback123' expected_config_requests: - path: "data/openconfig-interfaces:interfaces/interface=Eth1%2F1/config" method: "patch" data: openconfig-interfaces:config: - enabled: true + enabled: false description: '' mtu: 9100 - - path: "data/openconfig-interfaces:interfaces/interface=Loopback1" + - path: "data/openconfig-interfaces:interfaces/interface=Eth1%2F1/openconfig-if-ethernet:ethernet/config/port-speed" + method: "delete" + - path: "data/openconfig-interfaces:interfaces/interface=Eth1%2F1/openconfig-if-ethernet:ethernet/config/port-speed" + method: "get" + - path: "data/openconfig-interfaces:interfaces/interface=Loopback123" method: "delete" - data: replaced_01: module_args: state: replaced config: - name: Eth1/1 - mtu: 1600 + mtu: 5555 existing_interfaces_config: - path: "data/openconfig-interfaces:interfaces" response: @@ -89,14 +92,18 @@ replaced_01: method: "patch" data: openconfig-interfaces:config: - enabled: true + enabled: false description: '' mtu: 9100 - path: "data/openconfig-interfaces:interfaces/interface=Eth1%2F1/config" method: "patch" data: openconfig-interfaces:config: - mtu: 1600 + mtu: 5555 + - path: "data/openconfig-interfaces:interfaces/interface=Eth1%2F1/openconfig-if-ethernet:ethernet/config/port-speed" + method: "delete" + - path: "data/openconfig-interfaces:interfaces/interface=Eth1%2F1/openconfig-if-ethernet:ethernet/config/port-speed" + method: "get" overridden_01: module_args: