Units created using commit history.

Added support for the depth option in the CLI.
closes #2594

docs/user-guide/release-notes/1.3.x.rst

@@ -0,0 +1,31 @@
+=============================
+Pulp OSTree 1.3 Release Notes
+=============================
+
+Pulp OSTree 1.3.0
+=================
+
+New Features
+------------
+
+- The importer's algorithm for creating content units has been enhanced.
+  A content unit is created for each branch HEAD commit. The importer then walks up
+  the commit ancestry and creates content units for each parent commit. This ensure there
+  are no gaps in the branch history. The tree traversal ``depth`` may limit how much of the
+  history is available.  The importer does not support cases where the remote repository has
+  used ``ostree reset`` to update a branch HEAD.
+
+- The CLI now supports the ``depth`` option which is used to configure the tree traversal
+  depth for both the importer and distributor.
+
+
+API Changes
+-----------
+
+None
+
+
+Bugs Fixed
+----------
+
+You can see the :fixedbugs_pulp_ostree:`list of bugs fixed<1.3.0>`.

docs/user-guide/release-notes/index.rst

@@ -9,3 +9,4 @@ Contents:
    1.0.x
    1.1.x
    1.2.x
+   1.3.x

extensions_admin/pulp_ostree/extensions/admin/cudl.py

@@ -38,6 +38,10 @@
 OPT_GPG_KEY = PulpCliOption(
     '--gpg-key', description, aliases=['-k'], required=False, allow_multiple=True)
 
+description = _('Tree traversal depth. Default: 0')
+
+OPT_DEPTH = PulpCliOption('--depth', description, aliases=['-d'], required=False)
+
 
 IMPORTER_CONFIGURATION_FLAGS = dict(
     include_ssl=True,
@@ -75,6 +79,7 @@ def __init__(self, context):
         self.add_option(OPT_RELATIVE_PATH)
         self.add_option(OPT_BRANCH)
         self.add_option(OPT_GPG_KEY)
+        self.add_option(OPT_DEPTH)
         self.options_bundle.opt_feed.description = DESC_FEED
 
     def _describe_distributors(self, user_input):
@@ -108,6 +113,11 @@ def _describe_distributors(self, user_input):
             constants.DISTRIBUTOR_CONFIG_KEY_RELATIVE_PATH: relative_path
         }
 
