Sync v1 and v2 content when the registry support it.

plugins/pulp_docker/plugins/importers/importer.py

@@ -10,7 +10,7 @@
 
 from pulp_docker.common import constants, tarutils
 from pulp_docker.common.models import Image, Manifest, Blob
-from pulp_docker.plugins.importers import sync, upload, v1_sync
+from pulp_docker.plugins.importers import sync, upload
 
 
 _logger = logging.getLogger(__name__)
@@ -80,19 +80,9 @@ def sync_repo(self, repo, sync_conduit, config):
         """
         working_dir = tempfile.mkdtemp(dir=repo.working_dir)
         try:
-            try:
-                # This will raise NotImplementedError if the config's feed_url is determined not to
-                # support the Docker v2 API.
-                self.sync_step = sync.SyncStep(repo=repo, conduit=sync_conduit, config=config,
-                                               working_dir=working_dir)
-            except NotImplementedError:
-                # Since the feed_url was determined not to support the Docker v2 API, let's use the
-                # old v1 SyncStep instead.
-                self.sync_step = v1_sync.SyncStep(repo=repo, conduit=sync_conduit, config=config,
-                                                  working_dir=working_dir)
-
+            self.sync_step = sync.SyncStep(repo=repo, conduit=sync_conduit, config=config,
+                                           working_dir=working_dir)
             return self.sync_step.sync()
-
         finally:
             shutil.rmtree(working_dir, ignore_errors=True)
 

plugins/pulp_docker/plugins/importers/sync.py

@@ -13,6 +13,7 @@
 
 from pulp_docker.common import constants, models
 from pulp_docker.plugins import registry
+from pulp_docker.plugins.importers import v1_sync
 
 
 _logger = logging.getLogger(__name__)
@@ -50,6 +51,75 @@ def __init__(self, repo=None, conduit=None, config=None,
                                        working_dir, constants.IMPORTER_TYPE_ID)
         self.description = _('Syncing Docker Repository')
 
+        download_config = nectar_config.importer_config_to_nectar_config(config.flatten())
+        upstream_name = config.get(constants.CONFIG_KEY_UPSTREAM_NAME)
+        url = config.get(importer_constants.KEY_FEED)
+
+        # Create a Repository object to interact with.
+        self.index_repository = registry.V2Repository(
+            upstream_name, download_config, url, working_dir)
+        self.v1_index_repository = registry.V1Repository(upstream_name, download_config, url,
+                                                         working_dir)
+
+        v2_found = self.index_repository.api_version_check()
+        v1_found = self.v1_index_repository.api_version_check()
+
+        if v2_found:
+            _logger.debug(_('v2 API found'))
+            self.add_child(V2SyncStep(repo=repo, conduit=conduit, config=config,
+                                      working_dir=working_dir))
+        if v1_found:
+            _logger.debug(_('v1 API found'))
+            self.add_child(v1_sync.SyncStep(repo=repo, conduit=conduit, config=config,
+                                            working_dir=working_dir))
+        if not any((v1_found, v2_found)):
+            msg = _('This feed URL is not a Docker v1 or v2 endpoint: %(url)s'.format(url=url))
+            _logger.error(msg)
+            raise ValueError(msg)
+
+    def sync(self):
+        """
+        actually initiate the sync
+
+        :return:    a final sync report
+        :rtype:     pulp.plugins.model.SyncReport
+        """
+        self.process_lifecycle()
+        return self._build_final_report()
+
+
+class V2SyncStep(PluginStep):
+    """
+    This PluginStep is the primary entry point into a repository sync against a Docker v2 registry.
+    """
+    # The sync will fail if these settings are not provided in the config
+    required_settings = (constants.CONFIG_KEY_UPSTREAM_NAME, importer_constants.KEY_FEED)
+
+    def __init__(self, repo=None, conduit=None, config=None,
+                 working_dir=None):
+        """
+        This method initializes the SyncStep. It first validates the config to ensure that the
+        required keys are present. It then constructs some needed items (such as a download config),
+        and determines whether the feed URL is a Docker v2 registry or not. If it is, it
+        instantiates child tasks that are appropriate for syncing a v2 registry, and if it is not it
+        raises a NotImplementedError.
+
+        :param repo:        repository to sync
+        :type  repo:        pulp.plugins.model.Repository
+        :param conduit:     sync conduit to use
+        :type  conduit:     pulp.plugins.conduits.repo_sync.RepoSyncConduit
+        :param config:      config object for the sync
+        :type  config:      pulp.plugins.config.PluginCallConfiguration
+        :param working_dir: full path to the directory in which transient files
+                            should be stored before being moved into long-term
+                            storage. This should be deleted by the caller after
+                            step processing is complete.
+        :type  working_dir: basestring
+        """
+        super(V2SyncStep, self).__init__(constants.SYNC_STEP_MAIN, repo, conduit, config,
+                                         working_dir, constants.IMPORTER_TYPE_ID)
+        self.description = _('Syncing Docker Repository')
+
         self._validate(config)
         download_config = nectar_config.importer_config_to_nectar_config(config.flatten())
         upstream_name = config.get(constants.CONFIG_KEY_UPSTREAM_NAME)
@@ -93,16 +163,6 @@ def generate_download_requests(self):
             yield self.index_repository.create_blob_download_request(digest,
                                                                      self.get_working_dir())
 
-    def sync(self):
-        """
-        actually initiate the sync
-
-        :return:    a final sync report
-        :rtype:     pulp.plugins.model.SyncReport
-        """
-        self.process_lifecycle()
-        return self._build_final_report()
-
     @classmethod
     def _validate(cls, config):
         """

