Merge branch 'ansible-collections:main' into vrrp_bug_fix

ansible-collections · Sep 26, 2024 · 24d3d53 · 24d3d53
2 parents 9596929 + 6eafbcd
commit 24d3d53
Show file tree

Hide file tree

Showing 15 changed files with 688 additions and 405 deletions.
diff --git a/changelogs/fragments/440-bgp-aann-support-merged-fix.yaml b/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).
diff --git a/changelogs/fragments/456-tests-sanity-ignore-action-plugin-for-ansible-2.19.yaml b/changelogs/fragments/456-tests-sanity-ignore-action-plugin-for-ansible-2.19.yaml
@@ -0,0 +1,3 @@
+---
+trivial:
+  - tests - Skip Ansible 2.19 checking for a module correspnding to the sonic.py action plugin to avoid invalid sanity failures (https://github.com/ansible-collections/dellemc.enterprise_sonic/pull/456).
diff --git a/plugins/module_utils/network/sonic/argspec/bgp_communities/bgp_communities.py b/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
diff --git a/plugins/module_utils/network/sonic/config/bgp_communities/bgp_communities.py b/plugins/module_utils/network/sonic/config/bgp_communities/bgp_communities.py
diff --git a/plugins/module_utils/network/sonic/facts/bgp_communities/bgp_communities.py b/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

diff --git a/plugins/modules/sonic_bgp_communities.py b/plugins/modules/sonic_bgp_communities.py
@@ -57,22 +57,17 @@
         type: str
         description:
         - Whether it is a standard or expanded community-list entry.
+        - If unspecified, operational default value is C(standard).
         required: False
         choices:
         - standard
         - expanded
-        default: standard
       permit:
         required: False
         type: bool
         description:
         - Permits or denies this community.
-        - Default value while adding a new community-list is C(False).
-      aann:
-        required: False
-        type: str
-        description:
-        - Community number aa:nn format 0..65535:0..65535; applicable for standard BGP community type.
+        - If unspecified, operational default value is C(False).
       local_as:
         required: False
         type: bool
@@ -97,6 +92,13 @@
         required: False
         type: dict
         suboptions:
+          aann:
+            required: False
+            type: list
+            elements: str
+            version_added: 3.0.0
+            description:
+            - Community number aa:nn format 0..65535:0..65535; applicable for standard BGP community type.
           regex:
             type: list
             elements: str
@@ -110,10 +112,10 @@
         type: str
         description:
         - Matches any/all of the members.
+        - If unspecified, operational default value is C(ANY).
         choices:
         - ALL
         - ANY
-        default: ANY
   state:
     description:
     - The state of the configuration after module completion.
@@ -147,7 +149,7 @@
         permit: false
         members:
           regex:
-          - 302
+            - 302
     state: deleted
 
 # After state:
@@ -256,15 +258,22 @@
 #     permit 101
 #     permit 302
 
-- name: Add a new BGP community-list
+- name: Add new BGP community-lists
   dellemc.enterprise_sonic.sonic_bgp_communities:
     config:
       - name: test2
         type: expanded
         permit: true
         members:
           regex:
-          - 909
+            - 909
+      - name: test3
+        type: standard
+        permit: true
+        no_peer: true
+        members:
+          aann:
+            - 1000:10
     state: merged
 
 # After state:
@@ -276,6 +285,9 @@
 #     permit 302
 # Expanded community list test2:   match: ANY
 #     permit 909
+# Standard community list test3:  match: ANY
+#     permit 1000:10
+#     permit no-peer
 
 
 # Using replaced
@@ -298,7 +310,13 @@
         type: expanded
         members:
           regex:
-          - 301
+            - 301
+      - name: test2
+        type: standard
+        members:
+          aann:
+            - 1000:10
+            - 2000:20
       - name: test3
         type: standard
         no_advertise: true
@@ -316,6 +334,9 @@
 # Expanded community list test1:   match: ANY
 #     deny 101
 #     deny 302
+# Standard community list test2:  match: ANY
+#     deny 1000:10
+#     deny 2000:10
 # Standard community list test3:  match: ALL
 #     deny no-advertise
 #     deny no-peer
@@ -341,7 +362,7 @@
         type: expanded
         members:
           regex:
-          - 301
+            - 301
     state: overridden
 
 # After state:
@@ -350,8 +371,7 @@
 # show bgp community-list
 # Expanded community list test3:   match: ANY
 #     deny 301
-
-
+#
 """
 RETURN = """
 before:

diff --git a/tests/regression/roles/common/tasks/action.facts.report.yaml b/tests/regression/roles/common/tasks/action.facts.report.yaml
@@ -1,6 +1,6 @@
 - set_fact:
     ansible_facts:
-      test_reports: "{{ ansible_facts['test_reports']| default({})| combine({module_name: {item.name+'.1': {
+      test_reports: "{{ ansible_facts['test_reports']| default({})| combine({ ansible_parent_role_names[0].split('sonic_')[-1]: {item.name+'.1': {
         'status': action_condition, 
         'module_stderr': action_task_output.module_stderr | default(action_task_output.msg | default('No Error')),
         'before': action_task_output.before | default('Not defined'),

diff --git a/tests/regression/roles/common/tasks/idempotent.facts.report.yaml b/tests/regression/roles/common/tasks/idempotent.facts.report.yaml
@@ -1,6 +1,6 @@
 - set_fact:
     ansible_facts:
-      test_reports: "{{ ansible_facts['test_reports']| default({})| combine({module_name: {item.name+'.2': {
+      test_reports: "{{ ansible_facts['test_reports']| default({})| combine({ ansible_parent_role_names[0].split('sonic_')[-1]: {item.name+'.2': {
         'status': idempotnet_condition, 
         'module_stderr': idempotent_task_output.module_stderr | default(idempotent_task_output.msg | default('No Error')),
         'before': idempotent_task_output.before | default('Not defined'),
@@ -9,4 +9,4 @@
         'configs': item.input  | default('Not defined'),
         'msg': idempotent_task_output.msg  | default('Not defined'),
         }}}, recursive=True) }}"
-  # no_log: true
+  # no_log: true