Update subnet module to be compatible with new sdk

Change-Id: Iba1604ee9c0b922b8fb7c6a278acf90d080a63e7

.zuul.yaml

@@ -99,6 +99,7 @@
         recordset
         role_assignment
         security_group
+        subnet
         subnet_pool
         user
         user_group
@@ -110,7 +111,6 @@
       # neutron_rbac
       # router
       # server
-      # subnet
 
 - job:
     name: ansible-collections-openstack-functional-devstack-octavia-base

ci/roles/subnet/defaults/main.yml

@@ -1,2 +1,26 @@
-subnet_name: shade_subnet
 enable_subnet_dhcp: false
+expected_fields:
+  - allocation_pools
+  - cidr
+  - created_at
+  - description
+  - dns_nameservers
+  - gateway_ip
+  - host_routes
+  - id
+  - ip_version
+  - ipv6_address_mode
+  - ipv6_ra_mode
+  - is_dhcp_enabled
+  - name
+  - network_id
+  - prefix_length
+  - project_id
+  - revision_number
+  - segment_id
+  - service_types
+  - subnet_pool_id
+  - tags
+  - updated_at
+  - use_default_subnet_pool
+subnet_name: shade_subnet

ci/roles/subnet/tasks/main.yml

@@ -1,4 +1,16 @@
 ---
+- name: Delete subnet {{ subnet_name }} before test
+  openstack.cloud.subnet:
+    cloud: "{{ cloud }}"
+    name: "{{ subnet_name }}"
+    state: absent
+
+- name: Delete network {{ network_name }} before test
+  openstack.cloud.network:
+    cloud: "{{ cloud }}"
+    name: "{{ network_name }}"
+    state: absent
+
 - name: Create network {{ network_name }}
   openstack.cloud.network:
     cloud: "{{ cloud }}"
@@ -19,6 +31,36 @@
     gateway_ip: 192.168.0.1
     allocation_pool_start: 192.168.0.2
     allocation_pool_end: 192.168.0.254
+  register: subnet
+
+- name: Assert changed
+  assert:
+    that: subnet is changed
+
+- name: assert subnet fields
+  assert:
+    that: item in subnet.subnet
+  loop: "{{ expected_fields }}"
+
+- name: Create subnet {{ subnet_name }} on network {{ network_name }} again
+  openstack.cloud.subnet:
+    cloud: "{{ cloud }}"
+    network_name: "{{ network_name }}"
+    name: "{{ subnet_name }}"
+    state: present
+    enable_dhcp: "{{ enable_subnet_dhcp }}"
+    dns_nameservers:
+      - 8.8.8.7
+      - 8.8.8.8
+    cidr: 192.168.0.0/24
+    gateway_ip: 192.168.0.1
+    allocation_pool_start: 192.168.0.2
+    allocation_pool_end: 192.168.0.254
+  register: subnet
+
+- name: Assert not changed
+  assert:
+    that: subnet is not changed
 
 - name: Update subnet
   openstack.cloud.subnet:
@@ -29,12 +71,33 @@
     dns_nameservers:
       - 8.8.8.7
     cidr: 192.168.0.0/24
+  register: subnet
+
+- name: Assert changed
+  assert:
+    that: subnet is changed
 
 - name: Delete subnet {{ subnet_name }}
   openstack.cloud.subnet:
     cloud: "{{ cloud }}"
     name: "{{ subnet_name }}"
     state: absent
+  register: subnet
+
+- name: Assert changed
+  assert:
+    that: subnet is changed
+
+- name: Delete subnet {{ subnet_name }} again
+  openstack.cloud.subnet:
+    cloud: "{{ cloud }}"
+    name: "{{ subnet_name }}"
+    state: absent
+  register: subnet
+
+- name: Assert not changed
+  assert:
+    that: subnet is not changed
 
 - name: Delete network {{ network_name }}
   openstack.cloud.network:

plugins/modules/subnet.py

@@ -18,17 +18,15 @@ options:
         choices: ['present', 'absent']
         default: present
         type: str
-   network_name:
+    allocation_pool_start:
         description:
-        - Name of the network to which the subnet should be attached
+            - From the subnet pool the starting address from which the IP
-        - Required when I(state) is 'present'
+              should be allocated.
         type: str
