From 84c4e791b6ea4e6d488acf29491ef8c901011aa3 Mon Sep 17 00:00:00 2001 From: spisarski Date: Fri, 28 Jul 2017 10:40:49 -0600 Subject: Removed floating IP list from OpenStackVmInstance. There was a list and dict both holding the same floating IP objects which has been problematic especially when trying to initialize the object with a VM instance that already exists. JIRA: SNAPS-149 Change-Id: If4af6dfef04a40b9c8cd7a8add484c9ec03f1ef8 Signed-off-by: spisarski --- docs/how-to-use/IntegrationTests.rst | 3 ++ snaps/openstack/create_instance.py | 45 ++++++++++++-------------- snaps/openstack/tests/create_instance_tests.py | 45 ++++++++++++++++++++++++++ snaps/openstack/utils/neutron_utils.py | 22 ++++++++++--- 4 files changed, 87 insertions(+), 28 deletions(-) diff --git a/docs/how-to-use/IntegrationTests.rst b/docs/how-to-use/IntegrationTests.rst index 4cdd94f..981560f 100644 --- a/docs/how-to-use/IntegrationTests.rst +++ b/docs/how-to-use/IntegrationTests.rst @@ -296,6 +296,9 @@ create_instance_tests.py - CreateInstanceSingleNetworkTests | test_ssh_client_fip_after_active | Nova 2 | Ensures that an instance can be reached over SSH when the | | | Neutron 2 | floating IP is assigned after to the VM becoming ACTIVE | +---------------------------------------+---------------+-----------------------------------------------------------+ +| test_ssh_client_fip_second_creator | Nova 2 | Ensures that an instance can be reached over SSH via a | +| | Neutron 2 | second identical creator object | ++---------------------------------------+---------------+-----------------------------------------------------------+ create_instance_tests.py - CreateInstancePortManipulationTests -------------------------------------------------------------- diff --git a/snaps/openstack/create_instance.py b/snaps/openstack/create_instance.py index 997b5a5..252f2fe 100644 --- a/snaps/openstack/create_instance.py +++ b/snaps/openstack/create_instance.py @@ -57,9 +57,6 @@ class OpenStackVmInstance: self.image_settings = image_settings self.keypair_settings = keypair_settings - # TODO - get rid of FIP list and only use the dict(). Need to fix - # populating this object when already exists - self.__floating_ips = list() self.__floating_ip_dict = dict() # Instantiated in self.create() @@ -102,13 +99,14 @@ class OpenStackVmInstance: logger.info( 'Found existing machine with name - %s', self.instance_settings.name) - fips = neutron_utils.get_floating_ips(self.__neutron) - for fip in fips: - for subnet_name, ips in server.networks.items(): - if fip.ip in ips: - self.__floating_ips.append(fip) - # TODO - Determine a means to associate to the FIP - # configuration and add to FIP map + + fips = neutron_utils.get_floating_ips(self.__neutron, + self.__ports) + for port_name, fip in fips: + settings = self.instance_settings.floating_ip_settings + for fip_setting in settings: + if port_name == fip_setting.port_name: + self.__floating_ip_dict[fip_setting.name] = fip def __create_vm(self, block=False): """ @@ -170,7 +168,6 @@ class OpenStackVmInstance: self.__neutron, floating_ip_setting.subnet_name) floating_ip = neutron_utils.create_floating_ip( self.__neutron, ext_gateway) - self.__floating_ips.append(floating_ip) self.__floating_ip_dict[floating_ip_setting.name] = floating_ip logger.info( @@ -204,13 +201,12 @@ class OpenStackVmInstance: """ # Cleanup floating IPs - for floating_ip in self.__floating_ips: + for name, floating_ip in self.__floating_ip_dict.items(): try: logger.info('Deleting Floating IP - ' + floating_ip.ip) neutron_utils.delete_floating_ip(self.__neutron, floating_ip) except Exception as e: logger.error('Error deleting Floating IP - ' + str(e)) - self.__floating_ips = list() self.__floating_ip_dict = dict() # Cleanup ports @@ -266,7 +262,7 @@ class OpenStackVmInstance: port = neutron_utils.get_port_by_name(self.__neutron, port_setting.name) if port: - ports.append((port_setting.name, {'port': port})) + ports.append((port_setting.name, port)) elif not cleanup: # Exception will be raised when port with same name already # exists @@ -287,8 +283,8 @@ class OpenStackVmInstance: if subnet: # Take IP of subnet if there is one configured on which to place # the floating IP - for fixed_ip in port.fixed_ips: - if fixed_ip['subnet_id'] == subnet['subnet']['id']: + for fixed_ip in port.ips: + if fixed_ip['subnet_id'] == subnet.id: ip = fixed_ip['ip_address'] break else: @@ -408,7 +404,7 @@ class OpenStackVmInstance: more than one configured port :return: the value returned by ansible_utils.apply_ansible_playbook() """ - if len(self.__ports) > 1 and len(self.__floating_ips) > 0: + if len(self.__ports) > 1 and len(self.__floating_ip_dict) > 0: if self.vm_active(block=True) and self.vm_ssh_active(block=True): for key, port in self.__ports: port_index = self.__ports.index((key, port)) @@ -432,8 +428,9 @@ class OpenStackVmInstance: fip = self.__floating_ip_dict.get(floating_ip_setting.name) if fip: return fip - elif len(self.__floating_ips) > 0: - return self.__floating_ips[0] + elif len(self.__floating_ip_dict) > 0: + for key, fip in self.__floating_ip_dict.items(): + return fip def __config_nic(self, nic_name, port, ip): """ @@ -615,7 +612,7 @@ class OpenStackVmInstance: Returns True when can create a SSH session else False :return: T/F """ - if len(self.__floating_ips) > 0: + if len(self.__floating_ip_dict) > 0: ssh = self.ssh_client() if ssh: ssh.close() @@ -632,9 +629,8 @@ class OpenStackVmInstance: fip = None if fip_name and self.__floating_ip_dict.get(fip_name): return self.__floating_ip_dict.get(fip_name) - if not fip and len(self.__floating_ips) > 0: - return self.__floating_ips[0] - return None + if not fip: + return self.__get_first_provisioning_floating_ip() def ssh_client(self, fip_name=None): """ @@ -646,7 +642,8 @@ class OpenStackVmInstance: fip = self.get_floating_ip(fip_name) if fip: return ansible_utils.ssh_client( - self.__floating_ips[0].ip, self.get_image_user(), + self.__get_first_provisioning_floating_ip().ip, + self.get_image_user(), self.keypair_settings.private_filepath, proxy_settings=self.__os_creds.proxy_settings) else: diff --git a/snaps/openstack/tests/create_instance_tests.py b/snaps/openstack/tests/create_instance_tests.py index 1922146..54b6e53 100644 --- a/snaps/openstack/tests/create_instance_tests.py +++ b/snaps/openstack/tests/create_instance_tests.py @@ -746,6 +746,51 @@ class CreateInstanceSingleNetworkTests(OSIntegrationTestCase): self.assertTrue(validate_ssh_client(inst_creator)) + def test_ssh_client_fip_second_creator(self): + """ + Tests the ability to access a VM via SSH and a floating IP via a + creator that is identical to the original creator. + """ + port_settings = PortSettings( + name=self.port_1_name, + network_name=self.pub_net_config.network_settings.name) + + instance_settings = VmInstanceSettings( + name=self.vm_inst_name, + flavor=self.flavor_creator.flavor_settings.name, + port_settings=[port_settings], + floating_ip_settings=[FloatingIpSettings( + name=self.floating_ip_name, port_name=self.port_1_name, + router_name=self.pub_net_config.router_settings.name)]) + + inst_creator = OpenStackVmInstance( + self.os_creds, instance_settings, + self.image_creator.image_settings, + keypair_settings=self.keypair_creator.keypair_settings) + self.inst_creators.append(inst_creator) + + # block=True will force the create() method to block until the + vm_inst = inst_creator.create(block=True) + self.assertIsNotNone(vm_inst) + + self.assertTrue(inst_creator.vm_active(block=True)) + + ip = inst_creator.get_port_ip(port_settings.name) + self.assertTrue(check_dhcp_lease(inst_creator, ip)) + + inst_creator.add_security_group( + self.sec_grp_creator.get_security_group()) + self.assertEqual(vm_inst, inst_creator.get_vm_inst()) + + self.assertTrue(validate_ssh_client(inst_creator)) + + inst_creator2 = OpenStackVmInstance( + self.os_creds, instance_settings, + self.image_creator.image_settings, + keypair_settings=self.keypair_creator.keypair_settings) + inst_creator2.create() + self.assertTrue(validate_ssh_client(inst_creator2)) + class CreateInstancePortManipulationTests(OSIntegrationTestCase): """ diff --git a/snaps/openstack/utils/neutron_utils.py b/snaps/openstack/utils/neutron_utils.py index 2f8ef7e..19325a8 100644 --- a/snaps/openstack/utils/neutron_utils.py +++ b/snaps/openstack/utils/neutron_utils.py @@ -448,17 +448,31 @@ def get_external_networks(neutron): return out -def get_floating_ips(neutron): +def get_floating_ips(neutron, ports=None): """ Returns all of the floating IPs + When ports is not None, FIPs returned must be associated with one of the + ports in the list and a tuple 2 where the first element being the port's + name and the second being the FloatingIp SNAPS-OO domain object. + When ports is None, all known FloatingIp SNAPS-OO domain objects will be + returned in a list :param neutron: the Neutron client - :return: a list of SNAPS FloatingIp objects + :param ports: a list of SNAPS-OO Port objects to join + :return: a list of tuple 2 (port_name, SNAPS FloatingIp) objects when ports + is not None else a list of Port objects """ out = list() fips = neutron.list_floatingips() for fip in fips['floatingips']: - out.append(FloatingIp(inst_id=fip['id'], - ip=fip['floating_ip_address'])) + if ports: + for port_name, port in ports: + if fip['port_id'] == port.id: + out.append((port.name, FloatingIp( + inst_id=fip['id'], ip=fip['floating_ip_address']))) + break + else: + out.append(FloatingIp(inst_id=fip['id'], + ip=fip['floating_ip_address'])) return out -- cgit 1.2.3-korg