Release 1.5.1 version

Change-Id: Ib87202496ed05a6d2717d13a787f389a9c8d1336

.zuul.yaml

@@ -18,6 +18,7 @@
       - tools/run-ansible-sanity.sh
       - tests/sanity/.*
       - contrib/.*
+      - .zuul.yaml
     vars:
       zuul_work_dir: src/opendev.org/openstack/ansible-collections-openstack
       tox_envlist: ansible
@@ -48,6 +49,7 @@
       - ^plugins/modules/lb_pool.py
       - ^plugins/modules/loadbalancer.py
     vars:
+      configure_swap_size: 8192
       tox_envlist: ansible
       devstack_plugins:
         designate: https://opendev.org/openstack/designate
@@ -259,7 +261,7 @@
 - job:
     name: openstack-tox-linters-ansible-devel
     parent: openstack-tox-linters
-    nodeset: ubuntu-bionic
+    nodeset: ubuntu-focal
     description: |
       Run openstack collections linter tests using the devel branch of ansible
     # non-voting because we can't prevent ansible devel from breaking us
@@ -269,17 +271,23 @@
         override-checkout: devel
     vars:
       tox_envlist: linters-2.11
+      python_version: 3.8
+      bindep_profile: test py38
 
 - job:
     name: openstack-tox-linters-ansible-2.11
     parent: openstack-tox-linters
-    nodeset: ubuntu-bionic
+    nodeset: ubuntu-focal
     description: |
       Run openstack collections linter tests using the 2.11 branch of ansible
-    voting: false
+    voting: true
     required-projects:
       - name: github.com/ansible/ansible
         override-checkout: stable-2.11
+    vars:
+      tox_envlist: linters-2.11
+      python_version: 3.8
+      bindep_profile: test py38
 
 - job:
     name: openstack-tox-linters-ansible-2.9
@@ -370,7 +378,7 @@
             dependencies: &deps_unit_lint
               - tox-pep8
               - openstack-tox-linters-ansible-2.9
-              # - openstack-tox-linters-ansible-2.11
+              - openstack-tox-linters-ansible-2.11
 
         - ansible-collections-openstack-functional-devstack-releases:
             dependencies: *deps_unit_lint
@@ -413,7 +421,7 @@
     gate:
       jobs:
         - tox-pep8
-        # - openstack-tox-linters-ansible-2.11
+        - openstack-tox-linters-ansible-2.11
         - openstack-tox-linters-ansible-2.9
         - ansible-collections-openstack-functional-devstack
         - ansible-collections-openstack-functional-devstack-releases
@@ -423,7 +431,7 @@
         - ansible-collections-openstack-functional-devstack-victoria-ansible-2.11
         - ansible-collections-openstack-functional-devstack-ussuri-ansible-2.11
         - ansible-collections-openstack-functional-devstack-train-ansible-2.11
-        # - ansible-collections-openstack-functional-devstack-queens-ansible-2.11
+        - ansible-collections-openstack-functional-devstack-queens-ansible-2.11
         - ansible-collections-openstack-functional-devstack-octavia
         - tripleo-ci-centos-8-standalone-osa
 

CHANGELOG.rst

@@ -5,6 +5,30 @@ Openstack Cloud Ansilbe modules Release Notes
 .. contents:: Topics
 
 
+v1.5.1
+======
+
+Release Summary
+---------------
+
+Bugfixes for networking modules
+
+Minor Changes
+-------------
+
+- Changed minversion in tox to 3.18.0
+- Update IRC server in README
+
+Bugfixes
+--------
+
+- Add mandatory requires_ansible version to metadata
+- Add protocol listener octavia
+- Add support check mode for all info modules
+- Allow to attach multiple floating ips to a server
+- Only add or remove router interfaces when needed
+- Wait for pool to be active and online
+
 v1.5.0
 ======
 
@@ -74,9 +98,9 @@ Bugfixes
 New Modules
 -----------
 
+- openstack.cloud.address_scope - Create or delete address scopes from OpenStack
 - openstack.cloud.dns_zone_info - Getting information about dns zones
 - openstack.cloud.floating_ip_info - Get information about floating ips
-- openstack.cloud.address_scope - Create or delete address scopes from OpenStack
 
 v1.4.0
 ======

changelogs/changelog.yaml

@@ -222,13 +222,27 @@ releases:
       - image - Add support to setting image tags
       release_summary: New modules for DNS and FIPs and bugfixes.
     modules:
+    - description: Create or delete address scopes from OpenStack
+      name: address_scope
+      namespace: ''
     - description: Getting information about dns zones
       name: dns_zone_info
       namespace: ''
     - description: Get information about floating ips
       name: floating_ip_info
       namespace: ''
-    - description: Create or delete address scopes from OpenStack
-      name: address_scope
-      namespace: ''
     release_date: '2021-06-23'
+  1.5.1:
+    changes:
+      bugfixes:
+      - Add mandatory requires_ansible version to metadata
+      - Add protocol listener octavia
+      - Add support check mode for all info modules
+      - Allow to attach multiple floating ips to a server
+      - Only add or remove router interfaces when needed
+      - Wait for pool to be active and online
+      minor_changes:
+      - Changed minversion in tox to 3.18.0
+      - Update IRC server in README
+      release_summary: Bugfixes for networking modules
+    release_date: '2021-09-02'

ci/roles/floating_ip/tasks/main.yml