-   name:
+    allocation_pool_end:
         description:
-       - The name of the subnet that should be created. Although Neutron
+            - From the subnet pool the last IP that should be assigned to the
-         allows for non-unique subnet names, this module enforces subnet
+              virtual machines.
-         name uniqueness.
-     required: true
         type: str
     cidr:
         description:
@@ -36,41 +34,28 @@ options:
               the subnet. Required when I(state) is 'present' and a subnetpool
               is not specified.
         type: str
-   ip_version:
     description:
-        - The IP version of the subnet 4 or 6
+        description:
-     default: '4'
+            - Description of the subnet
         type: str
-     choices: ['4', '6']
+    disable_gateway_ip:
-   enable_dhcp:
-     description:
-        - Whether DHCP should be enabled for this subnet.
-     type: bool
-     default: 'yes'
-   gateway_ip:
-     description:
-        - The ip that would be assigned to the gateway for this subnet
-     type: str
-   no_gateway_ip:
         description:
             - The gateway IP would not be assigned for this subnet
         type: bool
+        aliases: ['no_gateway_ip']
         default: 'no'
     dns_nameservers:
         description:
             - List of DNS nameservers for this subnet.
         type: list
         elements: str
-   allocation_pool_start:
+    extra_attrs:
         description:
-        - From the subnet pool the starting address from which the IP should
+            - Dictionary with extra key/value pairs passed to the API
-          be allocated.
+        required: false
-     type: str
+        aliases: ['extra_specs']
-   allocation_pool_end:
+        default: {}
-     description:
+        type: dict
-        - From the subnet pool the last IP that should be assigned to the
-          virtual machines.
-     type: str
     host_routes:
         description:
             - A list of host route dictionaries for the subnet.
@@ -85,6 +70,22 @@ options:
                 description: The next hop (aka gateway) for the I(destination).
                 type: str
                 required: true
+    gateway_ip:
+        description:
+            - The ip that would be assigned to the gateway for this subnet
+        type: str
+    ip_version:
+        description:
+            - The IP version of the subnet 4 or 6
+        default: 4
+        type: int
+        choices: [4, 6]
+    is_dhcp_enabled:
+        description:
+            - Whether DHCP should be enabled for this subnet.
+        type: bool
+        aliases: ['enable_dhcp']
+        default: 'yes'
     ipv6_ra_mode:
         description:
             - IPv6 router advertisement mode
@@ -95,21 +96,37 @@ options:
             - IPv6 address mode
         choices: ['dhcpv6-stateful', 'dhcpv6-stateless', 'slaac']
         type: str
-   use_default_subnetpool:
+    name:
         description:
-        - Use the default subnetpool for I(ip_version) to obtain a CIDR.
+            - The name of the subnet that should be created. Although Neutron
-     type: bool
+              allows for non-unique subnet names, this module enforces subnet
-     default: 'no'
+              name uniqueness.
+        required: true
+        type: str
+    network:
+        description:
+            - Name or id of the network to which the subnet should be attached
+            - Required when I(state) is 'present'
+        aliases: ['network_name']
+        type: str
     project:
         description:
             - Project name or ID containing the subnet (name admin-only)
         type: str
-   extra_specs:
+    prefix_length:
         description:
-        - Dictionary with extra key/value pairs passed to the API
+            - The prefix length to use for subnet allocation from a subnet pool
+        type: str
+    use_default_subnet_pool:
+        description:
+            - Use the default subnetpool for I(ip_version) to obtain a CIDR.
+        type: bool
+        aliases: ['use_default_subnetpool']
+    subnet_pool:
+        description:
+            - The subnet pool name or ID from which to obtain a CIDR
+        type: str
         required: false
-     default: {}
-     type: dict
 requirements:
     - "python >= 3.6"
     - "openstacksdk"
@@ -153,6 +170,120 @@ EXAMPLES = '''
     ipv6_address_mode: dhcpv6-stateless
 '''
 
