ACI: Fixes to recent change to parameter choices (#35968)

This PR includes: - Fixes related to the recent merge of #31637 and #34537 - A generic fix for a reference for assignment issue - Fixes to aci.boolean() in order to catch exception
2018-02-09 21:21:56 +01:00 · 2018-02-09 21:21:56 +01:00 · 01ba3a4efc
commit 01ba3a4efc
parent d76db835ff
8 changed files with 42 additions and 36 deletions
--- a/lib/ansible/module_utils/network/aci/aci.py
+++ b/lib/ansible/module_utils/network/aci/aci.py
@ -169,24 +169,34 @@ class ACIModule(object):

    def boolean(self, value, true='yes', false='no'):
        ''' Return an acceptable value back '''
+
+        # When we expect value is of type=bool
        if value is None:
            return None
        elif value is True:
            return true
        elif value is False:
            return false
-        elif boolean(value) is True:  # When type=raw, this supports Ansible booleans
-            return true
-        elif boolean(value) is False:  # When type=raw, this supports Ansible booleans
-            return false
-        elif value == true:  # When type=raw, this supports the original boolean values
-            self.module.deprecate("Boolean value '%s' is no longer valid, please use 'yes' as a boolean value." % value, '2.9')
-            return true
-        elif value == false:  # When type=raw, this supports the original boolean values
-            self.module.deprecate("Boolean value '%s' is no longer valid, please use 'no' as a boolean value." % value, '2.9')
-            return false
-        else:  # When type=raw, escalate back to user
-            self.module.fail_json(msg="Boolean value '%s' is an invalid ACI boolean value.")
+
+        # When we expect value is of type=raw, deprecate in Ansible v2.8 (and all modules use type=bool)
+        try:
+            # This supports all Ansible boolean types
+            bool_value = boolean(value)
+            if bool_value is True:
+                return true
+            elif bool_value is False:
+                return false
+        except:
+            # This provides backward compatibility to Ansible v2.4, deprecate in Ansible v2.8
+            if value == true:
+                self.module.deprecate("Boolean value '%s' is no longer valid, please use 'yes' as a boolean value." % value, '2.9')
+                return true
+            elif value == false:
+                self.module.deprecate("Boolean value '%s' is no longer valid, please use 'no' as a boolean value." % value, '2.9')
+                return false
+
+        # If all else fails, escalate back to user
+        self.module.fail_json(msg="Boolean value '%s' is an invalid ACI boolean value.")

    def iso8601_format(self, dt):
        ''' Return an ACI-compatible ISO8601 formatted time: 2123-12-12T00:00:00.000+00:00 '''
--- a/lib/ansible/modules/network/aci/aci_aaa_user.py
+++ b/lib/ansible/modules/network/aci/aci_aaa_user.py
@ -269,28 +269,25 @@ def main():
        ],
    )

+    aci = ACIModule(module)
+
    if not HAS_DATEUTIL:
        module.fail_json(msg='dateutil required for this module')

    aaa_password = module.params['aaa_password']
    aaa_password_lifetime = module.params['aaa_password_lifetime']
-    aaa_password_update_required = module.params['aaa_password_update_required']
+    aaa_password_update_required = aci.boolean(module.params['aaa_password_update_required'])
    aaa_user = module.params['aaa_user']
    clear_password_history = module.params['clear_password_history']
    description = module.params['description']
    email = module.params['email']
-    enabled = module.params['enabled']
+    enabled = aci.boolean(module.params['enabled'], 'active', 'inactive')
+    expires = aci.boolean(module.params['expires'])
    first_name = module.params['first_name']
    last_name = module.params['last_name']
    phone = module.params['phone']
    state = module.params['state']

-    aci = ACIModule(module)
-
-    aaa_password_update_required = aci.boolean(module.params['aaa_password_update_required'])
-    enabled = aci.boolean(module.params['enabled'], 'active', 'inactive')
-    expires = aci.boolean(module.params['expires'])
-
    expiration = module.params['expiration']
    if expiration is not None and expiration != 'never':
        try:
--- a/lib/ansible/modules/network/aci/aci_bd_subnet.py
+++ b/lib/ansible/modules/network/aci/aci_bd_subnet.py
@ -310,7 +310,7 @@ SUBNET_CONTROL_MAPPING = dict(nd_ra='nd', no_gw='no-default-gateway', querier_ip


 from ansible.module_utils.network.aci.aci import ACIModule, aci_argument_spec
-from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.basic import AnsibleModule, SEQUENCETYPE


 def main():
@ -326,10 +326,7 @@ def main():
        preferred=dict(type='bool'),
        route_profile=dict(type='str'),
        route_profile_l3_out=dict(type='str'),
-        scope=dict(
-            type='list',
-            choices=[['private'], ['public'], ['shared'], ['private', 'shared'], ['shared', 'private'], ['public', 'shared'], ['shared', 'public']],
-        ),
+        scope=dict(type='list', choices=['private', 'public', 'shared']),
        subnet_control=dict(type='str', choices=['nd_ra', 'no_gw', 'querier_ip', 'unspecified']),
        state=dict(type='str', default='present', choices=['absent', 'present', 'query']),
        tenant=dict(type='str', aliases=['tenant_name']),
@ -366,13 +363,11 @@ def main():
    route_profile = module.params['route_profile']
    route_profile_l3_out = module.params['route_profile_l3_out']
    scope = module.params['scope']
-    if scope:
-        if len(scope) == 1:
-            scope = scope[0]
-        elif 'public' in scope:
-            scope = 'public,shared'
+    if isinstance(scope, SEQUENCETYPE):
+        if 'private' in scope and 'public' in scope:
+            module.fail_json(msg="Parameter 'scope' cannot be both 'private' and 'public', got: %s" % scope)
        else:
-            scope = 'private,shared'
+            scope = ','.join(sorted(scope))
    state = module.params['state']
    subnet_control = module.params['subnet_control']
    if subnet_control:
--- a/lib/ansible/modules/network/aci/aci_interface_policy_l2.py
+++ b/lib/ansible/modules/network/aci/aci_interface_policy_l2.py
@ -199,6 +199,8 @@ def main():
        ],
    )

+    aci = ACIModule(module)
+
    l2_policy = module.params['l2_policy']
    vlan_scope = module.params['vlan_scope']
    qinq = module.params['qinq']
@ -208,7 +210,6 @@ def main():
    description = module.params['description']
    state = module.params['state']

-    aci = ACIModule(module)
    aci.construct_url(
        root_class=dict(
            aci_class='l2IfPol',
--- a/lib/ansible/modules/network/aci/aci_interface_policy_lldp.py
+++ b/lib/ansible/modules/network/aci/aci_interface_policy_lldp.py
@ -194,13 +194,14 @@ def main():
        ],
    )

+    aci = ACIModule(module)
+
    lldp_policy = module.params['lldp_policy']
    description = module.params['description']
    receive_state = aci.boolean(module.params['receive_state'], 'enabled', 'disabled')
    transmit_state = aci.boolean(module.params['transmit_state'], 'enabled', 'disabled')
    state = module.params['state']

-    aci = ACIModule(module)
    aci.construct_url(
        root_class=dict(
            aci_class='lldpIfPol',
--- a/lib/ansible/modules/network/aci/aci_interface_policy_mcp.py
+++ b/lib/ansible/modules/network/aci/aci_interface_policy_mcp.py
@ -185,12 +185,13 @@ def main():
        ],
    )

+    aci = ACIModule(module)
+
    mcp = module.params['mcp']
    description = module.params['description']
    admin_state = aci.boolean(module.params['admin_state'], 'enabled', 'disabled')
    state = module.params['state']

-    aci = ACIModule(module)
    aci.construct_url(
        root_class=dict(
            aci_class='mcpIfPol',
--- a/lib/ansible/modules/network/aci/aci_tenant_span_src_group.py
+++ b/lib/ansible/modules/network/aci/aci_tenant_span_src_group.py
@ -197,6 +197,8 @@ def main():
        ],
    )

+    aci = ACIModule(module)
+
    admin_state = aci.boolean(module.params['admin_state'], 'enabled', 'disabled')
    description = module.params['description']
    dst_group = module.params['dst_group']
@ -204,7 +206,6 @@ def main():
    state = module.params['state']
    tenant = module.params['tenant']

-    aci = ACIModule(module)
    aci.construct_url(
        root_class=dict(
            aci_class='fvTenant',
--- a/test/integration/targets/aci_bd_subnet/tasks/main.yml
+++ b/test/integration/targets/aci_bd_subnet/tasks/main.yml
@ -109,7 +109,7 @@
      - modify_subnet.changed != modify_subnet.proposed
      - 'modify_subnet.sent == {"fvSubnet": {"attributes": {"ctrl": "querier", "scope": "public,shared"}}}'
      - create_bad_scope.failed == true
-      - 'create_bad_scope.msg.startswith("value of scope must be one of")'
+      - create_bad_scope.msg.startswith("Parameter 'scope' cannot be both 'private' and 'public'")
      - create_incomplete_data.failed == true
      - 'create_incomplete_data.msg == "state is present but all of the following are missing: bd"'