Update router for 2.0.0

- Change sdk calls to use proxies
- Convert return values to
- Update module docs
- Change argspec to more closely match the new sdk (and therefore the
  API) without breaking backward compatibility

Change-Id: I0f9bc573fd0c69cab65bd808145d628732bb0830

ci/roles/router/defaults/main.yml

@@ -6,7 +6,6 @@ expected_fields:
   - external_gateway_info
   - flavor_id
   - id
-  - interfaces_info
   - is_admin_state_up
   - is_distributed
   - is_ha
@@ -18,6 +17,28 @@ expected_fields:
   - tags
   - tenant_id
   - updated_at
+network_name: ansible_net
 external_network_name: ansible_external_net
 network_external: true
 router_name: ansible_router
+test_subnets:
+  - cloud: "{{ cloud }}"
+    state: present
+    network_name: "{{ network_name }}"
+    name: shade_subnet1
+    cidr: 10.7.7.0/24
+  - cloud: "{{ cloud }}"
+    state: present
+    network_name: "{{ network_name }}"
+    name: shade_subnet2
+    cidr: 10.8.8.0/24
+  - cloud: "{{ cloud }}"
+    state: present
+    network_name: "{{ network_name }}"
+    name: shade_subnet3
+    cidr: 10.9.9.0/24
+  - cloud: "{{ cloud }}"
+    state: present
+    network_name: "{{ network_name }}"
+    name: shade_subnet4
+    cidr: 10.10.10.0/24

ci/roles/router/tasks/main.yml

@@ -6,44 +6,51 @@
      state: present
      name: "{{ network_name }}"
      external: false
+  register: internal_net
 
-- name: Create subnet1
+- name: Ensure router doesn't exist before tests
-  openstack.cloud.subnet:
+  openstack.cloud.router:
      cloud: "{{ cloud }}"
-     state: present
+     state: absent
-     network_name: "{{ network_name }}"
+     name: "{{ router_name }}"
-     name: shade_subnet1
-     cidr: 10.7.7.0/24
 
-- name: Create subnet2
+- name: Get ports in internal network
-  openstack.cloud.subnet:
+  openstack.cloud.port_info:
-     cloud: "{{ cloud }}"
+    cloud: "{{ cloud }}"
-     state: present
+    filters:
-     network_name: "{{ network_name }}"
+      network_id: "{{ internal_net.id }}"
-     name: shade_subnet2
+  register: existing_ports
-     cidr: 10.8.8.0/24
 
-- name: Create subnet3
+- name: Ensure ports don't exist before tests
-  openstack.cloud.subnet:
+  openstack.cloud.port:
-     cloud: "{{ cloud }}"
+    cloud: "{{ cloud }}"
-     state: present
+    name: "{{ item.id }}"
-     network_name: "{{ network_name }}"
+    state: absent
-     name: shade_subnet3
+  loop: "{{ existing_ports.ports }}"
-     cidr: 10.9.9.0/24
 
-- name: Create subnet4
+- name: Delete subnets before tests
   openstack.cloud.subnet:
-     cloud: "{{ cloud }}"
+    cloud: "{{ cloud }}"
-     state: present
+    state: absent
-     network_name: "{{ network_name }}"
+    name: "{{ item.name }}"
-     name: shade_subnet4
+  loop: "{{ test_subnets }}"
-     cidr: 10.10.10.0/24
+
+- name: Create subnets 1-4
+  openstack.cloud.subnet: "{{ item }}"
+  loop: "{{ test_subnets }}"
 
 - name: Create router
   openstack.cloud.router:
      cloud: "{{ cloud }}"
      state: present
      name: "{{ router_name }}"
+  register: result
+
+- name: Verify returned values
+  assert:
+    that:
+      - item in result.router
+  loop: "{{ expected_fields }}"
 
 - name: Update router (add interface)
   openstack.cloud.router:
@@ -80,6 +87,12 @@
       - "result.routers.0.name == router_name"
       - (result.routers.0.interfaces_info|length) == 1
 
+- name: Verify existence of return values
+  assert:
+    that:
+      - item in result.routers[0]
+  loop: "{{ expected_fields + ['interfaces_info'] }}"
+
 - name: Gather routers info with filters
   openstack.cloud.routers_info:
       cloud: "{{ cloud }}"
@@ -94,12 +107,6 @@
       - "result_filter.routers.0.name == router_name"
       - (result_filter.routers.0.interfaces_info|length) == 1
 
-- name: Verify returned values
-  assert:
-    that:
-      - item in result.routers[0]
-  loop: '{{ expected_fields }}'
-
 - name: Update router (change interfaces)
   openstack.cloud.router:
      cloud: "{{ cloud }}"