+RETURN = '''
+id:
+    description: Id of subnet
+    returned: On success when subnet exists.
+    type: str
+subnet:
+    description: Dictionary describing the subnet.
+    returned: On success when subnet exists.
+    type: dict
+    contains:
+        allocation_pools:
+            description: Allocation pools associated with this subnet.
+            returned: success
+            type: list
+            elements: dict
+        cidr:
+            description: Subnet's CIDR.
+            returned: success
+            type: str
+        created_at:
+            description: Created at timestamp
+            type: str
+        description:
+            description: Description
+            type: str
+        dns_nameservers:
+            description: DNS name servers for this subnet.
+            returned: success
+            type: list
+            elements: str
+        dns_publish_fixed_ip:
+            description: Whether to publish DNS records for fixed IPs.
+            returned: success
+            type: bool
+        gateway_ip:
+            description: Subnet's gateway ip.
+            returned: success
+            type: str
+        host_routes:
+            description: A list of host routes.
+            returned: success
+            type: str
+        id:
+            description: Unique UUID.
+            returned: success
+            type: str
+        ip_version:
+            description: IP version for this subnet.
+            returned: success
+            type: int
+        ipv6_address_mode:
+            description: |
+                The IPv6 address modes which are 'dhcpv6-stateful',
+                'dhcpv6-stateless' or 'slaac'.
+            returned: success
+            type: str
+        ipv6_ra_mode:
+            description: |
+                The IPv6 router advertisements modes which can be 'slaac',
+                'dhcpv6-stateful', 'dhcpv6-stateless'.
+            returned: success
+            type: str
+        is_dhcp_enabled:
+            description: DHCP enable flag for this subnet.
+            returned: success
+            type: bool
+        name:
+            description: Name given to the subnet.
+            returned: success
+            type: str
+        network_id:
+            description: Network ID this subnet belongs in.
+            returned: success
+            type: str
+        prefix_length:
+            description: |
+                The prefix length to use for subnet allocation from a subnet
+                pool.
+            returned: success
+            type: str
+        project_id:
+            description: Project id associated with this subnet.
+            returned: success
+            type: str
+        revision_number:
+            description: Revision number of the resource
+            returned: success
+            type: int
+        segment_id:
+            description: The ID of the segment this subnet is associated with.
+            returned: success
+            type: str
+        service_types:
+            description: Service types for this subnet
+            returned: success
+            type: list
+        subnet_pool_id:
+            description: The subnet pool ID from which to obtain a CIDR.
+            returned: success
+            type: str
+        tags:
+            description: Tags
+            type: str
+        updated_at:
+            description: Timestamp when the subnet was last updated.
+            returned: success
+            type: str
+        use_default_subnet_pool:
+            description: |
+                Whether to use the default subnet pool to obtain a CIDR.
+            returned: success
+            type: bool
+'''
+
 from ansible_collections.openstack.cloud.plugins.module_utils.openstack import OpenStackModule
 
 
@@ -160,198 +291,175 @@ class SubnetModule(OpenStackModule):
     ipv6_mode_choices = ['dhcpv6-stateful', 'dhcpv6-stateless', 'slaac']
     argument_spec = dict(
         name=dict(type='str', required=True),
-        network_name=dict(type='str'),
+        network=dict(type='str', aliases=['network_name']),
         cidr=dict(type='str'),
-        ip_version=dict(type='str', default='4', choices=['4', '6']),
+        description=dict(type='str'),
-        enable_dhcp=dict(type='bool', default=True),
+        ip_version=dict(type='int', default=4, choices=[4, 6]),
+        is_dhcp_enabled=dict(type='bool', default=True,
+                             aliases=['enable_dhcp']),
         gateway_ip=dict(type='str'),
-        no_gateway_ip=dict(type='bool', default=False),
+        disable_gateway_ip=dict(
+            type='bool', default=False, aliases=['no_gateway_ip']),
         dns_nameservers=dict(type='list', default=None, elements='str'),
         allocation_pool_start=dict(type='str'),
         allocation_pool_end=dict(type='str'),
         host_routes=dict(type='list', default=None, elements='dict'),
         ipv6_ra_mode=dict(type='str', choices=ipv6_mode_choices),
         ipv6_address_mode=dict(type='str', choices=ipv6_mode_choices),
-        use_default_subnetpool=dict(type='bool', default=False),
+        subnet_pool=dict(type='str'),
-        extra_specs=dict(type='dict', default=dict()),
+        prefix_length=dict(type='str'),
-        state=dict(type='str', default='present', choices=['absent', 'present']),
+        use_default_subnet_pool=dict(
+            type='bool', aliases=['use_default_subnetpool']),
+        extra_attrs=dict(type='dict', default=dict(), aliases=['extra_specs']),
+        state=dict(type='str', default='present',
+                   choices=['absent', 'present']),
         project=dict(type='str'),
     )
 
     module_kwargs = dict(
         supports_check_mode=True,
-        required_together=[['allocation_pool_end', 'allocation_pool_start']]
+        required_together=[['allocation_pool_end', 'allocation_pool_start']],
+        required_if=[
+            ('state', 'present', ('network',)),
+            ('state', 'present',
+             ('cidr', 'use_default_subnet_pool', 'subnet_pool'), True),
+        ],
+        mutually_exclusive=[
+            ('cidr', 'use_default_subnet_pool', 'subnet_pool')
+        ]
     )
 