@@ -0,0 +1,466 @@
+---
+# Prepare environment
+- name: Gather information about public network
+  openstack.cloud.networks_info:
+    cloud: "{{ cloud }}"
+    name: public
+  register: public_network
+
+- name: Assert that public network exists
+  assert:
+    that: public_network.openstack_networks|length == 1
+
+- name: Create external network
+  openstack.cloud.network:
+     cloud: "{{ cloud }}"
+     state: present
+     name: ansible_external
+     external: true
+
+- name: Create external subnet
+  openstack.cloud.subnet:
+     cloud: "{{ cloud }}"
+     state: present
+     network_name: ansible_external
+     name: ansible_external_subnet
+     cidr: 10.6.6.0/24
+
+- name: Create external port 1
+  openstack.cloud.port:
+     cloud: "{{ cloud }}"
+     state: present
+     name: ansible_external_port1
+     network: ansible_external
+     fixed_ips:
+       - ip_address: 10.6.6.50
+
+- name: Create external port 2
+  openstack.cloud.port:
+     cloud: "{{ cloud }}"
+     state: present
+     name: ansible_external_port2
+     network: ansible_external
+     fixed_ips:
+       - ip_address: 10.6.6.51
+
+- name: Create internal network
+  openstack.cloud.network:
+     cloud: "{{ cloud }}"
+     state: present
+     name: ansible_internal
+     external: false
+
+- name: Create internal subnet
+  openstack.cloud.subnet:
+     cloud: "{{ cloud }}"
+     state: present
+     network_name: ansible_internal
+     name: ansible_internal_subnet
+     cidr: 10.7.7.0/24
+
+- name: Create internal port 1
+  openstack.cloud.port:
+     cloud: "{{ cloud }}"
+     state: present
+     name: ansible_internal_port1
+     network: ansible_internal
+     fixed_ips:
+       - ip_address: 10.7.7.100
+
+- name: Create internal port 2
+  openstack.cloud.port:
+     cloud: "{{ cloud }}"
+     state: present
+     name: ansible_internal_port2
+     network: ansible_internal
+     fixed_ips:
+       - ip_address: 10.7.7.101
+
+- name: Create internal port 3
+  openstack.cloud.port:
+     cloud: "{{ cloud }}"
+     state: present
+     name: ansible_internal_port3
+     network: ansible_internal
+     fixed_ips:
+       - ip_address: 10.7.7.102
+
+- name: Create router 1
+  openstack.cloud.router:
+     cloud: "{{ cloud }}"
+     state: present
+     name: ansible_router1
+     network: ansible_external
+     external_fixed_ips:
+        - subnet: ansible_external_subnet
+          ip: 10.6.6.10
+     interfaces:
+         - net: ansible_internal
+           subnet: ansible_internal_subnet
+           portip: 10.7.7.1
+
+# Router 2 is required for the simplest, first test that assigns a new floating IP to server
+# from first available external network or nova pool which is DevStack's public network
+- name: Create router 2
+  openstack.cloud.router:
+     cloud: "{{ cloud }}"
+     state: present
+     name: ansible_router2
+     network: public
+     interfaces:
+         - net: ansible_internal
+           subnet: ansible_internal_subnet
+           portip: 10.7.7.10
+
+- name: Get all floating ips
+  openstack.cloud.floating_ip_info:
+     cloud: "{{ cloud }}"
+  register: fips
+
+- name: Check if public network has any floating ips
+  set_fact:
+     public_network_had_fips: "{{ fips.floating_ips|
+                                  selectattr('floating_network_id', '==', public_network.openstack_networks.0.id)|
+                                  list|length > 0 }}"
+
+# TODO: Replace with appropriate Ansible module once available
+- name: Create a floating ip on public network (required for simplest, first floating ip test)
+  command: openstack --os-cloud={{ cloud }} floating ip create public
+  when: not public_network_had_fips
+
+# TODO: Replace with appropriate Ansible module once available
+- name: Create floating ip 1 on external network
+  command: >
+    openstack --os-cloud={{ cloud }} floating ip create
+        --subnet ansible_external_subnet
+        --floating-ip-address 10.6.6.150
+        ansible_external
+  when: fips.floating_ips|length == 0 or
+        "10.6.6.150" not in fips.floating_ips|map(attribute="floating_ip_address")|list
+
+- name: Create server with one nic
+  openstack.cloud.server:
+     cloud: "{{ cloud }}"
+     state: present
+     name: ansible_server1
+     image: "{{ image }}"
+     flavor: m1.tiny
+     nics:
+         # one nic only else simple, first floating ip test does not work
+         - port-name: ansible_internal_port1
+     auto_ip: false
+     wait: true
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: ansible_server1
+  register: info
+
+- name: Assert one internal port and no floating ips on server 1
+  # If this assertion fails because server has an public ipv4 address (public_v4) then make sure
+  # that no floating ip on public network is associated with "10.7.7.100" before running this role
+  assert:
+    that:
+      - info.openstack_servers|length == 1
+      - info.openstack_servers.0.public_v4|length == 0
+      - info.openstack_servers.0.public_v6|length == 0
+      - info.openstack_servers.0.addresses.ansible_internal|length == 1
+      - info.openstack_servers.0.addresses.ansible_internal|map(attribute="addr")|sort|list == ["10.7.7.100"]
+
+- name: Create server with two nics
+  openstack.cloud.server:
+     cloud: "{{ cloud }}"
+     state: present
+     name: ansible_server2
+     image: "{{ image }}"
+     flavor: m1.tiny
+     nics:
+         - port-name: ansible_internal_port2
+         - port-name: ansible_internal_port3
+     auto_ip: false
+     wait: true
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: ansible_server2
+  register: info
+
+- name: Assert two internal ports and no floating ips on server 2
+  assert:
+    that:
+      - info.openstack_servers|length == 1
+      - info.openstack_servers.0.public_v4|length == 0
+      - info.openstack_servers.0.public_v6|length == 0
+      - info.openstack_servers.0.addresses.ansible_internal|length == 2
+      - info.openstack_servers.0.addresses.ansible_internal|map(attribute="addr")|sort|list ==
+        ["10.7.7.101", "10.7.7.102"]
+
+# Tests
+- name: Assign new floating IP to server from first available external network or nova pool
+  openstack.cloud.floating_ip:
+     cloud: "{{ cloud }}"
+     state: present
+     server: ansible_server1
+     wait: true
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: ansible_server1
+  register: info
+
+- name: Assert one internal port and one floating ip on server 1
+  assert:
+    that:
+      - info.openstack_servers.0.addresses.ansible_internal|length == 2
+      - info.openstack_servers.0.addresses.ansible_internal|map(attribute="OS-EXT-IPS:type")|sort|list ==
+        ["fixed", "floating"]
+
+- name: Detach floating IP from server
+  openstack.cloud.floating_ip:
+     cloud: "{{ cloud }}"
+     state: absent
+     server: ansible_server1
+     network: public
+     floating_ip_address: "{{ (info.openstack_servers.0.addresses.ansible_internal|
+                               selectattr('OS-EXT-IPS:type', '==', 'floating')|map(attribute='addr')|list)[0] }}"
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: ansible_server1
+  register: info
+  # When detaching a floating ip from an instance there might be a delay until openstack.cloud.server_info
+  # does not list it any more in info.openstack_servers.0.addresses.ansible_internal, so retry if necessary.
+  retries: 10
+  delay: 3
+  until: info.openstack_servers.0.addresses.ansible_internal|length == 1
+
+- name: Assert one internal port on server 1
+  assert:
+    that:
+      - info.openstack_servers.0.addresses.ansible_internal|length == 1
+      - info.openstack_servers.0.addresses.ansible_internal|map(attribute="addr")|list == ["10.7.7.100"]
+
+- name: Assign floating IP to server
+  openstack.cloud.floating_ip:
+     cloud: "{{ cloud }}"
+     state: present
+     reuse: yes
+     server: ansible_server2
+     network: public
+     fixed_address: 10.7.7.101
+     wait: true
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: ansible_server2
+  register: info
+
+- name: Assert two internal ports and one floating ip on server 2
+  assert:
+    that:
+      - info.openstack_servers.0.addresses.ansible_internal|length == 3
+      - info.openstack_servers.0.addresses.ansible_internal|map(attribute="OS-EXT-IPS:type")|sort|list ==
+        ["fixed", "fixed", "floating"]
+
+- name: Assign a second, specific floating IP to server
+  openstack.cloud.floating_ip:
+     cloud: "{{ cloud }}"
+     state: present
+     reuse: yes
+     server: ansible_server2
+     network: ansible_external
+     fixed_address: 10.7.7.102
+     floating_ip_address: "10.6.6.150"
+
+# We cannot wait for second floating ip to be attached because OpenStackSDK checks only for first floating ip
+# Ref.: https://github.com/openstack/openstacksdk/blob/e0372b72af8c5f471fc17e53434d7a814ca958bd/openstack/cloud/_floating_ip.py#L733
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: ansible_server2
+  register: info
+  # retry because we cannot wait for second floating ip
+  retries: 10
+  delay: 3
+  until: info.openstack_servers.0.addresses.ansible_internal|length == 4
+
+- name: Assert two internal ports and two floating ips on server 2
+  assert:
+    that:
+      - info.openstack_servers.0.addresses.ansible_internal|length == 4
+      - ("10.6.6.150" in info.openstack_servers.0.addresses.ansible_internal|map(attribute="addr")|sort|list)
+
+- name: Detach second floating IP from server
+  openstack.cloud.floating_ip:
+     cloud: "{{ cloud }}"
+     state: absent
+     server: ansible_server2
+     network: ansible_external
+     floating_ip_address: "10.6.6.150"
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: ansible_server2
+  register: info
+  # When detaching a floating ip from an instance there might be a delay until openstack.cloud.server_info
+  # does not list it any more in info.openstack_servers.0.addresses.ansible_internal, so retry if necessary.
+  retries: 10
+  delay: 3
+  until: info.openstack_servers.0.addresses.ansible_internal|length == 3
+
+- name: Assert two internal ports and one floating ip on server 2
+  assert:
+    that:
+      - info.openstack_servers.0.addresses.ansible_internal|length == 3
+
+- name: Detach remaining floating IP from server
+  openstack.cloud.floating_ip:
+     cloud: "{{ cloud }}"
+     state: absent
+     server: ansible_server2
+     network: public
+     floating_ip_address: "{{ (info.openstack_servers.0.addresses.ansible_internal|
+                               selectattr('OS-EXT-IPS:type', '==', 'floating')|map(attribute='addr')|list)[0] }}"
+
+- name: Get info about server
+  openstack.cloud.server_info:
+    cloud: "{{ cloud }}"
+    server: ansible_server2
+  register: info
+  # When detaching a floating ip from an instance there might be a delay until openstack.cloud.server_info
+  # does not list it any more in info.openstack_servers.0.addresses.ansible_internal, so retry if necessary.
+  retries: 10
+  delay: 3
+  until: info.openstack_servers.0.addresses.ansible_internal|length == 2
+
+- name: Assert two internal ports on server 2
+  assert:
+    that:
+      - info.openstack_servers.0.addresses.ansible_internal|length == 2
+      - info.openstack_servers.0.addresses.ansible_internal|map(attribute="addr")|list == ["10.7.7.101", "10.7.7.102"]
+
+# Clean environment
+- name: Delete server with two nics
+  openstack.cloud.server:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: ansible_server2
+     wait: true
+
+- name: Delete server with one nic
+  openstack.cloud.server:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: ansible_server1
+     wait: true
+
+- name: Get all floating ips
+  openstack.cloud.floating_ip_info:
+     cloud: "{{ cloud }}"
+  register: fips
+
+# TODO: Replace with appropriate Ansible module once available
+- name: Delete floating ip on public network if we created it
+  when: not public_network_had_fips
+  command: >
+    openstack --os-cloud={{ cloud }} floating ip delete
+        {{ fips.floating_ips|selectattr('floating_network_id', '==', public_network.openstack_networks.0.id)|
+           map(attribute="floating_ip_address")|list|join(' ') }}
+
+# TODO: Replace with appropriate Ansible module once available
+- name: Delete floating ip 1
+  command: openstack --os-cloud={{ cloud }} floating ip delete 10.6.6.150
+  when: fips.floating_ips|length > 0 and "10.6.6.150" in fips.floating_ips|map(attribute="floating_ip_address")|list
+
+- name: Get remaining floating ips on external network
+  openstack.cloud.floating_ip_info:
+     cloud: "{{ cloud }}"
+     floating_network: ansible_external
+  register: fips
+
+# TODO: Replace with appropriate Ansible module once available
+# The first, simple floating ip test might have allocated a floating ip on the external network.
+# This floating ip must be removed before external network can be deleted.
+- name: Delete remaining floating ips on external network
+  when: fips.floating_ips|length > 0
+  command: >
+    openstack --os-cloud={{ cloud }} floating ip delete
+        {{ fips.floating_ips|map(attribute="floating_ip_address")|list|join(' ') }}
+
+# Remove routers after floating ips have been detached and disassociated else removal fails with
+#  Error detaching interface from router ***: Client Error for url: ***,
+#  Router interface for subnet *** on router *** cannot be deleted, 
+#  as it is required by one or more floating IPs.
+
+- name: Delete router 2
+  openstack.cloud.router:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: ansible_router2
+
+- name: Delete router 1
+  openstack.cloud.router:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: ansible_router1
+
+- name: Delete internal port 3
+  openstack.cloud.port:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: ansible_internal_port3
+
+- name: Delete internal port 2
+  openstack.cloud.port:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: ansible_internal_port2
+
+- name: Delete internal port 1
+  openstack.cloud.port:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: ansible_internal_port1
+
+- name: Delete internal subnet
+  openstack.cloud.subnet:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: ansible_internal_subnet
+
+- name: Delete internal network
+  openstack.cloud.network:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: ansible_internal
+
+- name: Delete external port 2
+  openstack.cloud.port:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: ansible_external_port2
+
+- name: Delete external port 1
+  openstack.cloud.port:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: ansible_external_port1
+
+- name: Delete external subnet
+  openstack.cloud.subnet:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: ansible_external_subnet
+
+- name: Delete external network
+  openstack.cloud.network:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: ansible_external

