Consolidate the ssh code

The entire mechanism for ssh validation is a mess. The fact that there
is a parameter which is server_or_ip, so you never know if it's an
object or an address, and that every one of 7 layers need to handle
either case, makes doing any enhancements here for failure detection
impossible.

This change implements a get_server_ip method that gets the fixed or
floating IP address before passing it to the remote client.

Change-Id: I76d818bf9e1bd26b1e499464fc9213764afee279
diff --git a/tempest/api/compute/base.py b/tempest/api/compute/base.py
index 6d19ca7..0856983 100644
--- a/tempest/api/compute/base.py
+++ b/tempest/api/compute/base.py
@@ -23,6 +23,7 @@
 from tempest.common.utils import data_utils
 from tempest.common import waiters
 from tempest import config
+from tempest import exceptions
 import tempest.test
 
 CONF = config.CONF
@@ -356,16 +357,19 @@
     def get_server_ip(cls, server):
         """Get the server fixed or floating IP.
 
-        For the floating IP, the address created by the validation resources
-        is returned.
-        For the fixed IP, the server is returned and the current mechanism of
-        address extraction in the remote_client is followed.
+        Based on the configuration we're in, return a correct ip
+        address for validating that a guest is up.
         """
         if CONF.validation.connect_method == 'floating':
-            ip_or_server = cls.validation_resources['floating_ip']['ip']
+            return cls.validation_resources['floating_ip']['ip']
         elif CONF.validation.connect_method == 'fixed':
-            ip_or_server = server
-        return ip_or_server
+            addresses = server['addresses'][CONF.validation.network_for_ssh]
+            for address in addresses:
+                if address['version'] == CONF.validation.ip_version_for_ssh:
+                    return address['addr']
+            raise exceptions.ServerUnreachable()
+        else:
+            raise exceptions.InvalidConfiguration()
 
 
 class BaseV2ComputeAdminTest(BaseV2ComputeTest):
diff --git a/tempest/common/utils/linux/remote_client.py b/tempest/common/utils/linux/remote_client.py
index b76c356..2e233c5 100644
--- a/tempest/common/utils/linux/remote_client.py
+++ b/tempest/common/utils/linux/remote_client.py
@@ -15,7 +15,6 @@
 import time
 
 from oslo_log import log as logging
-import six
 from tempest_lib.common import ssh
 
 from tempest import config
@@ -28,22 +27,10 @@
 
 class RemoteClient(object):
 
-    # NOTE(afazekas): It should always get an address instead of server
-    def __init__(self, server, username, password=None, pkey=None):
+    def __init__(self, ip_address, username, password=None, pkey=None):
         ssh_timeout = CONF.validation.ssh_timeout
-        network = CONF.validation.network_for_ssh
-        ip_version = CONF.validation.ip_version_for_ssh
         connect_timeout = CONF.validation.connect_timeout
-        if isinstance(server, six.string_types):
-            ip_address = server
-        else:
-            addresses = server['addresses'][network]
-            for address in addresses:
-                if address['version'] == ip_version:
-                    ip_address = address['addr']
-                    break
-            else:
-                raise exceptions.ServerUnreachable()
+
         self.ssh_client = ssh.Client(ip_address, username, password,
                                      ssh_timeout, pkey=pkey,
                                      channel_timeout=connect_timeout)
diff --git a/tempest/scenario/manager.py b/tempest/scenario/manager.py
index 6776220..cd36f38 100644
--- a/tempest/scenario/manager.py
+++ b/tempest/scenario/manager.py
@@ -352,25 +352,15 @@
 
         return secgroup
 
-    def get_remote_client(self, server_or_ip, username=None, private_key=None):
+    def get_remote_client(self, ip_address, username=None, private_key=None):
         """Get a SSH client to a remote server
 
-        @param server_or_ip a server object as returned by Tempest compute
-            client or an IP address to connect to
+        @param ip_address the server floating or fixed IP address to use
+                          for ssh validation
         @param username name of the Linux account on the remote server
         @param private_key the SSH private key to use
         @return a RemoteClient object
         """
-        if isinstance(server_or_ip, six.string_types):
-            ip = server_or_ip
-        else:
-            addrs = server_or_ip['addresses'][CONF.validation.network_for_ssh]
-            try:
-                ip = (addr['addr'] for addr in addrs if
-                      netaddr.valid_ipv4(addr['addr'])).next()
-            except StopIteration:
-                raise lib_exc.NotFound("No IPv4 addresses to use for SSH to "
-                                       "remote server.")
 
         if username is None:
             username = CONF.validation.image_ssh_user
@@ -383,14 +373,15 @@
         else:
             password = CONF.validation.image_ssh_password
             private_key = None
-        linux_client = remote_client.RemoteClient(ip, username,
+        linux_client = remote_client.RemoteClient(ip_address, username,
                                                   pkey=private_key,
                                                   password=password)
         try:
             linux_client.validate_authentication()
         except Exception as e:
             message = ('Initializing SSH connection to %(ip)s failed. '
-                       'Error: %(error)s' % {'ip': ip, 'error': e})
+                       'Error: %(error)s' % {'ip_address': ip_address,
+                                             'error': e})
             caller = misc_utils.find_test_caller()
             if caller:
                 message = '(%s) %s' % (caller, message)