plugins/pulp_docker/plugins/registry.py

@@ -25,6 +25,7 @@ class V1Repository(object):
     DOCKER_ENDPOINT_HEADER = 'x-docker-endpoints'
     IMAGES_PATH = '/v1/repositories/%s/images'
     TAGS_PATH = '/v1/repositories/%s/tags'
+    API_VERSION_CHECK_PATH = '/v1/_ping'
 
     def __init__(self, name, download_config, registry_url, working_dir):
         """
@@ -104,6 +105,23 @@ def _parse_response_headers(self, headers):
         if self.DOCKER_ENDPOINT_HEADER in headers:
             self.endpoint = headers[self.DOCKER_ENDPOINT_HEADER]
 
+    def api_version_check(self):
+        """
+        Make a call to the registry URL's /v1/_ping API call to determine if the registry supports
+        API v1.
+
+        :return: True if the v1 API is found, else False
+        :rtype:  bool
+        """
+        _logger.debug('Determining if the registry URL can do v1 of the Docker API.')
+
+        try:
+            self._get_single_path(self.API_VERSION_CHECK_PATH)
+        except IOError:
+            return False
+
+        return True
+
     def add_auth_header(self, request):
         """
         Given a download request, add an Authorization header if we have an
@@ -267,26 +285,30 @@ def api_version_check(self):
         """
         Make a call to the registry URL's /v2/ API call to determine if the registry supports API
         v2. If it does not, raise NotImplementedError. If it does, return.
+
+        :return: True if the v2 API is found, else False
+        :rtype:  bool
         """
         _logger.debug('Determining if the registry URL can do v2 of the Docker API.')
-        exception = NotImplementedError('%s is not a Docker v2 registry.' % self.registry_url)
 
         try:
             headers, body = self._get_path(self.API_VERSION_CHECK_PATH)
         except IOError:
-            raise exception
+            return False
 
         try:
             version = headers['Docker-Distribution-API-Version']
             if version != "registry/2.0":