@@ -270,12 +277,13 @@
      name: "{{ router_name }}"
      interfaces:
          - shade_subnet1
-     network: "{{ external_network_name }}"
+     external_gateway_info:
-     external_fixed_ips:
+        network: "{{ external_network_name }}"
-        - subnet: shade_subnet5
+        external_fixed_ips:
-          ip: 10.6.6.100
+           - subnet: shade_subnet5
-        - subnet: shade_subnet5
+             ip: 10.6.6.100
-          ip: 10.6.6.101
+           - subnet: shade_subnet5
+             ip: 10.6.6.101
   when:
     - network_external
 
@@ -368,35 +376,145 @@
   when:
     - network_external
 
+- name: Update router (disable external snat)
+  openstack.cloud.router:
+     cloud: "{{ cloud }}"
+     state: present
+     name: "{{ router_name }}"
+     enable_snat: false
+     interfaces:
+         - shade_subnet1
+     network: "{{ external_network_name }}"
+     external_fixed_ips:
+        - subnet: shade_subnet5
+          ip: 10.6.6.101
+  when:
+    - network_external
+  register: result
+
+- name: Gather routers info
+  openstack.cloud.routers_info:
+     cloud: "{{ cloud }}"
+     name: "{{ router_name }}"
+     filters:
+       admin_state_up: true
+  register: result
+- name: Verify routers info
+  assert:
+    that:
+      - "result.routers.0.name == router_name"
+      - "not result.routers.0.external_gateway_info.enable_snat"
+  when:
+    - network_external
+
+- name: Update router (disable external snat) again
+  openstack.cloud.router:
+     cloud: "{{ cloud }}"
+     state: present
+     name: "{{ router_name }}"
+     enable_snat: false
+     interfaces:
+         - shade_subnet1
+     network: "{{ external_network_name }}"
+     external_fixed_ips:
+        - subnet: shade_subnet5
+          ip: 10.6.6.101
+  when:
+    - network_external
+  register: result
+
+- name: Assert idempotent module
+  assert:
+    that: result is not changed
+
 - name: Delete router
   openstack.cloud.router:
      cloud: "{{ cloud }}"
      state: absent
      name: "{{ router_name }}"
 
-- name: Delete subnet1
+- name: Delete router again
-  openstack.cloud.subnet:
+  openstack.cloud.router:
      cloud: "{{ cloud }}"
      state: absent
-     name: shade_subnet1
+     name: "{{ router_name }}"
+  register: result
 
-- name: Delete subnet2
+- name: Assert idempotent module
-  openstack.cloud.subnet:
+  assert:
-     cloud: "{{ cloud }}"
+    that: result is not changed
-     state: absent
-     name: shade_subnet2
 
-- name: Delete subnet3
+- name: Create router with simple interface
-  openstack.cloud.subnet:
+  openstack.cloud.router:
      cloud: "{{ cloud }}"
-     state: absent
+     state: present
-     name: shade_subnet3
+     name: "{{ router_name }}"
+     interfaces:
+         - shade_subnet1
+  register: result
 
-- name: Delete subnet4
+- name: Assert changed
-  openstack.cloud.subnet:
+  assert:
+    that: result is changed
+
+- name: Set portip in already assigned subnet
+  openstack.cloud.router:
+     cloud: "{{ cloud }}"
+     state: present
+     name: "{{ router_name }}"
+     interfaces:
+         - subnet: shade_subnet1
+           net: "{{ network_name }}"
+           portip: 10.7.7.42
+  register: result
+
+- name: Assert changed
+  assert:
+    that: result is changed
+
+- name: Gather routers info
+  openstack.cloud.routers_info:
+     cloud: "{{ cloud }}"
+     name: "{{ router_name }}"
+  register: result
+
+- name: Verify routers info
+  assert:
+    that: "'10.7.7.42' in result.routers[0].interfaces_info|map(attribute='ip_address')"
+
+- name: Unset portip in already assigned subnet
+  openstack.cloud.router:
+     cloud: "{{ cloud }}"
+     state: present
+     name: "{{ router_name }}"
+     interfaces:
+         - subnet: shade_subnet1
+           net: "{{ network_name }}"
+  register: result
+
+- name: Assert not changed
+  assert:
+    that: result is not changed
+
+- name: Delete router
+  openstack.cloud.router:
      cloud: "{{ cloud }}"
      state: absent