-    def _can_update(self, subnet, filters=None):
+    # resource attributes obtainable directly from params
+    attr_params = ('cidr', 'description',
+                   'dns_nameservers', 'gateway_ip', 'host_routes',
+                   'ip_version', 'ipv6_address_mode', 'ipv6_ra_mode',
+                   'is_dhcp_enabled', 'name', 'prefix_length',
+                   'use_default_subnet_pool',)
+
+    def _validate_update(self, subnet, update):
         """ Check for differences in non-updatable values """
-        network_name = self.params['network_name']
+        # Ref.: https://docs.openstack.org/api-ref/network/v2/index.html#update-subnet
-        ip_version = int(self.params['ip_version'])
+        for attr in ('cidr', 'ip_version', 'ipv6_ra_mode', 'ipv6_address_mode',
-        ipv6_ra_mode = self.params['ipv6_ra_mode']
+                     'prefix_length', 'use_default_subnet_pool'):
-        ipv6_a_mode = self.params['ipv6_address_mode']
+            if attr in update and update[attr] != subnet[attr]:
+                self.fail_json(
+                    msg='Cannot update {0} in existing subnet'.format(attr))
 
-        if network_name:
+    def _system_state_change(self, subnet, network, project, subnet_pool):
-            network = self.conn.get_network(network_name, filters)
-            if network:
-                netid = network['id']
-                if netid != subnet['network_id']:
-                    self.fail_json(msg='Cannot update network_name in existing subnet')
-            else:
-                self.fail_json(msg='No network found for %s' % network_name)
-
-        if ip_version and subnet['ip_version'] != ip_version:
-            self.fail_json(msg='Cannot update ip_version in existing subnet')
-        if ipv6_ra_mode and subnet.get('ipv6_ra_mode', None) != ipv6_ra_mode:
-            self.fail_json(msg='Cannot update ipv6_ra_mode in existing subnet')
-        if ipv6_a_mode and subnet.get('ipv6_address_mode', None) != ipv6_a_mode:
-            self.fail_json(msg='Cannot update ipv6_address_mode in existing subnet')
-
-    def _needs_update(self, subnet, filters=None):
-        """Check for differences in the updatable values."""
-
-        # First check if we are trying to update something we're not allowed to
-        self._can_update(subnet, filters)
-
-        # now check for the things we are allowed to update
-        enable_dhcp = self.params['enable_dhcp']
-        subnet_name = self.params['name']
-        pool_start = self.params['allocation_pool_start']
-        pool_end = self.params['allocation_pool_end']
-        gateway_ip = self.params['gateway_ip']
-        no_gateway_ip = self.params['no_gateway_ip']
-        dns = self.params['dns_nameservers']
-        host_routes = self.params['host_routes']
-        if pool_start and pool_end:
-            pool = dict(start=pool_start, end=pool_end)
-        else:
-            pool = None
-
-        changes = dict()
-        if subnet['enable_dhcp'] != enable_dhcp:
-            changes['enable_dhcp'] = enable_dhcp
-        if subnet_name and subnet['name'] != subnet_name:
-            changes['subnet_name'] = subnet_name
-        if pool and (not subnet['allocation_pools'] or subnet['allocation_pools'] != [pool]):
-            changes['allocation_pools'] = [pool]
-        if gateway_ip and subnet['gateway_ip'] != gateway_ip:
-            changes['gateway_ip'] = gateway_ip
-        if dns and sorted(subnet['dns_nameservers']) != sorted(dns):
-            changes['dns_nameservers'] = dns
-        if host_routes:
-            curr_hr = sorted(subnet['host_routes'], key=lambda t: t.keys())
-            new_hr = sorted(host_routes, key=lambda t: t.keys())
-            if curr_hr != new_hr:
-                changes['host_routes'] = host_routes
-        if no_gateway_ip and subnet['gateway_ip']:
-            changes['disable_gateway_ip'] = no_gateway_ip
-        return changes
-
-    def _system_state_change(self, subnet, filters=None):
         state = self.params['state']
