Various small fixes to MSC modules and tests (#48417)

lib/ansible/module_utils/network/aci/msc.py

@@ -186,9 +186,9 @@ class MSCModule(object):
             except:
                 payload = json.loads(info['body'])
             if 'code' in payload:
-                self.fail_json(msg='MSC Error {code}: {message} [{info}]'.format(**payload), payload=data)
+                self.fail_json(msg='MSC Error {code}: {message}'.format(**payload), data=data, info=info, payload=payload)
             else:
-                self.fail_json(msg='MSC Error:'.format(**payload), info=info, output=output)
+                self.fail_json(msg='MSC Error:'.format(**payload), data=data, info=info, payload=payload)
 
         return {}
 
@@ -213,18 +213,20 @@ class MSCModule(object):
             self.fail_json(msg='More than one object matches unique filter: {0}'.format(kwargs))
         return objs[0]
 
-    def sanitize(self, updates, collate=False):
+    def sanitize(self, updates, collate=False, required_keys=None):
+        if required_keys is None:
+            required_keys = []
         self.proposed = deepcopy(self.existing)
         self.sent = deepcopy(self.existing)
 
         # Clean up self.sent
         for key in updates:
             # Always retain 'id'
-            if key in ('id'):
+            if key in required_keys:
                 pass
 
             # Remove unspecified values
-            elif updates[key] is None:
+            elif not collate and updates[key] is None:
                 if key in self.existing:
                     del(self.sent[key])
                 continue
@@ -265,8 +267,8 @@ class MSCModule(object):
 
         if self.module._diff:
             self.result['diff'] = dict(
-                before=self.existing,
-                after=self.sent,
+                before=self.previous,
+                after=self.existing,
             )
 
         self.result.update(**kwargs)

lib/ansible/modules/network/aci/msc_label.py

@@ -29,9 +29,6 @@ options:
     - The name of the label.
     required: yes
     aliases: [ label_name, name ]
-  display_name:
-    description:
-    - The name of the label displayed in the web UI.
   type:
     description:
     - The type of the label.
@@ -99,7 +96,6 @@ def main():
     argument_spec.update(
         label=dict(type='str', required=False, aliases=['name', 'label_name']),
         label_id=dict(type='str', required=False),
-        display_name=dict(type='str'),
         type=dict(type='str', default='site', choices=['site']),
         state=dict(type='str', default='present', choices=['absent', 'present', 'query']),
     )
@@ -116,7 +112,6 @@ def main():
     label = module.params['label']
     label_id = module.params['label_id']
     label_type = module.params['type']
-    display_name = module.params['display_name']
     state = module.params['state']
 
     msc = MSCModule(module)
@@ -158,7 +153,7 @@ def main():
 
         msc.sanitize(dict(
             id=label_id,
-            displayName=display_name,
+            displayName=label,
             type=label_type,
         ), collate=True)
 

lib/ansible/modules/network/aci/msc_site.py

@@ -23,20 +23,28 @@ options:
   apic_password:
     description:
     - The password for the APICs.
+    type: str
+    required: yes
   apic_site_id:
     description:
     - The site ID of the APICs.
+    type: str
+    required: yes
   apic_username:
     description:
     - The username for the APICs.
+    type: str
+    required: yes
     default: admin
   site_id:
     description:
     - The ID of the site.
+    type: str
     required: yes
   site:
     description:
     - The name of the site.
+    type: str
     required: yes
     aliases: [ name, site_name ]
   labels:
@@ -51,6 +59,7 @@ options:
     description:
     - Use C(present) or C(absent) for adding or removing.
     - Use C(query) for listing an object or multiple objects.
+    type: str
     choices: [ absent, present, query ]
     default: present
 extends_documentation_fragment: msc
@@ -122,7 +131,7 @@ def main():
         supports_check_mode=True,
         required_if=[
             ['state', 'absent', ['site']],
-            ['state', 'present', ['site']],
+            ['state', 'present', ['apic_site_id', 'site']],
         ],
     )
 