-     name: shade_subnet4
+     name: "{{ router_name }}"
+
+- name: Get ports in internal network
+  openstack.cloud.port_info:
+    cloud: "{{ cloud }}"
+    filters:
+      network_id: "{{ internal_net.id }}"
+  register: existing_ports
+
+- name: Clean up ports
+  openstack.cloud.port:
+    cloud: "{{ cloud }}"
+    name: "{{ item.id }}"
+    state: absent
+  loop: "{{ existing_ports.ports }}"
 
 - name: Delete subnet5
   openstack.cloud.subnet:
@@ -406,6 +524,13 @@
   when:
     - network_external
 
+- name: Delete subnets 1-4
+  openstack.cloud.subnet:
+    cloud: "{{ cloud }}"
+    state: absent
+    name: "{{ item.name }}"
+  loop: "{{ test_subnets }}"
+
 - name: Delete internal network
   openstack.cloud.network:
      cloud: "{{ cloud }}"

plugins/modules/router.py

@@ -25,15 +25,16 @@ options:
         - Name to be give to the router
      required: true
      type: str
-   admin_state_up:
-     description:
-        - Desired admin state of the created or existing router.
-     type: bool
-     default: 'yes'
    enable_snat:
      description:
         - Enable Source NAT (SNAT) attribute.
      type: bool
+   is_admin_state_up:
+     description:
+        - Desired admin state of the created or existing router.
+     type: bool
+     default: 'yes'
+     aliases: [admin_state_up]
    network:
      description:
         - Unique name or ID of the external gateway network.
@@ -43,6 +44,39 @@ options:
      description:
         - Unique name or ID of the project.
      type: str
+   external_gateway_info:
+     description:
+       - Information about the router's external gateway
+     type: dict
+     suboptions:
+       network:
+         description:
+            - Unique name or ID of the external gateway network.
+            - required I(interfaces) or I(enable_snat) are provided.
+         type: str
+       enable_snat:
+         description:
+            - Unique name or ID of the external gateway network.
+            - required I(interfaces) or I(enable_snat) are provided.
+         type: bool
+       external_fixed_ips:
+         description:
+            - The IP address parameters for the external gateway network. Each
+              is a dictionary with the subnet name or ID (subnet) and the IP
+              address to assign on the subnet (ip). If no IP is specified,
+              one is automatically assigned from that subnet.
+         type: list
+         elements: dict
+         suboptions:
+            ip_address:
+               description: The fixed IP address to attempt to allocate.
+               type: str
+               aliases: [ip]
+            subnet_id:
+               description: The subnet to attach the IP address to.
+               required: true
+               type: str
+               aliases: [subnet]
    external_fixed_ips:
      description:
         - The IP address parameters for the external gateway network. Each
@@ -52,13 +86,15 @@ options:
      type: list
      elements: dict
      suboptions:
-        ip:
+        ip_address:
            description: The fixed IP address to attempt to allocate.
+           type: str
+           aliases: [ip]
+        subnet_id:
+           description: The subnet to attach the IP address to.
            required: true
            type: str
-        subnet:
+           aliases: [subnet]
-           description: The subnet to attach the IP address to.
-           type: str
    interfaces:
      description:
         - List of subnets to attach to the router internal interface. Default
@@ -172,7 +208,7 @@ RETURN = '''
 router:
     description: Dictionary describing the router.
     returned: On success when I(state) is 'present'
-    type: complex
+    type: dict
     contains:
         id:
             description: Router ID.
@@ -212,256 +248,327 @@ router:
 '''
 
 from ansible_collections.openstack.cloud.plugins.module_utils.openstack import OpenStackModule
-import itertools
+from collections import defaultdict
+
+
+ext_ips_spec = dict(type='list', elements='dict', options=dict(
+    ip_address=dict(aliases=["ip"]),
+    subnet_id=dict(required=True, aliases=["subnet"]),
+))
 
 
 class RouterModule(OpenStackModule):
     argument_spec = dict(
         state=dict(default='present', choices=['absent', 'present']),
         name=dict(required=True),
-        admin_state_up=dict(type='bool', default=True),
+        is_admin_state_up=dict(type='bool', default=True,
+                               aliases=['admin_state_up']),
         enable_snat=dict(type='bool'),
         network=dict(),
         interfaces=dict(type='list', elements='raw'),
-        external_fixed_ips=dict(type='list', elements='dict'),
+        external_fixed_ips=ext_ips_spec,
+        external_gateway_info=dict(type='dict', options=dict(
+            network=dict(),
+            enable_snat=dict(type='bool'),
+            external_fixed_ips=ext_ips_spec,
+        )),
         project=dict()
     )
 
