From a660a35d8689c970b5025306b7d6b7baf8359056 Mon Sep 17 00:00:00 2001
From: Jakob Meng <code@jakobmeng.de>
Date: Thu, 4 Aug 2022 10:52:23 +0200
Subject: [PATCH] Replaced expensive get_server() and fixed issues in server
module
openstacksdk's get_server() [1] calls add_server_interfaces() [2]
which queries OpenStack APIs several times to get all ports and
floating ips attached to a server.
Now we call openstacksdk's compute.find_server() [3] and compute.\
get_server() [4] which result in two API calls, in order to fill
server['addresses'] attribute which we later use to get floating ip
addresses attached to the server.
Do an extra call to compute.get_server() in order to return a pristine
server resource, because openstacksdk's create_server() might call
meta.add_server_interfaces() which alters server attributes such as
server['addresses'] [5].
Fail if options 'auto_ip', 'floating_ips' or 'floating_ip_pools'
are specified but 'wait' is not set to true, because openstacksdk
will add floating ip addresses only if we wait until the server
has been created [6]. This conditional fail will help users to not
shoot their foot.
Marked floating ip support unstable in this module due to various
unresolved issues in openstacksdk's add_ips_to_server() function
such as [9] and [10].
For Zuul CI job ansible-collections-openstack-functional-devstack-\
releases to pass, the minimum required openstacksdk release must be
0.101.0 because [7],[8] are available since that release only.
[1] https://opendev.org/openstack/openstacksdk/src/commit/3f81d0001dd994cde990d38f6e2671ee0694d7d5/openstack/cloud/_compute.py#L484
[2] https://opendev.org/openstack/openstacksdk/src/commit/3f81d0001dd994cde990d38f6e2671ee0694d7d5/openstack/cloud/meta.py#L439
[3] https://opendev.org/openstack/openstacksdk/src/commit/3f81d0001dd994cde990d38f6e2671ee0694d7d5/openstack/compute/v2/_proxy.py#L652
[4] https://opendev.org/openstack/openstacksdk/src/commit/3f81d0001dd994cde990d38f6e2671ee0694d7d5/openstack/compute/v2/_proxy.py#L666
[5] https://opendev.org/openstack/openstacksdk/src/commit/3f81d0001dd994cde990d38f6e2671ee0694d7d5/openstack/cloud/_compute.py#L942
[6] https://opendev.org/openstack/openstacksdk/src/commit/3f81d0001dd994cde990d38f6e2671ee0694d7d5/openstack/cloud/_compute.py#L945
[7] https://review.opendev.org/c/openstack/openstacksdk/+/851976
[8] https://github.com/openstack/openstacksdk/commit/0ded7ac398843b6b1ce46668eb3b45ce02628428
[9] https://storyboard.openstack.org/#!/story/2010352
[10] https://storyboard.openstack.org/#!/story/2010153
Change-Id: I6a5663433b1b9529f99d5eced22a28c692a1d288
---
ci/roles/server/tasks/main.yml | 184 ++++++++++++++++++++++++++++++---
plugins/modules/server.py | 77 +++++++++-----
2 files changed, 217 insertions(+), 44 deletions(-)
diff --git a/ci/roles/server/tasks/main.yml b/ci/roles/server/tasks/main.yml
index b10b3df..d5f0eef 100644
--- a/ci/roles/server/tasks/main.yml
+++ b/ci/roles/server/tasks/main.yml
@@ -29,6 +29,18 @@
network_name: "{{ server_alt_network }}"
state: present
+- name: Create router 1 (for attaching floating ips)
+ openstack.cloud.router:
+ cloud: "{{ cloud }}"
+ state: present
+ name: ansible_router1
+ network: public
+ interfaces:
+ - net: "{{ server_network }}"
+ subnet: "{{ server_subnet }}"
+ - net: "{{ server_alt_network }}"
+ subnet: "{{ server_alt_subnet }}"
+
- name: Create security group for server
openstack.cloud.security_group:
cloud: "{{ cloud }}"
@@ -284,6 +296,68 @@
name: "{{ server_name }}"
wait: true
+- name: Create server on private network with auto_ip
+ openstack.cloud.server:
+ auto_ip: true
+ cloud: "{{ cloud }}"
+ flavor: "{{ flavor }}"
+ image: "{{ image }}"
+ name: "{{ server_name }}"
+ nics:
+ - net-name: "{{ server_network }}"
+ reuse_ips: false
+ state: present
+ wait: true
+ register: server
+
+- name: Assert server on private network with auto_ip
+ assert:
+ that:
+ - server.server.addresses.values()
+ |flatten(levels=1)
+ |selectattr('OS-EXT-IPS:type', 'equalto', 'floating')
+ |map(attribute='addr')
+ |list|length == 1
+
+- name: Delete server on private network with auto_ip
+ openstack.cloud.server:
+ cloud: "{{ cloud }}"
+ state: absent
+ name: "{{ server_name }}"
+ wait: true
+
+- name: Create server on public network
+ openstack.cloud.server:
+ auto_ip: false
+ cloud: "{{ cloud }}"
+ flavor: "{{ flavor }}"
+ image: "{{ image }}"
+ name: "{{ server_name }}"
+ nics:
+ - net-name: 'public'
+ reuse_ips: false
+ state: present
+ wait: true
+ register: server
+
+- debug: var=server
+
+- name: Assert server on public network
+ assert:
+ that:
+ - server.server.addresses.values()
+ |flatten(levels=1)
+ |selectattr('OS-EXT-IPS:type', 'equalto', 'floating')
+ |map(attribute='addr')
+ |list|length == 0
+
+- name: Delete server on public network
+ openstack.cloud.server:
+ cloud: "{{ cloud }}"
+ state: absent
+ name: "{{ server_name }}"
+ wait: true
+
- name: Create port to be attached to server
openstack.cloud.port:
cloud: "{{ cloud }}"
@@ -310,18 +384,46 @@
key2: value2
name: "{{ server_name }}"
nics:
- - net-name: 'public'
- net-name: "{{ server_network }}"
- - port-id: '{{ port.port.id }}'
+ - port-id: "{{ port.port.id }}"
+ reuse_ips: false
state: present
wait: true
register: server
- debug: var=server
+- name: Assert server is not on public network and does not have a floating ip
+ assert:
+ that:
+ - server.server.addresses.keys()|sort == [server_network]|sort
+ - server.server.addresses.values()
+ |flatten(levels=1)
+ |selectattr('OS-EXT-IPS:type', 'equalto', 'floating')
+ |map(attribute='addr')
+ |list|length == 0
+
+- name: Find all floating ips for debugging
+ openstack.cloud.floating_ip_info:
+ cloud: "{{ cloud }}"
+ register: fips
+
+- name: Print all floating ips for debugging
+ debug: var=fips
+
+- name: Find all servers for debugging
+ openstack.cloud.server_info:
+ cloud: "{{ cloud }}"
+ register: servers
+
+- name: Print all servers for debugging
+ debug: var=servers
+
- name: Update server
openstack.cloud.server:
- auto_ip: true
+ # TODO: Change auto_ip to true once openstacksdk's issue #2010352 has been solved
+ # Ref.: https://storyboard.openstack.org/#!/story/2010352
+ auto_ip: false
cloud: "{{ cloud }}"
description: "This server got updated"
# flavor cannot be updated but must be present
@@ -334,9 +436,9 @@
name: "{{ server_name }}"
# nics cannot be updated
nics:
- - net-name: 'public'
- net-name: "{{ server_network }}"
- - port-id: '{{ port.port.id }}'
+ - port-id: "{{ port.port.id }}"
+ reuse_ips: false
security_groups:
- '{{ server_security_group }}'
- '{{ server_alt_security_group }}'
@@ -355,18 +457,28 @@
- "'key1' not in server_updated.server.metadata"
- server_updated.server.metadata['key2'] == 'value2'
- server_updated.server.metadata['key3'] == 'value3'
- - server_updated.server.addresses.keys()|sort == [server_network,'public']
- - server_updated.server.addresses[server_network]|length == 2
- - server_updated.server.addresses.public|length > 0
- - port.port.fixed_ips[0].ip_address in
- server_updated.server.addresses[server_network]|map(attribute='addr')
- server_updated.server.security_groups|map(attribute='name')|unique|length == 2
- security_group.secgroup.name in server_updated.server.security_groups|map(attribute='name')
- security_group_alt.secgroup.name in server_updated.server.security_groups|map(attribute='name')
+ - server_network in server_updated.server.addresses.keys()|list|sort
+ - server_updated.server.addresses[server_network]|length == 2
+ - port.port.fixed_ips[0].ip_address in
+ server_updated.server.addresses[server_network]|map(attribute='addr')
+ # TODO: Verify networks once openstacksdk's issue #2010352 has been solved
+ # Ref.: https://storyboard.openstack.org/#!/story/2010352
+ #- server_updated.server.addresses.public|length > 0
+ #- (server_updated.server.addresses.keys()|sort == ([server_network, 'public']|sort))
+ # or (server_updated.server.addresses.values()
+ # |flatten(levels=1)
+ # |selectattr('OS-EXT-IPS:type', 'equalto', 'floating')
+ # |map(attribute='addr')
+ # |list|length > 0)
- name: Update server again
openstack.cloud.server:
- auto_ip: true
+ # TODO: Change auto_ip to true once openstacksdk's issue #2010352 has been solved
+ # Ref.: https://storyboard.openstack.org/#!/story/2010352
+ auto_ip: false
cloud: "{{ cloud }}"
description: "This server got updated"
# flavor cannot be updated but must be present
@@ -379,21 +491,55 @@
name: "{{ server_name }}"
# nics cannot be updated
nics:
- - net-name: 'public'
- net-name: "{{ server_network }}"
- - port-id: '{{ port.port.id }}'
+ - port-id: "{{ port.port.id }}"
+ reuse_ips: false
security_groups:
- '{{ server_security_group }}'
- '{{ server_alt_security_group }}'
state: present
wait: true
- register: server_again
+ register: server_updated_again
- name: Assert server did not change
assert:
that:
- - server.server.id == server_again.server.id
- - server_again is not changed
+ - server.server.id == server_updated_again.server.id
+ - server_updated_again is not changed
+
+# TODO: Drop failure test once openstacksdk's issue #2010352 has been solved
+# Ref.: https://storyboard.openstack.org/#!/story/2010352
+- name: Update server again with auto_ip set to true
+ openstack.cloud.server:
+ auto_ip: true
+ cloud: "{{ cloud }}"
+ description: "This server got updated"
+ # flavor cannot be updated but must be present
+ flavor: "{{ flavor }}"
+ # image cannot be updated but must be present
+ image: "{{ image }}"
+ metadata:
+ key2: value2
+ key3: value3
+ name: "{{ server_name }}"
+ # nics cannot be updated
+ nics:
+ - net-name: "{{ server_network }}"
+ - port-id: "{{ port.port.id }}"
+ reuse_ips: false
+ security_groups:
+ - '{{ server_security_group }}'
+ - '{{ server_alt_security_group }}'
+ state: present
+ wait: true
+ register: server_updated_again
+ ignore_errors: true
+
+- name: Assert server update succeeded or failed with expected error
+ assert:
+ that:
+ - not server_updated_again.failed
+ or ('was found matching your NAT destination network' in server_updated_again.msg)
- name: Delete updated server
openstack.cloud.server:
@@ -421,6 +567,12 @@
state: absent
name: "{{ server_security_group }}"
+- name: Delete router 1
+ openstack.cloud.router:
+ cloud: "{{ cloud }}"
+ state: absent
+ name: ansible_router1
+
- name: Delete second subnet for server
openstack.cloud.subnet:
cloud: "{{ cloud }}"
diff --git a/plugins/modules/server.py b/plugins/modules/server.py
index cd09a8f..fabfcbb 100644
--- a/plugins/modules/server.py
+++ b/plugins/modules/server.py
@@ -18,6 +18,10 @@ options:
auto_ip:
description:
- Ensure instance has public ip however the cloud wants to do that.
+ - For example, the cloud could add a floating ip for the server or
+ attach the server to a public network.
+ - Requires I(wait) to be C(True) during server creation.
+ - Floating IP support is unstable in this module, use with caution.
type: bool
default: 'yes'
aliases: ['auto_floating_ip', 'public_ip']
@@ -50,6 +54,7 @@ options:
description:
- When I(state) is C(absent) and this option is true, any floating IP
address associated with this server will be deleted along with it.
+ - Floating IP support is unstable in this module, use with caution.
type: bool
aliases: ['delete_fip']
default: 'no'
@@ -84,11 +89,15 @@ options:
floating_ip_pools:
description:
- Name of floating IP pool from which to choose a floating IP.
+ - Requires I(wait) to be C(True) during server creation.
+ - Floating IP support is unstable in this module, use with caution.
type: list
elements: str
floating_ips:
description:
- list of valid floating IPs that pre-exist to assign to this node.
+ - Requires I(wait) to be C(True) during server creation.
+ - Floating IP support is unstable in this module, use with caution.
type: list
elements: str
image:
@@ -158,6 +167,7 @@ options:
concurrent server creation, it is highly recommended to set this to
false and to delete the floating ip associated with a server when
the server is deleted using I(delete_ips).
+ - Floating IP support is unstable in this module, use with caution.
- This server attribute cannot be updated.
type: bool
default: 'yes'
@@ -777,6 +787,7 @@ server:
type: list
'''
from ansible_collections.openstack.cloud.plugins.module_utils.openstack import OpenStackModule
+import copy
class ServerModule(OpenStackModule):
@@ -831,10 +842,10 @@ class ServerModule(OpenStackModule):
def run(self):
state = self.params['state']
- # self.conn.get_server is required for server.addresses and
- # server.interface_ip which self.conn.compute.find_server
- # does not return
- server = self.conn.get_server(self.params['name'])
+ server = self.conn.compute.find_server(self.params['name'])
+ if server:
+ # fetch server details such as server['addresses']
+ server = self.conn.compute.get_server(server)
if self.ansible.check_mode:
self.exit_json(changed=self._will_change(state, server))
@@ -850,16 +861,6 @@ class ServerModule(OpenStackModule):
update = self._build_update(server)
if update:
server = self._update(server, update)
- else:
- # drop attributes added in function
- # openstacksdk.meta.add_server_interfaces()
- # because all other branches do not return them
- #
- # addresses will be expanded by get_server's call to
- # openstacksdk.meta.add_server_interfaces() but we
- # cannot easily undo this so we ignore it
- for k in ['public_v4', 'public_v6', 'interface_ip']:
- del server[k]
self.exit_json(changed=bool(update),
server=server.to_dict(computed=False))
@@ -893,20 +894,18 @@ class ServerModule(OpenStackModule):
# do not add or remove any floating ip.
return {}
- if (auto_ip and server['interface_ip']
- and not (floating_ip_pools or floating_ips)):
+ # Get floating ip addresses attached to the server
+ ips = [interface_spec['addr']
+ for v in server['addresses'].values()
+ for interface_spec in v
+ if interface_spec.get('OS-EXT-IPS:type', None) == 'floating']
+
+ if (auto_ip and ips and not floating_ip_pools and not floating_ips):
# Server has a floating ip address attached and
# no specific floating ip has been requested,
# so nothing to change.
return {}
- # Get floating ip addresses attached to the server
- ips = [interface_spec['addr']
- for v in server.addresses.values()
- for interface_spec in v
- if ('OS-EXT-IPS:type' in interface_spec
- and interface_spec['OS-EXT-IPS:type'] == 'floating')]
-
if not ips:
# One or multiple floating ips have been requested,
# but none have been attached, so attach them.
@@ -1024,6 +1023,15 @@ class ServerModule(OpenStackModule):
return update
def _create(self):
+ for k in ['auto_ip', 'floating_ips', 'floating_ip_pools']:
+ if self.params[k] is not None \
+ and self.params['wait'] is False:
+ # floating ip addresses will only be added if
+ # we wait until the server has been created
+ # Ref.: https://opendev.org/openstack/openstacksdk/src/commit/3f81d0001dd994cde990d38f6e2671ee0694d7d5/openstack/cloud/_compute.py#L945
+ self.fail_json(
+ msg="Option '{0}' requires 'wait: yes'".format(k))
+
flavor_name_or_id = self.params['flavor']
image_id = None
@@ -1063,7 +1071,12 @@ class ServerModule(OpenStackModule):
server = self.conn.create_server(**args)
- return server
+ # openstacksdk's create_server() might call meta.add_server_interfaces(
+ # ) which alters server attributes such as server['addresses']. So we
+ # do an extra call to compute.get_server() to return a clean server
+ # resource.
+ # Ref.: https://opendev.org/openstack/openstacksdk/src/commit/3f81d0001dd994cde990d38f6e2671ee0694d7d5/openstack/cloud/_compute.py#L942
+ return self.conn.compute.get_server(server)
def _delete(self, server):
self.conn.delete_server(
@@ -1077,15 +1090,15 @@ class ServerModule(OpenStackModule):
server = self._update_server(server, update)
# Refresh server attributes after security groups etc. have changed
#
- # self.conn.get_server() is unnecessary because server.addresses and
- # server.interface_ip are computed and hence not returned anyway
- return self.conn.compute.find_server(name_or_id=server.id)
+ # Use compute.get_server() instead of compute.find_server()
+ # to include server details
+ return self.conn.compute.get_server(server)
def _update_ips(self, server, update):
args = dict((k, self.params[k]) for k in ['wait', 'timeout'])
ips = update.get('ips')
if ips:
- return self.conn.add_ips_to_server(server, **ips, **args)
+ server = self.conn.add_ips_to_server(server, **ips, **args)
add_ips = update.get('add_ips')
if add_ips:
@@ -1182,6 +1195,10 @@ class ServerModule(OpenStackModule):
net['net-name'], ignore_missing=False).id
# Replace net-name with net-id and keep optional nic args
# Ref.: https://github.com/ansible/ansible/pull/20969
+ #
+ # Delete net-name from a copy else it will
+ # disappear from Ansible's debug output
+ net = copy.deepcopy(net)
del net['net-name']
net['net-id'] = network_id
nics.append(net)
@@ -1192,6 +1209,10 @@ class ServerModule(OpenStackModule):
net['port-name'], ignore_missing=False).id
# Replace net-name with net-id and keep optional nic args
# Ref.: https://github.com/ansible/ansible/pull/20969
+ #
+ # Delete net-name from a copy else it will
+ # disappear from Ansible's debug output
+ net = copy.deepcopy(net)
del net['port-name']
net['port-id'] = port_id
nics.append(net)
--
GitLab