From 8b4dbd7c2e7887dd7d5e253c6c5c13042e4be2b9 Mon Sep 17 00:00:00 2001 From: Steve Baker Date: Wed, 27 Feb 2019 10:02:16 +1300 Subject: [PATCH] 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 | 1 - tripleo_common/image/image_uploader.py | 143 +------- tripleo_common/image/kolla_builder.py | 4 +- .../tests/image/test_image_uploader.py | 336 +----------------- 4 files changed, 9 insertions(+), 475 deletions(-) diff --git a/requirements.txt b/requirements.txt index 6628c2fca..515e15564 100644 --- a/requirements.txt +++ b/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 diff --git a/tripleo_common/image/image_uploader.py b/tripleo_common/image/image_uploader.py index 8a5df1a8e..3815801ea 100644 --- a/tripleo_common/image/image_uploader.py +++ b/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""" diff --git a/tripleo_common/image/kolla_builder.py b/tripleo_common/image/kolla_builder.py index 3f4169824..4468fa3cd 100644 --- a/tripleo_common/image/kolla_builder.py +++ b/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): diff --git a/tripleo_common/tests/image/test_image_uploader.py b/tripleo_common/tests/image/test_image_uploader.py index fe9e157a1..86dd78f7d 100644 --- a/tripleo_common/tests/image/test_image_uploader.py +++ b/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') - assert isinstance(uploader, image_uploader.DockerImageUploader) + uploader = manager.get_uploader('python') + 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):