-    def _get_subnet_ids_from_ports(self, ports):
+    module_kwargs = dict(
-        return [fixed_ip['subnet_id'] for fixed_ip in
+        mutually_exclusive=[
-                itertools.chain.from_iterable(port['fixed_ips'] for port in ports if 'fixed_ips' in port)]
+            ('external_gateway_info', 'network'),
+            ('external_gateway_info', 'external_fixed_ips'),
+            ('external_gateway_info', 'enable_snat'),
+        ],
+        required_by=dict(
+            external_fixed_ips='network',
+        ),
+    )
 
-    def _needs_update(self, router, net,
+    def _needs_update(self, router, kwargs, external_fixed_ips, to_add,
-                      missing_port_ids,
+                      to_remove, missing_port_ids):
-                      requested_subnet_ids,
-                      existing_subnet_ids,
-                      router_ifs_cfg):
         """Decide if the given router needs an update."""
-        if router['admin_state_up'] != self.params['admin_state_up']:
+        if router['is_admin_state_up'] != self.params['is_admin_state_up']:
             return True
-        if router['external_gateway_info']:
+
-            # check if enable_snat is set in module params
+        cur_ext_gw_info = router['external_gateway_info']
-            if self.params['enable_snat'] is not None:
+        if 'external_gateway_info' in kwargs:
-                if router['external_gateway_info'].get('enable_snat', True) != self.params['enable_snat']:
+            if cur_ext_gw_info is None:
+                return True
+            update = kwargs['external_gateway_info']
+            for attr in ('enable_snat', 'network_id'):
+                if attr in update and cur_ext_gw_info[attr] != update[attr]:
                     return True
-        if net:
-            if not router['external_gateway_info']:
-                return True
-            elif router['external_gateway_info']['network_id'] != net['id']:
-                return True
 
-        # check if external_fixed_ip has to be added
+        cur_ext_gw_info = router['external_gateway_info']
-        for external_fixed_ip in router_ifs_cfg['external_fixed_ips']:
+        cur_ext_fips = (cur_ext_gw_info or {}) \
-            exists = False
+            .get('external_fixed_ips', [])
+        # map of external fixed ip subnets to addresses
+        cur_fip_map = defaultdict(set)
+        for p in cur_ext_fips:
+            if 'ip_address' in p:
+                cur_fip_map[p['subnet_id']].add(p['ip_address'])
+        req_fip_map = defaultdict(set)
+        for p in external_fixed_ips:
+            if 'ip_address' in p:
+                req_fip_map[p['subnet_id']].add(p['ip_address'])
 
-            # compare the requested interface with existing, looking for an existing match
+        # Check if external ip addresses need to be added
-            for existing_if in router['external_gateway_info']['external_fixed_ips']:
+        for fip in external_fixed_ips:
-                if existing_if['subnet_id'] == external_fixed_ip['subnet_id']:
+            subnet = fip['subnet_id']
-                    if 'ip' in external_fixed_ip:
+            ip = fip.get('ip_address', None)
-                        if existing_if['ip_address'] == external_fixed_ip['ip']:
+            if subnet in cur_fip_map:
-                            # both subnet id and ip address match
+                if ip is not None and ip not in cur_fip_map[subnet]:
-                            exists = True
-                            break
-                    else:
-                        # only the subnet was given, so ip doesn't matter
-                        exists = True
-                        break
-
-            # this interface isn't present on the existing router
-            if not exists:
-                return True
-
-        # check if external_fixed_ip has to be removed
-        if router_ifs_cfg['external_fixed_ips']:
-            for external_fixed_ip in router['external_gateway_info']['external_fixed_ips']:
-                obsolete = True
-
-                # compare the existing interface with requested, looking for an requested match
-                for requested_if in router_ifs_cfg['external_fixed_ips']:
-                    if external_fixed_ip['subnet_id'] == requested_if['subnet_id']:
-                        if 'ip' in requested_if:
-                            if external_fixed_ip['ip_address'] == requested_if['ip']:
-                                # both subnet id and ip address match
-                                obsolete = False
-                                break
-                        else:
-                            # only the subnet was given, so ip doesn't matter
-                            obsolete = False
-                            break
-
-                # this interface isn't present on the existing router
-                if obsolete:
                     return True
-        else:
+            else:
-            # no external fixed ips requested
+                return True
-            if router['external_gateway_info'] \
+        # Check if external ip addresses need to be removed
-               and router['external_gateway_info']['external_fixed_ips'] \
+        for fip in cur_ext_fips:
-               and len(router['external_gateway_info']['external_fixed_ips']) > 1:
+            subnet = fip['subnet_id']
-                # but router has several external fixed ips
+            ip = fip['ip_address']
+            if subnet in req_fip_map:
+                if ip not in req_fip_map[subnet]:
+                    return True
+            else:
                 return True
 
