[virt-tools-list] [virt-bootstrap] [PATCH v5 12/13] Add flag --faster

Radostin Stoyanov rstoyanov1 at gmail.com
Fri Aug 4 14:30:48 UTC 2017


When using this flag with format='dir' the overhead when extracting the
root file system of container image will be reduce by running `tar` on
the host instead of in sandbox.

Update the tests of safe_untar() to match the behaviour of untar().

Rename 'safe_untar' to 'untar' in the test suite and add the `faster`
class variable when mocking out DockerSource and FileSource.
---
 src/virtBootstrap/sources/docker_source.py |  9 +++-
 src/virtBootstrap/sources/file_source.py   |  4 +-
 src/virtBootstrap/utils.py                 | 30 +++++++------
 src/virtBootstrap/virt_bootstrap.py        |  6 +++
 tests/test_docker_source.py                |  3 +-
 tests/test_file_source.py                  | 12 +++---
 tests/test_utils.py                        | 68 ++++++++++++++++++++++--------
 7 files changed, 94 insertions(+), 38 deletions(-)

diff --git a/src/virtBootstrap/sources/docker_source.py b/src/virtBootstrap/sources/docker_source.py
index 207d166..675466a 100644
--- a/src/virtBootstrap/sources/docker_source.py
+++ b/src/virtBootstrap/sources/docker_source.py
@@ -53,6 +53,7 @@ class DockerSource(object):
         @param gid_map: Mappings for GID of files in rootfs
         @param fmt: Format used to store image [dir, qcow2]
         @param not_secure: Do not require HTTPS and certificate verification
+        @param faster: Reduce the overhead when untar images to rootfs
         @param no_cache: Whether to store downloaded images or not
         @param progress: Instance of the progress module
 
@@ -68,6 +69,7 @@ class DockerSource(object):
         self.root_password = kwargs.get('root_password', None)
         self.output_format = kwargs.get('fmt', utils.DEFAULT_OUTPUT_FORMAT)
         self.insecure = kwargs.get('not_secure', False)
+        self.faster = kwargs.get('faster', False)
         self.no_cache = kwargs.get('no_cache', False)
         self.progress = kwargs['progress'].update_progress
         self.images_dir = utils.get_image_dir(self.no_cache)
@@ -271,7 +273,12 @@ class DockerSource(object):
             if self.output_format == 'dir':
                 self.progress("Extracting container layers", value=50,
                               logger=logger)
-                utils.untar_layers(self.layers, dest, self.progress)
+                utils.untar_layers(
+                    self.layers,
+                    dest,
+                    self.progress,
+                    self.faster
+                )
             elif self.output_format == 'qcow2':
                 self.progress("Extracting container layers into qcow2 images",
                               value=50, logger=logger)
diff --git a/src/virtBootstrap/sources/file_source.py b/src/virtBootstrap/sources/file_source.py
index ef03741..fdb2794 100644
--- a/src/virtBootstrap/sources/file_source.py
+++ b/src/virtBootstrap/sources/file_source.py
@@ -43,6 +43,7 @@ class FileSource(object):
         @param fmt: Format used to store image [dir, qcow2]
         @param uid_map: Mappings for UID of files in rootfs
         @param gid_map: Mappings for GID of files in rootfs
+        @param faster: Reduce the overhead when untar images to rootfs
         @param progress: Instance of the progress module
         """
         self.path = kwargs['uri'].path
@@ -50,6 +51,7 @@ class FileSource(object):
         self.uid_map = kwargs.get('uid_map', None)
         self.gid_map = kwargs.get('gid_map', None)
         self.root_password = kwargs.get('root_password', None)
+        self.faster = kwargs.get('faster', False)
         self.progress = kwargs['progress'].update_progress
 
     def unpack(self, dest):
@@ -65,7 +67,7 @@ class FileSource(object):
         if self.output_format == 'dir':
             self.progress("Extracting files into destination directory",
                           value=0, logger=logger)
-            utils.safe_untar(self.path, dest)
+            utils.untar(self.path, dest, self.faster)
 
         elif self.output_format == 'qcow2':
             utils.Build_QCOW2_Image(
diff --git a/src/virtBootstrap/utils.py b/src/virtBootstrap/utils.py
index 0328bbd..258b82b 100644
--- a/src/virtBootstrap/utils.py
+++ b/src/virtBootstrap/utils.py
@@ -299,18 +299,24 @@ def execute(cmd):
         raise subprocess.CalledProcessError(proc.returncode, cmd_str)
 
 
-def safe_untar(src, dest):
+def untar(src, dest, faster=False):
     """