@@ -178,7 +187,7 @@ def main():
             urls=urls,
             username=apic_username,
             password=apic_password,
-        ))
+        ), collate=True)
 
         if msc.existing:
             if not issubset(msc.sent, msc.existing):

lib/ansible/modules/network/aci/msc_tenant.py

@@ -23,22 +23,28 @@ options:
   tenant_id:
     description:
     - The ID of the tenant.
+    type: str
     required: yes
   tenant:
     description:
     - The name of the tenant.
+    type: str
     required: yes
     aliases: [ name, tenant_name ]
   display_name:
     description:
     - The name of the tenant to be displayed in the web UI.
+    type: str
+    required: yes
   description:
     description:
     - The description for this tenant.
+    type: str
   state:
     description:
     - Use C(present) or C(absent) for adding or removing.
     - Use C(query) for listing an object or multiple objects.
+    type: str
     choices: [ absent, present, query ]
     default: present
 extends_documentation_fragment: msc
@@ -52,7 +58,8 @@ EXAMPLES = r'''
     password: SomeSecretPassword
     tenant: north_europe
     tenant_id: 101
-    description: North European Datacenter
+    display_name: North European Datacenter
+    description: This tenant manages the NEDC environment.
     state: present
   delegate_to: localhost
 
@@ -161,7 +168,7 @@ def main():
             displayName=display_name,
             siteAssociations=[],
             userAssociations=[dict(userId="0000ffff0000000000000020")],
-        ))
+        ), collate=True)
 
         if msc.existing:
             if not issubset(msc.sent, msc.existing):

test/integration/targets/msc_label/tasks/main.yml

@@ -49,8 +49,8 @@
     that:
     - cm_add_label is changed
     - cm_add_label.previous == {}
-    - cm_add_label.current.id is not defined
     - cm_add_label.current.displayName == 'ansible_test'
+    - cm_add_label.current.id is not defined
     - cm_add_label.current.type == 'site'
 
 - name: Add label (normal mode)
@@ -62,8 +62,8 @@
     that:
     - nm_add_label is changed
     - nm_add_label.previous == {}
-    - nm_add_label.current.id is defined
     - nm_add_label.current.displayName == 'ansible_test'
+    - nm_add_label.current.id is defined
     - nm_add_label.current.type == 'site'
 
 - name: Add label again (check_mode)
@@ -77,8 +77,8 @@
     - cm_add_label_again is not changed
     - cm_add_label_again.previous.displayName == 'ansible_test'
     - cm_add_label_again.previous.type == 'site'
-    - cm_add_label_again.current.id == nm_add_label.current.id
     - cm_add_label_again.current.displayName == 'ansible_test'
+    - cm_add_label_again.current.id == nm_add_label.current.id
     - cm_add_label_again.current.type == 'site'
 
 - name: Add label again (normal mode)
@@ -91,8 +91,8 @@
     - nm_add_label_again is not changed
     - nm_add_label_again.previous.displayName == 'ansible_test'
     - nm_add_label_again.previous.type == 'site'
-    - nm_add_label_again.current.id == nm_add_label.current.id
     - nm_add_label_again.current.displayName == 'ansible_test'
+    - nm_add_label_again.current.id == nm_add_label.current.id
     - nm_add_label_again.current.type == 'site'
 
 
@@ -109,8 +109,8 @@
   assert:
     that:
     - cm_change_label is changed
-    - cm_change_label.current.id == nm_add_label.current.id
     - cm_change_label.current.displayName == 'ansible_test2'
+    - cm_change_label.current.id == nm_add_label.current.id
     - cm_change_label.current.type == 'site'
 
 - name: Change label (normal mode)
@@ -125,8 +125,8 @@
   assert:
     that:
     - nm_change_label is changed
+    - cm_change_label.current.displayName == 'ansible_test2'
     - nm_change_label.current.id == nm_add_label.current.id
-    - nm_change_label.current.displayName == 'ansible_test2'
     - nm_change_label.current.type == 'site'
 
 - name: Change label again (check_mode)
@@ -141,8 +141,8 @@
   assert:
     that:
     - cm_change_label_again is not changed
-    - cm_change_label_again.current.id == nm_add_label.current.id
     - cm_change_label_again.current.displayName == 'ansible_test2'
+    - cm_change_label_again.current.id == nm_add_label.current.id
     - cm_change_label_again.current.type == 'site'
 
 - name: Change label again (normal mode)
@@ -156,8 +156,8 @@
   assert:
     that:
     - nm_change_label_again is not changed
-    - nm_change_label_again.current.id == nm_add_label.current.id
     - nm_change_label_again.current.displayName == 'ansible_test2'
+    - nm_change_label_again.current.id == nm_add_label.current.id
     - nm_change_label_again.current.type == 'site'
 
 
@@ -206,12 +206,12 @@
   assert:
     that:
     - cm_query_label is not changed
-    - cm_query_label.current.id == nm_add_label.current.id
     - cm_query_label.current.displayName == 'ansible_test2'
+    - cm_query_label.current.id == nm_add_label.current.id
     - cm_query_label.current.type == 'site'
     - nm_query_label is not changed
-    - nm_query_label.current.id == nm_add_label.current.id
     - nm_query_label.current.displayName == 'ansible_test2'
+    - nm_query_label.current.id == nm_add_label.current.id
     - nm_query_label.current.type == 'site'
     - cm_query_label == nm_query_label
 

test/integration/targets/msc_role/tasks/main.yml

@@ -51,9 +51,9 @@
     that:
     - cm_add_role is changed
     - cm_add_role.previous == {}
-    - cm_add_role.current.id is not defined
-    - cm_add_role.current.displayName == 'ansible_test'
     - cm_add_role.current.description == 'Ansible test role'
+    - cm_add_role.current.displayName == 'ansible_test'
+    - cm_add_role.current.id is not defined
 
 - name: Add role (normal mode)
   msc_role: *role_present
@@ -64,9 +64,9 @@
     that:
     - nm_add_role is changed
     - nm_add_role.previous == {}
-    - nm_add_role.current.id is defined
-    - nm_add_role.current.displayName == 'ansible_test'
     - nm_add_role.current.description == 'Ansible test role'
+    - nm_add_role.current.displayName == 'ansible_test'
+    - nm_add_role.current.id is defined
 
 - name: Add role again (check_mode)
   msc_role: *role_present
@@ -77,11 +77,11 @@
   assert:
     that:
     - cm_add_role_again is not changed
-    - cm_add_role_again.previous.displayName == 'ansible_test'
     - cm_add_role_again.previous.description == 'Ansible test role'
-    - cm_add_role_again.current.id == nm_add_role.current.id
-    - cm_add_role_again.current.displayName == 'ansible_test'
+    - cm_add_role_again.previous.displayName == 'ansible_test'
     - cm_add_role_again.current.description == 'Ansible test role'
+    - cm_add_role_again.current.displayName == 'ansible_test'
+    - cm_add_role_again.current.id == nm_add_role.current.id
 
 - name: Add role again (normal mode)
   msc_role: *role_present
@@ -91,11 +91,11 @@
   assert:
     that:
     - nm_add_role_again is not changed
-    - nm_add_role_again.previous.displayName == 'ansible_test'
     - nm_add_role_again.previous.description == 'Ansible test role'
-    - nm_add_role_again.current.id == nm_add_role.current.id
-    - nm_add_role_again.current.displayName == 'ansible_test'
+    - nm_add_role_again.previous.displayName == 'ansible_test'
     - nm_add_role_again.current.description == 'Ansible test role'
+    - nm_add_role_again.current.displayName == 'ansible_test'
+    - nm_add_role_again.current.id == nm_add_role.current.id
 
 
 # CHANGE ROLE
@@ -112,9 +112,9 @@
   assert:
     that:
     - cm_change_role is changed
-    - cm_change_role.current.id == nm_add_role.current.id
-    - cm_change_role.current.displayName == 'ansible_test2'
     - cm_change_role.current.description == 'Ansible test role 2'
+    - cm_change_role.current.displayName == 'ansible_test2'
+    - cm_change_role.current.id == nm_add_role.current.id
 
 - name: Change role (normal mode)
   msc_role:
@@ -129,9 +129,9 @@
   assert:
     that:
     - nm_change_role is changed
-    - nm_change_role.current.id == nm_add_role.current.id
-    - nm_change_role.current.displayName == 'ansible_test2'
     - nm_change_role.current.description == 'Ansible test role 2'
+    - nm_change_role.current.displayName == 'ansible_test2'
+    - nm_change_role.current.id == nm_add_role.current.id
 
 - name: Change role again (check_mode)
   msc_role:
@@ -146,9 +146,9 @@
   assert:
     that:
     - cm_change_role_again is not changed
-    - cm_change_role_again.current.id == nm_add_role.current.id
-    - cm_change_role_again.current.displayName == 'ansible_test2'
     - cm_change_role_again.current.description == 'Ansible test role 2'
+    - cm_change_role_again.current.displayName == 'ansible_test2'
+    - cm_change_role_again.current.id == nm_add_role.current.id
 
 - name: Change role again (normal mode)
   msc_role:
@@ -162,9 +162,9 @@
   assert:
     that:
     - nm_change_role_again is not changed
-    - nm_change_role_again.current.id == nm_add_role.current.id
-    - nm_change_role_again.current.displayName == 'ansible_test2'
     - nm_change_role_again.current.description == 'Ansible test role 2'
+    - nm_change_role_again.current.displayName == 'ansible_test2'
+    - nm_change_role_again.current.id == nm_add_role.current.id
 
 
 # QUERY ALL ROLES
@@ -212,13 +212,13 @@
   assert:
     that:
     - cm_query_role is not changed
-    - cm_query_role.current.id == nm_add_role.current.id
-    - cm_query_role.current.displayName == 'ansible_test2'
     - cm_query_role.current.description == 'Ansible test role 2'
+    - cm_query_role.current.displayName == 'ansible_test2'
+    - cm_query_role.current.id == nm_add_role.current.id
     - nm_query_role is not changed
-    - nm_query_role.current.id == nm_add_role.current.id
-    - nm_query_role.current.displayName == 'ansible_test2'
     - nm_query_role.current.description == 'Ansible test role 2'
+    - nm_query_role.current.displayName == 'ansible_test2'
+    - nm_query_role.current.id == nm_add_role.current.id
     - cm_query_role == nm_query_role
 
 

test/integration/targets/msc_site/tasks/main.yml

@@ -42,6 +42,7 @@
     site: ansible_test
     apic_username: admin
     apic_password: '{{ apic_password }}'
+    apic_site_id: 101
     urls:
     - https://{{ apic_hostname }}/
     state: present

test/integration/targets/msc_tenant/tasks/main.yml

@@ -40,6 +40,7 @@
     use_proxy: '{{ msc_use_proxy | default(true) }}'
     output_level: '{{ msc_output_level | default("info") }}'
     tenant: ansible_test
+    display_name: Ansible test title
     description: Ansible test tenant
     state: present
   check_mode: yes
@@ -51,7 +52,7 @@
     - cm_add_tenant is changed
     - cm_add_tenant.previous == {}
     - cm_add_tenant.current.id is not defined
-    - cm_add_tenant.current.displayName == 'ansible_test'
+    - cm_add_tenant.current.name == 'ansible_test'
     - cm_add_tenant.current.description == 'Ansible test tenant'
 
 - name: Add tenant (normal mode)
@@ -64,7 +65,7 @@
     - nm_add_tenant is changed
     - nm_add_tenant.previous == {}
     - nm_add_tenant.current.id is defined
-    - nm_add_tenant.current.displayName == 'ansible_test'
+    - nm_add_tenant.current.name == 'ansible_test'
     - nm_add_tenant.current.description == 'Ansible test tenant'
 
 - name: Add tenant again (check_mode)
@@ -76,10 +77,10 @@
   assert:
     that:
     - cm_add_tenant_again is not changed
-    - cm_add_tenant_again.previous.displayName == 'ansible_test'
+    - cm_add_tenant_again.previous.name == 'ansible_test'
     - cm_add_tenant_again.previous.description == 'Ansible test tenant'
     - cm_add_tenant_again.current.id == nm_add_tenant.current.id
-    - cm_add_tenant_again.current.displayName == 'ansible_test'
+    - cm_add_tenant_again.current.name == 'ansible_test'
     - cm_add_tenant_again.current.description == 'Ansible test tenant'
 
 - name: Add tenant again (normal mode)
@@ -90,10 +91,10 @@
   assert:
     that:
     - nm_add_tenant_again is not changed
-    - nm_add_tenant_again.previous.displayName == 'ansible_test'
+    - nm_add_tenant_again.previous.name == 'ansible_test'
     - nm_add_tenant_again.previous.description == 'Ansible test tenant'
     - nm_add_tenant_again.current.id == nm_add_tenant.current.id
-    - nm_add_tenant_again.current.displayName == 'ansible_test'
+    - nm_add_tenant_again.current.name == 'ansible_test'
     - nm_add_tenant_again.current.description == 'Ansible test tenant'
 
 
@@ -112,7 +113,7 @@
     that:
     - cm_change_tenant is changed
     - cm_change_tenant.current.id == nm_add_tenant.current.id
-    - cm_change_tenant.current.displayName == 'ansible_test2'
+    - cm_change_tenant.current.name == 'ansible_test2'
     - cm_change_tenant.current.description == 'Ansible test tenant 2'
 
 - name: Change tenant (normal mode)
@@ -129,7 +130,7 @@
     that:
     - nm_change_tenant is changed
     - nm_change_tenant.current.id == nm_add_tenant.current.id
-    - nm_change_tenant.current.displayName == 'ansible_test2'
+    - nm_change_tenant.current.name == 'ansible_test2'
     - nm_change_tenant.current.description == 'Ansible test tenant 2'
 
 - name: Change tenant again (check_mode)
@@ -146,7 +147,7 @@
     that:
     - cm_change_tenant_again is not changed
     - cm_change_tenant_again.current.id == nm_add_tenant.current.id
-    - cm_change_tenant_again.current.displayName == 'ansible_test2'
+    - cm_change_tenant_again.current.name == 'ansible_test2'
     - cm_change_tenant_again.current.description == 'Ansible test tenant 2'
 
 - name: Change tenant again (normal mode)
@@ -162,7 +163,7 @@
     that:
     - nm_change_tenant_again is not changed
     - nm_change_tenant_again.current.id == nm_add_tenant.current.id
-    - nm_change_tenant_again.current.displayName == 'ansible_test2'
+    - nm_change_tenant_again.current.name == 'ansible_test2'
     - nm_change_tenant_again.current.description == 'Ansible test tenant 2'
 
 
@@ -212,11 +213,11 @@
     that:
     - cm_query_tenant is not changed
     - cm_query_tenant.current.id == nm_add_tenant.current.id
-    - cm_query_tenant.current.displayName == 'ansible_test2'
+    - cm_query_tenant.current.name == 'ansible_test2'
     - cm_query_tenant.current.description == 'Ansible test tenant 2'
     - nm_query_tenant is not changed
     - nm_query_tenant.current.id == nm_add_tenant.current.id
-    - nm_query_tenant.current.displayName == 'ansible_test2'
+    - nm_query_tenant.current.name == 'ansible_test2'
     - nm_query_tenant.current.description == 'Ansible test tenant 2'
     - cm_query_tenant == nm_query_tenant