From 24d7b4a66051cda190dd2eb22e9c90b4b2d25d3f Mon Sep 17 00:00:00 2001 From: Dag Wieers Date: Tue, 13 Nov 2018 09:01:45 +0100 Subject: [PATCH] Various small fixes to MSC modules and tests (#48417) --- lib/ansible/module_utils/network/aci/msc.py | 16 ++++--- lib/ansible/modules/network/aci/msc_label.py | 7 +-- lib/ansible/modules/network/aci/msc_site.py | 13 +++++- lib/ansible/modules/network/aci/msc_tenant.py | 11 ++++- .../targets/msc_label/tasks/main.yml | 20 ++++----- .../targets/msc_role/tasks/main.yml | 44 +++++++++---------- .../targets/msc_site/tasks/main.yml | 1 + .../targets/msc_tenant/tasks/main.yml | 25 ++++++----- 8 files changed, 76 insertions(+), 61 deletions(-) diff --git a/lib/ansible/module_utils/network/aci/msc.py b/lib/ansible/module_utils/network/aci/msc.py index 62e5d951249..c7b0a613635 100644 --- a/lib/ansible/module_utils/network/aci/msc.py +++ b/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) diff --git a/lib/ansible/modules/network/aci/msc_label.py b/lib/ansible/modules/network/aci/msc_label.py index 44e974c57fa..4fde746ef6c 100644 --- a/lib/ansible/modules/network/aci/msc_label.py +++ b/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) diff --git a/lib/ansible/modules/network/aci/msc_site.py b/lib/ansible/modules/network/aci/msc_site.py index 9aa4f614122..a268e542cdc 100644 --- a/lib/ansible/modules/network/aci/msc_site.py +++ b/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): diff --git a/lib/ansible/modules/network/aci/msc_tenant.py b/lib/ansible/modules/network/aci/msc_tenant.py index 9a33d069d0a..5417d35844f 100644 --- a/lib/ansible/modules/network/aci/msc_tenant.py +++ b/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): diff --git a/test/integration/targets/msc_label/tasks/main.yml b/test/integration/targets/msc_label/tasks/main.yml index 142acf16e1e..707bec2b455 100644 --- a/test/integration/targets/msc_label/tasks/main.yml +++ b/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 diff --git a/test/integration/targets/msc_role/tasks/main.yml b/test/integration/targets/msc_role/tasks/main.yml index 1ebb4e234c1..12cba5b1ae2 100644 --- a/test/integration/targets/msc_role/tasks/main.yml +++ b/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 diff --git a/test/integration/targets/msc_site/tasks/main.yml b/test/integration/targets/msc_site/tasks/main.yml index fd906e2251a..fb5490cc59d 100644 --- a/test/integration/targets/msc_site/tasks/main.yml +++ b/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 diff --git a/test/integration/targets/msc_tenant/tasks/main.yml b/test/integration/targets/msc_tenant/tasks/main.yml index 23380b2b6f6..57cea1b3e80 100644 --- a/test/integration/targets/msc_tenant/tasks/main.yml +++ b/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