-    Extract tarball within LXC container for safety.
-    """
-    virt_sandbox = ['virt-sandbox',
-                    '-c', LIBVIRT_CONN,
-                    '-m', 'host-bind:/mnt=' + dest]  # Bind destination folder
+    Extract tarball to destination path.
 
-    # Compression type is auto detected from tar
-    # Exclude files under /dev to avoid "Cannot mknod: Operation not permitted"
-    params = ['--', '/bin/tar', 'xf', src, '-C', '/mnt', '--exclude', 'dev/*']
-    execute(virt_sandbox + params)
+    @param faster: If True reduce the overhead by running tar on the host
+    """
+    cmd = ['/bin/tar',
+           'xf', src,
+           '-C', dest if faster else '/mnt',
+           '--exclude', 'dev/*']
+    if not faster:
+        cmd = ['virt-sandbox',
+               # Set connection
+               '-c', LIBVIRT_CONN,
+               # Bind destination folder
+               '-m', 'host-bind:/mnt=' + dest,
+               '--'] + cmd
+    execute(cmd)
 
 
 def bytes_to_size(number):
@@ -357,7 +363,7 @@ def log_layer_extract(layer, current, total, progress):
     logger.debug('Untar layer: (%s:%s) %s', sum_type, sum_value, layer_file)
 
 
-def untar_layers(layers_list, dest_dir, progress):
+def untar_layers(layers_list, dest_dir, progress, faster=False):
     """
     Untar each of layers from container image.
     """
@@ -367,7 +373,7 @@ def untar_layers(layers_list, dest_dir, progress):
         layer_file = layer[2]
 
         # Extract layer tarball into destination directory
-        safe_untar(layer_file, dest_dir)
+        untar(layer_file, dest_dir, faster)
 
         # Update progress value
         progress(value=(float(index + 1) / nlayers * 50) + 50)
diff --git a/src/virtBootstrap/virt_bootstrap.py b/src/virtBootstrap/virt_bootstrap.py
index cbd9f0c..118127f 100755
--- a/src/virtBootstrap/virt_bootstrap.py
+++ b/src/virtBootstrap/virt_bootstrap.py
@@ -100,6 +100,7 @@ def bootstrap(uri, dest,
               uid_map=None,
               gid_map=None,
               not_secure=False,
+              faster=None,
               no_cache=False,
               progress_cb=None):
     """
@@ -127,6 +128,7 @@ def bootstrap(uri, dest,
            uid_map=uid_map,
            gid_map=gid_map,
            not_secure=not_secure,
+           faster=faster,
            no_cache=no_cache,
            root_password=root_password,
            progress=prog).unpack(dest)
@@ -210,6 +212,9 @@ def main():
     parser.add_argument("--status-only", action="store_const",
                         const=utils.write_progress,
                         help=_("Show only progress information"))
+    parser.add_argument("--faster", action="store_true",
+                        help=_("Reduce the overhead when extracting layers "
+                               "(use this option only for trusted images)"))
 
     try:
         args = parser.parse_args()
@@ -234,6 +239,7 @@ def main():
                   root_password=args.root_password,
                   uid_map=uid_map,
                   gid_map=gid_map,
+                  faster=args.faster,
                   not_secure=args.not_secure,
                   no_cache=args.no_cache,
                   progress_cb=args.status_only)
diff --git a/tests/test_docker_source.py b/tests/test_docker_source.py
index 3865be6..d9fe32f 100644
--- a/tests/test_docker_source.py
+++ b/tests/test_docker_source.py
@@ -47,6 +47,7 @@ class TestDockerSource(unittest.TestCase):
         m_self.url = "docker://test"
         m_self.images_dir = "/images_path"
         m_self.insecure = True
+        m_self.faster = False
         m_self.username = 'user'
         m_self.password = 'password'
         m_self.layers = [
@@ -512,7 +513,7 @@ class TestDockerSource(unittest.TestCase):
                 sources.DockerSource.unpack(m_self, dest)
 
             mocked.assert_called_once_with(m_self.layers, dest,
-                                           m_self.progress)
+                                           m_self.progress, m_self.faster)
         else:
             sources.DockerSource.unpack(m_self, dest)
 
diff --git a/tests/test_file_source.py b/tests/test_file_source.py
index a55ae4e..b850a7c 100644
--- a/tests/test_file_source.py
+++ b/tests/test_file_source.py
@@ -71,21 +71,22 @@ class TestFileSource(unittest.TestCase):
 
     def test_unpack_to_dir(self):
         """