-        # check if internal port has to be added
+        if not external_fixed_ips and len(cur_ext_fips) > 1:
-        if router_ifs_cfg['internal_ports_missing']:
+            # no external fixed ips requested but router has several external
+            # fixed ips
             return True
 
-        if missing_port_ids:
+        # check if internal interfaces need update
-            return True
+        if to_add or to_remove or missing_port_ids:
-
-        # check if internal subnet has to be added or removed
-        if set(requested_subnet_ids) != set(existing_subnet_ids):
             return True
 
         return False
 
-    def _build_kwargs(self, router, net):
+    def _build_kwargs(self, router, network, ext_fixed_ips):
         kwargs = {
-            'admin_state_up': self.params['admin_state_up'],
+            'is_admin_state_up': self.params['is_admin_state_up'],
         }
 
-        if router:
+        if not router:
-            kwargs['name_or_id'] = router['id']
-        else:
             kwargs['name'] = self.params['name']
 
-        if net:
+        curr_ext_gw_info = None
-            kwargs['ext_gateway_net_id'] = net['id']
+        if router:
-            # can't send enable_snat unless we have a network
+            curr_ext_gw_info = router['external_gateway_info']
-            if self.params.get('enable_snat') is not None:
+        curr_ext_fixed_ips = []
-                kwargs['enable_snat'] = self.params['enable_snat']
+        if curr_ext_gw_info:
+            curr_ext_fixed_ips = curr_ext_gw_info.get('external_fixed_ips', [])
 
-        if self.params['external_fixed_ips']:
+        external_gateway_info = {}
-            kwargs['ext_fixed_ips'] = []
+        if network:
-            for iface in self.params['external_fixed_ips']:
+            external_gateway_info['network_id'] = network.id
-                subnet = self.conn.get_subnet(iface['subnet'])
+            # can't send enable_snat unless we have a network
-                d = {'subnet_id': subnet['id']}
+            if self.params['enable_snat'] is not None:
-                if 'ip' in iface:
+                external_gateway_info['enable_snat'] = \
-                    d['ip_address'] = iface['ip']
+                    self.params['enable_snat']
-                kwargs['ext_fixed_ips'].append(d)
+        if ext_fixed_ips:
-        else:
+            external_gateway_info['external_fixed_ips'] = ext_fixed_ips
-            # no external fixed ips requested
+        if external_gateway_info:
-            if router \
+            kwargs['external_gateway_info'] = external_gateway_info
-               and router['external_gateway_info'] \
+
-               and router['external_gateway_info']['external_fixed_ips'] \
+        if 'external_fixed_ips' not in external_gateway_info:
-               and len(router['external_gateway_info']['external_fixed_ips']) > 1:
+            if len(curr_ext_fixed_ips) > 1:
-                # but router has several external fixed ips
+                fip = curr_ext_fixed_ips[0]
-                # keep first external fixed ip only
+                external_gateway_info['external_fixed_ips'] = [fip]
-                fip = router['external_gateway_info']['external_fixed_ips'][0]
-                kwargs['ext_fixed_ips'] = [fip]
 
         return kwargs
 
     def _build_router_interface_config(self, filters=None):
+        if filters is None:
+            filters = {}
         external_fixed_ips = []
-        internal_subnets = []
-        internal_ports = []
         internal_ports_missing = []
+        internal_ifaces = []
 
         # Build external interface configuration
-        if self.params['external_fixed_ips']:
+        ext_fixed_ips = None
-            for iface in self.params['external_fixed_ips']:
+        if self.params['external_gateway_info']:
-                subnet = self.conn.get_subnet(iface['subnet'], filters)
+            ext_fixed_ips = self.params['external_gateway_info'] \
+                .get('external_fixed_ips')
+        ext_fixed_ips = ext_fixed_ips or self.params['external_fixed_ips']
+        if ext_fixed_ips:
+            for iface in ext_fixed_ips:
+                subnet = self.conn.network.find_subnet(
+                    iface['subnet'], **filters)
                 if not subnet:
                     self.fail(msg='subnet %s not found' % iface['subnet'])
-                new_external_fixed_ip = {'subnet_name': subnet.name, 'subnet_id': subnet.id}
+                fip = dict(subnet_id=subnet.id)
-                if 'ip' in iface:
+                if 'ip_address' in iface:
-                    new_external_fixed_ip['ip'] = iface['ip']
+                    fip['ip_address'] = iface['ip_address']
-                external_fixed_ips.append(new_external_fixed_ip)
+                external_fixed_ips.append(fip)
 
         # Build internal interface configuration
         if self.params['interfaces']:
             internal_ips = []
             for iface in self.params['interfaces']:
                 if isinstance(iface, str):