+        # traversal depth
+        depth = user_input.get(OPT_DEPTH.keyword)
+        if depth is not None:
+            config[constants.DISTRIBUTOR_CONFIG_KEY_DEPTH] = depth
+
         data = [
             dict(distributor_type_id=constants.WEB_DISTRIBUTOR_TYPE_ID,
                  distributor_config=config,
@@ -135,6 +145,9 @@ def _parse_importer_config(self, user_input):
         paths = user_input.pop(OPT_GPG_KEY.keyword, None)
         if paths:
             config[constants.IMPORTER_CONFIG_KEY_GPG_KEYS] = map(read, paths)
+        depth = user_input.get(OPT_DEPTH.keyword)
+        if depth is not None:
+            config[constants.IMPORTER_CONFIG_KEY_DEPTH] = depth
         return config
 
 
@@ -146,13 +159,19 @@ def __init__(self, context):
         self.add_option(OPT_AUTO_PUBLISH)
         self.add_option(OPT_BRANCH)
         self.add_option(OPT_GPG_KEY)
+        self.add_option(OPT_DEPTH)
         self.options_bundle.opt_feed.description = DESC_FEED
 
     def run(self, **kwargs):
         arg_utils.convert_removed_options(kwargs)
 
         importer_config = self.parse_user_input(kwargs)
 
+        # traversal depth
+        if OPT_DEPTH.keyword in kwargs:
+            value = kwargs.get(OPT_DEPTH.keyword)
+            importer_config[constants.IMPORTER_CONFIG_KEY_DEPTH] = value
+
         # branch list
         if OPT_BRANCH.keyword in kwargs:
             value = kwargs.pop(OPT_BRANCH.keyword)
@@ -171,7 +190,8 @@ def run(self, **kwargs):
 
         # Remove importer specific keys
         for key in importer_config.keys():
-            kwargs.pop(key, None)
+            if key not in (OPT_DEPTH.keyword,):
+                kwargs.pop(key, None)
 
         if importer_config:
             kwargs['importer_config'] = importer_config
@@ -183,6 +203,11 @@ def run(self, **kwargs):
         if value is not None:
             web_config['auto_publish'] = value
 
+        # traversal depth
+        depth = kwargs.pop(OPT_DEPTH.keyword, None)
+        if depth is not None:
+            web_config[constants.DISTRIBUTOR_CONFIG_KEY_DEPTH] = depth
+
         if web_config:
             kwargs['distributor_configs'] = {}
             kwargs['distributor_configs'][constants.CLI_WEB_DISTRIBUTOR_ID] = web_config

plugins/pulp_ostree/plugins/distributors/configuration.py

@@ -4,26 +4,26 @@
 
 from pulp_ostree.common import constants
 
-from mongoengine import Q
-from pulp.server.db import model
 
 _LOG = logging.getLogger(__name__)
 
 
-def validate_config(repo, config):
+def validate_config(repo, config, conduit):
     """
     Validate a configuration
 
     :param repo: metadata describing the repository to which the configuration applies
     :type  repo: pulp.plugins.model.Repository
     :param config: Pulp configuration for the distributor
     :type  config: pulp.plugins.config.PluginCallConfiguration
+    :param conduit: A configuration conduit.
+    :type conduit: pulp.plugins.conduits.repo_config.RepoConfigConduit
     :return: tuple of (bool, str) to describe the result
     :rtype:  tuple
     """
     repo_obj = repo.repo_obj
     relative_path = get_repo_relative_path(repo_obj, config)
-    error_msgs = _check_for_relative_path_conflicts(repo_obj.repo_id, relative_path)
+    error_msgs = _check_for_relative_path_conflicts(repo_obj.repo_id, relative_path, conduit)
 
     if error_msgs:
         return False, '\n'.join(error_msgs)
@@ -98,40 +98,30 @@ def get_repo_relative_path(repo, config):
     return path
 
 
-def _check_for_relative_path_conflicts(repo_id, relative_path):
+def _check_for_relative_path_conflicts(repo_id, relative_path, conduit):
     """
     Check that a relative path does not conflict with existing distributors' relative paths.
 
     :param repo_id: identifier of repository associated with the path to check
     :type  repo_id: basestring
     :param relative_path: relative path of the repository
     :type  relative_path: basestring
-    :return error_messages: a list of validation errors
+    :param conduit: A configuration conduit.
+    :type conduit: pulp.plugins.conduits.repo_config.RepoConfigConduit
+    :return: A list of validation error messages.
     :rtype: list
     """
-    current_url_pieces = [x for x in relative_path.split('/') if x]
-    matching_url_list = []
-    working_url = ''
-    for piece in current_url_pieces:
-        working_url = os.path.join(working_url, piece)
-        matching_url_list.append(working_url)
-        matching_url_list.append('/' + working_url)
-
-    # Search for all the sub urls as well as any url that would fall within the specified url.
-    # The regex here basically matches the a url if it starts with (optional preceding slash)
-    # the working url. Anything can follow as long as it is separated by a slash.
-    rel_url_match = Q(config__relative_path={'$regex': '^/?' + working_url + '(/.*|/?\z)'})
-    rel_url_in_list = Q(config__relative_path__in=matching_url_list)
-
-    conflicts = model.Distributor.objects(rel_url_match | rel_url_in_list).only('repo_id', 'config')
-    error_messages = []
-    for distributor in conflicts:
+    messages = []
+    distributors = conduit.get_repo_distributors_by_relative_url(relative_path, repo_id)
+    for distributor in distributors:
         conflicting_repo_id = distributor['repo_id']
-        conflicting_relative_url = None
         conflicting_relative_url = distributor['config']['relative_path']
-        msg = _('Relative path [{relative_path}] for repository [{repo_id}] conflicts with '
-                'existing relative path [{conflict_url}] for repository [{conflict_repo}]')
-        error_messages.append(msg.format(relative_path=relative_path, repo_id=repo_id,
-                                         conflict_url=conflicting_relative_url,
-                                         conflict_repo=conflicting_repo_id))
-    return error_messages
+        description = _('Relative path [{relative_path}] for repository [{repo_id}] conflicts '
+                        'with relative path [{conflict_url}] for repository [{conflict_repo}]')
+        msg = description.format(
+            relative_path=relative_path,
+            repo_id=repo_id,
+            conflict_url=conflicting_relative_url,
+            conflict_repo=conflicting_repo_id)
+        messages.append(msg)
+    return messages

plugins/pulp_ostree/plugins/distributors/web.py

@@ -161,4 +161,4 @@ def validate_config(self, repo, config, config_conduit):
 
         :raises: PulpCodedValidationException if any validations failed
         """
-        return configuration.validate_config(repo, config)
+        return configuration.validate_config(repo, config, config_conduit)

plugins/pulp_ostree/plugins/importers/steps.py

@@ -246,7 +246,7 @@ def __init__(self):
     def process_main(self, item=None):
         """
         Find all branch (heads) in the local repository and
-        create content units for them.
+        create content units for each branch and commit in the history.
         """
         lib_repository = lib.Repository(self.parent.storage_dir)
         for ref in lib_repository.list_refs():
@@ -257,16 +257,17 @@ def process_main(self, item=None):
                     # not listed
                     log.debug('skipping non-selected branch: {0}'.format(branch))
                     continue
-            unit = model.Branch(
-                remote_id=self.parent.remote_id,
-                branch=branch,
-                commit=ref.commit,
-                metadata=ref.metadata)
-            try:
-                unit.save()
-            except NotUniqueError:
-                unit = model.Branch.objects.get(**unit.unit_key)
-            associate_single_unit(self.get_repo().repo_obj, unit)
+            for commit in reversed(lib_repository.history(ref.commit)):
+                unit = model.Branch(
+                    remote_id=self.parent.remote_id,
+                    branch=branch,
+                    commit=commit.id,
+                    metadata=commit.metadata)
+                try:
+                    unit.save()
+                except NotUniqueError:
+                    unit = model.Branch.objects.get(**unit.unit_key)
+                associate_single_unit(self.get_repo().repo_obj, unit)
 
 
 class Clean(PluginStep):

plugins/pulp_ostree/plugins/lib.py

@@ -1,3 +1,4 @@
+from collections import namedtuple
 from logging import getLogger
 
 log = getLogger(__name__)
@@ -127,6 +128,12 @@ def dict(self):
         return dict(self.__dict__)
 
 
+# OSTree commit.
+# id (str): The commit hash.
+# metadata (dict): The commit metadata.
+Commit = namedtuple('Commit', ['id', 'metadata'])
+
+
 class Variant(object):
     """
     Variant encoding.
@@ -385,6 +392,36 @@ def pull_local(self, path, refs, depth=0):
         self.open()
         self.impl.pull_with_options(url, Variant.opt_dict(options), None, None)
 
+    def history(self, commit_id):
+        """
+        Get commit history.
+        Traversal of the commit hierarchy.
+        Depending on the traversal 'depth' setting, the entire commit
+        hierarchy may not have been pulled.  This is detected when GError
+        is raised.  Unfortunately, a more specific exception is not raised.
+
+        :param commit_id: A commit (hash) used as the starting point for the traversal.
+        :type  commit_id: str
+        :return: A list of: Commit.
+        :rtype: list
+        """
+        lib = Lib()
+        self.open()
+        history = []
+        while commit_id:
+            try:
+                _, commit = self.impl.load_variant(lib.OSTree.ObjectType.COMMIT, commit_id)
+            except lib.GLib.GError as le:
+                if history:
+                    # parent not pulled.
+                    log.debug(le)
+                    break
+                else:
+                    raise
+            history.append(Commit(id=commit_id, metadata=commit[0]))
+            commit_id = lib.OSTree.commit_get_parent(commit)
+        return history
+
 
 class Remote(object):
     """

plugins/test/unit/plugins/distributors/test_configuration.py

@@ -49,11 +49,12 @@ def test_get_repo_relative_path_when_passed(self):
         self.assertEquals(directory, relative_path[1:])
 
 
-@mock.patch('pulp_ostree.plugins.distributors.configuration.model.Distributor.objects')
 class TestValidateConfig(unittest.TestCase):
 
-    def test_server_url_fully_qualified(self, mock_dist_qs):
+    def test_server_url_fully_qualified(self):
         m_repo = mock.MagicMock()
+        conduit = mock.MagicMock()
+        conduit.get_repo_distributors_by_relative_url.return_value = []
         config = PluginCallConfiguration({}, {})
         self.assertEquals(
-            (True, None), configuration.validate_config(m_repo, config))
+            (True, None), configuration.validate_config(m_repo, config, conduit))

plugins/test/unit/plugins/distributors/test_web.py

@@ -84,6 +84,8 @@ def test_cancel_publish_repo(self):
     @patch('pulp_ostree.plugins.distributors.web.configuration.validate_config')
     def test_validate_config(self, mock_validate):
         m_repo = Mock()
-        value = self.distributor.validate_config(m_repo, 'foo', Mock())
-        mock_validate.assert_called_once_with(m_repo, 'foo')
+        conduit = Mock()
+        repository = Mock()
+        value = self.distributor.validate_config(m_repo, repository, conduit)
+        mock_validate.assert_called_once_with(m_repo, repository, conduit)
         self.assertEquals(value, mock_validate.return_value)