Merge "Change role loading to explicitly list roles"

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)
 

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):
+def load_roles(roles, seed_file=None, dry_run=False):
-    """Given a directory path, import the YAML role files into the
+    """Given a list of roles files import them into the
-    TemplateStore. When dry_run=True is passed, run through the roles but don't
+    add any to the store. TemplateStore. When dry_run=True is
-    add any to the store.
+    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
+    :param roles: A list of yaml files (as strings)
-    :type  directory: str
+    :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:
 

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)

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 import load_roles
-from tuskar.storage.load_roles import _list_roles
+from tuskar.storage import stores
-from tuskar.storage.load_roles import load_roles
-from tuskar.storage.stores import TemplateStore
 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']
+        roles_name = ['role1.yaml', 'role2.yml']
-        for role in roles:
+        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(
+        total, created, updated = load_roles.load_roles(
-            self.directory, dry_run=True)
+            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,
+        total, created, updated = load_roles.load_roles(self.roles,
-                                             seed_file=seed_file)
+                                                        seed_file=seed_file)
 
         # Verify
         self.assertEqual(['_master_seed', 'role1', 'role2'], sorted(total))