-                raise exception
+                return False
             _logger.debug(_('The docker registry is using API version: %(v)s') % {'v': version})
         except KeyError:
             # If the Docker-Distribution-API-Version header isn't present, we will assume that this
             # is a valid Docker 2.0 API server so that simple file-based webservers can serve as our
             # remote feed.
             pass
 
+        return True
+
     def create_blob_download_request(self, digest, destination_dir):
         """
         Return a DownloadRequest instance for the given blob digest.

plugins/test/unit/plugins/importers/test_importer.py

@@ -65,21 +65,6 @@ def test_calls_sync(self, mock_rmtree, mock_mkdtemp, mock_sync_step):
 
         mock_sync_step.return_value.sync.assert_called_once_with()
 
-    @mock.patch('pulp_docker.plugins.importers.v1_sync.SyncStep')
-    def test_fall_back_to_v1(self, v1_sync_step, mock_rmtree, mock_mkdtemp, mock_sync_step):
-        """
-        Ensure that the sync_repo() method falls back to Docker v1 if Docker v2 isn't available.
-        """
-        # Simulate the v2 API being unavailable
-        mock_sync_step.side_effect = NotImplementedError()
-
-        self.importer.sync_repo(self.repo, self.sync_conduit, self.config)
-
-        v1_sync_step.assert_called_once_with(
-            repo=self.repo, conduit=self.sync_conduit, config=self.config,
-            working_dir=mock_mkdtemp.return_value)
-        v1_sync_step.return_value.sync.assert_called_once_with()
-
     def test_makes_temp_dir(self, mock_rmtree, mock_mkdtemp, mock_sync_step):
         self.importer.sync_repo(self.repo, self.sync_conduit, self.config)
 

plugins/test/unit/plugins/importers/test_sync.py

@@ -92,7 +92,7 @@ def test_process_main_with_one_layer(self, super_process_main, from_json):
     def test_process_main_with_repeated_layers(self, super_process_main, from_json):
         """
         Test process_main() when the various tags contains some layers in common, which is a
-        typical pattern. The available_units set on the SyncStep should only have the layers once
+        typical pattern. The available_units set on the V2SyncStep should only have the layers once
         each so that we don't try to download them more than once.
         """
         repo = mock.MagicMock()
@@ -472,11 +472,11 @@ def test_process_main_no_units(self, _move_file):
         self.assertEqual(step.parent.get_conduit.return_value.save_unit.call_count, 0)
 
 
-class TestSyncStep(unittest.TestCase):
+class TestV2SyncStep(unittest.TestCase):
     """
-    This class contains tests for the SyncStep class.
+    This class contains tests for the V2SyncStep class.
     """
-    @mock.patch('pulp_docker.plugins.importers.sync.SyncStep._validate')
+    @mock.patch('pulp_docker.plugins.importers.sync.V2SyncStep._validate')
     @mock.patch('pulp_docker.plugins.registry.V2Repository.api_version_check')
     def test___init___with_v2_registry(self, api_version_check, _validate):
         """
@@ -491,7 +491,7 @@ def test___init___with_v2_registry(self, api_version_check, _validate):
              importer_constants.KEY_MAX_DOWNLOADS: 25})
         working_dir = '/some/path'
 
-        step = sync.SyncStep(repo, conduit, config, working_dir)
+        step = sync.V2SyncStep(repo, conduit, config, working_dir)
 
         self.assertEqual(step.description, _('Syncing Docker Repository'))
         # The config should get validated
@@ -529,26 +529,6 @@ def test___init___with_v2_registry(self, api_version_check, _validate):
         # And the final step
         self.assertEqual(step.children[3].working_dir, working_dir)
 
