ACI: Fixes to domain parameters dependencies

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

@@ -194,7 +194,7 @@ def main():
             ['state', 'present', ['aep', 'domain', 'domain_type']],
         ],
         required_together=[
-            ['domain', 'domain_type']
+            ['domain', 'domain_type'],
         ],
     )
 

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

@@ -260,7 +260,7 @@ def main():
                            'CS0', 'CS1', 'CS2', 'CS3', 'CS4', 'CS5', 'CS6', 'CS7', 'EF', 'VA', 'unspecified'],
                   aliases=['target']),
         domain=dict(type='str', aliases=['domain_name', 'domain_profile', 'name']),
-        domain_type=dict(type='str', choices=['fc', 'l2dom', 'l3dom', 'phys', 'vmm'], aliases=['type']),
+        domain_type=dict(type='str', required=True, choices=['fc', 'l2dom', 'l3dom', 'phys', 'vmm'], aliases=['type']),
         encap_mode=dict(type='str', choices=['unknown', 'vlan', 'vxlan']),
         multicast_address=dict(type='str'),
         state=dict(type='str', default='present', choices=['absent', 'present', 'query']),
@@ -291,13 +291,13 @@ def main():
 
     if domain_type != 'vmm':
         if vm_provider is not None:
-            module.fail_json(msg="Domain type '{0}' cannot have a 'vm_provider'".format(domain_type))
+            module.fail_json(msg="Domain type '{0}' cannot have parameter 'vm_provider'".format(domain_type))
         if encap_mode is not None:
-            module.fail_json(msg="Domain type '{0}' cannot have an 'encap_mode'".format(domain_type))
+            module.fail_json(msg="Domain type '{0}' cannot have parameter 'encap_mode'".format(domain_type))
         if multicast_address is not None:
-            module.fail_json(msg="Domain type '{0}' cannot have a 'multicast_address'".format(domain_type))
+            module.fail_json(msg="Domain type '{0}' cannot have parameter 'multicast_address'".format(domain_type))
         if vswitch is not None:
-            module.fail_json(msg="Domain type '{0}' cannot have a 'vswitch'".format(domain_type))
+            module.fail_json(msg="Domain type '{0}' cannot have parameter 'vswitch'".format(domain_type))
 
     if dscp is not None and domain_type not in ['l2dom', 'l3dom']:
         module.fail_json(msg="DSCP values can only be assigned to 'l2ext and 'l3ext' domains")
@@ -324,6 +324,10 @@ def main():
         domain_mo = 'uni/vmmp-{0}/dom-{1}'.format(VM_PROVIDER_MAPPING[vm_provider], domain)
         domain_rn = 'vmmp-{0}/dom-{1}'.format(VM_PROVIDER_MAPPING[vm_provider], domain)
 
