Use docker networks and aliases instead links in tests

.travis.yml

@@ -18,6 +18,11 @@ env:
     - LONG_PRODUCT_TEST_GROUP_PRESTO_ADMIN_AND_PRESTO="tests/product/test_collect.py tests/product/test_server_uninstall.py"
     - LONG_PRODUCT_TEST_GROUP_PRESTO_ADMIN_AND_PRESTO="tests/product/test_catalog.py"
     - LONG_PRODUCT_TEST_GROUP_PRESTO_ADMIN_AND_PRESTO="tests/product/test_control.py"
+before_install:
+  - curl -fsSL https://download.docker.com/linux/ubuntu/gpg | sudo apt-key add -
+  - sudo add-apt-repository "deb [arch=amd64] https://download.docker.com/linux/ubuntu $(lsb_release -cs) stable"
+  - sudo apt-get update
+  - sudo apt-get -y install docker-ce
 install:
   - pip install --upgrade pip==9.0.1
   - pip install -r requirements.txt

requirements.txt

@@ -9,7 +9,7 @@ virtualenv==12.0.7
 wheel==0.23.0
 fabric==1.10.1
 requests==2.7.0
-docker-py==1.5.0
+docker==2.5.1
 certifi==2015.4.28
 nose==1.3.7
 nose-timer==0.6

tests/bare_image_provider.py

@@ -21,7 +21,7 @@
 
 import abc
 
-from docker import Client
+from docker import DockerClient
 
 
 class BareImageProvider(object):