-                    subnet = self.conn.get_subnet(iface, filters)
+                    subnet = self.conn.network.find_subnet(iface, **filters)
                     if not subnet:
                         self.fail(msg='subnet %s not found' % iface)
-                    internal_subnets.append(subnet)
+                    internal_ifaces.append(dict(subnet_id=subnet.id))
 
                 elif isinstance(iface, dict):
-                    subnet = self.conn.get_subnet(iface['subnet'], filters)
+                    subnet = self.conn.network.find_subnet(iface['subnet'],
+                                                           **filters)
                     if not subnet:
                         self.fail(msg='subnet %s not found' % iface['subnet'])
 
-                    net = self.conn.get_network(iface['net'])
+                    # TODO: We allow passing a subnet without specifing a
-                    if not net:
+                    #       network in case iface is a string, hence we
-                        self.fail(msg='net %s not found' % iface['net'])
+                    #       should allow to omit the network here as well.
+                    if 'net' not in iface:
+                        self.fail(
+                            "Network name missing from interface definition")
+                    net = self.conn.network.find_network(iface['net'])
 
-                    if "portip" not in iface:
+                    if 'portip' not in iface:
                         # portip not set, add any ip from subnet
-                        internal_subnets.append(subnet)
+                        internal_ifaces.append(dict(subnet_id=subnet.id))
                     elif not iface['portip']:
                         # portip is set but has invalid value
-                        self.fail(msg='put an ip in portip or remove it from list to assign default port to router')
+                        self.fail(msg='put an ip in portip or remove it'
+                                  'from list to assign default port to router')
                     else:
                         # portip has valid value
-                        # look for ports whose fixed_ips.ip_address matchs portip
+                        # look for ports whose fixed_ips.ip_address matchs
-                        for existing_port in self.conn.list_ports(filters={'network_id': net.id}):
+                        # portip
-                            for fixed_ip in existing_port['fixed_ips']:
+                        portip = iface['portip']
-                                if iface['portip'] == fixed_ip['ip_address']:
+                        port_kwargs = ({'network_id': net.id}
-                                    # portip exists in net already
+                                       if net is not None else {})
-                                    internal_ports.append(existing_port)
+                        existing_ports = self.conn.network.ports(**port_kwargs)
-                                    internal_ips.append(fixed_ip['ip_address'])
+                        for port in existing_ports:
-                        if iface['portip'] not in internal_ips:
+                            for fip in port['fixed_ips']:
+                                if (fip['subnet_id'] != subnet.id
+                                        or fip['ip_address'] != portip):
+                                    continue
+                                internal_ips.append(fip['ip_address'])
+                                internal_ifaces.append(
+                                    dict(port_id=port.id,
+                                         subnet_id=subnet.id,
+                                         ip_address=portip))
+                        if portip not in internal_ips:
                             # no port with portip exists hence create a new port
                             internal_ports_missing.append({
-                                'network_id': net.id,
+                                'network_id': subnet.network_id,
-                                'fixed_ips': [{'ip_address': iface['portip'], 'subnet_id': subnet.id}]
+                                'fixed_ips': [{'ip_address': portip,
+                                               'subnet_id': subnet.id}]
                             })
 
         return {
             'external_fixed_ips': external_fixed_ips,
-            'internal_subnets': internal_subnets,
+            'internal_ports_missing': internal_ports_missing,
-            'internal_ports': internal_ports,
+            'internal_ifaces': internal_ifaces,
-            'internal_ports_missing': internal_ports_missing
         }
 
-    def run(self):
+    def _update_ifaces(self, router, to_add, to_remove, missing_ports):
+        for port in to_remove:
+            self.conn.network.remove_interface_from_router(router,
+                                                           port_id=port.id)
+        # create ports that are missing
+        for port in missing_ports:
+            p = self.conn.network.create_port(**port)
+            if p:
+                to_add.append(dict(port_id=p.id))
+        for iface in to_add:
+            self.conn.network.add_interface_to_router(router, **iface)
 
