Merge branch 'ansible-collections:main' into los_alarm

changelogs/fragments/440-bgp-aann-support-merged-fix.yaml

@@ -0,0 +1,5 @@
+---
+breaking_changes:
+  - sonic_bgp_communities - Change 'aann' option as a suboption of 'members' and update its type from string to list of strings (https://github.com/ansible-collections/dellemc.enterprise_sonic/pull/440).
+bugfixes:
+  - sonic_bgp_communities - Fix issues in merged state for standard community-lists (https://github.com/ansible-collections/dellemc.enterprise_sonic/pull/440).

changelogs/fragments/455-fix-vrrp-regression-failure.yaml

@@ -0,0 +1,4 @@
+---
+bugfixes:
+  - sonic_vrrp - Update delete handling to fix regression failure (https://github.com/ansible-collections/dellemc.enterprise_sonic/pull/455).
+  - Update 'update_url' method to handle multiple interface names (https://github.com/ansible-collections/dellemc.enterprise_sonic/pull/455).

plugins/module_utils/network/sonic/argspec/bgp_communities/bgp_communities.py

@@ -36,24 +36,37 @@ class Bgp_communitiesArgs(object):  # pylint: disable=R0903
     def __init__(self, **kwargs):
         pass
 
-    argument_spec = {'config': {'elements': 'dict',
-                                'options': {'aann': {'type': 'str'},
-                                            'local_as': {'type': 'bool'},
-                                            'match': {'choices': ['ALL', 'ANY'],
-                                                      'default': 'ANY',
-                                                      'type': 'str'},
-                                            'members': {'options': {'regex': {'elements': 'str',
-                                                                              'type': 'list'}},
-                                                        'type': 'dict'},
-                                            'name': {'required': True, 'type': 'str'},
-                                            'no_advertise': {'type': 'bool'},
-                                            'no_export': {'type': 'bool'},
-                                            'no_peer': {'type': 'bool'},
-                                            'permit': {'type': 'bool'},
-                                            'type': {'choices': ['standard', 'expanded'],
-                                                     'default': 'standard',
-                                                     'type': 'str'}},
-                                'type': 'list'},
-                     'state': {'choices': ['merged', 'deleted', 'replaced', 'overridden'],
-                               'default': 'merged',
-                               'type': 'str'}}  # pylint: disable=C0301
+    argument_spec = {
+        "config": {
+            "elements": "dict",
+            "options": {
+                "local_as": {"type": "bool"},
+                "match": {
+                    "choices": ["ALL", "ANY"],
+                    "type": "str"
+                },
+                "members": {
+                    "options": {
+                        "aann": {"elements": "str", "type": "list"},
+                        "regex": {"elements": "str", "type": "list"}
+                    },
+                    "type": "dict"
+                },
+                "name": {"required": True, "type": "str"},
+                "no_advertise": {"type": "bool"},
+                "no_export": {"type": "bool"},
+                "no_peer": {"type": "bool"},
+                "permit": {"type": "bool"},
+                "type": {
+                    "choices": ["standard", "expanded"],
+                    "type": "str"
+                }
+            },
+            "type": "list"
+        },
+        "state": {
+            "choices": ["merged", "deleted", "replaced", "overridden"],
+            "default": "merged",
+            "type": "str"
+        }
+    }  # pylint: disable=C0301

plugins/module_utils/network/sonic/config/vrrp/vrrp.py

@@ -388,12 +388,18 @@ def _get_replaced_overridden_config(self, want, have, state):
                         match_group = next((g for g in want_groups if g['virtual_router_id'] == vr_id and g['afi'] == afi), None)
                         if not match_group:
                             del_cfg.setdefault('group', []).append({'virtual_router_id': vr_id, 'afi': afi})
-                            commands, requests = self.get_delete_vrrp_group_command_request(name, vr_id, afi)
+                            commands, requests = self.get_delete_vrrp_group_command_request(name, group)
                             if commands:
                                 del_requests.extend(requests)
                     if del_cfg:
-                        del_cfg['name'] = name
-                        del_config.append(del_cfg)
+                        if not any(cfg.get('name') == name for cfg in del_config):
+                            del_cfg['name'] = name
+                            del_config.append(del_cfg)
+                        else:
+                            for cfg in del_config:
+                                if cfg['name'] == name:
+                                    cfg['group'].append(del_cfg)
+                                    break
         return add_config, del_config, del_requests
 
     def get_create_vrrp_requests(self, commands):
@@ -407,15 +413,14 @@ def get_create_vrrp_requests(self, commands):
 
         for cmd in commands:
             name = cmd.get('name', None)
-            intf_name = name.replace('/', '%2f')
             group_list = cmd.get('group', [])
             if group_list:
                 for group in group_list:
                     virtual_router_id = group.get('virtual_router_id', None)
-                    if 'Vlan' in intf_name:
-                        keypath = self.vrrp_vlan_path.format(intf_name=intf_name)
+                    if 'Vlan' in name:
+                        keypath = self.vrrp_vlan_path.format(intf_name=name)
                     else:
-                        parent_intf, sub_intf = intf_name.split('.') if '.' in intf_name else (intf_name, 0)
+                        parent_intf, sub_intf = name.split('.') if '.' in name else (name, 0)
                         keypath = self.vrrp_intf_path.format(intf_name=parent_intf, intf_index=sub_intf)
                     requests.extend(self.get_create_specific_vrrp_param_requests(virtual_router_id, group, keypath))
 
@@ -503,14 +508,13 @@ def get_delete_vrrp_commands_requests(self, want, have, is_delete_all):
         for cmd in want:
             del_cmd = {}
             name = cmd.get('name')
-            intf_name = name.replace('/', '%2f')
             match_have = next((cnf for cnf in have if cnf['name'] == name), None)
             group_list = [] if cmd.get('group') is None else cmd['group']
 
             if is_delete_all:
                 if match_have:
                     if match_have.get('group'):
-                        del_group, request = self.get_delete_all_vrrp_groups_commands_requests(match_have['group'], intf_name)
+                        del_group, request = self.get_delete_all_vrrp_groups_commands_requests(match_have['group'], name)
                         if del_group:
                             commands.append({'name': name})
                             requests.extend(request)
@@ -525,18 +529,18 @@ def get_delete_vrrp_commands_requests(self, want, have, is_delete_all):
                         match_group = next((g for g in match_group_list if g['virtual_router_id'] == virtual_router_id and g['afi'] == afi), None)
                         if match_group:
                             del_group = None
-                            if len(match_group.keys()) == 2:
-                                del_group, request = self.get_delete_vrrp_group_command_request(intf_name, virtual_router_id, afi)
+                            if len(group.keys()) == 2:
+                                del_group, request = self.get_delete_vrrp_group_command_request(name, group)
                             else:
-                                del_group, request = self.get_delete_specific_vrrp_param_commands_requests(match_group, virtual_router_id, group, intf_name)
+                                del_group, request = self.get_delete_specific_vrrp_param_commands_requests(match_group, virtual_router_id, group, name)
 
                             if del_group:
                                 del_groups.append(del_group)
                                 requests.extend(request)
                     if del_groups:
                         del_cmd['group'] = del_groups
                 elif match_group_list:
-                    del_group, request = self.get_delete_all_vrrp_groups_commands_requests(match_group_list, intf_name)
+                    del_group, request = self.get_delete_all_vrrp_groups_commands_requests(match_group_list, name)
                     if del_group:
                         commands.append({'name': name})
                         requests.extend(request)
@@ -551,11 +555,10 @@ def get_delete_all_vrrp_groups_commands_requests(self, groups, intf_name):
         groups = [] if groups is None else groups
         for group in groups:
             virtual_router_id = group.get('virtual_router_id')
-            afi = group.get('afi')
             # VRRP with VRRP ID 1 can be removed only if other VRRP
             # groups are removed first
             # Hence the check
-            command, request = self.get_delete_vrrp_group_command_request(intf_name, virtual_router_id, afi)
+            command, request = self.get_delete_vrrp_group_command_request(intf_name, group)
             if command:
                 commands.append(command)
                 if virtual_router_id == 1:
@@ -567,11 +570,13 @@ def get_delete_all_vrrp_groups_commands_requests(self, groups, intf_name):
 
         return commands, requests
 
-    def get_delete_vrrp_group_command_request(self, intf_name, virtual_router_id, afi):
+    def get_delete_vrrp_group_command_request(self, intf_name, group):
         """ Get list of requests to delete the entire VRRP and VRRP6 group configurations
         based on the specified interface
         """
         command, request = [], []
+        virtual_router_id = group.get('virtual_router_id')
+        afi = group.get('afi')
         if not virtual_router_id or not afi:
             return command, request
 
@@ -582,6 +587,14 @@ def get_delete_vrrp_group_command_request(self, intf_name, virtual_router_id, af
             parent_intf, sub_intf = intf_name.split('.') if '.' in intf_name else (intf_name, 0)
             keypath = self.vrrp_intf_path.format(intf_name=parent_intf, intf_index=sub_intf)
         url = '{0}{1}vrrp/vrrp-group={2}'.format(keypath, ip_path, virtual_router_id)
+
+        track_interfaces = self.get_track_interfaces(group.get('track_interface'))
+
+        for track_intf in track_interfaces:
+            if track_intf.get('interface'):
+                track_url = url + "/openconfig-interfaces-ext:vrrp-track/vrrp-track-interface={0}".format(track_intf.get('interface'))
+                request.append({'path': track_url, 'method': DELETE})
+
         command = {'virtual_router_id': virtual_router_id, 'afi': afi}
         request.append({'path': url, 'method': DELETE})
 
@@ -595,12 +608,7 @@ def get_delete_specific_vrrp_param_commands_requests(self, cfg_group, virtual_ro
 
         afi = group['afi']
         vip_addresses = self.get_vip_addresses(group.get('virtual_address'))
-        preempt = group.get('preempt')
         ip_path = IPV4_PATH if afi == 'ipv4' else IPV6_PATH
-        adv_interval = group.get('advertisement_interval')
-        priority = group.get('priority')
-        version = group.get('version')
-        use_v2_checksum = group.get('use_v2_checksum')
         track_interfaces = self.get_track_interfaces(group.get('track_interface'))
 
         if not virtual_router_id or not afi:
@@ -615,12 +623,6 @@ def get_delete_specific_vrrp_param_commands_requests(self, cfg_group, virtual_ro
             parent_intf, sub_intf = intf_name.split('.') if '.' in intf_name else (intf_name, 0)
             keypath = self.vrrp_intf_path.format(intf_name=parent_intf, intf_index=sub_intf)
 
-        if not (vip_addresses or preempt or adv_interval or priority or version or use_v2_checksum or track_interfaces):
-            url = keypath + ip_path + 'vrrp/vrrp-group={vrid}'.format(vrid=virtual_router_id)
-            commands = {'virtual_router_id': virtual_router_id, 'afi': afi}
-            requests.append({'path': url, 'method': DELETE})
-            return commands, requests
-
         if vip_addresses and cfg_vip_addresses:
             del_vip_list = []
             for addr in set(vip_addresses).intersection(set(cfg_vip_addresses)):
@@ -639,10 +641,8 @@ def get_delete_specific_vrrp_param_commands_requests(self, cfg_group, virtual_ro
             del_track_list = []
             for track in track_interfaces:
                 interface = track['interface']
-                interface = interface.replace('/', '%2f')
                 for cfg_track in cfg_track_interfaces:
                     cfg_interface = cfg_track['interface']
-                    cfg_interface = cfg_interface.replace('/', '%2f')
                     if interface == cfg_interface:
                         track_url = self.vrrp_config_path['track_interface'].format(vrid=virtual_router_id) + '/vrrp-track-interface=' + interface
                         requests.append({'path': keypath + ip_path + track_url, 'method': DELETE})

plugins/module_utils/network/sonic/facts/bgp_communities/bgp_communities.py

@@ -21,6 +21,9 @@
     to_request,
     edit_config
 )
+from ansible_collections.dellemc.enterprise_sonic.plugins.module_utils.network.sonic.utils.utils import (
+    remove_empties_from_list
+)
 from ansible.module_utils.connection import ConnectionError
 
 
@@ -82,10 +85,7 @@ def get_bgp_communities(self):
                 result['type'] = 'standard'
 
             if result['type'] == 'standard':
-                result['local_as'] = None
-                result['no_advertise'] = None
-                result['no_export'] = None
-                result['no_peer'] = None
+                aann = []
                 for i in members:
                     if "NO_EXPORT_SUBCONFED" in i:
                         result['local_as'] = True
@@ -95,6 +95,12 @@ def get_bgp_communities(self):
                         result['no_export'] = True
                     elif "NOPEER" in i:
                         result['no_peer'] = True
+                    else:
+                        aann.append(i)
+
+                if aann:
+                    aann.sort()
+                    result['members'] = {'aann': aann}
 
             bgp_communities_configs.append(result)
         return bgp_communities_configs
@@ -124,7 +130,7 @@ def populate_facts(self, connection, ansible_facts, data=None):
         facts = {}
         if objs:
             params = utils.validate_config(self.argument_spec, {'config': objs})
-            facts['bgp_communities'] = params['config']
+            facts['bgp_communities'] = remove_empties_from_list(params['config'])
 
         ansible_facts['ansible_network_resources'].update(facts)
         return ansible_facts

plugins/module_utils/network/sonic/sonic.py

@@ -41,8 +41,7 @@
 from ansible_collections.ansible.netcommon.plugins.module_utils.network.common.config import NetworkConfig, ConfigLine
 
 _DEVICE_CONFIGS = {}
-STANDARD_ETH_REGEXP = r"Eth\d+(/\d+)+"
-PATTERN = re.compile(STANDARD_ETH_REGEXP)
+STANDARD_ETH_REGEXP = r"(Eth\d+(/\d+)+)"
 
 
 def get_connection(module):
@@ -158,12 +157,13 @@ def edit_config_reboot(module, commands, skip_code=None):
 
 
 def update_url(url):
-    match = re.search(STANDARD_ETH_REGEXP, url)
+    match = re.findall(STANDARD_ETH_REGEXP, url)
     ret_url = url
     if match:
-        interface_name = match.group()
-        interface_name = interface_name.replace("/", "%2f")
-        ret_url = PATTERN.sub(interface_name, url)
+        for item in match:
+            interface_name = item[0]
+            update_interface_name = interface_name.replace("/", "%2f")
+            ret_url = ret_url.replace(interface_name, update_interface_name)
     return ret_url