@@ -628,9 +619,9 @@
             floating_ip['ip'], thing['id'])
         return floating_ip
 
-    def create_timestamp(self, server_or_ip, dev_name=None, mount_path='/mnt',
+    def create_timestamp(self, ip_address, dev_name=None, mount_path='/mnt',
                          private_key=None):
-        ssh_client = self.get_remote_client(server_or_ip,
+        ssh_client = self.get_remote_client(ip_address,
                                             private_key=private_key)
         if dev_name is not None:
             ssh_client.make_fs(dev_name)
@@ -643,9 +634,9 @@
             ssh_client.umount(mount_path)
         return timestamp
 
-    def get_timestamp(self, server_or_ip, dev_name=None, mount_path='/mnt',
+    def get_timestamp(self, ip_address, dev_name=None, mount_path='/mnt',
                       private_key=None):
-        ssh_client = self.get_remote_client(server_or_ip,
+        ssh_client = self.get_remote_client(ip_address,
                                             private_key=private_key)
         if dev_name is not None:
             ssh_client.mount(dev_name, mount_path)
@@ -655,12 +646,25 @@
             ssh_client.umount(mount_path)
         return timestamp
 
-    def get_server_or_ip(self, server):
+    def get_server_ip(self, server):
+        """Get the server fixed or floating IP.
+
+        Based on the configuration we're in, return a correct ip
+        address for validating that a guest is up.
+        """
         if CONF.validation.connect_method == 'floating':
-            ip = self.create_floating_ip(server)['ip']
+            # The tests calling this method don't have a floating IP
+            # and can't make use of the validattion resources. So the
+            # method is creating the floating IP there.
+            return self.create_floating_ip(server)['ip']
+        elif CONF.validation.connect_method == 'fixed':
+            addresses = server['addresses'][CONF.validation.network_for_ssh]
+            for address in addresses:
+                if address['version'] == CONF.validation.ip_version_for_ssh:
+                    return address['addr']
+            raise exceptions.ServerUnreachable()
         else:
-            ip = server
-        return ip
+            raise exceptions.InvalidConfiguration()
 
 
 class NetworkScenarioTest(ScenarioTest):
@@ -1316,13 +1320,6 @@
     def add_keypair(self):
         self.keypair = self.create_keypair()
 
-    def verify_connectivity(self, ip=None):
-        if ip:
-            dest = self.get_remote_client(ip)
-        else:
-            dest = self.get_remote_client(self.instance)
-        dest.validate_authentication()
-
     def boot_instance(self):
         self.instance = self.create_server(
             key_name=self.keypair['name'])
diff --git a/tempest/scenario/test_baremetal_basic_ops.py b/tempest/scenario/test_baremetal_basic_ops.py
index 93b32f7..15d9b66 100644
--- a/tempest/scenario/test_baremetal_basic_ops.py
+++ b/tempest/scenario/test_baremetal_basic_ops.py
@@ -112,12 +112,9 @@
         self.add_keypair()
         self.boot_instance()
         self.validate_ports()
-        self.verify_connectivity()
-        if CONF.validation.connect_method == 'floating':
-            floating_ip = self.create_floating_ip(self.instance)['ip']
-            self.verify_connectivity(ip=floating_ip)
-
-        vm_client = self.get_remote_client(self.instance)
+        ip_address = self.get_server_ip(self.instance)
+        self.get_remote_client(ip_address).validate_authentication()
+        vm_client = self.get_remote_client(ip_address)
 
         # We expect the ephemeral partition to be mounted on /mnt and to have
         # the same size as our flavor definition.
@@ -126,6 +123,6 @@
             self.verify_partition(vm_client, 'ephemeral0', '/mnt', eph_size)
             # Create the test file
             self.create_timestamp(
-                floating_ip, private_key=self.keypair['private_key'])
+                ip_address, private_key=self.keypair['private_key'])
 
         self.terminate_instance()
diff --git a/tempest/scenario/test_network_v6.py b/tempest/scenario/test_network_v6.py
index cc28873..effc42f 100644
--- a/tempest/scenario/test_network_v6.py
+++ b/tempest/scenario/test_network_v6.py
@@ -124,7 +124,7 @@
         fip = self.create_floating_ip(thing=srv)
         ips = self.define_server_ips(srv=srv)
         ssh = self.get_remote_client(
-            server_or_ip=fip.floating_ip_address,
+            ip_address=fip.floating_ip_address,
             username=username)
         return ssh, ips, srv["id"]
 
diff --git a/tempest/scenario/test_server_basic_ops.py b/tempest/scenario/test_server_basic_ops.py
index 6c24d04..dcb095b 100644
--- a/tempest/scenario/test_server_basic_ops.py
+++ b/tempest/scenario/test_server_basic_ops.py
@@ -75,7 +75,7 @@
             self.fip = self.create_floating_ip(self.instance)['ip']
             # Check ssh
             self.ssh_client = self.get_remote_client(
-                server_or_ip=self.fip,
+                ip_address=self.fip,
                 username=self.image_utils.ssh_user(self.image_ref),
                 private_key=keypair['private_key'])
 
diff --git a/tempest/scenario/test_shelve_instance.py b/tempest/scenario/test_shelve_instance.py
index 378ae9d..a91f9c5 100644
--- a/tempest/scenario/test_shelve_instance.py
+++ b/tempest/scenario/test_shelve_instance.py
@@ -80,7 +80,7 @@
                 security_groups=security_groups,
                 wait_until='ACTIVE')
 
-        instance_ip = self.get_server_or_ip(server)
+        instance_ip = self.get_server_ip(server)
         timestamp = self.create_timestamp(instance_ip,
                                           private_key=keypair['private_key'])
 
diff --git a/tempest/scenario/test_snapshot_pattern.py b/tempest/scenario/test_snapshot_pattern.py
index f3b6558..f4cc4e3 100644
--- a/tempest/scenario/test_snapshot_pattern.py
+++ b/tempest/scenario/test_snapshot_pattern.py
@@ -52,7 +52,7 @@
             security_groups=[{'name': security_group['name']}],
             wait_until='ACTIVE')
 
-        instance_ip = self.get_server_or_ip(server)
+        instance_ip = self.get_server_ip(server)
         timestamp = self.create_timestamp(instance_ip,
                                           private_key=keypair['private_key'])
 
@@ -67,7 +67,7 @@
             wait_until='ACTIVE')
 
         # check the existence of the timestamp file in the second instance
