Expunge the docker uploader

The docker based image uploader can no longer be used because the
docker service has been removed from the undercloud.

This change removes the DockerImageUploader class, relieving us of the
burden of maintaining unused code.

Change-Id: I7be5915a0aee561250c16487c1fee7afd28b9036
Blueprint: podman-support

requirements.txt

@@ -4,7 +4,6 @@
 
 pbr!=2.1.0,>=2.0.0 # Apache-2.0
 Babel!=2.4.0,>=2.3.4 # BSD
-docker>=2.4.2 # Apache-2.0
 GitPython>=1.0.1 # BSD License (3 clause)
 python-heatclient>=1.10.0 # Apache-2.0
 oslo.config>=5.2.0 # Apache-2.0

tripleo_common/image/image_uploader.py

@@ -30,13 +30,6 @@ import tempfile
 import tenacity
 import yaml
 
-# Docker in TripleO is deprecated in Stein
-try:
-    import docker
-    from docker import APIClient as Client
-except ImportError:
-    pass
-
 from oslo_concurrency import processutils
 from oslo_log import log as logging
 from tripleo_common.actions import ansible
@@ -117,7 +110,6 @@ class ImageUploadManager(BaseImageManager):
             config_files = []
         super(ImageUploadManager, self).__init__(config_files)
         self.uploaders = {
-            'docker': DockerImageUploader(),
             'skopeo': SkopeoImageUploader(),
             'python': PythonImageUploader()
         }
@@ -139,6 +131,9 @@ class ImageUploadManager(BaseImageManager):
             raise ImageUploaderException('Unknown image uploader type')
         return self.uploaders[uploader]
 
+    def get_uploader(self, uploader):
+        return self.uploader(uploader)
+
     def get_push_destination(self, item):
         push_destination = item.get('push_destination')
         if not push_destination:
@@ -183,13 +178,6 @@ class ImageUploadManager(BaseImageManager):
 
         return upload_images  # simply to make test validation easier
 
-    def get_uploader(self, uploader):
-        if uploader == 'docker':
-            return DockerImageUploader()
-        if uploader == 'skopeo':
-            return SkopeoImageUploader()
-        raise ImageUploaderException('Unknown image uploader type')
-
 
 class BaseImageUploader(object):
 
@@ -223,7 +211,7 @@ class BaseImageUploader(object):
     @classmethod
     def run_modify_playbook(cls, modify_role, modify_vars,
                             source_image, target_image, append_tag,
-                            container_build_tool='docker'):
+                            container_build_tool='buildah'):
         vars = {}
         if modify_vars:
             vars.update(modify_vars)
@@ -617,129 +605,6 @@ class BaseImageUploader(object):
                 LOG.debug('%s %s' % (r.status_code, r.reason))
 
 