ci/roles/router/tasks/main.yml

@@ -15,6 +15,30 @@
      name: shade_subnet1
      cidr: 10.7.7.0/24
 
+- name: Create subnet2
+  openstack.cloud.subnet:
+     cloud: "{{ cloud }}"
+     state: present
+     network_name: "{{ network_name }}"
+     name: shade_subnet2
+     cidr: 10.8.8.0/24
+
+- name: Create subnet3
+  openstack.cloud.subnet:
+     cloud: "{{ cloud }}"
+     state: present
+     network_name: "{{ network_name }}"
+     name: shade_subnet3
+     cidr: 10.9.9.0/24
+
+- name: Create subnet4
+  openstack.cloud.subnet:
+     cloud: "{{ cloud }}"
+     state: present
+     network_name: "{{ network_name }}"
+     name: shade_subnet4
+     cidr: 10.10.10.0/24
+
 - name: Create router
   openstack.cloud.router:
      cloud: "{{ cloud }}"
@@ -29,6 +53,19 @@
      interfaces:
          - shade_subnet1
 
+- name: Update router (add interface) again
+  openstack.cloud.router:
+     cloud: "{{ cloud }}"
+     state: present
+     name: "{{ router_name }}"
+     interfaces:
+         - shade_subnet1
+  register: result
+
+- name: Assert idempotent module
+  assert:
+    that: result is not changed
+
 - name: Gather routers info
   openstack.cloud.routers_info:
      cloud: "{{ cloud }}"
@@ -43,6 +80,93 @@
       - "result.openstack_routers.0.name == router_name"
       - (result.openstack_routers.0.interfaces_info|length) == 1
 
+- name: Update router (change interfaces)
+  openstack.cloud.router:
+     cloud: "{{ cloud }}"
+     state: present
+     name: "{{ router_name }}"
+     interfaces:
+         - net: '{{ network_name }}'
+           subnet: shade_subnet2
+           portip: 10.8.8.1
+         - net: '{{ network_name }}'
+           subnet: shade_subnet3
+         - shade_subnet4
+
+- name: Update router (change interfaces) again
+  openstack.cloud.router:
+     cloud: "{{ cloud }}"
+     state: present
+     name: "{{ router_name }}"
+     interfaces:
+         - net: '{{ network_name }}'
+           subnet: shade_subnet2
+           portip: 10.8.8.1
+         - net: '{{ network_name }}'
+           subnet: shade_subnet3
+         - shade_subnet4
+  register: result
+
+- name: Assert idempotent module
+  assert:
+    that: result is not changed
+
+- 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.openstack_routers.0.name == router_name"
+      - (result.openstack_routers.0.interfaces_info|length) == 3
+      - result.openstack_routers.0.interfaces_info|map(attribute='ip_address')|sort|list ==
+        ['10.10.10.1', '10.8.8.1', '10.9.9.1']
+
+- name: Update router (remove interface)
+  openstack.cloud.router:
+     cloud: "{{ cloud }}"
+     state: present
+     name: "{{ router_name }}"
+     interfaces:
+         - net: '{{ network_name }}'
+           subnet: shade_subnet1
+           portip: 10.7.7.1
+
+- name: Update router (remove interface) again
+  openstack.cloud.router:
+     cloud: "{{ cloud }}"
+     state: present
+     name: "{{ router_name }}"
+     interfaces:
+         - net: '{{ network_name }}'
+           subnet: shade_subnet1
+           portip: 10.7.7.1
+  register: result
+
+- name: Assert idempotent module
+  assert:
+    that: result is not changed
+
+- 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.openstack_routers.0.name == router_name"
+      - (result.openstack_routers.0.interfaces_info|length) == 1
+      - result.openstack_routers.0.interfaces_info.0.ip_address == '10.7.7.1'
+
 # Admin operation
 - name: Create external network
   openstack.cloud.network:
@@ -53,12 +177,12 @@
   when:
     - network_external
 
-- name: Create subnet2
+- name: Create subnet5
   openstack.cloud.subnet:
      cloud: "{{ cloud }}"
      state: present
      network_name: "{{ external_network_name }}"
-     name: shade_subnet2
+     name: shade_subnet5
      cidr: 10.6.6.0/24
   when:
     - network_external
@@ -88,6 +212,142 @@
       - "result.openstack_routers.0.name == router_name"
       - (result.openstack_routers.0.interfaces_info|length) == 1
 
+- name: Update router (change external fixed ips)
+  openstack.cloud.router:
+     cloud: "{{ cloud }}"
+     state: present
+     name: "{{ router_name }}"
+     interfaces:
+         - shade_subnet1
+     network: "{{ external_network_name }}"
+     external_fixed_ips:
+        - subnet: shade_subnet5
+          ip: 10.6.6.100
+  when:
+    - network_external
+
+- 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.openstack_routers.0.name == router_name"
+      - (result.openstack_routers.0.external_gateway_info.external_fixed_ips|length) == 1
+      - result.openstack_routers.0.external_gateway_info.external_fixed_ips.0.ip_address == "10.6.6.100"
+  when:
+    - network_external
+
+- name: Update router (add external fixed ips)
+  openstack.cloud.router:
+     cloud: "{{ cloud }}"
+     state: present
+     name: "{{ router_name }}"
+     interfaces:
+         - shade_subnet1
+     network: "{{ external_network_name }}"
+     external_fixed_ips:
+        - subnet: shade_subnet5
+          ip: 10.6.6.100
+        - subnet: shade_subnet5
+          ip: 10.6.6.101
+  when:
+    - network_external
+
+- name: Update router (add external fixed ips) again
+  openstack.cloud.router:
+     cloud: "{{ cloud }}"
+     state: present
+     name: "{{ router_name }}"
+     interfaces:
+         - shade_subnet1
+     network: "{{ external_network_name }}"
+     external_fixed_ips:
+        - subnet: shade_subnet5
+          ip: 10.6.6.100
+        - subnet: shade_subnet5
+          ip: 10.6.6.101
+  when:
+    - network_external
+  register: result
+
+- name: Assert idempotent module
+  assert:
+    that: result is not changed
+
+- 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.openstack_routers.0.name == router_name"
+      - (result.openstack_routers.0.external_gateway_info.external_fixed_ips|length) == 2
+      - result.openstack_routers.0.external_gateway_info.external_fixed_ips|map(attribute='ip_address')|sort|list ==
+        ["10.6.6.100", "10.6.6.101"]
+  when:
+    - network_external
+
+- name: Update router (remove external fixed ips)
+  openstack.cloud.router:
+     cloud: "{{ cloud }}"
+     state: present
+     name: "{{ router_name }}"
+     interfaces:
+         - shade_subnet1
+     network: "{{ external_network_name }}"
+     external_fixed_ips:
+        - subnet: shade_subnet5
+          ip: 10.6.6.101
+  when:
+    - network_external
+
+- name: Update router (remove external fixed ips) again
+  openstack.cloud.router:
+     cloud: "{{ cloud }}"
+     state: present
+     name: "{{ router_name }}"
+     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: 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.openstack_routers.0.name == router_name"
+      - (result.openstack_routers.0.external_gateway_info.external_fixed_ips|length) == 1
+      - result.openstack_routers.0.external_gateway_info.external_fixed_ips.0.ip_address == "10.6.6.101"
+  when:
+    - network_external
+
 - name: Delete router
   openstack.cloud.router:
      cloud: "{{ cloud }}"
@@ -105,6 +365,24 @@
      cloud: "{{ cloud }}"
      state: absent
      name: shade_subnet2
+
+- name: Delete subnet3
+  openstack.cloud.subnet:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: shade_subnet3
+
+- name: Delete subnet4
+  openstack.cloud.subnet:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: shade_subnet4
+
+- name: Delete subnet5
+  openstack.cloud.subnet:
+     cloud: "{{ cloud }}"
+     state: absent
+     name: shade_subnet5
   when:
     - network_external
 

ci/run-collection.yml

@@ -50,3 +50,4 @@
       when: sdk_version is version("0.53.0", '>=')
     - role: loadbalancer
       tags: loadbalancer
+    - { role: floating_ip, tags: floating_ip }

galaxy.yml

@@ -33,4 +33,4 @@ build_ignore:
   - ansible_collections_openstack.egg-info
   - contrib
   - changelogs
-version: 1.5.1-dev
+version: 1.5.1

plugins/modules/baremetal_node.py

@@ -299,7 +299,10 @@ def main():
             required=False,
             type='bool',
             aliases=['skip_update_of_driver_password'],
-            deprecated_aliases=[dict(name='skip_update_of_driver_password', version='2.0.0')]
+            deprecated_aliases=[dict(
+                name='skip_update_of_driver_password',
+                version='2.0.0',
+                collection_name='openstack.cloud')]
         ),
         state=dict(required=False, default='present', choices=['present', 'absent'])
     )

plugins/modules/compute_flavor_info.py

@@ -174,7 +174,8 @@ class ComputeFlavorInfoModule(OpenStackModule):
             ['name', 'ram'],
             ['name', 'vcpus'],
             ['name', 'ephemeral']
-        ]
+        ],
+        supports_check_mode=True
     )
 
     deprecated_names = ('openstack.cloud.compute_flavor_facts')

plugins/modules/dns_zone_info.py

@@ -136,6 +136,9 @@ class DnsZoneInfoModule(OpenStackModule):
         description=dict(required=False, type='str'),
         ttl=dict(required=False, type='int')
     )
+    module_kwargs = dict(
+        supports_check_mode=True
+    )
 
     def run(self):
 

plugins/modules/floating_ip.py

@@ -11,6 +11,7 @@ short_description: Add/Remove floating IP from an instance
 description:
    - Add or Remove a floating IP to an instance.
    - Returns the floating IP when attaching only if I(wait=true).
+   - When detaching a floating IP there might be a delay until an instance does not list the floating IP any more.
 options:
    server:
      description:
@@ -24,9 +25,9 @@ options:
      type: str
    floating_ip_address:
      description:
-        - A floating IP address to attach or to detach. Required only if I(state)
-          is absent. When I(state) is present can be used to specify a IP address
-          to attach.
+        - A floating IP address to attach or to detach. When I(state) is present
+          can be used to specify a IP address to attach. I(floating_ip_address)
+          requires I(network) to be set.
      type: str
    reuse:
      description:
@@ -49,7 +50,7 @@ options:
    wait:
      description:
         - When attaching a floating IP address, specify whether to wait for it to appear as attached.
-        - Must be set to C(yes) for the module to return the value of the floating IP.
+        - Must be set to C(yes) for the module to return the value of the floating IP when attaching.
      type: bool
      default: 'no'
    timeout:
@@ -118,8 +119,8 @@ EXAMPLES = '''
      server: cattle001
 '''
 
-from ansible.module_utils.basic import remove_values
 from ansible_collections.openstack.cloud.plugins.module_utils.openstack import OpenStackModule
+import itertools
 
 
 class NetworkingFloatingIPModule(OpenStackModule):
@@ -137,16 +138,57 @@ class NetworkingFloatingIPModule(OpenStackModule):
         purge=dict(required=False, type='bool', default=False),
     )
 