-        if state == 'present':
+        if state == 'absent':
+            return subnet is not None
+        # else state is present
         if not subnet:
             return True
-            return bool(self._needs_update(subnet, filters))
+        params = self._build_params(network, project, subnet_pool)
-        if state == 'absent' and subnet:
+        updates = self._build_updates(subnet, params)
-            return True
+        self._validate_update(subnet, updates)
-        return False
+        return bool(updates)
+
+    def _build_pool(self):
+        pool_start = self.params['allocation_pool_start']
+        pool_end = self.params['allocation_pool_end']
+        if pool_start:
+            return [dict(start=pool_start, end=pool_end)]
+        return None
+
+    def _build_params(self, network, project, subnet_pool):
+        params = {attr: self.params[attr] for attr in self.attr_params}
+        params['network_id'] = network.id
+        if project:
+            params['project_id'] = project.id
+        if subnet_pool:
+            params['subnet_pool_id'] = subnet_pool.id
+        params['allocation_pools'] = self._build_pool()
+        params = self._add_extra_attrs(params)
+        params = {k: v for k, v in params.items() if v is not None}
+        return params
+
+    def _build_updates(self, subnet, params):
+        # Sort lists before doing comparisons comparisons
+        if 'dns_nameservers' in params:
+            params['dns_nameservers'].sort()
+            subnet['dns_nameservers'].sort()
+
+        if 'host_routes' in params:
+            params['host_routes'].sort(key=lambda r: sorted(r.items()))
+            subnet['host_routes'].sort(key=lambda r: sorted(r.items()))
+
+        updates = {k: params[k] for k in params if params[k] != subnet[k]}
+        if self.params['disable_gateway_ip'] and subnet.gateway_ip:
+            updates['gateway_ip'] = None
+        return updates
+
+    def _add_extra_attrs(self, params):
+        duplicates = set(self.params['extra_attrs']) & set(params)
+        if duplicates:
+            self.fail_json(msg='Duplicate key(s) {0} in extra_specs'
+                           .format(list(duplicates)))
+        params.update(self.params['extra_attrs'])
+        return params
 
     def run(self):
-
         state = self.params['state']
-        network_name = self.params['network_name']
+        network_name_or_id = self.params['network']
-        cidr = self.params['cidr']
+        project_name_or_id = self.params['project']
-        ip_version = self.params['ip_version']
+        subnet_pool_name_or_id = self.params['subnet_pool']
-        enable_dhcp = self.params['enable_dhcp']
         subnet_name = self.params['name']
         gateway_ip = self.params['gateway_ip']
-        no_gateway_ip = self.params['no_gateway_ip']
+        disable_gateway_ip = self.params['disable_gateway_ip']
-        dns = self.params['dns_nameservers']
-        pool_start = self.params['allocation_pool_start']
-        pool_end = self.params['allocation_pool_end']
-        host_routes = self.params['host_routes']
-        ipv6_ra_mode = self.params['ipv6_ra_mode']
-        ipv6_a_mode = self.params['ipv6_address_mode']
-        use_default_subnetpool = self.params['use_default_subnetpool']
-        project = self.params.pop('project')
-        extra_specs = self.params['extra_specs']
 
