[virt-tools-list] [virt-bootstrap] [PATCH v7 06/26] DockerSource: Split checksum and layers

[Cedric Bosdonnat]

On Sat, 2017-08-26 at 21:41 +0100, Radostin Stoyanov wrote:
> The current implementation store in one list:
>     - checksum
>     - checksum type
>     - file path
>     - file size
> However, the information about checksum and checksum type is only used
> to verify the content of tarball before it is being extracted. Splitting
> these into separate lists would allow us to reuse the function
> untar_layers() with FileSource.
> ---
>  src/virtBootstrap/sources/docker_source.py | 12 ++++++++++--
>  src/virtBootstrap/sources/file_source.py   |  3 ++-
>  src/virtBootstrap/utils.py                 | 19 +++++++++----------
>  tests/docker_source.py                     | 12 ++++++------
>  4 files changed, 27 insertions(+), 19 deletions(-)
> 
> diff --git a/src/virtBootstrap/sources/docker_source.py b/src/virtBootstrap/sources/docker_source.py
> index 54d8903..246356a 100644
> --- a/src/virtBootstrap/sources/docker_source.py
> +++ b/src/virtBootstrap/sources/docker_source.py
> @@ -65,6 +65,7 @@ class DockerSource(object):
>          self.images_dir = utils.get_image_dir(self.no_cache)
>          self.manifest = None
>          self.layers = []
> +        self.checksums = []
>  
>          if self.username and not self.password:
>              self.password = getpass.getpass()
> @@ -94,9 +95,13 @@ class DockerSource(object):
>              layer_digest = layer[digest_field]
>              layer_size = layer['size'] if 'size' in layer else None
>  
> +            # Store checksums of layers
>              sum_type, layer_sum = layer_digest.split(':')
> +            self.checksums.append([sum_type, layer_sum])
> +
> +            # Store file path and size of each layer
>              file_path = os.path.join(self.images_dir, layer_sum + '.tar')
> -            self.layers.append([sum_type, layer_sum, file_path, layer_size])
> +            self.layers.append([file_path, layer_size])
>  
>      def gen_valid_uri(self, uri):
>          """
> @@ -230,7 +235,10 @@ class DockerSource(object):
>          and have valid hash sum.
>          """
>          self.progress("Checking cached layers", value=0, logger=logger)
> -        for sum_type, sum_expected, path, _ignore in self.layers:
> +        for index, checksum in enumerate(self.checksums):
> +            path = self.layers[index][0]
> +            sum_type, sum_expected = checksum
> +
>              logger.debug("Checking layer: %s", path)
>              if not (os.path.exists(path)
>                      and utils.checksum(path, sum_type, sum_expected)):
> diff --git a/src/virtBootstrap/sources/file_source.py b/src/virtBootstrap/sources/file_source.py
> index c02f735..412db8a 100644
> --- a/src/virtBootstrap/sources/file_source.py
> +++ b/src/virtBootstrap/sources/file_source.py
> @@ -57,10 +57,11 @@ class FileSource(object):
>          if not os.path.isfile(self.path):
>              raise Exception('Invalid file source "%s"' % self.path)
>  
> +        layer = [[self.path, os.path.getsize(self.path)]]
>          if self.output_format == 'dir':
>              self.progress("Extracting files into destination directory",
>                            value=0, logger=logger)
> -            utils.safe_untar(self.path, dest)
> +            utils.untar_layers(layer, dest, self.progress)
>  
>          elif self.output_format == 'qcow2':
>              # Remove the old path
> diff --git a/src/virtBootstrap/utils.py b/src/virtBootstrap/utils.py
> index 9d477db..a04627e 100644
> --- a/src/virtBootstrap/utils.py
> +++ b/src/virtBootstrap/utils.py
> @@ -146,17 +146,16 @@ def size_to_bytes(number, fmt):
>              else False)
>  
>  
> -def log_layer_extract(layer, current, total, progress):
> +def log_layer_extract(tar_file, tar_size, current, total, progress):
>      """
>      Create log message on layer extract.
>      """
> -    sum_type, sum_value, layer_file, layer_size = layer
>      msg = 'Extracting layer (%s/%s)' % (current, total)
>  
> -    if layer_size:
> -        msg += " with size: %s" % bytes_to_size(layer_size)
> +    if tar_size:
> +        msg += " with size: %s" % bytes_to_size(tar_size)
>      progress(msg, logger=logger)
> -    logger.debug('Untar layer: (%s:%s) %s', sum_type, sum_value, layer_file)
> +    logger.debug('Untar layer: %s', tar_file)
>  
>  
>  def untar_layers(layers_list, dest_dir, progress):
> @@ -165,11 +164,11 @@ def untar_layers(layers_list, dest_dir, progress):
>      """
>      nlayers = len(layers_list)
>      for index, layer in enumerate(layers_list):
> -        log_layer_extract(layer, index + 1, nlayers, progress)
> -        layer_file = layer[2]
> +        tar_file, tar_size = layer
> +        log_layer_extract(tar_file, tar_size, index + 1, nlayers, progress)
>  
>          # Extract layer tarball into destination directory
> -        safe_untar(layer_file, dest_dir)
> +        safe_untar(tar_file, dest_dir)
>  
>          # Update progress value
>          progress(value=(float(index + 1) / nlayers * 50) + 50)
> @@ -246,8 +245,8 @@ def extract_layers_in_qcow2(layers_list, dest_dir, progress):
>  
>      nlayers = len(layers_list)
>      for index, layer in enumerate(layers_list):
> -        log_layer_extract(layer, index + 1, nlayers, progress)
> -        tar_file = layer[2]
> +        tar_file, tar_size = layer
> +        log_layer_extract(tar_file, tar_size, index + 1, nlayers, progress)
>  
>          # Name format for the qcow2 image
>          qcow2_layer_file = "{}/layer-{}.qcow2".format(dest_dir, index)
> diff --git a/tests/docker_source.py b/tests/docker_source.py
> index 5f9e6fe..9dc25d9 100644
> --- a/tests/docker_source.py
> +++ b/tests/docker_source.py
> @@ -333,9 +333,9 @@ class TestDockerSource(unittest.TestCase):
>          }
>  
>          expected_result = [
> -            ['sha256', 'a7050fc1', '/images_path/a7050fc1.tar', None],
> -            ['sha256', 'c6ff40b6', '/images_path/c6ff40b6.tar', None],
> -            ['sha256', '75c416ea', '/images_path/75c416ea.tar', None]
> +            ['/images_path/a7050fc1.tar', None],
> +            ['/images_path/c6ff40b6.tar', None],
> +            ['/images_path/75c416ea.tar', None]
>          ]
>  
>          with mock.patch('os.path.getsize') as m_getsize:
> @@ -364,9 +364,9 @@ class TestDockerSource(unittest.TestCase):
>          }
>  
>          expected_result = [
> -            ['sha256', '75c416ea', '/images_path/75c416ea.tar', 47103294],
> -            ['sha256', 'c6ff40b6', '/images_path/c6ff40b6.tar', 814],
> -            ['sha256', 'a7050fc1', '/images_path/a7050fc1.tar', 513]
> +            ['/images_path/75c416ea.tar', 47103294],
> +            ['/images_path/c6ff40b6.tar', 814],
> +            ['/images_path/a7050fc1.tar', 513]
>          ]
>  
>          src_instance = self._mock_retrieve_layers_info(manifest, kwargs)[0]

ACK

--
Cedric