-    module_kwargs = dict()
+    module_kwargs = dict(
+        required_if=[
+            ['state', 'absent', ['floating_ip_address']]
+        ],
+        required_by=dict(
+            floating_ip_address=('network',)
+        )
+    )
 
     def _get_floating_ip(self, floating_ip_address):
         f_ips = self.conn.search_floating_ips(
             filters={'floating_ip_address': floating_ip_address})
+
         if not f_ips:
             return None
 
         return f_ips[0]
 
+    def _list_floating_ips(self, server):
+        return itertools.chain.from_iterable([
+            (addr['addr'] for addr in server.addresses[net] if addr['OS-EXT-IPS:type'] == 'floating')
+            for net in server.addresses
+        ])
+
+    def _match_floating_ip(self, server,
+                           floating_ip_address,
+                           network_id,
+                           fixed_address,
+                           nat_destination):
+
+        if floating_ip_address:
+            return self._get_floating_ip(floating_ip_address)
+        elif not fixed_address and nat_destination:
+            nat_destination_name = self.conn.get_network(nat_destination)['name']
+            return next(
+                (self._get_floating_ip(addr['addr'])
+                 for addr in server.addresses.get(nat_destination_name, [])
+                 if addr['OS-EXT-IPS:type'] == 'floating'),
+                None)
+        else:
+            # not floating_ip_address and (fixed_address or not nat_destination)
+
+            # get any of the floating ips that matches fixed_address and/or network
+            f_ip_addrs = self._list_floating_ips(server)
+            f_ips = [f_ip for f_ip in self.conn.list_floating_ips() if f_ip['floating_ip_address'] in f_ip_addrs]
+            return next(
+                (f_ip for f_ip in f_ips
+                 if ((fixed_address and f_ip.fixed_ip_address == fixed_address) or not fixed_address)
+                 and ((network_id and f_ip.network == network_id) or not network_id)),
+                None)
+
     def run(self):
         server_name_or_id = self.params['server']
         state = self.params['state']
@@ -160,83 +202,96 @@ class NetworkingFloatingIPModule(OpenStackModule):
         purge = self.params['purge']
 
         server = self.conn.get_server(server_name_or_id)
-        if server is None:
+        if not server:
             self.fail_json(
                 msg="server {0} not found".format(server_name_or_id))
 
+        # Extract floating ips from server
+        f_ip_addrs = self._list_floating_ips(server)
+
+        # Get details about requested floating ip
+        f_ip = self._get_floating_ip(floating_ip_address) if floating_ip_address else None
+
+        if network:
+            network_id = self.conn.get_network(name_or_id=network)["id"]
+        else:
+            network_id = None
+
         if state == 'present':
-            # If f_ip already assigned to server, check that it matches
-            # requirements.
-            public_ip = self.conn.get_server_public_ip(server)
-            f_ip = self._get_floating_ip(public_ip) if public_ip else public_ip
-            if f_ip:
-                if network:
-                    network_id = self.conn.get_network(name_or_id=network)["id"]
-                else:
-                    network_id = None
-                # check if we have floating ip on given nat_destination network
-                if nat_destination:
-                    nat_floating_addrs = [
-                        addr for addr in server.addresses.get(
-                            self.conn.get_network(nat_destination)['name'], [])
-                        if addr['addr'] == public_ip
-                        and addr['OS-EXT-IPS:type'] == 'floating'
-                    ]
+            if floating_ip_address and f_ip and floating_ip_address in f_ip_addrs:
+                # Floating ip address has been assigned to server
+                self.exit_json(changed=False, floating_ip=f_ip)
 
-                    if len(nat_floating_addrs) == 0:
-                        self.fail_json(
-                            msg="server {server} already has a "
-                            "floating-ip on a different "
-                            "nat-destination than '{nat_destination}'"
-                            .format(server=server_name_or_id,
-                                    nat_destination=nat_destination))
+            if f_ip and f_ip['attached'] and floating_ip_address not in f_ip_addrs:
+                # Requested floating ip has been attached to different server
+                self.fail_json(msg="floating-ip {floating_ip_address} already has been attached to different server"
+                                   .format(floating_ip_address=floating_ip_address))
 
-                if all([fixed_address, f_ip.fixed_ip_address == fixed_address,
-                        network, f_ip.network != network_id]):
-                    # Current state definitely conflicts with requirements
-                    self.fail_json(
-                        msg="server {server} already has a "
-                        "floating-ip on requested "
-                        "interface but it doesn't match "
-                        "requested network {network}: {fip}"
-                        .format(server=server_name_or_id,
-                                network=network,
-                                fip=remove_values(f_ip, self.no_log_values)))
-                if not network or f_ip.network == network_id:
-                    # Requirements are met
-                    self.exit_json(changed=False, floating_ip=f_ip)
+            if not floating_ip_address:
+                # No specific floating ip requested, i.e. if any floating ip is already assigned to server,
+                # check that it matches requirements.
 
-                # Requirements are vague enough to ignore existing f_ip and try
-                # to create a new f_ip to the server.
+                if not fixed_address and nat_destination:
+                    # Check if we have any floating ip on the given nat_destination network
+                    nat_destination_name = self.conn.get_network(nat_destination)['name']
+                    for addr in server.addresses.get(nat_destination_name, []):
+                        if addr['OS-EXT-IPS:type'] == 'floating':
+                            # A floating ip address has been assigned to the requested nat_destination
+                            f_ip = self._get_floating_ip(addr['addr'])
+                            self.exit_json(changed=False, floating_ip=f_ip)
+                # else fixed_address or not nat_destination, hence an
+                # analysis of all floating ips of server is required
+                f_ips = [f_ip for f_ip in self.conn.list_floating_ips() if f_ip['floating_ip_address'] in f_ip_addrs]
+                for f_ip in f_ips:
+                    if network_id and f_ip.network != network_id:
+                        # requested network does not match network of floating ip
+                        continue
 
+                    if not fixed_address and not nat_destination:
+                        # any floating ip will fullfil these requirements
+                        self.exit_json(changed=False, floating_ip=f_ip)
+
+                    if fixed_address and f_ip.fixed_ip_address == fixed_address:
+                        # a floating ip address has been assigned that points to the requested fixed_address
+                        self.exit_json(changed=False, floating_ip=f_ip)
+
+            if floating_ip_address and not f_ip:
+                # openstacksdk's create_ip requires floating_ip_address and floating_network_id to be set
+                self.conn.network.create_ip(floating_ip_address=floating_ip_address, floating_network_id=network_id)
+            # Else floating ip either does not exist or has not been attached yet
+
+            # Both floating_ip_address and network are mutually exclusive in add_ips_to_server, i.e.
+            # add_ips_to_server will ignore floating_ip_address if network is set
+            # Ref.: https://github.com/openstack/openstacksdk/blob/a6b0ece2821ea79330c4067100295f6bdcbe456e/openstack/cloud/_floating_ip.py#L987
             server = self.conn.add_ips_to_server(
-                server=server, ips=floating_ip_address, ip_pool=network,
-                reuse=reuse, fixed_address=fixed_address, wait=wait,
+                server=server,
+                ips=floating_ip_address,
+                ip_pool=network if not floating_ip_address else None,
+                reuse=reuse,
+                fixed_address=fixed_address,
+                wait=wait,
                 timeout=timeout, nat_destination=nat_destination)
-            fip_address = self.conn.get_server_public_ip(server)
-            # Update the floating IP status
-            f_ip = self._get_floating_ip(fip_address)
+
+            # Update the floating ip status
+            f_ip = self._match_floating_ip(server, floating_ip_address, network_id, fixed_address, nat_destination)
             self.exit_json(changed=True, floating_ip=f_ip)
 
         elif state == 'absent':