-        # Check for required parameters when state == 'present'
+        # fail early if incompatible options have been specified
-        if state == 'present':
+        if disable_gateway_ip and gateway_ip:
-            if not self.params['network_name']:
-                self.fail(msg='network_name required with present state')
-            if (
-                not self.params['cidr']
-                and not use_default_subnetpool
-                and not extra_specs.get('subnetpool_id', False)
-            ):
-                self.fail(msg='cidr or use_default_subnetpool or '
-                          'subnetpool_id required with present state')
-
-        if pool_start and pool_end:
-            pool = [dict(start=pool_start, end=pool_end)]
-        else:
-            pool = None
-
-        if no_gateway_ip and gateway_ip:
             self.fail_json(msg='no_gateway_ip is not allowed with gateway_ip')
 
-        if project is not None:
+        subnet_pool_filters = {}
-            proj = self.conn.get_project(project)
+        filters = {}
-            if proj is None:
-                self.fail_json(msg='Project %s could not be found' % project)
-            project_id = proj['id']
-            filters = {'tenant_id': project_id}
-        else:
-            project_id = None
-            filters = None
 
-        subnet = self.conn.get_subnet(subnet_name, filters=filters)
+        project = None
+        if project_name_or_id:
+            project = self.conn.identity.find_project(project_name_or_id,
+                                                      ignore_missing=False)
+            subnet_pool_filters['project_id'] = project.id
+            filters['project_id'] = project.id
+
+        network = None
+        if network_name_or_id:
+            # At this point filters can only contain project_id
+            network = self.conn.network.find_network(network_name_or_id,
+                                                     ignore_missing=False,
+                                                     **filters)
+            filters['network_id'] = network.id
+
+        subnet_pool = None
+        if subnet_pool_name_or_id:
+            subnet_pool = self.conn.network.find_subnet_pool(
+                subnet_pool_name_or_id,
+                ignore_missing=False,
+                **subnet_pool_filters)
+            filters['subnet_pool_id'] = subnet_pool.id
+
+        subnet = self.conn.network.find_subnet(subnet_name, **filters)
 
         if self.ansible.check_mode:
-            self.exit_json(changed=self._system_state_change(subnet, filters))
+            self.exit_json(changed=self._system_state_change(
+                subnet, network, project, subnet_pool))
 
+        changed = False
         if state == 'present':
-            if not subnet:
+            params = self._build_params(network, project, subnet_pool)
-                kwargs = dict(
+            if subnet is None:
-                    cidr=cidr,
+                subnet = self.conn.network.create_subnet(**params)
-                    ip_version=ip_version,
-                    enable_dhcp=enable_dhcp,
-                    subnet_name=subnet_name,
-                    gateway_ip=gateway_ip,
-                    disable_gateway_ip=no_gateway_ip,
-                    dns_nameservers=dns,
-                    allocation_pools=pool,
-                    host_routes=host_routes,
-                    ipv6_ra_mode=ipv6_ra_mode,
-                    ipv6_address_mode=ipv6_a_mode,
-                    tenant_id=project_id)
-                dup_args = set(kwargs.keys()) & set(extra_specs.keys())
-                if dup_args:
-                    raise ValueError('Duplicate key(s) {0} in extra_specs'
-                                     .format(list(dup_args)))
-                if use_default_subnetpool:
-                    kwargs['use_default_subnetpool'] = use_default_subnetpool
-                kwargs = dict(kwargs, **extra_specs)
-                subnet = self.conn.create_subnet(network_name, **kwargs)
                 changed = True
             else:
-                changes = self._needs_update(subnet, filters)
+                updates = self._build_updates(subnet, params)
-                if changes:
+                if updates:
-                    subnet = self.conn.update_subnet(subnet['id'], **changes)
+                    self._validate_update(subnet, updates)
+                    subnet = self.conn.network.update_subnet(subnet, **updates)
                     changed = True
-                else:
+            self.exit_json(changed=changed, subnet=subnet, id=subnet.id)
-                    changed = False
+        elif state == 'absent' and subnet is not None:
-            self.exit_json(changed=changed,
+            self.conn.network.delete_subnet(subnet)
-                           subnet=subnet,
-                           id=subnet['id'])
-
-        elif state == 'absent':
-            if not subnet:
-                changed = False
-            else:
             changed = True
-                self.conn.delete_subnet(subnet_name)
         self.exit_json(changed=changed)