+    def _get_external_gateway_network_name(self):
+        network_name_or_id = self.params['network']
+        if self.params['external_gateway_info']:
+            network_name_or_id = \
+                self.params['external_gateway_info']['network']
+        return network_name_or_id
+
+    def _get_port_changes(self, router, ifs_cfg):
+        requested_subnet_ids = [iface['subnet_id'] for iface
+                                in ifs_cfg['internal_ifaces']]
+
+        router_ifs_internal = []
+        if router:
+            router_ifs_internal = self.conn.list_router_interfaces(
+                router, 'internal')
+
+        existing_subnet_ips = {}
+        for iface in router_ifs_internal:
+            if 'fixed_ips' not in iface:
+                continue
+            for fip in iface['fixed_ips']:
+                existing_subnet_ips[fip['subnet_id']] = (fip['ip_address'],
+                                                         iface)
+
+        obsolete_subnet_ids = (set(existing_subnet_ips.keys())
+                               - set(requested_subnet_ids))
+
+        internal_ifaces = ifs_cfg['internal_ifaces']
+        to_add = []
+        to_remove = []
+        for iface in internal_ifaces:
+            subnet_id = iface['subnet_id']
+            if subnet_id not in existing_subnet_ips:
+                iface.pop('ip_address', None)
+                to_add.append(iface)
+                continue
+            ip, existing_port = existing_subnet_ips[subnet_id]
+            if 'ip_address' in iface and ip != iface['ip_address']:
+                # Port exists for subnet but has the wrong ip. Schedule it for
+                # deletion
+                to_remove.append(existing_port)
+
+        for port in router_ifs_internal:
+            if 'fixed_ips' not in port:
+                continue
+            if any(fip['subnet_id'] in obsolete_subnet_ids
+                   for fip in port['fixed_ips']):
+                to_remove.append(port)
+        return dict(to_add=to_add, to_remove=to_remove,
+                    router_ifs_internal=router_ifs_internal)
+
+    def run(self):
         state = self.params['state']
         name = self.params['name']
-        network = self.params['network']
+        network_name_or_id = self._get_external_gateway_network_name()
-        project = self.params['project']
+        project_name_or_id = self.params['project']
 
-        if self.params['external_fixed_ips'] and not network:
+        if self.params['external_fixed_ips'] and not network_name_or_id:
-            self.fail(msg='network is required when supplying external_fixed_ips')
+            self.fail(
+                msg='network is required when supplying external_fixed_ips')
 
-        if project is not None:
+        query_filters = {}
-            proj = self.conn.get_project(project)
+        project = None
-            if proj is None:
+        project_id = None
-                self.fail(msg='Project %s could not be found' % project)
+        if project_name_or_id is not None:
-            project_id = proj['id']
+            project = self.conn.identity.find_project(project_name_or_id)
-            filters = {'tenant_id': project_id}
+            if project is None:
-        else:
+                self.fail(
-            project_id = None
+                    msg='Project %s could not be found' % project_name_or_id)
-            filters = None
+            project_id = project['id']
+            query_filters['project_id'] = project_id
 
-        router = self.conn.get_router(name, filters=filters)
+        router = self.conn.network.find_router(name, **query_filters)
-        net = None
+        network = None
-        if network:
+        if network_name_or_id:
-            net = self.conn.get_network(network)
+            network = self.conn.network.find_network(network_name_or_id,
-            if not net:
+                                                     **query_filters)
-                self.fail(msg='network %s not found' % network)
+            if not network:
+                self.fail(msg='network %s not found' % network_name_or_id)
 
         # Validate and cache the subnet IDs so we can avoid duplicate checks
         # and expensive API calls.
-        router_ifs_cfg = self._build_router_interface_config(filters)
+        router_ifs_cfg = self._build_router_interface_config(query_filters)
-        requested_subnet_ids = [subnet.id for subnet in router_ifs_cfg['internal_subnets']] + \
-            self._get_subnet_ids_from_ports(router_ifs_cfg['internal_ports'])
-        requested_port_ids = [i['id'] for i in router_ifs_cfg['internal_ports']]
 
-        if router:
+        missing_internal_ports = router_ifs_cfg['internal_ports_missing']
-            router_ifs_internal = self.conn.list_router_interfaces(router, 'internal')
-            existing_subnet_ids = self._get_subnet_ids_from_ports(router_ifs_internal)
-            obsolete_subnet_ids = set(existing_subnet_ids) - set(requested_subnet_ids)
-            existing_port_ids = [i['id'] for i in router_ifs_internal]
 
-        else:
+        port_changes = self._get_port_changes(router, router_ifs_cfg)
-            router_ifs_internal = []
+        to_add = port_changes['to_add']
-            existing_subnet_ids = []
+        to_remove = port_changes['to_remove']
-            obsolete_subnet_ids = []
+        router_ifs_internal = port_changes['router_ifs_internal']
-            existing_port_ids = []
 
-        missing_port_ids = set(requested_port_ids) - set(existing_port_ids)
+        external_fixed_ips = router_ifs_cfg['external_fixed_ips']
 
         if self.ansible.check_mode:
             # Check if the system state would be changed
