From dd4bd638933477c87f1c5cc2c9dfe08125c35197 Mon Sep 17 00:00:00 2001 From: Monty Taylor Date: Thu, 25 Aug 2016 14:54:50 -0500 Subject: [PATCH] Detect the need for FIPs better in auto_ip The current logic of looking at interface_ip to see if a FIP is needed is too simplistic and mixes two concerns. interface_ip is dependent on the network context of the calling host, which is great - but may not be the full story for all of the people who would want to interact with the server. Instead of creating a FIP if there is no interface_ip - introduce a richer method that looks at public and private v4 addresses as well as the existence of a FIP-able network. Change-Id: I687375a1433d643e2f1cf18d0bbc00d0e8a0abbf --- shade/openstackcloud.py | 55 ++++++++++++++++++++- shade/tests/unit/test_floating_ip_common.py | 5 +- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/shade/openstackcloud.py b/shade/openstackcloud.py index db5420553..7b5e64ef2 100644 --- a/shade/openstackcloud.py +++ b/shade/openstackcloud.py @@ -4101,11 +4101,64 @@ class OpenStackCloud(object): server, ips, wait=wait, timeout=timeout, fixed_address=fixed_address) elif auto_ip: - if not server['interface_ip']: + if self._needs_floating_ip(server, nat_destination): server = self._add_auto_ip( server, wait=wait, timeout=timeout, reuse=reuse) return server + def _needs_floating_ip(self, server, nat_destination): + """Figure out if auto_ip should add a floating ip to this server. + + If the server has a public_v4 it does not need a floating ip. + + If the server does not have a private_v4 it does not need a + floating ip. + + If self.private then the server does not need a floating ip. + + If the cloud runs nova, and the server has a private_v4 and not + a public_v4, then the server needs a floating ip. + + If the server has a private_v4 and no public_v4 and the cloud has + a network from which floating IPs come that is connected via a + router to the network from which the private_v4 address came, + then the server needs a floating ip. + + If the server has a private_v4 and no public_v4 and the cloud + does not have a network from which floating ips come, or it has + one but that network is not connected to the network from which + the server's private_v4 address came via a router, then the + server does not need a floating ip. + """ + if not self._has_floating_ips(): + return False + + if server['public_v4']: + return False + + if not server['private_v4']: + return False + + if self.private: + return False + + if not self.has_service('network'): + return True + + # No external IPv4 network - no FIPs + # TODO(mordred) THIS IS get_external_ipv4_networks IN THE NEXT PATCH + networks = self.get_external_networks() + if not networks: + return False + + (port_obj, fixed_ip_address) = self._get_free_fixed_port( + server, nat_destination=nat_destination) + + if not port_obj or not fixed_ip_address: + return False + + return True + def _get_boot_from_volume_kwargs( self, image, boot_from_volume, boot_volume, volume_size, terminate_volume, volumes, kwargs): diff --git a/shade/tests/unit/test_floating_ip_common.py b/shade/tests/unit/test_floating_ip_common.py index 72ea32de0..ff561c645 100644 --- a/shade/tests/unit/test_floating_ip_common.py +++ b/shade/tests/unit/test_floating_ip_common.py @@ -202,16 +202,19 @@ class TestFloatingIP(base.TestCase): mock_add_ip_list.assert_called_with( server_dict, ips, wait=False, timeout=60, fixed_address=None) + @patch.object(OpenStackCloud, '_needs_floating_ip') @patch.object(OpenStackCloud, 'nova_client') @patch.object(OpenStackCloud, '_add_auto_ip') def test_add_ips_to_server_auto_ip( - self, mock_add_auto_ip, mock_nova_client): + self, mock_add_auto_ip, mock_nova_client, mock_needs_floating_ip): server = FakeServer( id='server-id', name='test-server', status="ACTIVE", addresses={} ) server_dict = meta.obj_to_dict(server) mock_nova_client.servers.get.return_value = server + # TODO(mordred) REMOVE THIS MOCK WHEN THE NEXT PATCH LANDS + mock_needs_floating_ip.return_value = True self.cloud.add_ips_to_server(server_dict)