-            if floating_ip_address is None:
-                if not server_name_or_id:
-                    self.fail_json(msg="either server or floating_ip_address are required")
-                server = self.conn.get_server(server_name_or_id)
-                floating_ip_address = self.conn.get_server_public_ip(server)
-
-            f_ip = self._get_floating_ip(floating_ip_address)
-
+            f_ip = self._match_floating_ip(server, floating_ip_address, network_id, fixed_address, nat_destination)
             if not f_ip:
                 # Nothing to detach
                 self.exit_json(changed=False)
             changed = False
+
             if f_ip["fixed_ip_address"]:
-                self.conn.detach_ip_from_server(
-                    server_id=server['id'], floating_ip_id=f_ip['id'])
+                self.conn.detach_ip_from_server(server_id=server['id'], floating_ip_id=f_ip['id'])
+                # OpenStackSDK sets {"port_id": None} to detach a floating ip from an instance,
+                # but there might be a delay until a server does not list it in addresses any more.
+
                 # Update the floating IP status
                 f_ip = self.conn.get_floating_ip(id=f_ip['id'])
                 changed = True
+
             if purge:
                 self.conn.delete_floating_ip(f_ip['id'])
                 self.exit_json(changed=True)

plugins/modules/floating_ip_info.py

@@ -150,6 +150,9 @@ class FloatingIPInfoModule(OpenStackModule):
         router=dict(required=False),
         status=dict(required=False, choices=['active', 'down']),
     )
+    module_kwargs = dict(
+        supports_check_mode=True
+    )
 
     def run(self):
 

plugins/modules/identity_domain_info.py

@@ -92,7 +92,8 @@ class IdentityDomainInfoModule(OpenStackModule):
     module_kwargs = dict(
         mutually_exclusive=[
             ['name', 'filters'],
-        ]
+        ],
+        supports_check_mode=True
     )
 
     deprecated_names = ('openstack.cloud.identity_domain_facts')

plugins/modules/identity_group_info.py

@@ -119,6 +119,9 @@ class IdentityGroupInfoModule(OpenStackModule):
         domain=dict(required=False, default=None),
         filters=dict(required=False, type='dict', default=None),
     )
+    module_kwargs = dict(
+        supports_check_mode=True
+    )
 
     def run(self):
         name = self.params['name']

plugins/modules/identity_user_info.py

@@ -116,6 +116,9 @@ class IdentityUserInfoModule(OpenStackModule):
         domain=dict(required=False, default=None),
         filters=dict(required=False, type='dict', default=None),
     )
+    module_kwargs = dict(
+        supports_check_mode=True
+    )
 
     deprecated_names = ('openstack.cloud.identity_user_facts')
 

plugins/modules/image_info.py

@@ -157,6 +157,9 @@ class ImageInfoModule(OpenStackModule):
         image=dict(type='str', required=False),
         properties=dict(type='dict', required=False),
     )
+    module_kwargs = dict(
+        supports_check_mode=True
+    )
 
     def run(self):
 

plugins/modules/keypair_info.py

@@ -115,6 +115,9 @@ class KeyPairInfoModule(OpenStackModule):
         limit=dict(type='int', required=False),
         marker=dict(type='str', required=False)
     )
+    module_kwargs = dict(
+        supports_check_mode=True
+    )
 
     def run(self):
         name = self.params['name']

plugins/modules/lb_health_monitor.py

@@ -223,10 +223,10 @@ class HealthMonitorModule(OpenStackModule):
         expected_codes=dict(required=False, default="200"),
         admin_state_up=dict(required=False, default=True, type='bool'),
         state=dict(default='present', choices=['absent', 'present']),
-        http_method=dict(default="GET", requried=False, choices=["GET", "CONNECT", "DELETE",
+        http_method=dict(default="GET", required=False, choices=["GET", "CONNECT", "DELETE",
                                                                  "HEAD", "OPTIONS", "PATCH",
                                                                  "POST", "PUT", "TRACE"]),
-        url_path=dict(default="/", requires=False),
+        url_path=dict(default="/", required=False),
         type=dict(default='HTTP',
                   choices=['HTTP', 'HTTPS', 'PING', 'SCTP', 'TCP', 'TLS-HELLO', 'UDP-CONNECT']))
 

plugins/modules/networks_info.py

@@ -124,6 +124,9 @@ class NetworkInfoModule(OpenStackModule):
         name=dict(required=False, default=None),
         filters=dict(required=False, type='dict', default=None)
     )
+    module_kwargs = dict(
+        supports_check_mode=True
+    )
 
     def run(self):
 

plugins/modules/port_info.py

@@ -188,6 +188,7 @@ class NetworkPortInfoModule(OpenStackModule):
         filters=dict(type='dict', required=False),
     )
     module_kwargs = dict(
+        supports_check_mode=True
     )
 
     deprecated_names = ('openstack.cloud.port_facts')

plugins/modules/router.py

