From 2474839f2afb83f5d4032e8ac6f9a3f92e72b0bc Mon Sep 17 00:00:00 2001 From: Dougal Matthews Date: Tue, 23 Sep 2014 15:24:51 +0100 Subject: [PATCH] Change role loading to explicitly list roles This change comes after the previous approach caused issues for element creation. Change-Id: Ieaa9acb03cd9b00ba61eb05594b04456cf3683c3 --- tuskar/cmd/load_roles.py | 4 +- tuskar/storage/load_roles.py | 34 ++++----------- tuskar/tests/cmd/test_load_roles.py | 10 ++--- tuskar/tests/storage/test_load_roles.py | 56 ++++++++----------------- 4 files changed, 31 insertions(+), 73 deletions(-) diff --git a/tuskar/cmd/load_roles.py b/tuskar/cmd/load_roles.py index 24ee2238..ecbd2142 100644 --- a/tuskar/cmd/load_roles.py +++ b/tuskar/cmd/load_roles.py @@ -34,7 +34,7 @@ seed_help = ('Full path to the template that should be loaded ' 'as the master seed') cfg.CONF.register_cli_opt(cfg.StrOpt('master-seed', dest='master_seed', help=seed_help)) -cfg.CONF.register_cli_opt(cfg.StrOpt('directory', positional=True)) +cfg.CONF.register_cli_opt(cfg.MultiStrOpt('role', short='r')) def main(argv=None): @@ -44,7 +44,7 @@ def main(argv=None): service.prepare_service(argv) - all_roles, created, updated = load_roles(cfg.CONF.directory, + all_roles, created, updated = load_roles(cfg.CONF.role, seed_file=cfg.CONF.master_seed, dry_run=cfg.CONF.dry_run) diff --git a/tuskar/storage/load_roles.py b/tuskar/storage/load_roles.py index 4417bad1..cdf60f81 100644 --- a/tuskar/storage/load_roles.py +++ b/tuskar/storage/load_roles.py @@ -17,7 +17,6 @@ from __future__ import print_function -from os import listdir from os import path from tuskar.storage.exceptions import UnknownName @@ -28,25 +27,6 @@ from tuskar.storage.stores import TemplateStore MASTER_SEED_NAME = '_master_seed' -def _list_roles(directory): - """Scan a directory and yield a tuple for all the roles containing the - role name and the full path to the role. - """ - - if not path.isdir(directory): - raise ValueError("The given path is not a valid directory.") - - directory = path.abspath(directory) - - for filename in listdir(directory): - - if not filename.endswith("yaml") and not filename.endswith("yml"): - continue - - role_name = path.splitext(filename)[0] - yield role_name, path.join(directory, filename) - - def _load_file(role_path): with open(role_path) as role_file: @@ -69,18 +49,18 @@ def _create_or_update(name, contents, store=None): return True, store.create(name, contents) -def load_roles(directory, seed_file=None, dry_run=False): - """Given a directory path, import the YAML role files into the - TemplateStore. When dry_run=True is passed, run through the roles but don't - add any to the store. +def load_roles(roles, seed_file=None, dry_run=False): + """Given a list of roles files import them into the + add any to the store. TemplateStore. When dry_run=True is + passed, run through the roles but don't The returned tuple contains all the role names and then the names split over where were created and updated. On a dry run the first item will contain all of the roles found while the second two will be empty lists as no files were updated or created. - :param directory: Directory name containing the roles - :type directory: str + :param roles: A list of yaml files (as strings) + :type roles: [str] :param seed_file: full path to the template seed that should be used for plan master templates @@ -93,7 +73,7 @@ def load_roles(directory, seed_file=None, dry_run=False): all_roles, created, updated = [], [], [] - roles = _list_roles(directory) + roles = [(path.splitext(path.basename(r))[0], r) for r in roles] for name, role_path in roles: diff --git a/tuskar/tests/cmd/test_load_roles.py b/tuskar/tests/cmd/test_load_roles.py index 7375a7e7..a67a2778 100644 --- a/tuskar/tests/cmd/test_load_roles.py +++ b/tuskar/tests/cmd/test_load_roles.py @@ -21,16 +21,16 @@ from tuskar.tests.base import TestCase class LoadRoleTests(TestCase): - @patch('tuskar.storage.load_roles._list_roles', - return_value=[['role_name.yaml', '/path/role_name.yaml']]) @patch('tuskar.storage.load_roles._load_file', return_value="YAML") @patch('tuskar.cmd.load_roles._print_names') - def test_main(self, mock_print, mock_read, mock_list): + def test_main(self, mock_print, mock_read): # test - load_roles.main(argv="--master-seed=seed.yaml path".split()) + load_roles.main(argv=( + "--master-seed=seed.yaml -r role_name1.yaml " + "-r /path/role_name2.yaml").split()) # verify self.assertEqual([ - call('Created', ['role_name.yaml']) + call('Created', ['role_name1', 'role_name2']) ], mock_print.call_args_list) diff --git a/tuskar/tests/storage/test_load_roles.py b/tuskar/tests/storage/test_load_roles.py index 132a60e8..3c575286 100644 --- a/tuskar/tests/storage/test_load_roles.py +++ b/tuskar/tests/storage/test_load_roles.py @@ -16,10 +16,8 @@ from os import path from shutil import rmtree from tempfile import mkdtemp -from tuskar.storage.load_roles import _create_or_update -from tuskar.storage.load_roles import _list_roles -from tuskar.storage.load_roles import load_roles -from tuskar.storage.stores import TemplateStore +from tuskar.storage import load_roles +from tuskar.storage import stores from tuskar.tests.base import TestCase @@ -29,10 +27,12 @@ class LoadRoleTests(TestCase): super(LoadRoleTests, self).setUp() self.directory = mkdtemp() - self.store = TemplateStore() + self.store = stores.TemplateStore() - roles = ['role1.yaml', 'rubbish', 'role2.yml'] - for role in roles: + roles_name = ['role1.yaml', 'role2.yml'] + self.roles = [path.join(self.directory, role) for role in roles_name] + + for role in self.roles: self._create_role(role) def tearDown(self): @@ -43,37 +43,15 @@ class LoadRoleTests(TestCase): """Create a mock role file which simple contains it's own name as the file contents. """ - with open(path.join(self.directory, role), 'w') as f: + + with open(role, 'w') as f: f.write("CONTENTS FOR {0}".format(role)) - def test_list_roles(self): - - # test - roles = sorted(_list_roles(self.directory)) - - # verify - self.assertEqual([ - ('role1', path.join(self.directory, "role1.yaml")), - ('role2', path.join(self.directory, "role2.yml")), - ], roles) - - def test_list_roles_invalid(self): - - # setup - invalid_path = path.join(self.directory, "FAKEPATH/") - self.assertFalse(path.isdir(invalid_path)) - - # test - list_call = _list_roles(invalid_path) - - # verify - self.assertRaises(ValueError, list, list_call) - def test_dry_run(self): # test - total, created, updated = load_roles( - self.directory, dry_run=True) + total, created, updated = load_roles.load_roles( + self.roles, dry_run=True) # verify self.assertEqual(['role1', 'role2'], sorted(total)) @@ -83,7 +61,7 @@ class LoadRoleTests(TestCase): def test_import(self): # test - total, created, updated = load_roles(self.directory) + total, created, updated = load_roles.load_roles(self.roles) # verify self.assertEqual(['role1', 'role2'], sorted(total)) @@ -93,10 +71,10 @@ class LoadRoleTests(TestCase): def test_import_update(self): # setup - _create_or_update("role2", "contents") + load_roles._create_or_update("role2", "contents") # test - total, created, updated = load_roles(self.directory) + total, created, updated = load_roles.load_roles(self.roles) # verify self.assertEqual(['role1', 'role2'], sorted(total)) @@ -105,12 +83,12 @@ class LoadRoleTests(TestCase): def test_import_with_seed(self): # Setup - self._create_role('seed') + self._create_role(path.join(self.directory, 'seed')) # Test seed_file = path.join(self.directory, 'seed') - total, created, updated = load_roles(self.directory, - seed_file=seed_file) + total, created, updated = load_roles.load_roles(self.roles, + seed_file=seed_file) # Verify self.assertEqual(['_master_seed', 'role1', 'role2'], sorted(total))