From 0ec16fbeccf0c43444aabf4d1d5dfd5498bfcde4 Mon Sep 17 00:00:00 2001
From: Jakob Meng <code@jakobmeng.de>
Date: Thu, 4 Aug 2022 14:58:27 +0200
Subject: [PATCH] Dropped extra interfaces_info attribute from routers_info
module
routers_info's interfaces_info attribute is not provided by
openstacksdk, it is added to each router resource by the routers_info
module after retrieving the routers list. To get the required data
list_router_interfaces() [1] is being called for each router resource
which then retrieves all ports for each router. This requires extra
api calls which might be useless because we do not know whether the
user actually cares about the ports. For getting ports of a router
we have the openstack.cloud.ports module. So instead of proactively
retrieving the router ports we drop the interfaces_info attribute.
The interfaces_info attribute was introduced because retrieving
interfaces via openstacksdk and openstack.cloud modules was
complex in the past [2]. Nowadays, using openstack.cloud.ports
Ansible and Jinja2 filters retrieving ip addresses of a router
is straight forward. In case someone still needs the old
interfaces_info attribute, one can refer to the module example
to see how it could be reproduced. But in general, retrieving the
router interfaces is much easier as can be seen in the updated
integration tests.
[1] https://opendev.org/openstack/openstacksdk/src/commit/3f81d0001dd994cde990d38f6e2671ee0694d7d5/openstack/cloud/_network.py#L1926
[2] https://review.opendev.org/c/openstack/ansible-collections-openstack/+/703927/6/plugins/modules/os_routers_info.py
Change-Id: I7fbdf11d07c95421d3aee800bfeebb88ea829817
---
ci/roles/router/tasks/main.yml | 184 ++++++++++++++----
plugins/modules/routers_info.py | 95 ++++++---
.../cloud/openstack/test_routers_info.py | 87 +--------
3 files changed, 223 insertions(+), 143 deletions(-)
diff --git a/ci/roles/router/tasks/main.yml b/ci/roles/router/tasks/main.yml
index 32c8fc6..afbe654 100644
--- a/ci/roles/router/tasks/main.yml
+++ b/ci/roles/router/tasks/main.yml
@@ -74,10 +74,21 @@
- name: Verify routers info
assert:
that:
+ - info.routers|length == 1
- info.routers.0.name == router_name
- - (info.routers.0.interfaces_info|length) == 0
- info.routers.0.is_admin_state_up
+- name: List ports of router
+ openstack.cloud.port_info:
+ cloud: "{{ cloud }}"
+ filters:
+ device_id: "{{ router.router.id }}"
+ register: ports
+
+- name: Verify router ports
+ assert:
+ that: ports.ports|length == 0
+
- name: Update router (add interface)
openstack.cloud.router:
cloud: "{{ cloud }}"
@@ -109,13 +120,26 @@
assert:
that:
- info.routers.0.name == router_name
- - (info.routers.0.interfaces_info|length) == 1
- info.routers.0.is_admin_state_up
+- name: List ports of router
+ openstack.cloud.port_info:
+ cloud: "{{ cloud }}"
+ filters:
+ device_id: "{{ router.router.id }}"
+ register: ports
+
+- name: Verify router ports
+ assert:
+ that:
+ - ports.ports|length == 1
+ - (ports.ports.0.fixed_ips|map(attribute='ip_address')|sort|list == ['10.7.7.1']) or
+ ports.ports.0.fixed_ips|length > 0
+
- name: Verify existence of return values
assert:
that: item in info.routers[0]
- loop: "{{ expected_fields + ['interfaces_info'] }}"
+ loop: "{{ expected_fields }}"
- name: Gather routers info with filters
openstack.cloud.routers_info:
@@ -130,7 +154,6 @@
that:
- info.routers.0.name == router_name
- info.routers.0.id == router.router.id
- - (info.routers.0.interfaces_info|length) == 1
- name: Gather routers info with other filters
openstack.cloud.routers_info:
@@ -184,10 +207,22 @@
- name: Verify routers info
assert:
that:
+ - info.routers|length == 1
- info.routers.0.name == router_name
- info.routers.0.id == router.router.id
- - (info.routers.0.interfaces_info|length) == 3
- - info.routers.0.interfaces_info|map(attribute='ip_address')|sort|list ==
+
+- name: List ports of router
+ openstack.cloud.port_info:
+ cloud: "{{ cloud }}"
+ filters:
+ device_id: "{{ router.router.id }}"
+ register: ports
+
+- name: Verify router ports
+ assert:
+ that:
+ - ports.ports|rejectattr('device_owner', 'equalto', 'network:router_gateway')|sum(attribute='fixed_ips', start=[])|map(attribute='ip_address')|list|length == 3
+ - ports.ports|rejectattr('device_owner', 'equalto', 'network:router_gateway')|sum(attribute='fixed_ips', start=[])|map(attribute='ip_address')|sort|list ==
['10.10.10.1', '10.8.8.1', '10.9.9.1']
- name: Update router (remove interfaces)
@@ -222,8 +257,19 @@
that:
- info.routers.0.name == router_name
- info.routers.0.id == router.router.id
- - (info.routers.0.interfaces_info|length) == 1
- - info.routers.0.interfaces_info|map(attribute='ip_address')|sort|list ==
+
+- name: List ports of router
+ openstack.cloud.port_info:
+ cloud: "{{ cloud }}"
+ filters:
+ device_id: "{{ router.router.id }}"
+ register: ports
+
+- name: Verify router ports
+ assert:
+ that:
+ - ports.ports|rejectattr('device_owner', 'equalto', 'network:router_gateway')|sum(attribute='fixed_ips', start=[])|map(attribute='ip_address')|list|length == 1
+ - ports.ports|rejectattr('device_owner', 'equalto', 'network:router_gateway')|sum(attribute='fixed_ips', start=[])|map(attribute='ip_address')|sort|list ==
['10.10.10.1']
- name: Update router (replace interfaces)
@@ -236,18 +282,18 @@
subnet: shade_subnet1
portip: 10.7.7.1
-- name: Gather routers info
- openstack.cloud.routers_info:
- cloud: "{{ cloud }}"
- name: "{{ router_name }}"
- register: info
+- name: List ports of router
+ openstack.cloud.port_info:
+ cloud: "{{ cloud }}"
+ filters:
+ device_id: "{{ router.router.id }}"
+ register: ports
-- name: Verify routers info
+- name: Verify router ports
assert:
that:
- - info.routers.0.name == router_name
- - (info.routers.0.interfaces_info|length) == 1
- - info.routers.0.interfaces_info|map(attribute='ip_address')|sort|list ==
+ - ports.ports|rejectattr('device_owner', 'equalto', 'network:router_gateway')|sum(attribute='fixed_ips', start=[])|map(attribute='ip_address')|list|length == 1
+ - ports.ports|rejectattr('device_owner', 'equalto', 'network:router_gateway')|sum(attribute='fixed_ips', start=[])|map(attribute='ip_address')|sort|list ==
['10.7.7.1']
- name: Update router (replace interfaces) again
@@ -265,20 +311,19 @@
assert:
that: router is not changed
-- name: Gather routers info
- openstack.cloud.routers_info:
- cloud: "{{ cloud }}"
- name: "{{ router_name }}"
- filters:
- is_admin_state_up: true
- register: info
+- name: List ports of router
+ openstack.cloud.port_info:
+ cloud: "{{ cloud }}"
+ filters:
+ device_id: "{{ router.router.id }}"
+ register: ports
-- name: Verify routers info
+- name: Verify router ports
assert:
that:
- - info.routers.0.name == router_name
- - (info.routers.0.interfaces_info|length) == 1
- - info.routers.0.interfaces_info.0.ip_address == '10.7.7.1'
+ - ports.ports|rejectattr('device_owner', 'equalto', 'network:router_gateway')|sum(attribute='fixed_ips', start=[])|map(attribute='ip_address')|list|length == 1
+ - ports.ports|rejectattr('device_owner', 'equalto', 'network:router_gateway')|sum(attribute='fixed_ips', start=[])|map(attribute='ip_address')|sort|list ==
+ ['10.7.7.1']
# Admin operation
- name: Create external network
@@ -323,7 +368,18 @@
assert:
that:
- info.routers.0.name == router_name
- - (info.routers.0.interfaces_info|length) == 1
+
+- name: List ports of router
+ openstack.cloud.port_info:
+ cloud: "{{ cloud }}"
+ filters:
+ device_id: "{{ router.router.id }}"
+ register: ports
+
+- name: Verify router ports
+ assert:
+ that:
+ - ports.ports|rejectattr('device_owner', 'equalto', 'network:router_gateway')|sum(attribute='fixed_ips', start=[])|map(attribute='ip_address')|list|length == 1
- name: Update router (change external fixed ips)
openstack.cloud.router:
@@ -558,15 +614,19 @@
assert:
that: router is changed
-- name: Gather routers info
- openstack.cloud.routers_info:
- cloud: "{{ cloud }}"
- name: "{{ router_name }}"
- register: info
+- name: List ports of router
+ openstack.cloud.port_info:
+ cloud: "{{ cloud }}"
+ filters:
+ device_id: "{{ router.router.id }}"
+ register: ports
-- name: Verify routers info
+- name: Verify router ports
assert:
- that: "'10.7.7.42' in info.routers[0].interfaces_info|map(attribute='ip_address')"
+ that:
+ - ports.ports|rejectattr('device_owner', 'equalto', 'network:router_gateway')|sum(attribute='fixed_ips', start=[])|map(attribute='ip_address')|list|length == 1
+ - ports.ports|rejectattr('device_owner', 'equalto', 'network:router_gateway')|sum(attribute='fixed_ips', start=[])|map(attribute='ip_address')|sort|list ==
+ ['10.7.7.42']
- name: Unset portip in already assigned subnet
openstack.cloud.router:
@@ -582,6 +642,58 @@
assert:
that: router is not changed
+- name: List all routers
+ openstack.cloud.routers_info:
+ cloud: "{{ cloud }}"
+ register: routers
+
+- name: List ports of all routers
+ loop: "{{ routers.routers }}"
+ openstack.cloud.port_info:
+ cloud: "{{ cloud }}"
+ filters:
+ device_id: "{{ item['id'] }}"
+ register: ports
+
+- name: Transform ports for interfaces_info entries
+ loop: "{{ ports.results|map(attribute='ports')|list }}"
+ set_fact:
+ interfaces_info: |-
+ {% for port in item %}
+ {% if port.device_owner != "network:router_gateway" %}
+ {% for fixed_ip in port['fixed_ips'] %}
+ - port_id: {{ port.id }}
+ ip_address: {{ fixed_ip.ip_address }}
+ subnet_id: {{ fixed_ip.subnet_id }}
+ {% endfor %}
+ {% endif %}
+ {% endfor %}
+ register: interfaces
+
+- name: Combine router and interfaces_info entries
+ loop: "{{
+ routers.routers|zip(interfaces.results|map(attribute='ansible_facts'))|list
+ }}"
+ set_fact:
+ # underscore prefix to prevent overwriting facts outside of loop
+ _router: "{{
+ item.0|combine({'interfaces_info': (item.1.interfaces_info|from_yaml) })
+ }}"
+ register: routers
+
+- name: Remove set_fact artifacts from routers
+ set_fact:
+ routers: "{{ {
+ 'routers': routers.results|map(attribute='ansible_facts._router')|list
+ } }}"
+
+- debug: var=routers
+
+- name: Assert our router's interfaces_info
+ assert:
+ that:
+ - routers.routers|selectattr('id', 'equalto', router.router.id)|list|length == 1
+
# Cleanup environment
- name: Delete router
diff --git a/plugins/modules/routers_info.py b/plugins/modules/routers_info.py
index 22ac68e..4683e9b 100644
--- a/plugins/modules/routers_info.py
+++ b/plugins/modules/routers_info.py
@@ -19,14 +19,15 @@ options:
type: str
filters:
description:
- - A dictionary of meta data to use for further filtering. Elements of
+ - A dictionary of meta data to use for further filtering. Elements of
this dictionary may be additional dictionaries.
required: false
type: dict
suboptions:
project_id:
description:
- - Filter the list result by the ID of the project that owns the resource.
+ - Filter the list result by the ID of the project that owns the
+ resource.
type: str
aliases:
- tenant_id
@@ -36,11 +37,13 @@ options:
type: str
description:
description:
- - Filter the list result by the human-readable description of the resource.
+ - Filter the list result by the human-readable description of the
+ resource.
type: str
is_admin_state_up:
description:
- - Filter the list result by the administrative state of the resource, which is up (true) or down (false).
+ - Filter the list result by the administrative state of the
+ resource, which is up (true) or down (false).
type: bool
revision_number:
description:
@@ -48,7 +51,8 @@ options:
type: int
tags:
description:
- - A list of tags to filter the list result by. Resources that match all tags in this list will be returned.
+ - A list of tags to filter the list result by. Resources that match
+ all tags in this list will be returned.
type: list
elements: str
requirements:
@@ -100,6 +104,68 @@ EXAMPLES = '''
- name: Show openstack routers
debug:
msg: "{{ result.routers }}"
+
+- name: List all routers
+ openstack.cloud.routers_info:
+ cloud: devstack
+ register: routers
+
+- name: List ports of first router
+ openstack.cloud.port_info:
+ cloud: devstack
+ filters:
+ device_id: "{{ routers[0].router.id }}"
+ register: ports
+
+- name: Show first router's fixed ips
+ debug:
+ msg: "{{ ports.ports
+ |rejectattr('device_owner', 'equalto', 'network:router_gateway')
+ |sum(attribute='fixed_ips', start=[])
+ |map(attribute='ip_address')
+ |sort|list }}"
+
+- name: List ports of all routers
+ loop: "{{ routers.routers }}"
+ openstack.cloud.port_info:
+ cloud: devstack
+ filters:
+ device_id: "{{ item['id'] }}"
+ register: ports
+
+- name: Transform ports for interfaces_info entries
+ loop: "{{ ports.results|map(attribute='ports')|list }}"
+ set_fact:
+ interfaces_info: |-
+ {% for port in item %}
+ {% if port.device_owner != "network:router_gateway" %}
+ {% for fixed_ip in port['fixed_ips'] %}
+ - port_id: {{ port.id }}
+ ip_address: {{ fixed_ip.ip_address }}
+ subnet_id: {{ fixed_ip.subnet_id }}
+ {% endfor %}
+ {% endif %}
+ {% endfor %}
+ register: interfaces
+
+- name: Combine router and interfaces_info entries
+ loop: "{{
+ routers.routers|zip(interfaces.results|map(attribute='ansible_facts'))|list
+ }}"
+ set_fact:
+ # underscore prefix to prevent overwriting facts outside of loop
+ _router: "{{
+ item.0|combine({'interfaces_info': item.1.interfaces_info|from_yaml})
+ }}"
+ register: routers
+
+- name: Remove set_fact artifacts from routers
+ set_fact:
+ routers: "{{ {
+ 'routers': routers.results|map(attribute='ansible_facts._router')|list
+ } }}"
+
+- debug: var=routers
'''
RETURN = '''
@@ -137,10 +203,6 @@ routers:
description: Unique UUID.
returned: success
type: str
- interfaces_info:
- description: List of connected interfaces.
- returned: success
- type: list
is_admin_state_up:
description: Network administrative state
returned: success
@@ -206,21 +268,6 @@ class RouterInfoModule(OpenStackModule):
for router in self.conn.search_routers(
name_or_id=self.params['name'],
filters=self.params['filters'])]
-
- # append interfaces_info attribute for backward compatibility
- for router in routers:
- interfaces_info = []
- for port in self.conn.list_router_interfaces(router):
- if port.device_owner != "network:router_gateway":
- for ip_spec in port.fixed_ips:
- int_info = {
- 'port_id': port.id,
- 'ip_address': ip_spec.get('ip_address'),
- 'subnet_id': ip_spec.get('subnet_id')
- }
- interfaces_info.append(int_info)
- router['interfaces_info'] = interfaces_info
-
self.exit(changed=False, routers=routers)
diff --git a/tests/unit/modules/cloud/openstack/test_routers_info.py b/tests/unit/modules/cloud/openstack/test_routers_info.py
index 0259889..9ccf9db 100644
--- a/tests/unit/modules/cloud/openstack/test_routers_info.py
+++ b/tests/unit/modules/cloud/openstack/test_routers_info.py
@@ -1,10 +1,8 @@
import munch
-from mock import patch
-
from ansible_collections.openstack.cloud.plugins.modules import routers_info
-from ansible_collections.openstack.cloud.tests.unit.modules.utils import set_module_args, ModuleTestCase, AnsibleExitJson
+from ansible_collections.openstack.cloud.tests.unit.modules.utils import ModuleTestCase
def openstack_cloud_from_module(module, **kwargs):
@@ -108,64 +106,17 @@ class FakeCloud(object):
]
if name_or_id is not None:
- return [munch.Munch(router) for router in test_routers if router["name"] == name_or_id]
+ return [munch.Munch(router) for router in test_routers
+ if router["name"] == name_or_id]
else:
return [munch.Munch(router) for router in test_routers]
- def list_router_interfaces(self, router):
- test_ports = [
- {
- "device_id": "d3f70ce4-7ab1-46a7-9bec-498c9d8a2483",
- "device_owner": "network:router_interface",
- "fixed_ips": [
- {
- "ip_address": "192.168.1.254",
- "subnet_id": "0624c75f-0574-41b5-a8d1-92e6e3a9e51d"
- }
- ],
- "id": "92eeeca3-225d-46b8-a857-ede6c4f05484",
- },
- {
- "device_id": "b869307c-a1f9-4956-a993-8a90fc7cc01d",
- "device_owner": "network:router_gateway",
- "fixed_ips": [
- {
- "ip_address": "172.24.4.10",
- "subnet_id": "b42b8057-5b3b-4aa3-949a-eaaee2032462"
- },
- ],
- "id": "ab45060c-98fd-42a3-a1aa-8d5a03554bef",
- },
- {
- "device_id": "98bce30e-c912-4490-85eb-b22d650721e6",
- "device_owner": "network:router_interface",
- "fixed_ips": [
- {
- "ip_address": "192.168.1.1",
- "subnet_id": "0624c75f-0574-41b5-a8d1-92e6e3a9e51d"
- }
- ],
- "id": "c9fb53f1-d43e-4588-a223-0e8bf8a79715",
- },
- {
- "device_id": "98bce30e-c912-4490-85eb-b22d650721e6",
- "device_owner": "network:router_gateway",
- "fixed_ips": [
- {
- "ip_address": "172.24.4.234",
- "subnet_id": "b42b8057-5b3b-4aa3-949a-eaaee2032462"
- },
- ],
- "id": "0271878e-4be8-433c-acdc-52823b41bcbf",
- },
- ]
- return [munch.Munch(port) for port in test_ports if port["device_id"] == router.id]
-
class TestRoutersInfo(ModuleTestCase):
'''This class calls the main function of the
openstack.cloud.routers_info module.
'''
+
def setUp(self):
super(TestRoutersInfo, self).setUp()
self.module = routers_info
@@ -174,33 +125,3 @@ class TestRoutersInfo(ModuleTestCase):
with self.assertRaises(exit_exc) as exc:
self.module.main()
return exc.exception.args[0]
-
- @patch('ansible_collections.openstack.cloud.plugins.modules.routers_info.openstack_cloud_from_module', side_effect=openstack_cloud_from_module)
- def test_main_with_router_interface(self, *args):
-
- set_module_args({'name': 'router1'})
- result = self.module_main(AnsibleExitJson)
- self.assertIs(type(result.get('openstack_routers')[0].get('interfaces_info')), list)
- self.assertEqual(len(result.get('openstack_routers')[0].get('interfaces_info')), 1)
- self.assertEqual(result.get('openstack_routers')[0].get('interfaces_info')[0].get('port_id'), '92eeeca3-225d-46b8-a857-ede6c4f05484')
- self.assertEqual(result.get('openstack_routers')[0].get('interfaces_info')[0].get('ip_address'), '192.168.1.254')
- self.assertEqual(result.get('openstack_routers')[0].get('interfaces_info')[0].get('subnet_id'), '0624c75f-0574-41b5-a8d1-92e6e3a9e51d')
-
- @patch('ansible_collections.openstack.cloud.plugins.modules.routers_info.openstack_cloud_from_module', side_effect=openstack_cloud_from_module)
- def test_main_with_router_gateway(self, *args):
-
- set_module_args({'name': 'router2'})
- result = self.module_main(AnsibleExitJson)
- self.assertIs(type(result.get('openstack_routers')[0].get('interfaces_info')), list)
- self.assertEqual(len(result.get('openstack_routers')[0].get('interfaces_info')), 0)
-
- @patch('ansible_collections.openstack.cloud.plugins.modules.routers_info.openstack_cloud_from_module', side_effect=openstack_cloud_from_module)
- def test_main_with_router_interface_and_router_gateway(self, *args):
-
- set_module_args({'name': 'router3'})
- result = self.module_main(AnsibleExitJson)
- self.assertIs(type(result.get('openstack_routers')[0].get('interfaces_info')), list)
- self.assertEqual(len(result.get('openstack_routers')[0].get('interfaces_info')), 1)
- self.assertEqual(result.get('openstack_routers')[0].get('interfaces_info')[0].get('port_id'), 'c9fb53f1-d43e-4588-a223-0e8bf8a79715')
- self.assertEqual(result.get('openstack_routers')[0].get('interfaces_info')[0].get('ip_address'), '192.168.1.1')
- self.assertEqual(result.get('openstack_routers')[0].get('interfaces_info')[0].get('subnet_id'), '0624c75f-0574-41b5-a8d1-92e6e3a9e51d')
--
GitLab