From 27d675ea13ed1642f4c1d17da3fba495f5e5e492 Mon Sep 17 00:00:00 2001
From: Mohammed Naser <mnaser@vexxhost.com>
Date: Thu, 6 Apr 2017 13:37:38 -0400
Subject: [PATCH] Switch kolla_docker to rely on SHA256 for image changes

At the moment, the process to determine if an image has
changed or not relies on the Docker API which depending
on the Docker release server can return different results.

This patch addresses this issue by grabbing the SHA256
of the image before pulling (defaulting to None if it does
not exist) and then comparing it after the pull is complete
which should always be successful at determining if the
image did change or not.

The test for unknown status images is removed because this
is not a possible scenario as we do not rely on status
anymore except for failures (which are still tested).

Change-Id: Ia60a7f34420b02f50597dddb96a4c36ff3996612
Closes-Bug: #1668059
---
 ansible/library/kolla_docker.py | 26 ++++++++-------------
 tests/test_kolla_docker.py      | 40 ++++++++++++++++++++-------------
 2 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/ansible/library/kolla_docker.py b/ansible/library/kolla_docker.py
index d521e2e8fd..38a7a4db18 100644
--- a/ansible/library/kolla_docker.py
+++ b/ansible/library/kolla_docker.py
@@ -436,6 +436,12 @@ class DockerWorker(object):
         else:
             return full_image, 'latest'
 
+    def get_image_id(self):
+        full_image = self.params.get('image')
+
+        image = self.dc.images(name=full_image, quiet=True)
+        return image[0] if len(image) == 1 else None
+
     def pull_image(self):
         if self.params.get('auth_username'):
             self.dc.login(
@@ -446,6 +452,7 @@ class DockerWorker(object):
             )
 
         image, tag = self.parse_image()
+        old_image_id = self.get_image_id()
 
         statuses = [
             json.loads(line.strip().decode('utf-8')) for line in self.dc.pull(
@@ -468,23 +475,8 @@ class DockerWorker(object):
                         failed=True
                     )
 
-            if status and status.get('status'):
-                # NOTE(SamYaple): This allows us to use v1 and v2 docker
-                # registries.  Eventually docker will stop supporting v1
-                # registries and when that happens we can remove this.
-                if 'legacy registry' in status['status']:
-                    continue
-                elif 'Downloaded newer image for' in status['status']:
-                    self.changed = True
-                    return
-                elif 'Image is up to date for' in status['status']:
-                    return
-                else:
-                    self.module.fail_json(
-                        msg="Unknown status message: {}".format(
-                            status['status']),
-                        failed=True
-                    )
+        new_image_id = self.get_image_id()
+        self.changed = old_image_id != new_image_id
 
     def remove_container(self):
         if self.check_container():
diff --git a/tests/test_kolla_docker.py b/tests/test_kolla_docker.py
index 65874f179d..b97a2d2c8f 100644
--- a/tests/test_kolla_docker.py
+++ b/tests/test_kolla_docker.py
@@ -481,6 +481,22 @@ class TestImage(base.BaseTestCase):
         self.dw.dc.images.assert_called_once_with()
         self.assertTrue(return_data)
 
+    def test_get_image_id_not_exists(self):
+        self.dw = get_DockerWorker(
+            {'image': 'myregistrydomain.com:5000/ubuntu:16.04'})
+        self.dw.dc.images.return_value = []
+
+        return_data = self.dw.get_image_id()
+        self.assertIsNone(return_data)
+
+    def test_get_image_id_exists(self):
+        self.dw = get_DockerWorker(
+            {'image': 'myregistrydomain.com:5000/ubuntu:16.04'})
+        self.dw.dc.images.return_value = ['sha256:47c3bdbcf99f0c1a36e4db']
+
+        return_data = self.dw.get_image_id()
+        self.assertEqual('sha256:47c3bdbcf99f0c1a36e4db', return_data)
+
     def test_pull_image_new(self):
         self.dw = get_DockerWorker(
             {'image': 'myregistrydomain.com:5000/ubuntu:16.04',
@@ -494,6 +510,10 @@ class TestImage(base.BaseTestCase):
             b'{"status":"Digest: sha256:47c3bdbcf99f0c1a36e4db"}\r\n',
             b'{"status":"Downloaded newer image for ubuntu:16.04"}\r\n'
         ]
+        self.dw.dc.images.side_effect = [
+            [],
+            ['sha256:47c3bdbcf99f0c1a36e4db']
+        ]
 
         self.dw.pull_image()
         self.dw.dc.pull.assert_called_once_with(
@@ -510,6 +530,10 @@ class TestImage(base.BaseTestCase):
             b'{"status":"Digest: sha256:47c3bdbf0c1a36e4db"}\r\n',
             b'{"status":"mage is up to date for ubuntu:16.04"}\r\n'
         ]
+        self.dw.dc.images.side_effect = [
+            ['sha256:47c3bdbcf99f0c1a36e4db'],
+            ['sha256:47c3bdbcf99f0c1a36e4db']
+        ]
 
         self.dw.pull_image()
         self.dw.dc.pull.assert_called_once_with(
@@ -518,22 +542,6 @@ class TestImage(base.BaseTestCase):
             stream=True)
         self.assertFalse(self.dw.changed)
 
-    def test_pull_image_unknown_status(self):
-        self.dw = get_DockerWorker(
-            {'image': 'myregistrydomain.com:5000/ubuntu:16.04'})
-        self.dw.dc.pull.return_value = [
-            b'{"status": "some random message"}\r\n']
-
-        self.dw.pull_image()
-        self.dw.dc.pull.assert_called_once_with(
-            repository='myregistrydomain.com:5000/ubuntu',
-            tag='16.04',
-            stream=True)
-        self.assertFalse(self.dw.changed)
-        self.dw.module.fail_json.assert_called_with(
-            msg='Unknown status message: some random message',
-            failed=True)
-
     def test_pull_image_not_exists(self):
         self.dw = get_DockerWorker(
             {'image': 'unknown:16.04'})