Merge pull request #240 from rnc/ISSUE169

Don't clean up if tmp_dir (aka development mode) is used

Author: rnc

README.rst

@@ -62,7 +62,6 @@ Usage
       -h, --help            show this help message and exit
       -v, --verbose         Verbose output
       --version             Show version and exit
-      -d, --development     Does not clean up after failure for easier debugging
       -f FROM_LAYER, --from-layer FROM_LAYER
                             Number of layers to squash or ID of the layer (or image ID or image name) to squash from.
                             In case the provided value is an integer, specified number of layers will be squashed.
@@ -71,7 +70,8 @@ Usage
       -m MESSAGE, --message MESSAGE
                             Specify a commit message (comment) for the new image.
       -c, --cleanup         Remove source image from Docker after squashing
-      --tmp-dir TMP_DIR     Temporary directory to be created and used
+      --tmp-dir TMP_DIR     Temporary directory to be created and used. This will NOT be deleted afterwards for
+                            easier debugging.
       --output-path OUTPUT_PATH
                             Path where the image may be stored after squashing.
       --load-image [LOAD_IMAGE]

docker_squash/cli.py

@@ -71,12 +71,6 @@ def run(self):
         )
 
         parser.add_argument("image", help="Image to be squashed")
-        parser.add_argument(
-            "-d",
-            "--development",
-            action="store_true",
-            help="Does not clean up after failure for easier debugging",
-        )
         parser.add_argument(
             "-f",
             "--from-layer",
@@ -100,7 +94,8 @@ def run(self):
             help="Remove source image from Docker after squashing",
         )
         parser.add_argument(
-            "--tmp-dir", help="Temporary directory to be created and used"
+            "--tmp-dir",
+            help="Temporary directory to be created and used. This will NOT be deleted afterwards for easier debugging.",
         )
         parser.add_argument(
             "--output-path",
@@ -133,7 +128,6 @@ def run(self):
                 output_path=args.output_path,
                 load_image=args.load_image,
                 tmp_dir=args.tmp_dir,
-                development=args.development,
                 cleanup=args.cleanup,
             ).run()
         except KeyboardInterrupt:
@@ -142,7 +136,7 @@ def run(self):
         except Exception:
             e = sys.exc_info()[1]
 
-            if args.development or args.verbose:
+            if args.verbose:
                 self.log.exception(e)
             else:
                 self.log.error(str(e))

docker_squash/squash.py

@@ -24,7 +24,6 @@ def __init__(
         tmp_dir=None,
         output_path=None,
         load_image=True,
-        development=False,
         cleanup=False,
     ):
         self.log = log
@@ -36,9 +35,11 @@ def __init__(
         self.tmp_dir = tmp_dir
         self.output_path = output_path
         self.load_image = load_image
-        self.development = development
         self.cleanup = cleanup
+        self.development = False
 
+        if tmp_dir:
+            self.development = True
         if not docker:
             self.docker = common.docker_client(self.log)
 

tests/test_integ_squash.py

@@ -2,13 +2,15 @@
 import json
 import logging
 import os
+import re
 import shutil
 import tarfile
 import unittest
 import uuid
 from io import BytesIO
 
 import mock
+import pytest
 
 from docker_squash.errors import SquashError, SquashUnnecessaryError
 from docker_squash.lib import common
@@ -125,7 +127,6 @@ def __init__(
             numeric=False,
             tmp_dir=None,
             log=None,
-            development=False,
             tag=True,
         ):
             self.image = image
@@ -140,7 +141,6 @@ def __init__(
             self.load_image = load_image
             self.numeric = numeric
             self.tmp_dir = tmp_dir
-            self.development = development
 
         def __enter__(self):
             from_layer = self.number_of_layers
@@ -159,7 +159,6 @@ def __enter__(self):
                 output_path=self.output_path,
                 load_image=self.load_image,
                 tmp_dir=self.tmp_dir,
-                development=self.development,
             )
 
             self.image_id = squash.run()
@@ -278,6 +277,10 @@ def assertFileDoesNotExist(self, name):
 
 
 class TestIntegSquash(IntegSquash):
+    @pytest.fixture(autouse=True)
+    def inject_fixtures(self, caplog):
+        self.caplog = caplog
+
     def test_all_files_should_be_in_squashed_layer(self):
         """
         We squash all layers in RUN, all files should be in the resulting squashed layer.
@@ -754,35 +757,29 @@ def test_should_squash_every_layer(self):
 
     # https://github.com/goldmann/docker-scripts/issues/44
     def test_remove_tmp_dir_after_failure(self):
+        self.caplog.set_level(logging.DEBUG, logger="cekit")
         dockerfile = """
         FROM busybox:1.24.0
         LABEL foo bar
         """
 
-        tmp_dir = "/tmp/docker-squash-integ-tmp-dir"
-        log = mock.Mock()
-        shutil.rmtree(tmp_dir, ignore_errors=True)
-
-        self.assertFalse(os.path.exists(tmp_dir))
-
         with self.Image(dockerfile) as image:
             with self.assertRaisesRegex(
                 SquashError,
                 r"Cannot squash 20 layers, the .* image contains only \d " r"layers",
             ):
-                with self.SquashedImage(
-                    image, 20, numeric=True, tmp_dir=tmp_dir, log=log
-                ):
+                with self.SquashedImage(image, 20, numeric=True):
                     pass
-
-        log.debug.assert_any_call(
-            "Using /tmp/docker-squash-integ-tmp-dir as the temporary directory"
+        tmp_location = re.search(
+            ".*Using (.*)as the temporary directory.*", self.caplog.text
         )
-        log.debug.assert_any_call(
-            "Cleaning up /tmp/docker-squash-integ-tmp-dir temporary directory"
+
+        assert tmp_location
+        assert "Cleaning up %s temporary directory", (
+            tmp_location.group(1) in self.caplog.text
         )
 
-        self.assertFalse(os.path.exists(tmp_dir))
+        self.assertFalse(os.path.exists(tmp_location.group(1)))
 
     def test_should_not_remove_tmp_dir_after_failure_if_development_mode_is_on(self):
         dockerfile = """
@@ -802,7 +799,7 @@ def test_should_not_remove_tmp_dir_after_failure_if_development_mode_is_on(self)
                 r"Cannot squash 20 layers, the .* image contains only \d " r"layers",
             ):
                 with self.SquashedImage(
-                    image, 20, numeric=True, tmp_dir=tmp_dir, log=log, development=True
+                    image, 20, numeric=True, tmp_dir=tmp_dir, log=log
                 ):
                     pass