@@ -211,13 +211,7 @@ router:
 '''
 
 from ansible_collections.openstack.cloud.plugins.module_utils.openstack import OpenStackModule
-
-
-ROUTER_INTERFACE_OWNERS = set([
-    'network:router_interface',
-    'network:router_interface_distributed',
-    'network:ha_router_replicated_interface'
-])
+import itertools
 
 
 class RouterModule(OpenStackModule):
@@ -232,14 +226,17 @@ class RouterModule(OpenStackModule):
         project=dict(default=None)
     )
 
-    def _router_internal_interfaces(self, router):
-        for port in self.conn.list_router_interfaces(router, 'internal'):
-            if port['device_owner'] in ROUTER_INTERFACE_OWNERS:
-                yield port
+    def _get_subnet_ids_from_ports(self, ports):
+        return [fixed_ip['subnet_id'] for fixed_ip in
+                itertools.chain.from_iterable(port['fixed_ips'] for port in ports if 'fixed_ips' in port)]
 
-    def _needs_update(self, router, network, internal_subnet_ids, internal_port_ids, filters=None):
-        """Decide if the given router needs an update.
-        """
+    def _needs_update(self, router, net,
+                      missing_port_ids,
+                      requested_subnet_ids,
+                      existing_subnet_ids,
+                      router_ifs_cfg,
+                      filters=None):
+        """Decide if the given router needs an update."""
         if router['admin_state_up'] != self.params['admin_state_up']:
             return True
         if router['external_gateway_info']:
@@ -247,68 +244,76 @@ class RouterModule(OpenStackModule):
             if self.params['enable_snat'] is not None:
                 if router['external_gateway_info'].get('enable_snat', True) != self.params['enable_snat']:
                     return True
-        if network:
+        if net:
             if not router['external_gateway_info']:
                 return True
-            elif router['external_gateway_info']['network_id'] != network['id']:
+            elif router['external_gateway_info']['network_id'] != net['id']:
                 return True
 
-        # check external interfaces
-        if self.params['external_fixed_ips']:
-            for new_iface in self.params['external_fixed_ips']:
-                subnet = self.conn.get_subnet(new_iface['subnet'], filters)
-                exists = False
+        # check if external_fixed_ip has to be added
+        for external_fixed_ip in router_ifs_cfg['external_fixed_ips']:
+            exists = False
 
-                # compare the requested interface with existing, looking for an existing match
-                for existing_iface in router['external_gateway_info']['external_fixed_ips']:
-                    if existing_iface['subnet_id'] == subnet['id']:
-                        if 'ip' in new_iface:
-                            if existing_iface['ip_address'] == new_iface['ip']:
+            # compare the requested interface with existing, looking for an existing match
+            for existing_if in router['external_gateway_info']['external_fixed_ips']:
+                if existing_if['subnet_id'] == external_fixed_ip['subnet_id']:
+                    if 'ip' in external_fixed_ip:
+                        if existing_if['ip_address'] == external_fixed_ip['ip']:
+                            # both subnet id and ip address match
+                            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
-                                exists = True
+                                obsolete = False
                                 break
                         else:
                             # only the subnet was given, so ip doesn't matter
-                            exists = True
+                            obsolete = False
                             break
 
                 # this interface isn't present on the existing router
-                if not exists:
+                if obsolete:
                     return True
-
-        # check internal interfaces
-        if self.params['interfaces']:
-            existing_subnet_ids = []
-            for port in self._router_internal_interfaces(router):
-                if 'fixed_ips' in port:
-                    for fixed_ip in port['fixed_ips']:
-                        existing_subnet_ids.append(fixed_ip['subnet_id'])
-
-            for iface in self.params['interfaces']:
-                if isinstance(iface, dict):
-                    for p_id in internal_port_ids:
-                        p = self.conn.get_port(name_or_id=p_id)
-                        if 'fixed_ips' in p:
-                            for fip in p['fixed_ips']:
-                                internal_subnet_ids.append(fip['subnet_id'])
-
-            if set(internal_subnet_ids) != set(existing_subnet_ids):
+        else:
+            # no external fixed ips requested
+            if router['external_gateway_info'] \
+               and router['external_gateway_info']['external_fixed_ips'] \
+               and len(router['external_gateway_info']['external_fixed_ips']) > 1:
+                # but router has several external fixed ips
                 return True
 
-        return False
-
-    def _system_state_change(self, router, network, internal_ids, internal_portids, filters=None):
-        """Check if the system state would be changed."""
-        state = self.params['state']
-        if state == 'absent' and router:
+        # check if internal port has to be added
+        if router_ifs_cfg['internal_ports_missing']:
             return True
-        if state == 'present':
-            if not router:
-                return True
-            return self._needs_update(router, network, internal_ids, internal_portids, filters)
+
+        if missing_port_ids:
+            return True
+
+        # 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, network):
+    def _build_kwargs(self, router, net):
         kwargs = {
             'admin_state_up': self.params['admin_state_up'],
         }
@@ -318,8 +323,8 @@ class RouterModule(OpenStackModule):
         else:
             kwargs['name'] = self.params['name']
 
-        if network:
-            kwargs['ext_gateway_net_id'] = network['id']
+        if net:
+            kwargs['ext_gateway_net_id'] = net['id']
             # can't send enable_snat unless we have a network
             if self.params.get('enable_snat') is not None:
                 kwargs['enable_snat'] = self.params['enable_snat']
@@ -332,56 +337,83 @@ class RouterModule(OpenStackModule):
                 if 'ip' in iface:
                     d['ip_address'] = iface['ip']
                 kwargs['ext_fixed_ips'].append(d)
+        else:
+            # no external fixed ips requested
+            if router \
+               and router['external_gateway_info'] \
+               and router['external_gateway_info']['external_fixed_ips'] \
+               and len(router['external_gateway_info']['external_fixed_ips']) > 1:
+                # but router has several external fixed ips
+                # keep first external fixed ip only
+                fip = router['external_gateway_info']['external_fixed_ips'][0]
+                kwargs['ext_fixed_ips'] = [fip]
 
         return kwargs
 
-    def _validate_subnets(self, filters=None):
-        external_subnet_ids = []
-        internal_subnet_ids = []
-        internal_port_ids = []
-        existing_port_ips = []
+    def _build_router_interface_config(self, filters=None):
+        external_fixed_ips = []
+        internal_subnets = []
+        internal_ports = []
+        internal_ports_missing = []
+
+        # Build external interface configuration
         if self.params['external_fixed_ips']:
             for iface in self.params['external_fixed_ips']:
-                subnet = self.conn.get_subnet(iface['subnet'])
+                subnet = self.conn.get_subnet(iface['subnet'], filters)
                 if not subnet:
-                    self.fail_json(msg='subnet %s not found' % iface['subnet'])
-                external_subnet_ids.append(subnet['id'])
+                    self.fail(msg='subnet %s not found' % iface['subnet'])
+                new_external_fixed_ip = {'subnet_name': subnet.name, 'subnet_id': subnet.id}
+                if 'ip' in iface:
+                    new_external_fixed_ip['ip'] = iface['ip']
+                external_fixed_ips.append(new_external_fixed_ip)
 
+        # 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)
                     if not subnet:
                         self.fail(msg='subnet %s not found' % iface)
-                    internal_subnet_ids.append(subnet['id'])
+                    internal_subnets.append(subnet)
+
                 elif isinstance(iface, dict):
                     subnet = self.conn.get_subnet(iface['subnet'], filters)
                     if not subnet:
                         self.fail(msg='subnet %s not found' % iface['subnet'])
+
                     net = self.conn.get_network(iface['net'])
                     if not net:
                         self.fail(msg='net %s not found' % iface['net'])
+
                     if "portip" not in iface:
-                        internal_subnet_ids.append(subnet['id'])
+                        # portip not set, add any ip from subnet
+                        internal_subnets.append(subnet)
                     elif not iface['portip']:
-                        self.fail(msg='put an ip in portip or  remove it from list to assign default port to router')
+                        # 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')
                     else:
+                        # portip has valid value
+                        # look for ports whose fixed_ips.ip_address matchs portip
                         for existing_port in self.conn.list_ports(filters={'network_id': net.id}):
                             for fixed_ip in existing_port['fixed_ips']:
                                 if iface['portip'] == fixed_ip['ip_address']:
-                                    internal_port_ids.append(existing_port.id)
-                                    existing_port_ips.append(fixed_ip['ip_address'])
-                        if iface['portip'] not in existing_port_ips:
-                            p = self.conn.create_port(network_id=net.id, fixed_ips=[
-                                {
-                                    'ip_address': iface['portip'],
-                                    'subnet_id': subnet.id
-                                }
-                            ])
-                            if p:
-                                internal_port_ids.append(p.id)
+                                    # portip exists in net already
+                                    internal_ports.append(existing_port)
+                                    internal_ips.append(fixed_ip['ip_address'])
+                        if iface['portip'] not in internal_ips:
+                            # no port with portip exists hence create a new port
+                            internal_ports_missing.append({
+                                'network_id': net.id,
+                                'fixed_ips': [{'ip_address': iface['portip'], 'subnet_id': subnet.id}]
+                            })
 
-        return external_subnet_ids, internal_subnet_ids, internal_port_ids
+        return {
+            'external_fixed_ips': external_fixed_ips,
+            'internal_subnets': internal_subnets,
+            'internal_ports': internal_ports,
+            'internal_ports_missing': internal_ports_missing
+        }
 
     def run(self):
 
@@ -391,7 +423,7 @@ class RouterModule(OpenStackModule):
         project = self.params['project']
 
         if self.params['external_fixed_ips'] and not network:
-            self.fail_json(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:
             proj = self.conn.get_project(project)
@@ -412,67 +444,125 @@ class RouterModule(OpenStackModule):
 
         # Validate and cache the subnet IDs so we can avoid duplicate checks
         # and expensive API calls.
-        external_ids, subnet_internal_ids, internal_portids = self._validate_subnets(filters)
+        router_ifs_cfg = self._build_router_interface_config(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:
+            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:
+            router_ifs_internal = []
+            existing_subnet_ids = []
+            obsolete_subnet_ids = []
+            existing_port_ids = []
+
+        missing_port_ids = set(requested_port_ids) - set(existing_port_ids)
+
         if self.ansible.check_mode:
-            self.exit_json(
-                changed=self._system_state_change(router, net, subnet_internal_ids, internal_portids, filters)
-            )
+            # Check if the system state would be changed
+            if state == 'absent' and router:
+                changed = True
+            elif state == 'absent' and not router:
+                changed = False
+            elif state == 'present' and not router:
+                changed = True
+            else:  # if state == 'present' and router
+                changed = self._needs_update(router, net,
+                                             missing_port_ids,
+                                             requested_subnet_ids,
+                                             existing_subnet_ids,
+                                             router_ifs_cfg,
+                                             filters)
+            self.exit_json(changed=changed)
 
         if state == 'present':
             changed = False
 
             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)
-                for int_s_id in subnet_internal_ids:
-                    self.conn.add_router_interface(router, subnet_id=int_s_id)
-                # add interface by port id as well
-                for int_p_id in internal_portids:
-                    self.conn.add_router_interface(router, port_id=int_p_id)
-                changed = True
+
+                # 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)
+
             else:
-                if self._needs_update(router, net, subnet_internal_ids, internal_portids, filters):
+                if self._needs_update(router, net,
+                                      missing_port_ids,
+                                      requested_subnet_ids,
+                                      existing_subnet_ids,
+                                      router_ifs_cfg,
+                                      filters):
+                    changed = True
                     kwargs = self._build_kwargs(router, net)
                     updated_router = self.conn.update_router(**kwargs)
 
-                    # Protect against update_router() not actually
-                    # updating the router.
+                    # Protect against update_router() not actually updating the router.
                     if not updated_router:
                         changed = False
-
-                    # On a router update, if any internal interfaces were supplied,
-                    # just detach all existing internal interfaces and attach the new.
-                    if internal_portids or subnet_internal_ids:
+                    else:
                         router = updated_router
-                        ports = self._router_internal_interfaces(router)
-                        for port in ports:
-                            self.conn.remove_router_interface(router, port_id=port['id'])
-                    if internal_portids:
-                        external_ids, subnet_internal_ids, internal_portids = self._validate_subnets(filters)
-                        for int_p_id in internal_portids:
-                            self.conn.add_router_interface(router, port_id=int_p_id)
-                        changed = True
-                    if subnet_internal_ids:
-                        for s_id in subnet_internal_ids:
-                            self.conn.add_router_interface(router, subnet_id=s_id)
+
+                    # delete internal subnets i.e. ports
+                    if obsolete_subnet_ids:
+                        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
 
-            self.exit(changed=changed, router=router, id=router['id'])
+                    # 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:
-                self.exit(changed=False)
+                self.exit_json(changed=False)
             else:
-                # We need to detach all internal interfaces on a router before
-                # we will be allowed to delete it.
-                ports = self._router_internal_interfaces(router)
-                router_id = router['id']
-                for port in ports:
+                # We need to detach all internal interfaces on a router
+                # before we will be allowed to delete it. Deletion can
+                # 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.delete_router(router_id)
-                self.exit_json(changed=True)
+                self.conn.delete_router(router['id'])
+                self.exit_json(changed=True, router=router)
 
 
 def main():

plugins/modules/routers_info.py

@@ -155,6 +155,9 @@ class RouterInfoModule(OpenStackModule):
         name=dict(required=False, default=None),
         filters=dict(required=False, type='dict', default=None)
     )
+    module_kwargs = dict(
+        supports_check_mode=True
+    )
 
     def run(self):
 

plugins/modules/security_group_info.py

@@ -148,6 +148,9 @@ class SecurityGroupInfoModule(OpenStackModule):
         not_tags=dict(required=False, type='list', elements='str'),
         not_any_tags=dict(required=False, type='list', elements='str')
     )
+    module_kwargs = dict(
+        supports_check_mode=True
+    )
 
     def run(self):
         description = self.params['description']

plugins/modules/security_group_rule_info.py

@@ -177,7 +177,8 @@ class SecurityGroupRuleInfoModule(OpenStackModule):
     module_kwargs = dict(
         mutually_exclusive=[
             ['remote_ip_prefix', 'remote_group'],
-        ]
+        ],
+        supports_check_mode=True
     )
 
     def run(self):

plugins/modules/server_info.py

@@ -70,6 +70,9 @@ class ServerInfoModule(OpenStackModule):
         filters=dict(required=False, type='dict', default=None),
         all_projects=dict(required=False, type='bool', default=False),
     )
+    module_kwargs = dict(
+        supports_check_mode=True
+    )
 
     def run(self):
 

plugins/modules/stack_info.py

@@ -80,6 +80,9 @@ class StackInfoModule(OpenStackModule):
         project_id=dict(required=False, type='str'),
         owner_id=dict(required=False, type='str')
     )
+    module_kwargs = dict(
+        supports_check_mode=True
+    )
 
     def run(self):
         data = []

plugins/modules/subnets_info.py

@@ -140,6 +140,9 @@ class SubnetInfoModule(OpenStackModule):
         name=dict(required=False, default=None, aliases=['subnet']),
         filters=dict(required=False, type='dict', default=None)
     )
+    module_kwargs = dict(
+        supports_check_mode=True
+    )
 
     def run(self):
         kwargs = self.check_versioned(

plugins/modules/volume_backup_info.py

@@ -90,6 +90,9 @@ class VolumeBackupInfoModule(OpenStackModule):
         name=dict(required=False, type='str'),
         volume=dict(required=False, type='str')
     )
+    module_kwargs = dict(
+        supports_check_mode=True
+    )
 
     def run(self):
         name_filter = self.params['name']

plugins/modules/volume_info.py

@@ -124,6 +124,9 @@ class VolumeInfoModule(OpenStackModule):
         name=dict(type='str', required=False),
         status=dict(type='str', required=False),
     )
+    module_kwargs = dict(
+        supports_check_mode=True
+    )
 
     def run(self):
         kwargs = self.check_versioned(

plugins/modules/volume_snapshot_info.py

@@ -96,6 +96,9 @@ class VolumeSnapshotInfoModule(OpenStackModule):
                              'deleting', 'error_deleting', 'rollbacking',
                              'backing-up']),
     )
+    module_kwargs = dict(
+        supports_check_mode=True
+    )
 
     def run(self):
 

tools/run-ansible-sanity.sh

@@ -19,6 +19,9 @@ echo "Executing ansible-test sanity checks in ${ANSIBLE_COLLECTIONS_PATH}"
 
 trap "rm -rf ${ANSIBLE_COLLECTIONS_PATH}" err exit
 
+PY_VER=$(python3 -c "from platform import python_version;print(python_version())" | cut -f 1,2 -d".")
+echo "Running test with Python version ${PY_VER}"
+
 rm -rf "${ANSIBLE_COLLECTIONS_PATH}"
 mkdir -p ${ANSIBLE_COLLECTIONS_PATH}/ansible_collections/openstack/cloud
 cp -a ${TOXDIR}/{plugins,meta,scripts,tests,docs} ${ANSIBLE_COLLECTIONS_PATH}/ansible_collections/openstack/cloud
@@ -27,7 +30,7 @@ echo "Running ansible-test with version:"
 ansible --version
 ansible-test sanity -v \
     --venv \
-    --python 3.6 \
+    --python ${PY_VER} \
     --skip-test metaclass-boilerplate \
     --skip-test future-import-boilerplate \
-    plugins/ docs/ meta/ scripts/ tests/
+    plugins/ docs/ meta/ scripts/

tox.ini

@@ -1,5 +1,5 @@
 [tox]
-minversion = 3.1
+minversion = 3.18.0
 envlist = pep8
 skipsdist = True
 ignore_basepython_conflict = True
@@ -15,7 +15,7 @@ setenv =
     VIRTUAL_ENV={envdir}
     LANG=en_US.UTF-8
     LANGUAGE=en_US:en
-    LC_ALL=C
+    LC_ALL=en_US.utf-8
     OS_LOG_CAPTURE={env:OS_LOG_CAPTURE:true}
     OS_STDOUT_CAPTURE={env:OS_STDOUT_CAPTURE:true}
     OS_STDERR_CAPTURE={env:OS_STDERR_CAPTURE:true}