From 7e0564106e7b01f6516c22b95082b04bc0c8fc63 Mon Sep 17 00:00:00 2001
From: Steve Baker <sbaker@redhat.com>
Date: Mon, 13 Nov 2017 10:06:58 +1300
Subject: [PATCH] Implement post-upload cleanup of docker images

The docker uploader will leave a copy of all uploaded images in the
local docker storage. This change will track those images and delete
them from the local docker after all uploads are complete.

If a local image is in use (for example, an already deployed
containerised undercloud) then the delete will fail. In this case,
only a warning is logged.

Change-Id: Ic0424638b9ddbf77e10cfe936d0b96ff2da1a59e
Closes-Bug: #1708965
Closes-Bug: #1694709
---
 tripleo_common/image/image_uploader.py        | 31 +++++++++++++++++++
 .../tests/image/test_image_uploader.py        | 21 +++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/tripleo_common/image/image_uploader.py b/tripleo_common/image/image_uploader.py
index 4c2d068c2..afb555ffa 100644
--- a/tripleo_common/image/image_uploader.py
+++ b/tripleo_common/image/image_uploader.py
@@ -21,6 +21,7 @@ import netifaces
 import six
 import time
 
+import docker
 try:
     from docker import APIClient as Client
 except ImportError:
@@ -76,6 +77,11 @@ class ImageUploadManager(BaseImageManager):
             self.uploader(uploader).upload_image(
                 image_name, pull_source, push_destination)
 
+        # Do cleanup after all the uploads so common layers don't get deleted
+        # repeatedly
+        for uploader in self.uploaders.values():
+            uploader.cleanup()
+
         return upload_images  # simply to make test validation easier
 
     def get_ctrl_plane_ip(self):
@@ -107,12 +113,20 @@ class ImageUploader(object):
         """Discover a versioned tag for an image"""
         pass
 
+    @abc.abstractmethod
+    def cleanup(self):
+        """Remove unused images or temporary files from upload"""
+        pass
+
 
 class DockerImageUploader(ImageUploader):
     """Upload images using docker push"""
 
     logger = logging.getLogger(__name__ + '.DockerImageUploader')
 
+    def __init__(self):
+        self.local_images = set()
+
     def upload_image(self, image_name, pull_source, push_destination):
         dockerc = Client(base_url='unix://var/run/docker.sock', version='auto')
         if ':' in image_name:
@@ -129,13 +143,16 @@ class DockerImageUploader(ImageUploader):
         self._pull_retry(dockerc, repo, tag=tag)
 
         full_image = repo + ':' + tag
+        self.local_images.add(full_image)
         new_repo = push_destination + '/' + repo.partition('/')[2]
+        full_new_repo = new_repo + ':' + tag
         response = dockerc.tag(image=full_image, repository=new_repo,
                                tag=tag, force=True)
         self.logger.debug(response)
 
         response = [line for line in dockerc.push(new_repo,
                     tag=tag, stream=True)]
+        self.local_images.add(full_new_repo)
         self.logger.debug(response)
 
         self.logger.info('Completed upload for docker image %s' % image_name)
@@ -196,3 +213,17 @@ class DockerImageUploader(ImageUploader):
         versioned_image = '%s:%s' % (image_name, tag_label)
         self._pull_retry(dockerc, versioned_image)
         return tag_label
+
+    def cleanup(self):
+        dockerc = Client(base_url='unix://var/run/docker.sock', version='auto')
+
+        for image in sorted(self.local_images):
+            self.logger.info('Removing local copy of %s' % image)
+            try:
+                dockerc.remove_image(image)
+            except docker.errors.APIError as e:
+                if e.explanation:
+                    self.logger.warning(e.explanation)
+                else:
+                    self.logger.warning(e)
+        self.local_images.clear()
diff --git a/tripleo_common/tests/image/test_image_uploader.py b/tripleo_common/tests/image/test_image_uploader.py
index 458c18c99..2ad1d9803 100644
--- a/tripleo_common/tests/image/test_image_uploader.py
+++ b/tripleo_common/tests/image/test_image_uploader.py
@@ -63,6 +63,27 @@ class TestImageUploadManager(base.TestCase):
                                     key=operator.itemgetter('imagename'))
         self.assertEqual(sorted_expected_data, sorted_parsed_data)
 
+        dockerc = mockdocker.return_value
+        dockerc.remove_image.assert_has_calls([
+            mock.call('docker.io/tripleoupstream'
+                      '/centos-binary-nova-compute:liberty'),
+            mock.call('docker.io/tripleoupstream'
+                      '/centos-binary-nova-libvirt:liberty'),
+            mock.call('docker.io/tripleoupstream'
+                      '/heat-docker-agents-centos:latest'),
+            mock.call('docker.io/tripleoupstream'
+                      '/image-with-missing-tag:latest'),
+
+            mock.call('localhost:8787/tripleoupstream'
+                      '/centos-binary-nova-compute:liberty'),
+            mock.call('localhost:8787/tripleoupstream'
+                      '/centos-binary-nova-libvirt:liberty'),
+            mock.call('localhost:8787/tripleoupstream'
+                      '/heat-docker-agents-centos:latest'),
+            mock.call('localhost:8787/tripleoupstream/'
+                      'image-with-missing-tag:latest'),
+        ])
+
 
 class TestImageUploader(base.TestCase):