-        server_from_snapshot_ip = self.get_server_or_ip(server_from_snapshot)
+        server_from_snapshot_ip = self.get_server_ip(server_from_snapshot)
         timestamp2 = self.get_timestamp(server_from_snapshot_ip,
                                         private_key=keypair['private_key'])
         self.assertEqual(timestamp, timestamp2)
diff --git a/tempest/scenario/test_stamp_pattern.py b/tempest/scenario/test_stamp_pattern.py
index f88fb14..799e049 100644
--- a/tempest/scenario/test_stamp_pattern.py
+++ b/tempest/scenario/test_stamp_pattern.py
@@ -80,9 +80,9 @@
         self.assertEqual(snapshot_name, snapshot['display_name'])
         return snapshot
 
-    def _wait_for_volume_available_on_the_system(self, server_or_ip,
+    def _wait_for_volume_available_on_the_system(self, ip_address,
                                                  private_key):
-        ssh = self.get_remote_client(server_or_ip, private_key=private_key)
+        ssh = self.get_remote_client(ip_address, private_key=private_key)
 
         def _func():
             part = ssh.get_partitions()
@@ -113,7 +113,7 @@
             wait_until='ACTIVE')
 
         # create and add floating IP to server1
-        ip_for_server = self.get_server_or_ip(server)
+        ip_for_server = self.get_server_ip(server)
 
         self.nova_volume_attach(server, volume)
         self._wait_for_volume_available_on_the_system(ip_for_server,
@@ -140,7 +140,7 @@
             security_groups=security_group)
 
         # create and add floating IP to server_from_snapshot
-        ip_for_snapshot = self.get_server_or_ip(server_from_snapshot)
+        ip_for_snapshot = self.get_server_ip(server_from_snapshot)
 
         # attach volume2 to instance2
         self.nova_volume_attach(server_from_snapshot, volume_from_snapshot)
diff --git a/tempest/scenario/test_volume_boot_pattern.py b/tempest/scenario/test_volume_boot_pattern.py
index 7b88025..39351f8 100644
--- a/tempest/scenario/test_volume_boot_pattern.py
+++ b/tempest/scenario/test_volume_boot_pattern.py
@@ -114,7 +114,7 @@
                                                        keypair, security_group)
 
         # write content to volume on instance
-        ip_instance_1st = self.get_server_or_ip(instance_1st)
+        ip_instance_1st = self.get_server_ip(instance_1st)
         timestamp = self.create_timestamp(ip_instance_1st,
                                           private_key=keypair['private_key'])
 
@@ -126,7 +126,7 @@
                                                        keypair, security_group)
 
         # check the content of written file
-        ip_instance_2nd = self.get_server_or_ip(instance_2nd)
+        ip_instance_2nd = self.get_server_ip(instance_2nd)
         timestamp2 = self.get_timestamp(ip_instance_2nd,
                                         private_key=keypair['private_key'])
         self.assertEqual(timestamp, timestamp2)
@@ -141,7 +141,7 @@
                                             keypair, security_group))
 
         # check the content of written file
-        server_from_snapshot_ip = self.get_server_or_ip(server_from_snapshot)
+        server_from_snapshot_ip = self.get_server_ip(server_from_snapshot)
         timestamp3 = self.get_timestamp(server_from_snapshot_ip,
                                         private_key=keypair['private_key'])
         self.assertEqual(timestamp, timestamp3)