@@ -472,82 +579,42 @@ class RouterModule(OpenStackModule):
             elif state == 'present' and not router:
                 changed = True
             else:  # if state == 'present' and router
-                changed = self._needs_update(router, net,
+                kwargs = self._build_kwargs(router, network,
-                                             missing_port_ids,
+                                            external_fixed_ips)
-                                             requested_subnet_ids,
+                changed = self._needs_update(
-                                             existing_subnet_ids,
+                    router, kwargs, external_fixed_ips, to_add, to_remove,
-                                             router_ifs_cfg)
+                    missing_internal_ports)
             self.exit_json(changed=changed)
 
         if state == 'present':
             changed = False
+            external_fixed_ips = router_ifs_cfg['external_fixed_ips']
+            internal_ifaces = router_ifs_cfg['internal_ifaces']
+            kwargs = self._build_kwargs(router, network,
+                                        external_fixed_ips)
 
             if not router:
                 changed = True
-
-                kwargs = self._build_kwargs(router, net)
                 if project_id:
                     kwargs['project_id'] = project_id
-                router = self.conn.create_router(**kwargs)
+                router = self.conn.network.create_router(**kwargs)
-
-                # add interface by subnet id, because user did not specify a port id
-                for subnet in router_ifs_cfg['internal_subnets']:
-                    self.conn.add_router_interface(router, subnet_id=subnet.id)
-
-                # add interface by port id if user did specify a valid port id
-                for port in router_ifs_cfg['internal_ports']:
-                    self.conn.add_router_interface(router, port_id=port.id)
-
-                # add port and interface if user did specify an ip address but port is missing yet
-                for missing_internal_port in router_ifs_cfg['internal_ports_missing']:
-                    p = self.conn.create_port(**missing_internal_port)
-                    if p:
-                        self.conn.add_router_interface(router, port_id=p.id)
 
+                self._update_ifaces(router, internal_ifaces, [],
+                                    missing_internal_ports)
             else:
-                if self._needs_update(router, net,
+
-                                      missing_port_ids,
+                if self._needs_update(router, kwargs, external_fixed_ips,
-                                      requested_subnet_ids,
+                                      to_add, to_remove,
-                                      existing_subnet_ids,
+                                      missing_internal_ports):
-                                      router_ifs_cfg):
                     changed = True
-                    kwargs = self._build_kwargs(router, net)
+                    router = self.conn.network.update_router(router, **kwargs)
-                    updated_router = self.conn.update_router(**kwargs)
 
-                    # Protect against update_router() not actually updating the router.
+                    if to_add or to_remove or missing_internal_ports:
-                    if not updated_router:
+                        self._update_ifaces(router, to_add, to_remove,
-                        changed = False
+                                            missing_internal_ports)
-                    else:
-                        router = updated_router
 
-                    # delete internal subnets i.e. ports
+            self.exit_json(changed=changed,
-                    if obsolete_subnet_ids:
+                           router=router.to_dict(computed=False))
-                        for port in router_ifs_internal:
-                            if 'fixed_ips' in port:
-                                for fip in port['fixed_ips']:
-                                    if fip['subnet_id'] in obsolete_subnet_ids:
-                                        self.conn.remove_router_interface(router, port_id=port['id'])
-                                        changed = True
-
-                    # add new internal interface by subnet id, because user did not specify a port id
-                    for subnet in router_ifs_cfg['internal_subnets']:
-                        if subnet.id not in existing_subnet_ids:
-                            self.conn.add_router_interface(router, subnet_id=subnet.id)
-                            changed = True
-
-                    # add new internal interface by port id if user did specify a valid port id
-                    for port_id in missing_port_ids:
-                        self.conn.add_router_interface(router, port_id=port_id)
-                        changed = True
-
-                    # add new port and new internal interface if user did specify an ip address but port is missing yet
-                    for missing_internal_port in router_ifs_cfg['internal_ports_missing']:
-                        p = self.conn.create_port(**missing_internal_port)
-                        if p:
-                            self.conn.add_router_interface(router, port_id=p.id)
-                            changed = True
-
-            self.exit_json(changed=changed, router=router)
 
         elif state == 'absent':
             if not router:
@@ -558,8 +625,9 @@ class RouterModule(OpenStackModule):
                 # still fail if e.g. floating ips are attached to the
                 # router.
                 for port in router_ifs_internal:
-                    self.conn.remove_router_interface(router, port_id=port['id'])
+                    self.conn.network.remove_interface_from_router(router,
-                self.conn.delete_router(router['id'])
+                                                                   port_id=port['id'])
+                self.conn.network.delete_router(router)
                 self.exit_json(changed=True, router=router)