-class DockerImageUploader(BaseImageUploader):
-    """Upload images using docker pull/tag/push"""
-
-    def upload_image(self, task):
-        t = task
-        LOG.info('imagename: %s' % t.image_name)
-
-        if t.dry_run:
-            return []
-
-        if t.modify_role:
-            target_session = self.authenticate(
-                t.target_image_url)
-            if self._image_exists(t.target_image,
-                                  session=target_session):
-                LOG.warning('Skipping upload for modified image %s' %
-                            t.target_image)
-                return []
-        else:
-            source_session = self.authenticate(
-                t.source_image_url)
-            if self._images_match(t.source_image, t.target_image,
-                                  session1=source_session):
-                LOG.warning('Skipping upload for image %s' % t.image_name)
-                return []
-
-        dockerc = Client(base_url='unix://var/run/docker.sock', version='auto')
-        self._pull(dockerc, t.repo, tag=t.source_tag)
-
-        if t.modify_role:
-            self.run_modify_playbook(
-                t.modify_role, t.modify_vars, t.source_image,
-                t.target_image_source_tag, t.append_tag)
-            # raise an exception if the playbook didn't tag
-            # the expected target image
-            dockerc.inspect_image(t.target_image)
-        else:
-            response = dockerc.tag(
-                image=t.source_image, repository=t.target_image_no_tag,
-                tag=t.target_tag, force=True
-            )
-            LOG.debug(response)
-
-        self._push(dockerc, t.target_image_no_tag, tag=t.target_tag)
-
-        LOG.warning('Completed upload for image %s' % t.image_name)
-        if t.cleanup == CLEANUP_NONE:
-            return []
-        if t.cleanup == CLEANUP_PARTIAL:
-            return [t.source_image]
-        return [t.source_image, t.target_image]
-
-    @classmethod
-    @tenacity.retry(  # Retry up to 5 times with jittered exponential backoff
-        reraise=True,
-        wait=tenacity.wait_random_exponential(multiplier=1, max=10),
-        stop=tenacity.stop_after_attempt(5)
-    )
-    def _pull(cls, dockerc, image, tag=None):
-        LOG.info('Pulling %s' % image)
-
-        for line in dockerc.pull(image, tag=tag, stream=True):
-            status = json.loads(line)
-            if 'error' in status:
-                LOG.warning('docker pull failed: %s' % status['error'])
-                raise ImageUploaderException('Could not pull image %s' % image)
-
-    @classmethod
-    @tenacity.retry(  # Retry up to 5 times with jittered exponential backoff
-        reraise=True,
-        wait=tenacity.wait_random_exponential(multiplier=1, max=10),
-        stop=tenacity.stop_after_attempt(5)
-    )
-    def _push(cls, dockerc, image, tag=None):
-        LOG.info('Pushing %s' % image)
-
-        for line in dockerc.push(image, tag=tag, stream=True):
-            status = json.loads(line)
-            if 'error' in status:
-                LOG.warning('docker push failed: %s' % status['error'])
-                raise ImageUploaderException('Could not push image %s' % image)
-
-    def cleanup(self, local_images):
-        if not local_images:
-            return []
-
-        dockerc = Client(base_url='unix://var/run/docker.sock', version='auto')
-        for image in sorted(local_images):
-            if not image:
-                continue
-            LOG.warning('Removing local copy of %s' % image)
-            try:
-                dockerc.remove_image(image)
-            except docker.errors.APIError as e:
-                if e.explanation:
-                    LOG.error(e.explanation)
-                else:
-                    LOG.error(e)
-
-    def run_tasks(self):
-        if not self.upload_tasks:
-            return
-        local_images = []
-
-        # Pull a single image first, to avoid duplicate pulls of the
-        # same base layers
-        uploader, first_task = self.upload_tasks.pop()
-        result = uploader.upload_image(first_task)
-        local_images.extend(result)
-
-        # workers will be based on CPU count with a min 4, max 12
-        workers = min(12, max(4, processutils.get_worker_count()))
-        p = futures.ThreadPoolExecutor(max_workers=workers)
-
-        for result in p.map(upload_task, self.upload_tasks):
-            local_images.extend(result)
-        LOG.info('result %s' % local_images)
-
-        # Do cleanup after all the uploads so common layers don't get deleted
-        # repeatedly
-        self.cleanup(local_images)
-
-
 class SkopeoImageUploader(BaseImageUploader):
     """Upload images using skopeo copy"""
 

tripleo_common/image/kolla_builder.py

@@ -274,7 +274,7 @@ def container_images_prepare(template_file=DEFAULT_TEMPLATE_FILE,
         filter=ffunc, **mapping_args)
 
     manager = image_uploader.ImageUploadManager(mirrors=mirrors)
-    uploader = manager.uploader('docker')
+    uploader = manager.uploader('python')
     images = [i.get('imagename', '') for i in result]
 
     if tag_from_label:
@@ -342,7 +342,7 @@ def detect_insecure_registries(params):
               merged into other parameters
     """
     insecure = set()
-    uploader = image_uploader.ImageUploadManager().uploader('docker')
+    uploader = image_uploader.ImageUploadManager().uploader('python')
     for image in params.values():
         host = image.split('/')[0]
         if uploader.is_insecure_registry(host):

tripleo_common/tests/image/test_image_uploader.py

@@ -24,7 +24,6 @@ from requests_mock.contrib import fixture as rm_fixture
 import six
 from six.moves.urllib.parse import urlparse
 import tempfile
-import urllib3
 import zlib
 
 from oslo_concurrency import processutils
@@ -139,10 +138,10 @@ class TestImageUploadManager(base.TestCase):
             manager.get_push_destination({'push_destination': None})
         )
 
-    def test_get_uploader_docker(self):
+    def test_get_uploader_python(self):
         manager = image_uploader.ImageUploadManager(self.filelist)
-        uploader = manager.get_uploader('docker')
+        uploader = manager.get_uploader('python')
-        assert isinstance(uploader, image_uploader.DockerImageUploader)
+        assert isinstance(uploader, image_uploader.PythonImageUploader)
 
     def test_get_uploader_skopeo(self):
         manager = image_uploader.ImageUploadManager(self.filelist)
@@ -633,335 +632,6 @@ class TestBaseImageUploader(base.TestCase):
         )
 
 
-class TestDockerImageUploader(base.TestCase):
-
-    def setUp(self):
-        super(TestDockerImageUploader, self).setUp()
-        self.uploader = image_uploader.DockerImageUploader()
-        self.uploader._push.retry.sleep = mock.Mock()
-        self.uploader._pull.retry.sleep = mock.Mock()
-        self.uploader._inspect.retry.sleep = mock.Mock()
-
-        self.patcher = mock.patch('tripleo_common.image.image_uploader.Client')
-        self.dockermock = self.patcher.start()
-
-    def tearDown(self):
-        super(TestDockerImageUploader, self).tearDown()
-        self.patcher.stop()
-
-    @mock.patch('tripleo_common.image.image_uploader.'
-                'BaseImageUploader.authenticate')
-    @mock.patch('tripleo_common.image.image_uploader.'
-                'BaseImageUploader._inspect')
-    def test_upload_image(self, mock_inspect, mock_auth):
-        result1 = {
-            'Digest': 'a'
-        }
-        result2 = {
-            'Digest': 'b'
-        }
-        mock_inspect.side_effect = [result1, result2]
-
-        image = 'docker.io/tripleomaster/heat-docker-agents-centos'
-        tag = 'latest'
-        push_destination = 'localhost:8787'
-        push_image = 'localhost:8787/tripleomaster/heat-docker-agents-centos'
-
-        self.assertEqual(
-            ['docker.io/tripleomaster/heat-docker-agents-centos:latest',
-             'localhost:8787/tripleomaster/heat-docker-agents-centos:latest'],
-            self.uploader.upload_image(image_uploader.UploadTask(
-                image + ':' + tag,
-                None,
-                push_destination,
-                None,
-                None,
-                None,
-                False,
-                'full')
-            )
-        )
-
-        self.dockermock.assert_called_once_with(
-            base_url='unix://var/run/docker.sock', version='auto')
-
-        self.dockermock.return_value.pull.assert_called_once_with(
-            image, tag=tag, stream=True)
-        self.dockermock.return_value.tag.assert_called_once_with(
-            image=image + ':' + tag,
-            repository=push_image,
-            tag=tag, force=True)
-        self.dockermock.return_value.push.assert_called_once_with(
-            push_image,
-            tag=tag, stream=True)
-
-    @mock.patch('tripleo_common.image.image_uploader.'
-                'BaseImageUploader.authenticate')
-    @mock.patch('tripleo_common.image.image_uploader.'
-                'BaseImageUploader._inspect')
-    def test_upload_image_existing(self, mock_inspect, mock_auth):
-        mock_inspect.return_value = {
-            'Digest': 'a'
-        }
-        image = 'docker.io/tripleomaster/heat-docker-agents-centos'
-        tag = 'latest'
-        push_destination = 'localhost:8787'
-
-        self.assertEqual(
-            [],
-            self.uploader.upload_image(image_uploader.UploadTask(
-                image + ':' + tag,
-                None,
-                push_destination,
-                None,
-                None,
-                None,
-                False,
-                'full')
-            )
-        )
-
-        # both digests are the same, no pull/push
-        self.dockermock.assert_not_called()
-        self.dockermock.return_value.pull.assert_not_called()
-        self.dockermock.return_value.tag.assert_not_called()
-        self.dockermock.return_value.push.assert_not_called()
-
-    @mock.patch('tripleo_common.image.image_uploader.'
-                'BaseImageUploader.authenticate')
-    @mock.patch('tripleo_common.image.image_uploader.'
-                'BaseImageUploader._inspect')
-    @mock.patch('tripleo_common.actions.'
-                'ansible.AnsiblePlaybookAction', autospec=True)
-    def test_modify_upload_image(self, mock_ansible, mock_inspect, mock_auth):
-        mock_inspect.side_effect = ImageNotFoundException()
-        mock_ansible.return_value.run.return_value = {}
-
-        image = 'docker.io/tripleomaster/heat-docker-agents-centos'
-        tag = 'latest'
-        append_tag = 'modify-123'
-        push_destination = 'localhost:8787'
-        push_image = 'localhost:8787/tripleomaster/heat-docker-agents-centos'
-        playbook = [{
-            'tasks': [{
-                'import_role': {
-                    'name': 'add-foo-plugin'
-                },
-                'name': 'Import role add-foo-plugin',
-                'vars': {
-                    'target_image': '%s:%s' % (push_image, tag),
-                    'modified_append_tag': append_tag,
-                    'source_image': '%s:%s' % (image, tag),
-                    'foo_version': '1.0.1',
-                    'container_build_tool': 'docker'
-                }
-            }],
-            'hosts': 'localhost'
-        }]
-
-        # test response for a partial cleanup
-        self.assertEqual(
-            ['docker.io/tripleomaster/heat-docker-agents-centos:latest'],
-            self.uploader.upload_image(image_uploader.UploadTask(
-                image + ':' + tag,
-                None,
-                push_destination,
-                append_tag,
-                'add-foo-plugin',
-                {'foo_version': '1.0.1'},
-                False,
-                'partial')
-            )
-        )
-
-        self.dockermock.assert_called_once_with(
-            base_url='unix://var/run/docker.sock', version='auto')
-
-        self.dockermock.return_value.pull.assert_called_once_with(
-            image, tag=tag, stream=True)
-        mock_ansible.assert_called_once_with(
-            playbook=playbook,
-            work_dir=mock.ANY,
-            verbosity=1,
-            extra_env_variables=mock.ANY,
-            override_ansible_cfg=(
-                "[defaults]\n"
-                "stdout_callback=yaml\n"
-            )
-        )
-        self.dockermock.return_value.tag.assert_not_called()
-        self.dockermock.return_value.push.assert_called_once_with(
-            push_image,
-            tag=tag + append_tag,
-            stream=True)
-
-    @mock.patch('tripleo_common.image.image_uploader.'
-                'BaseImageUploader.authenticate')
-    @mock.patch('tripleo_common.image.image_uploader.'
-                'BaseImageUploader._inspect')
-    @mock.patch('tripleo_common.actions.'
-                'ansible.AnsiblePlaybookAction', autospec=True)
-    def test_modify_image_failed(self, mock_ansible, mock_inspect, mock_auth):
-        mock_inspect.side_effect = ImageNotFoundException()
-
-        image = 'docker.io/tripleomaster/heat-docker-agents-centos'
-        tag = 'latest'
-        append_tag = 'modify-123'
-        push_destination = 'localhost:8787'
-        error = processutils.ProcessExecutionError(
-            '', 'ouch', -1, 'ansible-playbook')
-        mock_ansible.return_value.run.side_effect = error
-
-        self.assertRaises(
-            ImageUploaderException,
-            self.uploader.upload_image, image_uploader.UploadTask(
-                image + ':' + tag, None, push_destination,
-                append_tag, 'add-foo-plugin', {'foo_version': '1.0.1'},
-                False, 'full')
-        )
-
-        self.dockermock.assert_called_once_with(
-            base_url='unix://var/run/docker.sock', version='auto')
-
-        self.dockermock.return_value.pull.assert_called_once_with(
-            image, tag=tag, stream=True)
-        self.dockermock.return_value.tag.assert_not_called()
-        self.dockermock.return_value.push.assert_not_called()
-
-    @mock.patch('subprocess.Popen')
-    @mock.patch('tripleo_common.actions.'
-                'ansible.AnsiblePlaybookAction', autospec=True)
-    def test_modify_upload_image_dry_run(self, mock_ansible, mock_popen):
-        mock_process = mock.Mock()
-        mock_popen.return_value = mock_process
-
-        image = 'docker.io/tripleomaster/heat-docker-agents-centos'
-        tag = 'latest'
-        append_tag = 'modify-123'
-        push_destination = 'localhost:8787'
-
-        result = self.uploader.upload_image(image_uploader.UploadTask(
-            image + ':' + tag,
-            None,
-            push_destination,
-            append_tag,
-            'add-foo-plugin',
-            {'foo_version': '1.0.1'},
-            True,
-            'full')
-        )
-
-        self.dockermock.assert_not_called()
-        mock_ansible.assert_not_called()
-        mock_process.communicate.assert_not_called()
-        self.assertEqual([], result)
-
-    @mock.patch('tripleo_common.image.image_uploader.'
-                'BaseImageUploader.authenticate')
-    @mock.patch('tripleo_common.image.image_uploader.'
-                'BaseImageUploader._inspect')
-    @mock.patch('tripleo_common.actions.'
-                'ansible.AnsiblePlaybookAction', autospec=True)
-    def test_modify_image_existing(self, mock_ansible, mock_inspect,
-                                   mock_auth):
-        mock_inspect.return_value = {'Digest': 'a'}
-
-        image = 'docker.io/tripleomaster/heat-docker-agents-centos'
-        tag = 'latest'
-        append_tag = 'modify-123'
-        push_destination = 'localhost:8787'
-
-        result = self.uploader.upload_image(image_uploader.UploadTask(
-            image + ':' + tag,
-            None,
-            push_destination,
-            append_tag,
-            'add-foo-plugin',
-            {'foo_version': '1.0.1'},
-            False,
-            'full')
-        )
-
-        self.dockermock.assert_not_called()
-        mock_ansible.assert_not_called()
-
-        self.assertEqual([], result)
-
-    def test_pull_retry(self):
-        image = 'docker.io/tripleomaster/heat-docker-agents-centos'
-
-        dockerc = self.dockermock.return_value
-        dockerc.pull.side_effect = [
-            urllib3.exceptions.ReadTimeoutError('p', '/foo', 'ouch'),
-            urllib3.exceptions.ReadTimeoutError('p', '/foo', 'ouch'),
-            ['{"error": "ouch"}'],
-            ['{"error": "ouch"}'],
-            ['{"status": "done"}']
-        ]
-        self.uploader._pull(dockerc, image)
-
-        self.assertEqual(dockerc.pull.call_count, 5)
-        dockerc.pull.assert_has_calls([
-            mock.call(image, tag=None, stream=True)
-        ])
-
-    def test_pull_retry_failure(self):
-        image = 'docker.io/tripleomaster/heat-docker-agents-centos'
-
-        dockerc = self.dockermock.return_value
-        dockerc.pull.side_effect = [
-            urllib3.exceptions.ReadTimeoutError('p', '/foo', 'ouch'),
-            urllib3.exceptions.ReadTimeoutError('p', '/foo', 'ouch'),
-            urllib3.exceptions.ReadTimeoutError('p', '/foo', 'ouch'),
-            urllib3.exceptions.ReadTimeoutError('p', '/foo', 'ouch'),
-            urllib3.exceptions.ReadTimeoutError('p', '/foo', 'ouch'),
-        ]
-        self.assertRaises(urllib3.exceptions.ReadTimeoutError,
-                          self.uploader._pull, dockerc, image)
-
-        self.assertEqual(dockerc.pull.call_count, 5)
-        dockerc.pull.assert_has_calls([
-            mock.call(image, tag=None, stream=True)
-        ])
-
-    def test_push_retry(self):
-        image = 'docker.io/tripleoupstream/heat-docker-agents-centos'
-
-        dockerc = self.dockermock.return_value
-        dockerc.push.side_effect = [
-            ['{"error": "ouch"}'],
-            ['{"error": "ouch"}'],
-            ['{"error": "ouch"}'],
-            ['{"error": "ouch"}'],
-            ['{"status": "done"}']
-        ]
-        self.uploader._push(dockerc, image)
-
-        self.assertEqual(dockerc.push.call_count, 5)
-        dockerc.push.assert_has_calls([
-            mock.call(image, tag=None, stream=True)
-        ])
-
-    def test_push_retry_failure(self):
-        image = 'docker.io/tripleoupstream/heat-docker-agents-centos'
-
-        dockerc = self.dockermock.return_value
-        dockerc.push.side_effect = [
-            ['{"error": "ouch"}'],
-            ['{"error": "ouch"}'],
-            ['{"error": "ouch"}'],
-            ['{"error": "ouch"}'],
-            ['{"error": "ouch"}'],
-        ]
-        self.assertRaises(ImageUploaderException,
-                          self.uploader._push, dockerc, image)
-
-        self.assertEqual(dockerc.push.call_count, 5)
-        dockerc.push.assert_has_calls([
-            mock.call(image, tag=None, stream=True)
-        ])
-
-
 class TestSkopeoImageUploader(base.TestCase):
 
     def setUp(self):