-    @mock.patch('pulp_docker.plugins.importers.sync.SyncStep._validate')
-    def test___init___without_v2_registry(self, _validate):
-        """
-        Test the __init__() method when the V2Repository raises a NotImplementedError with the
-        api_version_check() method, indicating that the feed URL is not a Docker v2 registry.
-        """
-        repo = mock.MagicMock()
-        conduit = mock.MagicMock()
-        # This feed does not implement a registry, so it will raise the NotImplementedError
-        config = plugin_config.PluginCallConfiguration(
-            {},
-            {'feed': 'https://registry.example.com', 'upstream_name': 'busybox',
-             importer_constants.KEY_MAX_DOWNLOADS: 25})
-        working_dir = '/some/path'
-
-        self.assertRaises(NotImplementedError, sync.SyncStep, repo, conduit, config, working_dir)
-
-        # The config should get validated
-        _validate.assert_called_once_with(config)
-
     @mock.patch('pulp_docker.plugins.registry.V2Repository.api_version_check', mock.MagicMock())
     def test_generate_download_requests(self):
         """
@@ -561,7 +541,7 @@ def test_generate_download_requests(self):
             {'feed': 'https://registry.example.com', 'upstream_name': 'busybox',
              importer_constants.KEY_MAX_DOWNLOADS: 25})
         working_dir = '/some/path'
-        step = sync.SyncStep(repo, conduit, config, working_dir)
+        step = sync.V2SyncStep(repo, conduit, config, working_dir)
         step.step_get_local_units.units_to_download = [
             {'digest': i} for i in ['cool', 'stuff']]
 
@@ -582,7 +562,7 @@ def test_required_settings(self):
         """
         Assert that the required_settings class attribute is set correctly.
         """
-        self.assertEqual(sync.SyncStep.required_settings,
+        self.assertEqual(sync.V2SyncStep.required_settings,
                          (constants.CONFIG_KEY_UPSTREAM_NAME, importer_constants.KEY_FEED))
 
     @mock.patch('pulp_docker.plugins.registry.V2Repository.api_version_check', mock.MagicMock())
@@ -614,7 +594,7 @@ def test__validate_missing_one_key(self):
             {}, {'upstream_name': 'busybox', importer_constants.KEY_MAX_DOWNLOADS: 25})
 
         try:
-            sync.SyncStep._validate(config)
+            sync.V2SyncStep._validate(config)
             self.fail('An Exception should have been raised, but was not!')
         except exceptions.MissingValue as e:
             self.assertEqual(e.property_names, ['feed'])
@@ -627,7 +607,7 @@ def test__validate_missing_two_keys(self):
             {}, {importer_constants.KEY_MAX_DOWNLOADS: 25})
 
         try:
-            sync.SyncStep._validate(config)
+            sync.V2SyncStep._validate(config)
             self.fail('An Exception should have been raised, but was not!')
         except exceptions.MissingValue as e:
             self.assertEqual(set(e.property_names), set(['upstream_name', 'feed']))
@@ -642,4 +622,4 @@ def test__validate_success_case(self):
              importer_constants.KEY_MAX_DOWNLOADS: 25})
 
         # This should not raise an Exception
-        sync.SyncStep._validate(config)
+        sync.V2SyncStep._validate(config)

plugins/test/unit/plugins/test_registry.py

@@ -372,7 +372,7 @@ def download_one(request):
         r = registry.V2Repository(name, download_config, registry_url, working_dir)
         r.downloader.download_one = mock.MagicMock(side_effect=download_one)
 
-        self.assertRaises(NotImplementedError, r.api_version_check)
+        self.assertEqual(r.api_version_check(), False)
 
     def test_api_version_check_ioerror(self):
         """
@@ -384,8 +384,8 @@ def test_api_version_check_ioerror(self):
         working_dir = '/a/working/dir'
         r = registry.V2Repository(name, download_config, registry_url, working_dir)
 
-        # The IOError will be raised since registry_url isn't a real registry
-        self.assertRaises(NotImplementedError, r.api_version_check)
+        # False will be returned since registry_url isn't a real registry
+        self.assertEqual(r.api_version_check(), False)
 
     def test_api_version_check_missing_header(self):
         """