@@ -64,8 +64,8 @@ def __init__(
         super(TagBareImageProvider, self).__init__(tag_decoration)
         self.base_master_name = base_master_name + ":" + base_tag
         self.base_slave_name = base_slave_name + ":" + base_tag
-        self.client = Client()
+        self.client = DockerClient()
 
     def create_bare_images(self, cluster, master_name, slave_name):
-        self.client.tag(self.base_master_name, master_name)
-        self.client.tag(self.base_slave_name, slave_name)
+        self.client.api.tag(self.base_master_name, master_name)
+        self.client.api.tag(self.base_slave_name, slave_name)

tests/docker_cluster.py

@@ -25,11 +25,12 @@
 import sys
 import uuid
 
-from docker import Client
+from docker import DockerClient
 from docker.errors import APIError
 from docker.utils.utils import kwargs_from_env
-from prestoadmin import main_dir
 from retrying import retry
+
+from prestoadmin import main_dir
 from tests.base_cluster import BaseCluster
 from tests.product.constants import \
     DEFAULT_DOCKER_MOUNT_POINT, DEFAULT_LOCAL_MOUNT_POINT
@@ -78,8 +79,9 @@ def __init__(self, master_host, slave_hosts,
         if 'tls' in kwargs:
             kwargs['tls'].assert_hostname = False
         kwargs['timeout'] = 300
-        self.client = Client(**kwargs)
+        self.client = DockerClient(**kwargs)
         self._user = 'root'
+        self._network_name = 'presto-admin-test-' + str(uuid.uuid4())
 
         DockerCluster.__check_if_docker_exists()
 
@@ -118,21 +120,18 @@ def __check_if_docker_exists():
             sys.exit('Docker is not installed. Try installing it with '
                      'presto-admin/bin/install-docker.sh.')
 
-    def start_containers(self, master_image, slave_image=None,
-                         cmd=None, **kwargs):
+    def start_containers(self, master_image, slave_image=None, cmd=None, **kwargs):
         self._create_host_mount_dirs()
+        self._create_network()
 
-        self._create_and_start_containers(master_image, slave_image,
-                                          cmd, **kwargs)
+        self._create_and_start_containers(master_image, slave_image, cmd, **kwargs)
         self._ensure_docker_containers_started(master_image)
 
     def tear_down(self):
         for container_name in self.all_hosts():
             self._tear_down_container(container_name)
         self._remove_host_mount_dirs()
-        if self.client:
-            self.client.close()
-            self.client = None
+        self._remove_network()
 
     def _tear_down_container(self, container_name):
         try:
@@ -144,18 +143,21 @@ def _tear_down_container(self, container_name):
 
         try:
             self.stop_host(container_name)
-            self.client.remove_container(container_name, v=True, force=True)
+            container = self.client.containers.get(container_name)
+            container.remove(v=True, force=True)
         except APIError as e:
             # container does not exist
             if e.response.status_code != 404:
                 raise
 
     def stop_host(self, container_name):
-        self.client.stop(container_name)
-        self.client.wait(container_name)
+        container = self.client.containers.get(container_name)
+        container.stop()
+        container.wait()
 
     def start_host(self, container_name):
-        self.client.start(container_name)
+        container = self.client.containers.get(container_name)
+        container.start()
 
     def get_down_hostname(self, host_name):
         return host_name
@@ -180,66 +182,48 @@ def _create_host_mount_dirs(self):
                 if e.errno != errno.EEXIST:
                     raise
 
-    @staticmethod
-    def _execute_and_wait(func, *args, **kwargs):
-        ret = func(*args, **kwargs)
-        # go through all lines in returned stream to ensure func finishes
-        output = ''
-        for line in ret:
-            output += line
-        return output
+    def _create_network(self):
+        self.client.networks.create(self._network_name)
 
-    def _create_and_start_containers(self, master_image, slave_image=None,
-                                     cmd=None, **kwargs):
+    def _get_network(self):
+        return self.client.networks.get(self._network_name)
+
+    def _remove_network(self):
+        self._get_network().remove()
+
+    def _create_and_start_containers(self, master_image, slave_image=None, cmd=None, **kwargs):
         if slave_image:
             for container_name in self.slaves:
-                container_mount_dir = \
-                    self.get_local_mount_dir(container_name)
-                self._create_container(
-                    slave_image, container_name,
-                    container_name.split('-')[0], cmd
-                )
-                self.client.start(container_name,
-                                  binds={container_mount_dir:
-                                         {'bind': self.mount_dir,
-                                          'ro': False}},
-                                  **kwargs)
-
-        master_mount_dir = self.get_local_mount_dir(self.master)
+                self._create_container(slave_image, container_name, container_name.split('-')[0], cmd, **kwargs)
+                container = self.client.containers.get(container_name)
+                container.start()
+
         self._create_container(
-            master_image, self.master, hostname=self.internal_master,
-            cmd=cmd
-        )
-        self.client.start(self.master,
-                          binds={master_mount_dir:
-                                 {'bind': self.mount_dir,
-                                  'ro': False}},
-                          links=zip(self.slaves, self.slaves), **kwargs)
-        self._add_hostnames_to_slaves()
-
-    def _create_container(self, image, container_name, hostname=None,
-                          cmd=None):
-        self._execute_and_wait(self.client.create_container,
+            master_image,
+            self.master,
+            hostname=self.internal_master,
+            cmd=cmd,
+            **kwargs)
+
+        container = self.client.containers.get(self.master)
+        container.start()
+
+    def _create_container(self, image, container_name, hostname, cmd, **kwargs):
+        master_mount_dir = self.get_local_mount_dir(container_name)
+        self.client.containers.create(
                                image,
                                detach=True,
                                name=container_name,
                                hostname=hostname,
-                               volumes=self.local_mount_dir,
+                               volumes={master_mount_dir: {'bind': self.mount_dir, 'mode': 'rw'}},
                                command=cmd,
-                               host_config={'mem_limit': '2g'})
-
-    def _add_hostnames_to_slaves(self):
-        ips = self.get_ip_address_dict()
-        additions_to_etc_hosts = ''
-        for host in self.all_internal_hosts():
-            additions_to_etc_hosts += '%s\t%s\n' % (ips[host], host)
-
-        for host in self.slaves:
-            self.exec_cmd_on_host(
-                host,
-                'bin/bash -c \'echo "%s" >> /etc/hosts\''
-                % additions_to_etc_hosts
-            )
+                               mem_limit='2g',
+                               network=None,
+                               **kwargs)
+
+        self._get_network().connect(
+            container_name,
+            aliases=[hostname.split('-')[0]])
 
     @retry(stop_max_delay=_DOCKER_START_TIMEOUT, wait_fixed=_DOCKER_START_WAIT)
     def _ensure_docker_containers_started(self, image):
@@ -252,9 +236,7 @@ def _ensure_docker_containers_started(self, image):
         for host in host_started.keys():
             if host_started[host]:
                 continue
-            is_started = True
-            is_started &= \
-                self.client.inspect_container(host)['State']['Running']
+            is_started = self.client.containers.get(host).status == 'running'
             if is_started and image_no_tag not in NO_WAIT_SSH_IMAGES:
                 is_started &= self._are_centos_container_services_up(host)
             host_started[host] = is_started
@@ -307,10 +289,13 @@ def _are_centos_container_services_up(self, host):
 
     def exec_cmd_on_host(self, host, cmd, user=None, raise_error=True,
                          tty=False, invoke_sudo=False):
-        ex = self.client.exec_create(self.__get_unique_host(host), ['sh', '-c', cmd],
-                                     tty=tty, user=user)
-        output = self.client.exec_start(ex['Id'], tty=tty)
-        exit_code = self.client.exec_inspect(ex['Id'])['ExitCode']
+        ex = self.client.api.exec_create(
+            self.__get_unique_host(host),
+            ['sh', '-c', cmd],
+            tty=tty,
+            user=user)
+        output = self.client.api.exec_start(ex['Id'], tty=tty)
+        exit_code = self.client.api.exec_inspect(ex['Id'])['ExitCode']
         if raise_error and exit_code:
             raise OSError(exit_code, output)
         return output
@@ -369,25 +354,23 @@ def start_cluster(bare_image_provider, cluster_type, master_host='master',
     def _check_for_images(master_image_name, slave_image_name, tag='latest'):
         master_repotag = '%s:%s' % (master_image_name, tag)
         slave_repotag = '%s:%s' % (slave_image_name, tag)
-        with Client(timeout=180) as client:
-            images = client.images()
+        client = DockerClient(timeout=180)
+        images = client.images.list()
         has_master_image = False
         has_slave_image = False
         for image in images:
-            if image['RepoTags'] is not None and master_repotag in image['RepoTags']:
+            if master_repotag in image.tags:
                 has_master_image = True
-            if image['RepoTags'] is not None and slave_repotag in image['RepoTags']:
+            if slave_repotag in image.tags:
                 has_slave_image = True
         return has_master_image and has_slave_image
 
     def commit_images(self, bare_image_provider, cluster_type):
-        self.client.commit(self.master,
-                           self._get_master_image_name(bare_image_provider,
-                                                       cluster_type))
+        container = self.client.containers.get(self.master)
+        container.commit(self._get_master_image_name(bare_image_provider, cluster_type))
         if self.slaves:
-            self.client.commit(self.slaves[0],
-                               self._get_slave_image_name(bare_image_provider,
-                                                          cluster_type))
+            container = self.client.containers.get(self.slaves[0])
+            container.commit(self._get_slave_image_name(bare_image_provider, cluster_type))
 
     def run_script_on_host(self, script_contents, host, tty=True):
         temp_script = '/tmp/tmp.sh'
@@ -417,7 +400,7 @@ def get_ip_address_dict(self):
         ip_addresses = {}
         for host, internal_host in zip(self.all_hosts(),
                                        self.all_internal_hosts()):
-            inspect = self.client.inspect_container(host)
+            inspect = self.client.api.inspect_container(host)
             ip_addresses[host] = inspect['NetworkSettings']['IPAddress']
             ip_addresses[internal_host] = \
                 inspect['NetworkSettings']['IPAddress']

tests/product/base_product_case.py

@@ -88,28 +88,9 @@ class BaseProductTestCase(BaseTestCase):
     # The two strings below (down_node_connection_string and status_down_node_string) aggregate
     # all possible error messages one might encounter when trying to perform an action when a
     # node is not accessible. The variety in error messages comes from differences in the OS.
-    down_node_connection_string = r'(\nWarning: (\[%(host)s\] )?Low level ' \
-                                  r'socket error connecting to host ' \
-                                  r'%(host)s on port 22: No route to host ' \
-                                  r'\(tried 1 time\)\n\nUnderlying ' \
-                                  r'exception:\n    No route to host\n' \
-                                  r'|\nWarning: (\[%(host)s] )?Timed out ' \
-                                  r'trying to connect to %(host)s \(tried 1 ' \
-                                  r'time\)\n\nUnderlying exception:' \
-                                  r'\n    timed out\n)' \
-                                  r'|\nWarning: (\[%(host)s\] )?Low level ' \
-                                  r'socket error connecting to host ' \
-                                  r'%(host)s on port 22: Network is unreachable ' \
-                                  r'\(tried 1 time\)\n\nUnderlying ' \
-                                  r'exception:\n    Network is unreachable\n'
-
-    status_down_node_string = r'(\tLow level socket error connecting to ' \
-                              r'host %(host)s on port 22: No route to host ' \
-                              r'\(tried 1 time\)|\tTimed out trying to ' \
-                              r'connect to %(host)s \(tried 1 time\))' \
-                              r'|\tLow level socket error connecting to ' \
-                              r'host %(host)s on port 22: Network is unreachable ' \
-                              r'\(tried 1 time\)'
+    down_node_connection_string = r'\nWarning: (\[%(host)s\] )?Name lookup failed for %(host)s'
+
+    status_down_node_string = r'\tName lookup failed for %(host)s'
 
     len_down_node_error = 6