[virt-tools-list] [virt-bootstrap] [PATCH 1/1] docker-source: Get list of layers without `--raw`

[Radostin Stoyanov]

On 19/12/17 10:12, Cedric Bosdonnat wrote:
> On Mon, 2017-12-18 at 19:20 +0000, Radostin Stoyanov wrote:
>> When `skopeo inspect --raw docker://feodra` is used the returned
>> manifest content contains a list with manifests for specific
>> platforms [1] rather than a list with layers.
>>
>> By using `skopeo inpect docker://fedora` the correct manifest
>> content is retrieved and a list with layers is provided. In addition,
>> skopeo handles the difference between schemaVersion 1 and 2.
>>
>> [1] https://docs.docker.com/registry/spec/manifest-v2-2/#manifest-list-field-descriptions
>> ---
>>  src/virtBootstrap/sources/docker_source.py | 29 ++++++-------
>>  tests/docker_source.py                     | 66 +++++-------------------------
>>  2 files changed, 22 insertions(+), 73 deletions(-)
>>
>> diff --git a/src/virtBootstrap/sources/docker_source.py b/src/virtBootstrap/sources/docker_source.py
>> index ec1a812..0eace81 100644
>> --- a/src/virtBootstrap/sources/docker_source.py
>> +++ b/src/virtBootstrap/sources/docker_source.py
>> @@ -77,7 +77,7 @@ class DockerSource(object):
>>          self.no_cache = kwargs.get('no_cache', False)
>>          self.progress = kwargs['progress'].update_progress
>>          self.images_dir = utils.get_image_dir(self.no_cache)
>> -        self.manifest = None
>> +        self.image_details = None
>>          self.layers = []
>>          self.checksums = []
>>  
>> @@ -91,30 +91,25 @@ class DockerSource(object):
>>          Retrive manifest from registry and get layers' digest,
>>          sum_type, size and file_path in a list.
>>          """
>> -        self.manifest = utils.get_image_details(self.url, raw=True,
>> +        image_details = utils.get_image_details(self.url, raw=False,
>>                                                  insecure=self.insecure,
>>                                                  username=self.username,
>>                                                  password=self.password)
>>  
>> -        if self.manifest['schemaVersion'] == 1:
>> -            layers_list = self.manifest['fsLayers'][::-1]
>> -            digest_field = 'blobSum'
>> -        elif self.manifest['schemaVersion'] == 2:
>> -            layers_list = self.manifest['layers']
>> -            digest_field = 'digest'
>> -        else:
>> -            raise ValueError('Unsupported manifest schema.')
>> +        if not 'Layers' in image_details or not image_details['Layers']:
>> +            raise ValueError('No image layers.')
>>  
>> -        for layer in layers_list:
>> -            # Store checksums of layers
>> -            layer_digest = layer[digest_field]
>> +        # Layers are in order:
>> +        # - root layer first, and then successive layered layers
>> +        # Ref: https://github.com/containers/image/blob/master/image/oci.go
>> +        for layer_digest in image_details['Layers']:
>>              sum_type, layer_sum = layer_digest.split(':')
>> -            self.checksums.append([sum_type, layer_sum])
>> +            self.checksums.append([sum_type, layer_sum]) # Store checksums
>>  
>> -            # Store file path and size of each layer
>> +            # Layers are tar files with hashsum used as name
>>              file_path = os.path.join(self.images_dir, layer_sum + '.tar')
>> -            layer_size = layer.get('size', None)
>> -            self.layers.append([file_path, layer_size])
>> +            # Store 'file path' and set placeholder for 'size'
>> +            self.layers.append([file_path, None])
>>  
>>      def gen_valid_uri(self, uri):
>>          """
>> diff --git a/tests/docker_source.py b/tests/docker_source.py
>> index 9090988..c8f4e08 100644
>> --- a/tests/docker_source.py
>> +++ b/tests/docker_source.py
>> @@ -90,11 +90,8 @@ class CreateLayers(object):
>>          """
>>          return {
>>              "schemaVersion": 2,
>> -            "layers": [
>> -                {
>> -                    "digest":
>> -                    "sha256:" + os.path.basename(layer).split('.')[0]
>> -                }
>> +            "Layers": [
>> +                "sha256:" + os.path.basename(layer).split('.')[0]
>>                  for layer in self.layers
>>              ]
>>          }
>> @@ -340,7 +337,7 @@ class TestDockerSource(unittest.TestCase):
>>              'progress': mock.Mock()
>>          }
>>  
>> -        manifest = {'schemaVersion': 2, 'layers': []}
>> +        manifest = {'schemaVersion': 2, 'Layers': ['sha256:a7050fc1']}
>>          (src_instance,
>>           m_uri, m_utils) = self._mock_retrieve_layers_info(manifest,
>>                                                             src_kwargs)
>> @@ -349,7 +346,7 @@ class TestDockerSource(unittest.TestCase):
>>              'insecure': src_instance.insecure,
>>              'username': src_instance.username,
>>              'password': src_instance.password,
>> -            'raw': True
>> +            'raw': False
>>          }
>>          m_utils['get_image_details'].assert_called_once_with(m_uri(), **kwargs)
>>  
>> @@ -365,11 +362,11 @@ class TestDockerSource(unittest.TestCase):
>>          }
>>  
>>          manifest = {
>> -            'schemaVersion': 1,
>> -            'fsLayers': [
>> -                {'blobSum': 'sha256:75c416ea'},
>> -                {'blobSum': 'sha256:c6ff40b6'},
>> -                {'blobSum': 'sha256:a7050fc1'}
>> +            'schemaVersion': 2,
>> +            'Layers': [
>> +                'sha256:a7050fc1',
>> +                'sha256:c6ff40b6',
>> +                'sha256:75c416ea'
>>              ]
>>          }
>>  
>> @@ -382,47 +379,4 @@ class TestDockerSource(unittest.TestCase):
>>          with mock.patch('os.path.getsize') as m_getsize:
>>              m_getsize.return_value = None
>>              src_instance = self._mock_retrieve_layers_info(manifest, kwargs)[0]
>> -        self.assertEqual(src_instance.layers, expected_result)
>> -
>> -    def test_retrieve_layers_info_schema_version_2(self):
>> -        """
>> -        Ensures that retrieve_layers_info() extracts the layers' information
>> -        from manifest with schema version 2 a list with format:
>> -            ["digest", "sum_type", "file_path", "size"].
>> -        """
>> -        kwargs = {
>> -            'uri': '',
>> -            'progress': mock.Mock()
>> -        }
>> -
>> -        manifest = {
>> -            'schemaVersion': 2,
>> -            "layers": [
>> -                {"size": 47103294, "digest": "sha256:75c416ea"},
>> -                {"size": 814, "digest": "sha256:c6ff40b6"},
>> -                {"size": 513, "digest": "sha256:a7050fc1"}
>> -            ]
>> -        }
>> -
>> -        expected_result = [
>> -            ['/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]
>> -        self.assertEqual(src_instance.layers, expected_result)
>> -
>> -    def test_retrieve_layers_info_raise_error_on_invalid_schema_version(self):
>> -        """
>> -        Ensures that retrieve_layers_info() calls get_image_details()
>> -        with all passed arguments.
>> -        """
>> -        kwargs = {
>> -            'uri': '',
>> -            'progress': mock.Mock()
>> -        }
>> -
>> -        manifest = {'schemaVersion': 3}
>> -        with self.assertRaises(ValueError):
>> -            self._mock_retrieve_layers_info(manifest, kwargs)
>> +        self.assertEqual(src_instance.layers, expected_result)
>> \ No newline at end of file
> ACK
>
> --
> Cedric
Thanks, pushed now.