Repository: cekit/cekit

Issue:
Make it possible to disable squashing
The #329 PR added support for automatic squashing, we should be able to disable it in some cases, leaving the original image untouched for investigation purposes.

Discussion:

Patch:
diff --git a/cekit/builder.py b/cekit/builder.py
index 4f4ff2c..a96dbe4 100644
--- a/cekit/builder.py
+++ b/cekit/builder.py
@@ -38,5 +38,5 @@ class Builder(object):
         # we dont require prepare to be implemented by builder
         pass
 
-    def build(self, build_args):
-        raise CekitError("Buider.build() is not implemented!")
+    def build(self):
+        raise CekitError("Builder.build() is not implemented!")
diff --git a/cekit/builders/docker_builder.py b/cekit/builders/docker_builder.py
index 25a5227..618adb2 100644
--- a/cekit/builders/docker_builder.py
+++ b/cekit/builders/docker_builder.py
@@ -4,7 +4,6 @@ import re
 import yaml
 import sys
 import traceback
-import requests
 
 from cekit.builder import Builder
 from cekit.errors import CekitError
@@ -25,6 +24,14 @@ try:
 except ImportError:
     pass
 
+try:
+    # The requests library is an indirect dependency, we need to put it here
+    # so that the dependency mechanism can kick in and require the docker library
+    # first which will pull requests
+    import requests
+except ImportError:
+    pass
+
 try:
     # Docker Python library, the new one
     from docker.client import Client as APIClientClass
@@ -43,6 +50,7 @@ class DockerBuilder(Builder):
         self._tags = params.get('tags', [])
         self._pull = params.get('pull', False)
         self._base = params.get('base')
+        self._no_squash = params.get('no_squash', False)
         super(DockerBuilder, self).__init__(build_engine, target, params)
 
         # Default Docker daemon connection timeout 10 minutes
@@ -78,12 +86,10 @@ class DockerBuilder(Builder):
 
         return deps
 
-    def build(self, build_args=None):
-        self.docker_client = APIClientClass(version="1.22", timeout=self.timeout)
+    def _build_with_docker(self, docker_client):
+        # Custom tags for the container image
+        logger.debug("Building image with tags: '%s'" % "', '".join(self._tags))
 
-        """After the source files are generated, the container image can be built.
-        We're using Docker to build the image currently.
-        """
         args = {}
         args['path'] = os.path.join(self.target, 'image')
         args['tag'] = self._tags[0]
@@ -93,15 +99,8 @@ class DockerBuilder(Builder):
         build_log = [""]
         docker_layer_ids = []
 
-        # Custom tags for the container image
-        logger.debug("Building image with tags: '%s'" %
-                     "', '".join(self._tags))
-
-        logger.info("Building container image...")
-
         try:
-
-            out = self.docker_client.build(**args)
+            out = docker_client.build(**args)
             for line in out:
                 if b'stream' in line:
                     line = yaml.safe_load(line)['stream']
@@ -124,18 +123,6 @@ class DockerBuilder(Builder):
                         docker_layer_ids.append(line.split(' ')[-1].strip())
                     elif '---> Using cache' in build_log[-2]:
                         docker_layer_ids.append(line.split(' ')[-1].strip())
-
-            self.squash_image(docker_layer_ids[-1])
-
-            for tag in self._tags[1:]:
-                if ':' in tag:
-                    img_repo, img_tag = tag.split(":")
-                    self.docker_client.tag(self._tags[0], img_repo, tag=img_tag)
-                else:
-                    self.docker_client.tag(self._tags[0], tag)
-            logger.info("Image built and available under following tags: %s"
-                        % ", ".join(self._tags))
-
         except requests.ConnectionError as ex:
             exception_chain = traceback.format_exc()
             logger.debug("Caught ConnectionError attempting to communicate with Docker ", exc_info=1)
@@ -170,10 +157,33 @@ class DockerBuilder(Builder):
                       " definitions."
             raise CekitError(msg, ex)
 
-    def squash_image(self, layer_id):
-        logger.info("Squashing image %s..." % (layer_id))
-        # XXX: currently, cleanup throws a 409 error from the docker daemon.  this needs to be investigated in docker_squash
-        squash = Squash(docker=self.docker_client, log=logger, from_layer=self._base, image=layer_id,
+        return docker_layer_ids[-1]
+
+    def _squash(self, docker_client, image_id):
+        logger.info("Squashing image %s..." % (image_id))
+        squash = Squash(docker=docker_client, log=logger, from_layer=self._base, image=image_id,
                         tag=self._tags[0],
-                        cleanup=False)
+                        cleanup=True)
         squash.run()