-        Ensures that unpack() calls safe_untar() when the output format
+        Ensures that unpack() calls untar() when the output format
         is set to 'dir'.
         """
         m_self = mock.Mock(spec=sources.FileSource)
         m_self.progress = mock.Mock()
         m_self.path = 'foo'
+        m_self.faster = False
         m_self.output_format = 'dir'
         dest = 'bar'
 
         with mock.patch('os.path.isfile') as m_isfile:
             m_isfile.return_value = True
-            with mock.patch('virtBootstrap.utils.safe_untar') as m_untar:
+            with mock.patch('virtBootstrap.utils.untar') as m_untar:
                 sources.FileSource.unpack(m_self, dest)
 
-        m_untar.assert_called_once_with(m_self.path, dest)
+        m_untar.assert_called_once_with(m_self.path, dest, m_self.faster)
 
     def _unpack_raise_error_test(self,
                                  output_format,
@@ -100,6 +101,7 @@ class TestFileSource(unittest.TestCase):
         m_self.progress = mock.Mock()
         m_self.path = 'foo'
         m_self.output_format = output_format
+        m_self.faster = False
         dest = 'bar'
 
         with mock.patch.multiple('os.path',
@@ -125,11 +127,11 @@ class TestFileSource(unittest.TestCase):
 
     def test_unpack_raise_error_if_untar_fail(self):
         """
-        Ensures that unpack() throws an Exception when safe_untar()
+        Ensures that unpack() throws an Exception when untar()
         fails.
         """
         msg = 'Caught untar failure'
-        patch_method = 'virtBootstrap.utils.safe_untar'
+        patch_method = 'virtBootstrap.utils.untar'
         self._unpack_raise_error_test(output_format='dir',
                                       side_effect=Exception(msg),
                                       patch_method=patch_method,
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 56f3460..07bcd6e 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -115,29 +115,60 @@ class TestUtils(unittest.TestCase):
                 utils.execute(['foo'])
 
     ###################################
-    # Tests for: safe_untar()
+    # Tests for: untar()
     ###################################
-    def test_utils_safe_untar_calls_execute(self):
+    def _apply_test_to_untar(self, src, dest, faster, expected_call):
         """
-        Ensures that safe_untar() calls execute with virt-sandbox
-        command to extract source files to destination folder.
-        Test for users with EUID 0 and 1000.
+        This method contains common test pattern used in the next two
+        test cases. Test for users with EUID 0 and 1000.
         """
         with mock.patch('virtBootstrap.utils.os.geteuid') as m_geteuid:
             for uid in [0, 1000]:
+                # Set UID
                 m_geteuid.return_value = uid
                 reload(utils)
+
+                # If using virt-sandbox insert the LIBVIRT_CONN value
+                # in the expected call, after UID has been set.
+                if not faster:
+                    expected_call[2] = utils.LIBVIRT_CONN
+
                 with mock.patch('virtBootstrap.utils.execute') as m_execute:
-                    src, dest = 'foo', 'bar'
-                    utils.safe_untar('foo', 'bar')
-                    cmd = ['virt-sandbox',
-                           '-c', utils.LIBVIRT_CONN,
-                           '-m', 'host-bind:/mnt=' + dest,
-                           '--',
-                           '/bin/tar', 'xf', src,
-                           '-C', '/mnt',
-                           '--exclude', 'dev/*']
-                    m_execute.assert_called_once_with(cmd)
+                    utils.untar(src, dest, faster)
+                    m_execute.assert_called_once_with(expected_call)
+
+    def test_utils_untar_calls_execute_virt_sandbox(self):
+        """
+        Ensures that untar() calls execute with virt-sandbox
+        command when 'faster' is set to False.
+        """
+        src = 'foo'
+        dest = 'bar'
+        faster = False
+
+        cmd = ['virt-sandbox',
+               '-c', 'utils.LIBVIRT_CONN',
+               '-m', 'host-bind:/mnt=' + dest,
+               '--',
+               '/bin/tar', 'xf', src,
+               '-C', '/mnt',
+               '--exclude', 'dev/*']
+        self._apply_test_to_untar(src, dest, faster, cmd)
+
+    def test_utils_untar_calls_execute_tar(self):
+        """
+        Ensures that untar() calls execute with tar command when
+        faster is set to True.
+        """
+        src = 'foo'
+        dest = 'bar'
+        faster = True
+
+        cmd = ['/bin/tar',
+               'xf', src,
+               '-C', dest,
+               '--exclude', 'dev/*']
+        self._apply_test_to_untar(src, dest, faster, cmd)
 
     ###################################
     # Tests for: bytes_to_size()
@@ -218,12 +249,13 @@ class TestUtils(unittest.TestCase):
         layers = ['l1', 'l2', 'l3']
         layers_list = [['', '', layer] for layer in layers]
         dest_dir = '/foo'
-        expected_calls = [mock.call(layer, dest_dir) for layer in layers]
+        expected_calls = [mock.call(layer, dest_dir, False)
+                          for layer in layers]
         with mock.patch.multiple(utils,
-                                 safe_untar=mock.DEFAULT,
+                                 untar=mock.DEFAULT,
                                  log_layer_extract=mock.DEFAULT) as mocked:
             utils.untar_layers(layers_list, dest_dir, mock.Mock())
-        mocked['safe_untar'].assert_has_calls(expected_calls)
+        mocked['untar'].assert_has_calls(expected_calls)
 
     ###################################
     # Tests for: get_image_dir()
-- 
2.13.4




More information about the virt-tools-list mailing list