From 891709e5898a26d9f4f02828ff122f7600f6e35c Mon Sep 17 00:00:00 2001 From: Jeremy Cline Date: Wed, 30 Jul 2014 17:19:54 -0400 Subject: [PATCH] 1123446 - Syncing against a directory uses 'modulename' rather than 'authorname-modulename' as the module name --- .../pulp_puppet/common/model.py | 12 ++++++-- .../test/unit/test_common_model.py | 22 +++++++++++--- .../plugins/importers/directory.py | 2 +- .../unit/plugins/importer/test_directory.py | 17 +++++++---- .../tools/puppet_module_builder.py | 30 +++++++++---------- 5 files changed, 55 insertions(+), 28 deletions(-) diff --git a/pulp_puppet_common/pulp_puppet/common/model.py b/pulp_puppet_common/pulp_puppet/common/model.py index 9cd9e424..749b6ead 100644 --- a/pulp_puppet_common/pulp_puppet/common/model.py +++ b/pulp_puppet_common/pulp_puppet/common/model.py @@ -112,7 +112,10 @@ def from_unit(cls, pulp_unit): @classmethod def from_json(cls, module_json): """ - Converts a module's metadata.json to a Module representation. + Converts a module's metadata.json to a Module representation. This + method does not use the 'author' field of metadata.json. The author + is retrieved from the 'name' field, which is expected to be in the + format 'authorname-modulename' or 'authorname/modulename'. :param module_json: dict with values from metadata.json :type module_json: dict @@ -121,7 +124,12 @@ def from_json(cls, module_json): :rtype: Module """ # The unique identifier fields are all required and should be present - author, name = module_json.get('name').split("-", 1) + try: + author, name = module_json.get('name').split("-", 1) + except ValueError: + # This is the old naming format, but Puppet still allows it + author, name = module_json.get('name').split("/", 1) + version = module_json.get('version') module = cls(name, version, author) diff --git a/pulp_puppet_common/test/unit/test_common_model.py b/pulp_puppet_common/test/unit/test_common_model.py index a9b62f87..55929939 100644 --- a/pulp_puppet_common/test/unit/test_common_model.py +++ b/pulp_puppet_common/test/unit/test_common_model.py @@ -139,10 +139,6 @@ def test_to_json(self): class ModuleTests(unittest.TestCase): - def test_from_dict(self): - # Setup - pass - def test_update_from_json(self): # Setup module = Module('jdob-valid', '1.0.0', 'jdob') @@ -177,6 +173,24 @@ def test_from_json(self): module.name = "jdob-valid" # rename the module to use the assert self.assert_valid_module(module) + def test_from_json_old_name(self): + """ + Test that the Module.from_json method handles the old naming style + """ + # Setup + metadata = { + 'name': 'oldauthor/oldmodule', + 'version': '0.1.0', + } + + # Test + module = Module.from_json(metadata) + + # Verify + self.assertEqual(module.author, 'oldauthor') + self.assertEqual(module.name, 'oldmodule') + self.assertEqual(module.version, '0.1.0') + def assert_valid_module(self, module): self.assertEqual(module.name, 'jdob-valid') self.assertEqual(module.version, '1.0.0') diff --git a/pulp_puppet_plugins/pulp_puppet/plugins/importers/directory.py b/pulp_puppet_plugins/pulp_puppet/plugins/importers/directory.py index 6640c23d..aa9f418b 100644 --- a/pulp_puppet_plugins/pulp_puppet/plugins/importers/directory.py +++ b/pulp_puppet_plugins/pulp_puppet/plugins/importers/directory.py @@ -271,7 +271,7 @@ def _import_modules(self, inventory, module_paths): if self.canceled: return [] puppet_manifest = self._extract_metadata(module_path) - module = Module.from_dict(puppet_manifest) + module = Module.from_json(puppet_manifest) if inventory.already_associated(module): continue _LOG.info(IMPORT_MODULE % dict(mod=module_path)) diff --git a/pulp_puppet_plugins/test/unit/plugins/importer/test_directory.py b/pulp_puppet_plugins/test/unit/plugins/importer/test_directory.py index 047adec6..3df93997 100644 --- a/pulp_puppet_plugins/test/unit/plugins/importer/test_directory.py +++ b/pulp_puppet_plugins/test/unit/plugins/importer/test_directory.py @@ -399,14 +399,21 @@ def test_import_modules(self, mock_extract, mock_add): mock_inventory = Mock() mock_inventory.already_associated.side_effect = [False, True, False] + # These manifests represent the parsed metadata.json file. These contain a 'name' + # field, where we retrieve both the unit key's 'name' and 'author' field. manifests = [ + {'name': 'john-pulp1', 'author': 'Johnathon', 'version': '1.0'}, + {'name': 'john-pulp2', 'author': 'Johnathon', 'version': '2.0'}, + {'name': 'john/pulp3', 'author': 'Johnathon', 'version': '3.0'}, + ] + mock_extract.side_effect = manifests + + unit_keys = [ {'name': 'pulp1', 'author': 'john', 'version': '1.0'}, {'name': 'pulp2', 'author': 'john', 'version': '2.0'}, {'name': 'pulp3', 'author': 'john', 'version': '3.0'}, ] - mock_extract.side_effect = manifests - module_paths = [ '/tmp/module_1', '/tmp/module_2', @@ -414,19 +421,17 @@ def test_import_modules(self, mock_extract, mock_add): ] # test - method = SynchronizeWithDirectory(conduit, config) imported_modules = method._import_modules(mock_inventory, module_paths) # validation - mock_add.assert_any_with(module_paths[0], ANY) mock_add.assert_any_with(module_paths[2], ANY) # should only be modules 1 and 3. 2 already associated. self.assertEqual(len(imported_modules), 2) - self.assertEqual(imported_modules[0], manifests[0]) - self.assertEqual(imported_modules[1], manifests[2]) + self.assertEqual(imported_modules[0], unit_keys[0]) + self.assertEqual(imported_modules[1], unit_keys[2]) @patch('pulp_puppet.plugins.importers.directory.SynchronizeWithDirectory._extract_metadata') def test_import_modules_cancelled(self, mock_extract): diff --git a/pulp_puppet_tools/pulp_puppet/tools/puppet_module_builder.py b/pulp_puppet_tools/pulp_puppet/tools/puppet_module_builder.py index 5009b024..f619be74 100644 --- a/pulp_puppet_tools/pulp_puppet/tools/puppet_module_builder.py +++ b/pulp_puppet_tools/pulp_puppet/tools/puppet_module_builder.py @@ -47,20 +47,20 @@ DESCRIPTION = _( 'Build puppet modules.' - 'Search the working directory and build all puppet modules found. The working' - 'directory is the current directory unless the (-w|--working-dir) option is specified.' - 'The (-p|--path) option may be used to specify a directory to search/build and' - 'can be either an absolute path or a path relative to the working directory.' - 'The archive built using \'puppet module build\' is copied to the output directory' - 'The output directory is the current directory unless (-o|--output-dir) is' - 'specified. The output directory may be either an absolute path or a path that is' - 'relative to the working directory.' - '\nSeveral options are provided for working with git. Repositories may be cloned' - 'by specifying the (-u|--url) option. After cloning git repositories, the (-p|--path)' - 'is set to the root of the cloned repository unless specified explicitly.' - 'The repository branch may be selected by using the (-b|--branch) option.' - 'In all cases, when the working directory is a git repository, a \'git pull\' is' - 'performed to ensure that the repository is up to date.' + ' Search the working directory and build all puppet modules found. The working' + ' directory is the current directory unless the (-w|--working-dir) option is specified.' + ' The (-p|--path) option may be used to specify a directory to search/build and' + ' can be either an absolute path or a path relative to the working directory.' + ' The archive built using \'puppet module build\' is copied to the output directory' + ' The output directory is the current directory unless (-o|--output-dir) is' + ' specified. The output directory may be either an absolute path or a path that is' + ' relative to the working directory.' + ' \nSeveral options are provided for working with git. Repositories may be cloned' + ' by specifying the (-u|--url) option. After cloning git repositories, the (-p|--path)' + ' is set to the root of the cloned repository unless specified explicitly.' + ' The repository branch may be selected by using the (-b|--branch) option.' + ' In all cases, when the working directory is a git repository, a \'git pull\' is' + ' performed to ensure that the repository is up to date.' '\n') BAD_PATH = _('(-p|--path) must be a relative path') @@ -215,7 +215,7 @@ def git_checkout(options): def find_modules(): """ Search for puppet (source) modules to build and return a list of paths. - Puppet modules are identified by finding '/modules/init.pp' + Puppet modules are identified by finding '/manifests/init.pp' files. Once found, the *module* directory path is included in the result. :return: A set of puppet module directory paths.