openstack/shade
Code Issues Proposed changes

Skip an extra unneeded server get

Browse Source 
When we're attaching a floating ip, the wait loop gets the most up to
date server dict as part of waiting. In that case, there is no need to
do an additional get. If we're NOT waiting, it's also pointless to do
a single get, as it's not guaranteed to have the updated info.

Change-Id: I84bc54ce32a7d5bf10078027fdb29fab5e84bd39
This commit is contained in:
Monty Taylor
2015-11-01 11:03:17 +09:00
parent b6e918089c
commit 0085effd40
4 changed files with 52 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files

82
shade/openstackcloud.py
View File

@@ -2901,7 +2901,7 @@ class OpenStackCloud(object):
:param skip_attach: (optional) Skip the actual attach and just do
the wait. Defaults to False.
:returns: None
:returns: The server dict
:raises: OpenStackCloudException, on operation error.
"""
@@ -2909,7 +2909,7 @@ class OpenStackCloud(object):
# attached
ext_ip = meta.get_server_ip(server, ext_tag='floating')
if ext_ip == floating_ip['floating_ip_address']:
return
return server
if self.has_service('network'):
if not skip_attach:
@@ -2937,7 +2937,8 @@ class OpenStackCloud(object):
server = self.get_server_by_id(server_id)
ext_ip = meta.get_server_ip(server, ext_tag='floating')
if ext_ip == floating_ip['floating_ip_address']:
return
return server
return server
def _get_free_fixed_port(self, server, fixed_address=None):
ports = self.search_ports(filters={'device_id': server['id']})
@@ -3079,7 +3080,8 @@ class OpenStackCloud(object):
return True
def _add_ip_from_pool(
self, server, network, fixed_address=None, reuse=True):
self, server, network, fixed_address=None, reuse=True,
wait=False, timeout=60):
"""Add a floating IP to a sever from a given pool
This method reuses available IPs, when possible, or allocate new IPs
@@ -3091,38 +3093,44 @@ class OpenStackCloud(object):
:param network: Nova pool name or Neutron network name or id.
:param fixed_address: a fixed address
:param reuse: Try to reuse existing ips. Defaults to True.
:param wait: (optional) Wait for the address to appear as assigned
to the server in Nova. Defaults to False.
:param timeout: (optional) Seconds to wait, defaults to 60.
See the ``wait`` parameter.
:returns: the floating IP assigned
:returns: the update server dict
"""
if reuse:
f_ip = self.available_floating_ip(network=network)
else:
f_ip = self.create_floating_ip(network=network)
self._attach_ip_to_server(
server=server, floating_ip=f_ip, fixed_address=fixed_address)
return self._attach_ip_to_server(
server=server, floating_ip=f_ip, fixed_address=fixed_address,
wait=wait, timeout=timeout)
return f_ip
def add_ip_list(self, server, ips):
def add_ip_list(self, server, ips, wait=False, timeout=60):
"""Attach a list of IPs to a server.
:param server: a server object
:param ips: list of IP addresses (floating IPs)
:param ips: list of floating IP addresses or a single address
:param wait: (optional) Wait for the address to appear as assigned
to the server in Nova. Defaults to False.
:param timeout: (optional) Seconds to wait, defaults to 60.
See the ``wait`` parameter.
:returns: None
:returns: The updated server dict
:raises: ``OpenStackCloudException``, on operation error.
"""
# ToDo(dguerri): this makes no sense as we cannot attach multiple
# floating IPs to a single fixed_address (this is true for both
# neutron and nova). I will leave this here for the moment as we are
# refactoring floating IPs methods.
for ip in ips:
f_ip = self.get_floating_ip(
id=None, filters={'floating_ip_address': ip})
self._attach_ip_to_server(
server=server, floating_ip=f_ip)
if type(ips) == list:
ip = ips[0]
else:
ip = ips
f_ip = self.get_floating_ip(
id=None, filters={'floating_ip_address': ip})
return self._attach_ip_to_server(
server=server, floating_ip=f_ip, wait=wait, timeout=timeout)
def add_auto_ip(self, server, wait=False, timeout=60, reuse=True):
"""Add a floating IP to a server.
@@ -3146,6 +3154,11 @@ class OpenStackCloud(object):
:returns: Floating IP address attached to server.
"""
server = self._add_auto_ip(
server, wait=wait, timeout=timeout, reuse=reuse)
return self.get_server_public_ip(server)
def _add_auto_ip(self, server, wait=False, timeout=60, reuse=True):
skip_attach = False
if reuse:
f_ip = self.available_floating_ip()
@@ -3156,35 +3169,22 @@ class OpenStackCloud(object):
# but is only meaninful for the neutron logic branch
skip_attach = True
self._attach_ip_to_server(
return self._attach_ip_to_server(
server=server, floating_ip=f_ip, wait=wait, timeout=timeout,
skip_attach=skip_attach)
return f_ip
def add_ips_to_server(
self, server, auto_ip=True, ips=None, ip_pool=None,
wait=False, timeout=60, reuse=True):
if ip_pool:
self._add_ip_from_pool(server, ip_pool, reuse=reuse)
server = self._add_ip_from_pool(
server, ip_pool, reuse=reuse, wait=wait, timeout=timeout)
elif ips:
self.add_ip_list(server, ips)
server = self.add_ip_list(server, ips, wait=wait, timeout=timeout)
elif auto_ip:
if self.get_server_public_ip(server):
return server
self.add_auto_ip(
server, wait=wait, timeout=timeout, reuse=reuse)
else:
return server
# this may look redundant, but if there is now a
# floating IP, then it needs to be obtained from
# a recent server object if the above code path exec'd
try:
server = self.get_server_by_id(server['id'])
except Exception as e:
raise OpenStackCloudException(
"Error in getting info from instance: %s " % str(e))
if not self.get_server_public_ip(server):
server = self._add_auto_ip(
server, wait=wait, timeout=timeout, reuse=reuse)
return server
@_utils.valid_kwargs(

8
shade/tests/unit/test_floating_ip_common.py
View File

@@ -75,7 +75,8 @@ class TestFloatingIP(base.TestCase):
self.client.add_ips_to_server(server_dict, ip_pool=pool)
mock_add_ip_from_pool.assert_called_with(server_dict, pool, reuse=True)
mock_add_ip_from_pool.assert_called_with(
server_dict, pool, reuse=True, wait=False, timeout=60)
@patch.object(OpenStackCloud, 'nova_client')
@patch.object(OpenStackCloud, 'add_ip_list')
@@ -90,10 +91,11 @@ class TestFloatingIP(base.TestCase):
self.client.add_ips_to_server(server_dict, ips=ips)
mock_add_ip_list.assert_called_with(server_dict, ips)
mock_add_ip_list.assert_called_with(
server_dict, ips, wait=False, timeout=60)
@patch.object(OpenStackCloud, 'nova_client')
@patch.object(OpenStackCloud, 'add_auto_ip')
@patch.object(OpenStackCloud, '_add_auto_ip')
def test_add_ips_to_server_auto_ip(
self, mock_add_auto_ip, mock_nova_client):
server = FakeServer(

10
shade/tests/unit/test_floating_ip_neutron.py
View File

@@ -258,7 +258,7 @@ class TestFloatingIP(base.TestCase):
network_name_or_id='my-network', server=None)
mock_attach_ip_to_server.assert_called_once_with(
server={'id': '1234'}, fixed_address=None,
floating_ip=self.floating_ip)
floating_ip=self.floating_ip, wait=False, timeout=60)
@patch.object(OpenStackCloud, 'keystone_session')
@patch.object(OpenStackCloud, '_neutron_create_floating_ip')
@@ -376,13 +376,11 @@ class TestFloatingIP(base.TestCase):
mock_available_floating_ip.return_value = \
_utils.normalize_neutron_floating_ips([
self.mock_floating_ip_new_rep['floatingip']])[0]
mock_attach_ip_to_server.return_value = None
mock_attach_ip_to_server.return_value = self.fake_server
ip = self.client._add_ip_from_pool(
server = self.client._add_ip_from_pool(
server=self.fake_server,
network='network-name',
fixed_address='192.0.2.129')
self.assertEqual(
self.mock_floating_ip_new_rep['floatingip']['floating_ip_address'],
ip['floating_ip_address'])
self.assertEqual(server, self.fake_server)

4
shade/tests/unit/test_floating_ip_nova.py
View File

@@ -242,9 +242,9 @@ class TestFloatingIP(base.TestCase):
mock_has_service.side_effect = has_service_side_effect
mock_nova_client.floating_ips.list.return_value = self.floating_ips
ip = self.client._add_ip_from_pool(
server = self.client._add_ip_from_pool(
server=self.fake_server,
network='nova',
fixed_address='192.0.2.129')
self.assertEqual('203.0.113.1', ip['floating_ip_address'])
self.assertEqual(server, self.fake_server)