+    # Ensure that querying all objects works when only domain_type is provided
+    if domain is None:
+        domain_mo = None
+
     aci = ACIModule(module)
     aci.construct_url(
         root_class=dict(

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

@@ -186,13 +186,13 @@
 
 POOL_MAPPING = dict(
     vlan=dict(
-        aci_mo='uni/infra/vlanns-',
+        aci_mo='uni/infra/vlanns-{0}',
     ),
     vxlan=dict(
-        aci_mo='uni/infra/vxlanns-',
+        aci_mo='uni/infra/vxlanns-{0}',
     ),
     vsan=dict(
-        aci_mo='uni/infra/vsanns-',
+        aci_mo='uni/infra/vsanns-{0}',
     ),
 )
 
@@ -265,7 +265,11 @@ def main():
         domain_mo = 'uni/vmmp-{0}/dom-{1}'.format(VM_PROVIDER_MAPPING[vm_provider], domain)
         domain_rn = 'vmmp-{0}/dom-{1}'.format(VM_PROVIDER_MAPPING[vm_provider], domain)
 
-    pool_mo = POOL_MAPPING[pool_type]["aci_mo"] + pool_name
+    # Ensure that querying all objects works when only domain_type is provided
+    if domain is None:
+        domain_mo = None
+
+    pool_mo = POOL_MAPPING[pool_type]["aci_mo"].format(pool_name)
 
     aci = ACIModule(module)
     aci.construct_url(

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

@@ -122,6 +122,7 @@
     host: apic
     username: admin
     password: SomeSecretPassword
+    domain_type: phys
     state: query
 '''
 
@@ -234,7 +235,10 @@
 from ansible.module_utils.basic import AnsibleModule
 
 VM_PROVIDER_MAPPING = dict(
+    cloudfoundry='CloudFoundry',
+    kubernetes='Kubernetes',
     microsoft='Microsoft',
+    openshift='OpenShift',
     openstack='OpenStack',
     redhat='Redhat',
     vmware='VMware',
@@ -245,7 +249,7 @@ def main():
     argument_spec = aci_argument_spec()
     argument_spec.update(
         domain=dict(type='str', aliases=['domain_name', 'domain_profile']),
-        domain_type=dict(type='str', choices=['fc', 'l2dom', 'l3dom', 'phys', 'vmm']),
+        domain_type=dict(type='str', required=True, choices=['fc', 'l2dom', 'l3dom', 'phys', 'vmm']),
         pool=dict(type='str', aliases=['pool_name', 'vlan_pool']),
         pool_allocation_mode=dict(type='str', required=True, aliases=['allocation_mode', 'mode'], choices=['dynamic', 'static']),
         state=dict(type='str', default='present', choices=['absent', 'present', 'query']),
@@ -300,7 +304,11 @@ def main():
         domain_mo = 'uni/vmmp-{0}/dom-{1}'.format(VM_PROVIDER_MAPPING[vm_provider], domain)
         domain_rn = 'dom-{0}'.format(domain)
 
-    aci_mo = 'uni/infra/vlanns-' + pool_name
+    # Ensure that querying all objects works when only domain_type is provided
+    if domain is None:
+        domain_mo = None
+
+    aci_mo = 'uni/infra/vlanns-{0}'.format(pool_name)
 
     aci = ACIModule(module)
     aci.construct_url(

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

@@ -65,6 +65,15 @@
   aci_aep_to_domain: *binding_present
   register: nm_add_binding
 
+- name: Verify add_binding
+  assert:
+    that:
+    - cm_add_binding.changed == nm_add_binding.changed == true
+    - 'cm_add_binding.sent == nm_add_binding.sent == {"infraRsDomP": {"attributes": {"tDn": "uni/phys-phys_dom"}}}'
+    - 'cm_add_binding.proposed == nm_add_binding.proposed == {"infraRsDomP": {"attributes": {"tDn": "uni/phys-phys_dom"}}}'
+    - cm_add_binding.current == cm_add_binding.previous == nm_add_binding.previous == []
+    - 'nm_add_binding.current == [{"infraRsDomP": {"attributes": {"dn": "uni/infra/attentp-test_aep/rsdomP-[uni/phys-phys_dom]", "tDn": "uni/phys-phys_dom"}}}]'
+
 - name: Add AEP to domain binding again (check_mode)
   aci_aep_to_domain: *binding_present
   check_mode: yes
@@ -74,15 +83,10 @@
   aci_aep_to_domain: *binding_present
   register: nm_add_binding_again
 
-- name: Verify add_binding
+- name: Verify add_binding_again
   assert:
     that:
-    - cm_add_binding.changed == nm_add_binding.changed == true
     - cm_add_binding_again.changed == nm_add_binding_again.changed == false
-    - 'cm_add_binding.sent == nm_add_binding.sent == {"infraRsDomP": {"attributes": {"tDn": "uni/phys-phys_dom"}}}'
-    - 'cm_add_binding.proposed == nm_add_binding.proposed == {"infraRsDomP": {"attributes": {"tDn": "uni/phys-phys_dom"}}}'
-    - cm_add_binding.current == cm_add_binding.previous == nm_add_binding.previous == []
-    - 'nm_add_binding.current == [{"infraRsDomP": {"attributes": {"dn": "uni/infra/attentp-test_aep/rsdomP-[uni/phys-phys_dom]", "tDn": "uni/phys-phys_dom"}}}]'
 
 
 # QUERY ALL BINDINGS
@@ -148,6 +152,13 @@
   aci_aep_to_domain: *binding_absent
   register: nm_remove_binding
 
+- name: Verify remove_binding
+  assert:
+    that:
+    - cm_remove_binding.changed == nm_remove_binding.changed == true
+    - 'cm_remove_binding.current == cm_remove_binding.previous == nm_remove_binding.previous == [{"infraRsDomP": {"attributes": {"dn": "uni/infra/attentp-test_aep/rsdomP-[uni/phys-phys_dom]", "tDn": "uni/phys-phys_dom"}}}]'
+    - nm_remove_binding.current == []
+
 - name: Remove AEP to domain binding again (check_mode)
   aci_aep_to_domain: *binding_absent
   check_mode: yes
@@ -157,13 +168,10 @@
   aci_aep_to_domain: *binding_absent
   register: nm_remove_binding_again
 
-- name: Verify remove_binding
+- name: Verify remove_binding_again
   assert:
     that:
-    - cm_remove_binding.changed == nm_remove_binding.changed == true
     - cm_remove_binding_again.changed == nm_remove_binding_again.changed == false
-    - 'cm_remove_binding.current == cm_remove_binding.previous == nm_remove_binding.previous == [{"infraRsDomP": {"attributes": {"dn": "uni/infra/attentp-test_aep/rsdomP-[uni/phys-phys_dom]", "tDn": "uni/phys-phys_dom"}}}]'
-    - nm_remove_binding.current == []
 
 
 # QUERY NON-EXISTING BINDING

test/integration/targets/aci_domain/tasks/fc.yml

@@ -39,6 +39,15 @@
   aci_domain: *domain_present
   register: nm_add_domain
 
+- name: Verify add_domain
+  assert:
+    that:
+    - cm_add_domain.changed == nm_add_domain.changed == true
+    - 'cm_add_domain.sent == nm_add_domain.sent == {"fcDomP": {"attributes": {"name": "fc_dom"}}}'
+    - 'cm_add_domain.proposed == nm_add_domain.proposed == {"fcDomP": {"attributes": {"name": "fc_dom"}}}'
+    - cm_add_domain.current == cm_add_domain.previous == nm_add_domain.previous == []
+    - 'nm_add_domain.current == [{"fcDomP": {"attributes": {"dn": "uni/fc-fc_dom", "name": "fc_dom", "nameAlias": "", "ownerKey": "", "ownerTag": ""}}}]'
+
 - name: Add FC domain again (check_mode)
   aci_domain: *domain_present
   check_mode: yes
@@ -48,15 +57,10 @@
   aci_domain: *domain_present
   register: nm_add_domain_again
 
-- name: Verify add_domain
+- name: Verify add_domain_again
   assert:
     that:
-    - cm_add_domain.changed == nm_add_domain.changed == true
     - cm_add_domain_again.changed == nm_add_domain_again.changed == false
-    - 'cm_add_domain.sent == nm_add_domain.sent == {"fcDomP": {"attributes": {"name": "fc_dom"}}}'
-    - 'cm_add_domain.proposed == nm_add_domain.proposed == {"fcDomP": {"attributes": {"name": "fc_dom"}}}'
-    - cm_add_domain.current == cm_add_domain.previous == nm_add_domain.previous == []
-    - 'nm_add_domain.current == [{"fcDomP": {"attributes": {"dn": "uni/fc-fc_dom", "name": "fc_dom", "nameAlias": "", "ownerKey": "", "ownerTag": ""}}}]'
 
 
 # QUERY ALL DOMAINS
@@ -82,8 +86,8 @@
   assert:
     that:
     - cm_query_all_domains.changed == nm_query_all_domains.changed == false
-    # NOTE: Order of domains is not stable between calls
-    #- cm_query_all_domains == nm_query_all_domains
+    - cm_query_all_domains == nm_query_all_domains
+    - nm_query_all_domains.current|length >= 1
 
 
 # QUERY A DOMAIN
@@ -119,6 +123,13 @@
   aci_domain: *domain_absent
   register: nm_remove_domain
 
+- name: Verify remove_domain
+  assert:
+    that:
+    - cm_remove_domain.changed == nm_remove_domain.changed == true
+    - 'cm_remove_domain.current == cm_remove_domain.previous == nm_remove_domain.previous == [{"fcDomP": {"attributes": {"dn": "uni/fc-fc_dom", "name": "fc_dom", "nameAlias": "", "ownerKey": "", "ownerTag": ""}}}]'
+    - nm_remove_domain.current == []
+
 - name: Remove FC domain again (check_mode)
   aci_domain: *domain_absent
   check_mode: yes
@@ -128,23 +139,24 @@
   aci_domain: *domain_absent
   register: nm_remove_domain_again
 
-- name: Verify remove_domain
+- name: Verify remove_domain_again
   assert:
     that:
-    - cm_remove_domain.changed == nm_remove_domain.changed == true
     - cm_remove_domain_again.changed == nm_remove_domain_again.changed == false
-    - 'cm_remove_domain.current == cm_remove_domain.previous == nm_remove_domain.previous == [{"fcDomP": {"attributes": {"dn": "uni/fc-fc_dom", "name": "fc_dom", "nameAlias": "", "ownerKey": "", "ownerTag": ""}}}]'
-    - nm_remove_domain.current == []
 
 
 # QUERY NON-EXISTING DOMAIN
 - name: Query non-existing FC domain (check_mode)
-  aci_domain: *domain_query
+  aci_domain:
+    <<: *domain_query
+    domain: fc_dom
   check_mode: yes
   register: cm_query_non_domain
 
 - name: Query non-existing FC domain (normal mode)
-  aci_domain: *domain_query
+  aci_domain:
+    <<: *domain_query
+    domain: fc_dom
   register: nm_query_non_domain
 
 - name: Verify query_non_domain

test/integration/targets/aci_domain/tasks/l2dom.yml

@@ -39,6 +39,15 @@
   aci_domain: *domain_present
   register: nm_add_domain
 
+- name: Verify add_domain
+  assert:
+    that:
+    - cm_add_domain.changed == nm_add_domain.changed == true
+    - 'cm_add_domain.sent == nm_add_domain.sent == {"l2extDomP": {"attributes": {"name": "l2_dom"}}}'
+    - 'cm_add_domain.proposed == nm_add_domain.proposed == {"l2extDomP": {"attributes": {"name": "l2_dom"}}}'
+    - cm_add_domain.current == cm_add_domain.previous == nm_add_domain.previous == []
+    - 'nm_add_domain.current == [{"l2extDomP": {"attributes": {"dn": "uni/l2dom-l2_dom", "name": "l2_dom", "nameAlias": "", "ownerKey": "", "ownerTag": ""}}}]'
+
 - name: Add L2 domain again (check_mode)
   aci_domain: *domain_present
   check_mode: yes
@@ -48,15 +57,10 @@
   aci_domain: *domain_present
   register: nm_add_domain_again
 
-- name: Verify add_domain
+- name: Verify add_domain_again
   assert:
     that:
-    - cm_add_domain.changed == nm_add_domain.changed == true
     - cm_add_domain_again.changed == nm_add_domain_again.changed == false
-    - 'cm_add_domain.sent == nm_add_domain.sent == {"l2extDomP": {"attributes": {"name": "l2_dom"}}}'
-    - 'cm_add_domain.proposed == nm_add_domain.proposed == {"l2extDomP": {"attributes": {"name": "l2_dom"}}}'
-    - cm_add_domain.current == cm_add_domain.previous == nm_add_domain.previous == []
-    - 'nm_add_domain.current == [{"l2extDomP": {"attributes": {"dn": "uni/l2dom-l2_dom", "name": "l2_dom", "nameAlias": "", "ownerKey": "", "ownerTag": ""}}}]'
 
 
 # QUERY ALL DOMAINS
@@ -82,8 +86,8 @@
   assert:
     that:
     - cm_query_all_domains.changed == nm_query_all_domains.changed == false
-    # NOTE: Order of domains is not stable between calls
-    #- cm_query_all_domains == nm_query_all_domains
+    - cm_query_all_domains == nm_query_all_domains
+    - nm_query_all_domains.current|length >= 1
 
 
 # QUERY A DOMAIN
@@ -119,6 +123,13 @@
   aci_domain: *domain_absent
   register: nm_remove_domain
 
+- name: Verify remove_domain
+  assert:
+    that:
+    - cm_remove_domain.changed == nm_remove_domain.changed == true
+    - 'cm_remove_domain.current == cm_remove_domain.previous == nm_remove_domain.previous == [{"l2extDomP": {"attributes": {"dn": "uni/l2dom-l2_dom", "name": "l2_dom", "nameAlias": "", "ownerKey": "", "ownerTag": ""}}}]'
+    - nm_remove_domain.current == []
+
 - name: Remove L2 domain again (check_mode)
   aci_domain: *domain_absent
   check_mode: yes
@@ -128,23 +139,24 @@
   aci_domain: *domain_absent
   register: nm_remove_domain_again
 
-- name: Verify remove_domain
+- name: Verify remove_domain_again
   assert:
     that:
-    - cm_remove_domain.changed == nm_remove_domain.changed == true
     - cm_remove_domain_again.changed == nm_remove_domain_again.changed == false
-    - 'cm_remove_domain.current == cm_remove_domain.previous == nm_remove_domain.previous == [{"l2extDomP": {"attributes": {"dn": "uni/l2dom-l2_dom", "name": "l2_dom", "nameAlias": "", "ownerKey": "", "ownerTag": ""}}}]'
-    - nm_remove_domain.current == []
 
 
 # QUERY NON-EXISTING DOMAIN
 - name: Query non-existing L2 domain (check_mode)
-  aci_domain: *domain_query
+  aci_domain:
+    <<: *domain_query
+    domain: l2_dom
   check_mode: yes
   register: cm_query_non_domain
 
 - name: Query non-existing L2 domain (normal mode)
-  aci_domain: *domain_query
+  aci_domain:
+    <<: *domain_query
+    domain: l2_dom
   register: nm_query_non_domain
 
 - name: Verify query_non_domain