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 <s.pisarski@cablelabs.com>

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