+
+    def _tag(self, docker_client):
+        for tag in self._tags[1:]:
+            if ':' in tag:
+                img_repo, img_tag = tag.split(":")
+                docker_client.tag(self._tags[0], img_repo, tag=img_tag)
+            else:
+                docker_client.tag(self._tags[0], tag)
+
+    def build(self):
+        docker_client = APIClientClass(version="1.22", timeout=self.timeout)
+
+        logger.info("Building container image using Docker...")
+
+        image_id = self._build_with_docker(docker_client)
+
+        # Squash only if --build-docker-no-squash is NOT defined
+        if not self._no_squash:
+            self._squash(docker_client, image_id)
+
+        logger.info("Image built and available under following tags: %s" %
+                    ", ".join(self._tags))
diff --git a/cekit/cli.py b/cekit/cli.py
index 2b5bd4e..4df6a04 100644
--- a/cekit/cli.py
+++ b/cekit/cli.py
@@ -101,6 +101,11 @@ class Cekit(object):
                                  action='store_true',
                                  help='Always fetch latest base image during build')
 
+        build_group.add_argument('--build-docker-no-squash',
+                                 dest='build_docker_no_squash',
+                                 action='store_true',
+                                 help='do not squash after the image is built with Docker')
+
         build_group.add_argument('--build-osbs-release',
                                  dest='build_osbs_release',
                                  action='store_true',
@@ -245,6 +250,7 @@ class Cekit(object):
                           'nowait': self.args.build_osbs_nowait,
                           'stage': self.args.build_osbs_stage,
                           'release': self.args.build_osbs_release,
+                          'no_squash': self.args.build_docker_no_squash,
                           'tags': self.args.tags,
                           'pull': self.args.build_pull,
                           'redhat': config.get('common', 'redhat'),


Test patch:
diff --git a/tests/test_builder.py b/tests/test_builder.py
index f6be324..d72a6e1 100644
--- a/tests/test_builder.py
+++ b/tests/test_builder.py
@@ -1,4 +1,5 @@
 import glob
+import logging
 import os
 import pytest
 import subprocess
@@ -333,6 +334,63 @@ def test_docker_builder_defaults():
     builder = Builder('docker', 'tmp', params)
 
     assert builder._tags == ['foo', 'bar']
+    assert builder._no_squash == False
+
+
+def test_docker_squashing_enabled(mocker):
+    params = {'tags': ['foo', 'bar']}
+    builder = Builder('docker', 'tmp', params)
+
+    assert builder._no_squash == False
+
+    docker_client_class = mocker.patch('cekit.builders.docker_builder.APIClientClass')
+    docker_client = docker_client_class.return_value
+    mocker.patch.object(builder, '_build_with_docker')
+    mocker.patch.object(builder, '_squash')
+    builder._build_with_docker.return_value = "1654234sdf56"
+
+    builder.build()
+
+    builder._build_with_docker.assert_called_once_with(docker_client)
+    builder._squash.assert_called_once_with(docker_client, "1654234sdf56")
+
+
+def test_docker_squashing_disabled(mocker):
+    params = {'no_squash': True, 'tags': ['foo', 'bar']}
+    builder = Builder('docker', 'tmp', params)
+
+    assert builder._no_squash == True
+
+    docker_client_class = mocker.patch('cekit.builders.docker_builder.APIClientClass')
+    docker_client = docker_client_class.return_value
+    mocker.patch.object(builder, '_build_with_docker')
+    mocker.patch.object(builder, '_squash')
+    builder._build_with_docker.return_value = "1654234sdf56"
+
+    builder.build()
+
+    builder._build_with_docker.assert_called_once_with(docker_client)
+    builder._squash.assert_not_called()
+
+
+def test_docker_squashing_parameters(mocker):
+    params = {'tags': ['foo', 'bar']}
+    builder = Builder('docker', 'tmp', params)
+
+    assert builder._no_squash == False
+
+    docker_client_class = mocker.patch('cekit.builders.docker_builder.APIClientClass')
+    squash_class = mocker.patch('cekit.builders.docker_builder.Squash')
+    squash = squash_class.return_value
+    docker_client = docker_client_class.return_value
+    mocker.patch.object(builder, '_build_with_docker', return_value="1654234sdf56")
+
+    builder.build()
+
+    squash_class.assert_called_once_with(
+        cleanup=True, docker=docker_client, from_layer=None, image="1654234sdf56", log=logging.getLogger('cekit'), tag="foo")
+    squash.run.assert_called_once_with()
+    builder._build_with_docker.assert_called_once_with(docker_client)
 
 
 def test_buildah_builder_run(mocker):
diff --git a/tests/test_validate.py b/tests/test_validate.py
index 6ffd5a0..fce163a 100644
--- a/tests/test_validate.py
+++ b/tests/test_validate.py
@@ -2,12 +2,6 @@ import os
 import shutil
 import subprocess
 import sys
-
-try:
-    from unittest.mock import patch
-except ImportError:
-    from mock import patch
-
 import pytest
 import yaml
 
@@ -79,6 +73,7 @@ if sys.version_info.major == 2:
     PermissionError = ConnectionError
     FileNotFoundError = ConnectionError
 
+
 def run_cekit_cs_overrides(image_dir, mocker, overrides_descriptor):
     mocker.patch.object(sys, 'argv', ['cekit',
                                       '--config',
@@ -727,11 +722,12 @@ def test_override_modules_flat(tmpdir, mocker):
         yaml.dump(img_desc, fd, default_flow_style=False)
 
     run_cekit(image_dir)
-    
+
     assert check_dockerfile_text(image_dir, 'foo="mod_2"')
     assert not check_dockerfile_text(image_dir, "RUN yum makecache")
     assert not check_dockerfile_text(image_dir, "RUN yum clean all")
 
+
 def test_execution_order_flat(tmpdir, mocker):
     mocker.patch.object(sys, 'argv', ['cekit',
                                       '-v',
@@ -811,6 +807,7 @@ RUN [ "bash", "-x", "/tmp/scripts/mod_4/c" ]
     assert not check_dockerfile_text(image_dir, "RUN yum makecache")
     assert not check_dockerfile_text(image_dir, "RUN yum clean all")
 
+
 def test_package_related_commands_packages_in_module(tmpdir, mocker):
     mocker.patch.object(sys, 'argv', ['cekit',
                                       '-v',
@@ -850,7 +847,9 @@ RUN yum --setopt=tsflags=nodocs install -y wget mc \\
 
     assert check_dockerfile_text(image_dir, "RUN yum makecache")
     assert check_dockerfile_text(image_dir, expected_packages_order_install)
-    assert check_dockerfile_text(image_dir, "RUN yum clean all && [ ! -d /var/cache/yum ] || rm -rf /var/cache/yum")
+    assert check_dockerfile_text(
+        image_dir, "RUN yum clean all && [ ! -d /var/cache/yum ] || rm -rf /var/cache/yum")
+
 
 def test_package_related_commands_packages_in_image(tmpdir, mocker):
     mocker.patch.object(sys, 'argv', ['cekit',
@@ -878,6 +877,7 @@ RUN yum --setopt=tsflags=nodocs install -y wget mc \\
     assert check_dockerfile_text(image_dir, "RUN yum makecache")
     assert check_dockerfile_text(image_dir, expected_packages_install)
 
+
 def test_nonexisting_image_descriptor(mocker, tmpdir, caplog):
     mocker.patch.object(sys, 'argv', ['cekit',
                                       '-v',
@@ -932,8 +932,9 @@ def test_incorrect_override_file(mocker, tmpdir, caplog):
     assert "Schema validation failed" in caplog.text
     assert "Key 'wrong!' was not defined" in caplog.text
 
-@patch('urllib3.connectionpool.HTTPConnectionPool.urlopen', **{'side_effect': PermissionError()})
+
 def test_simple_image_build_no_docker_perm(tmpdir, mocker, caplog):
+    mocker.patch('urllib3.connectionpool.HTTPConnectionPool.urlopen', side_effect=PermissionError())
     mocker.patch.object(sys, 'argv', ['cekit',
                                       '-v',
                                       'build'])
@@ -951,8 +952,10 @@ def test_simple_image_build_no_docker_perm(tmpdir, mocker, caplog):
     else:
         assert "Unable to contact docker daemon. Is it correctly setup" in caplog.text
 
-@patch('urllib3.connectionpool.HTTPConnectionPool.urlopen', **{'side_effect': FileNotFoundError()})
+
 def test_simple_image_build_no_docker_start(tmpdir, mocker, caplog):
+    mocker.patch('urllib3.connectionpool.HTTPConnectionPool.urlopen',
+                 side_effect=FileNotFoundError())
     mocker.patch.object(sys, 'argv', ['cekit',
                                       '-v',
                                       'build'])
@@ -970,6 +973,7 @@ def test_simple_image_build_no_docker_start(tmpdir, mocker, caplog):
     else:
         assert "Unable to contact docker daemon. Is it started" in caplog.text
 
+
 def run_cekit(cwd):
     with Chdir(cwd):
         # run cekit and check it exits with 0