Add support for module ordering on apply

A method for specifying 'priority' modules plus a way to rank the order
in which modules are applied has been added.  Two new attributes
'priority_apply' and 'apply_order' are available in the payload
on create and update.  In addition, an is_admin flag was added as an
automatic attribute, to be set when someone with admin credentials
creates a module or updates an existing module with 'admin-only'
options.  This allows better control on the driver plugin
side with regards to security concerns, etc.  The attribute is
now passed in to the guest 'apply' interface for use by the driver.
All three of these attributes are stored in the Trove database.

An admin can create a 'non-admin' module by passing in --full_access
on the command line (or python interface).  This will cause an
error if any admin-only options are selected.

Scenario tests have been added to verify that the modules are
applied in the correct order.  The timestamp for the 'updated'
field on the guest was also enhanced to allow for fractional
seconds, since most applies take less than a second.

The issue where modules were allowed to be applied even if
they belonged to a different datastore has been fixed and
scenario tests added to check for this case.

Change-Id: I7fcd0cf12790564ba62e7d6451fff96f763e539d
Implements: blueprint module-management-ordering

releasenotes/notes/module-ordering-92b6445a8ac3a3bf.yaml

@@ -0,0 +1,9 @@
+---
+features:
+  - Modules can now be applied in a consistent order,
+    based on the new 'priority_apply' and 'apply_order'
+    attributes when creating them.
+    Blueprint module-management-ordering
+upgrade:
+  - For module ordering to work, db_upgrade must be run
+    on the Trove database.

tools/trove-pylint.config

@@ -717,6 +717,18 @@
       "No value for argument 'dml' in method call",
       "upgrade"
     ],
+    [
+      "trove/db/sqlalchemy/migrate_repo/versions/040_module_priority.py",
+      "E1101",
+      "Instance of 'Table' has no 'create_column' member",
+      "upgrade"
+    ],
+    [
+      "trove/db/sqlalchemy/migrate_repo/versions/040_module_priority.py",
+      "no-member",
+      "Instance of 'Table' has no 'create_column' member",
+      "upgrade"
+    ],
     [
       "trove/db/sqlalchemy/migration.py",
       "E0611",

trove/common/apischema.py

@@ -567,10 +567,16 @@ guest_log = {
 module_contents = {
     "type": "string",
     "minLength": 1,
-    "maxLength": 16777215,
+    "maxLength": 4294967295,
     "pattern": "^.*.+.*$"
 }
 
+module_apply_order = {
+    "type": "integer",
+    "minimum": 0,
+    "maximum": 9,
+}
+
 module = {
     "create": {
         "name": "module:create",
@@ -597,6 +603,9 @@ module = {
                     "all_tenants": boolean_string,
                     "visible": boolean_string,
                     "live_update": boolean_string,
+                    "priority_apply": boolean_string,
+                    "apply_order": module_apply_order,
+                    "full_access": boolean_string,
                 }
             }
         }
@@ -629,6 +638,9 @@ module = {
                     "all_datastore_versions": boolean_string,
                     "visible": boolean_string,
                     "live_update": boolean_string,
+                    "priority_apply": boolean_string,
+                    "apply_order": module_apply_order,
+                    "full_access": boolean_string,
                 }
             }
         }

trove/db/sqlalchemy/migrate_repo/versions/040_module_priority.py

@@ -0,0 +1,48 @@
+# Copyright 2016 Tesora, Inc.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+#
+
+from sqlalchemy.schema import Column
+from sqlalchemy.schema import MetaData
+from sqlalchemy.sql.expression import update
+
+from trove.db.sqlalchemy.migrate_repo.schema import Boolean
+from trove.db.sqlalchemy.migrate_repo.schema import Integer
+from trove.db.sqlalchemy.migrate_repo.schema import Table
+from trove.db.sqlalchemy.migrate_repo.schema import Text
+
+
+COLUMN_NAME_1 = 'priority_apply'
+COLUMN_NAME_2 = 'apply_order'
+COLUMN_NAME_3 = 'is_admin'
+
+
+def upgrade(migrate_engine):
+    meta = MetaData()
+    meta.bind = migrate_engine
+    modules = Table('modules', meta, autoload=True)
+    is_nullable = True if migrate_engine.name == "sqlite" else False
+    column = Column(COLUMN_NAME_1, Boolean(), nullable=is_nullable, default=0)
+    modules.create_column(column)
+    column = Column(COLUMN_NAME_2, Integer(), nullable=is_nullable, default=5)
+    modules.create_column(column)
+    column = Column(COLUMN_NAME_3, Boolean(), nullable=is_nullable, default=0)
+    modules.create_column(column)
+    modules.c.contents.alter(Text(length=4294967295))
+    # mark all non-visible, auto-apply and all-tenant modules as is_admin
+    update(table=modules,
+           values=dict(is_admin=1),
+           whereclause="visible=0 or auto_apply=1 or tenant_id is null"
+           ).execute()

trove/guestagent/datastore/manager.py

@@ -15,6 +15,7 @@
 #
 
 import abc
+import operator
 
 from oslo_config import cfg as oslo_cfg
 from oslo_log import log as logging
@@ -62,6 +63,8 @@ class Manager(periodic_task.PeriodicTasks):
     GUEST_LOG_DEFS_ERROR_LABEL = 'error'
     GUEST_LOG_DEFS_SLOW_QUERY_LABEL = 'slow_query'
 
+    MODULE_APPLY_TO_ALL = module_manager.ModuleManager.MODULE_APPLY_TO_ALL
+
     def __init__(self, manager_name):
         super(Manager, self).__init__(CONF)
 
@@ -644,18 +647,36 @@ class Manager(periodic_task.PeriodicTasks):
     def module_apply(self, context, modules=None):
         LOG.info(_("Applying modules."))
         results = []
-        for module_data in modules:
+        modules = [data['module'] for data in modules]
-            module = module_data['module']
+        try:
+            # make sure the modules are applied in the correct order
+            modules.sort(key=operator.itemgetter('apply_order'))
+            modules.sort(key=operator.itemgetter('priority_apply'),
+                         reverse=True)
+        except KeyError:
+            # If we don't have ordering info then maybe we're running
+            # a version of the module feature before ordering was
+            # introduced.  In that case, since we don't have any
+            # way to order the modules we should just continue.
+            pass
+        for module in modules:
             id = module.get('id', None)
             module_type = module.get('type', None)
             name = module.get('name', None)
-            tenant = module.get('tenant', None)
+            tenant = module.get('tenant', self.MODULE_APPLY_TO_ALL)
-            datastore = module.get('datastore', None)
+            datastore = module.get('datastore', self.MODULE_APPLY_TO_ALL)
-            ds_version = module.get('datastore_version', None)
+            ds_version = module.get('datastore_version',
+                                    self.MODULE_APPLY_TO_ALL)
             contents = module.get('contents', None)
             md5 = module.get('md5', None)
             auto_apply = module.get('auto_apply', True)
             visible = module.get('visible', True)
+            is_admin = module.get('is_admin', None)
+            if is_admin is None:
+                # fall back to the old method of checking for an admin option
+                is_admin = (tenant == self.MODULE_APPLY_TO_ALL or
+                            not visible or
+                            auto_apply)
             if not name:
                 raise AttributeError(_("Module name not specified"))
             if not contents:
@@ -665,9 +686,14 @@ class Manager(periodic_task.PeriodicTasks):
                 raise exception.ModuleTypeNotFound(
                     _("No driver implemented for module type '%s'") %
                     module_type)
+            if (datastore and datastore != self.MODULE_APPLY_TO_ALL and
+                    datastore != CONF.datastore_manager):
+                reason = (_("Module not valid for datastore %s") %
+                          CONF.datastore_manager)
+                raise exception.ModuleInvalid(reason=reason)
             result = module_manager.ModuleManager.apply_module(
                 driver, module_type, name, tenant, datastore, ds_version,
-                contents, id, md5, auto_apply, visible)
+                contents, id, md5, auto_apply, visible, is_admin)
             results.append(result)
         LOG.info(_("Returning list of modules: %s") % results)
         return results

trove/guestagent/module/module_manager.py

@@ -15,6 +15,7 @@
 #
 
 import datetime
+import operator
 import os
 
 from oslo_log import log as logging
@@ -41,12 +42,12 @@ class ModuleManager(object):
 
     @classmethod
     def get_current_timestamp(cls):
-        return datetime.datetime.now().strftime("%Y-%m-%d %H:%M:%S")
+        return datetime.datetime.now().strftime("%Y-%m-%d %H:%M:%S.%f")[0:22]
 
     @classmethod
     def apply_module(cls, driver, module_type, name, tenant,
                      datastore, ds_version, contents, module_id, md5,
-                     auto_apply, visible):
+                     auto_apply, visible, admin_module):
         tenant = tenant or cls.MODULE_APPLY_TO_ALL
         datastore = datastore or cls.MODULE_APPLY_TO_ALL
         ds_version = ds_version or cls.MODULE_APPLY_TO_ALL
@@ -57,9 +58,9 @@ class ModuleManager(object):
         now = cls.get_current_timestamp()
         default_result = cls.build_default_result(
             module_type, name, tenant, datastore,
-            ds_version, module_id, md5, auto_apply, visible, now)
+            ds_version, module_id, md5,
+            auto_apply, visible, now, admin_module)
         result = cls.read_module_result(module_dir, default_result)
-        admin_module = cls.is_admin_module(tenant, auto_apply, visible)
         try:
             driver.configure(name, datastore, ds_version, data_file)
             applied, message = driver.apply(
@@ -83,7 +84,7 @@ class ModuleManager(object):
             result['tenant'] = tenant
             result['auto_apply'] = auto_apply
             result['visible'] = visible
-            result['admin_only'] = admin_module
+            result['is_admin'] = admin_module
             cls.write_module_result(module_dir, result)
         return result
 
@@ -113,8 +114,7 @@ class ModuleManager(object):
     @classmethod
     def build_default_result(cls, module_type, name, tenant,
                              datastore, ds_version, module_id, md5,
-                             auto_apply, visible, now):
+                             auto_apply, visible, now, admin_module):
-        admin_module = cls.is_admin_module(tenant, auto_apply, visible)
         result = {
             'type': module_type,
             'name': name,
@@ -130,7 +130,7 @@ class ModuleManager(object):
             'removed': None,
             'auto_apply': auto_apply,
             'visible': visible,
-            'admin_only': admin_module,
+            'is_admin': admin_module,
             'contents': None,
         }
         return result
@@ -183,7 +183,9 @@ class ModuleManager(object):
                     (is_admin or result.get('visible'))):
                 if include_contents:
                     codec = stream_codecs.Base64Codec()
-                    if not is_admin and result.get('admin_only'):
+                    # keep admin_only for backwards compatibility
+                    if not is_admin and (result.get('is_admin') or
+                                         result.get('admin_only')):
                         contents = (
                             "Must be admin to retrieve contents for module %s"
                             % result.get('name', 'Unknown'))
@@ -195,6 +197,7 @@ class ModuleManager(object):
                         result['contents'] = operating_system.read_file(
                             contents_file, codec=codec, decode=False)
                 results.append(result)
+        results.sort(key=operator.itemgetter('updated'), reverse=True)
         return results
 
     @classmethod

trove/instance/models.py

@@ -564,6 +564,26 @@ def load_server_group_info(instance, context, compute_id):
         instance.locality = srv_grp.ServerGroup.get_locality(server_group)
 
 
+def validate_modules_for_apply(modules, datastore_id, datastore_version_id):
+    for module in modules:
+        if (module.datastore_id and
+                module.datastore_id != datastore_id):
+            reason = (_("Module '%(mod)s' cannot be applied "
+                        " (Wrong datastore '%(ds)s' - expected '%(ds2)s')")
+                      % {'mod': module.name, 'ds': module.datastore_id,
+                         'ds2': datastore_id})
+            raise exception.ModuleInvalid(reason=reason)
+        if (module.datastore_version_id and
+                module.datastore_version_id != datastore_version_id):
+            reason = (_("Module '%(mod)s' cannot be applied "
+                        " (Wrong datastore version '%(ver)s' "
+                        "- expected '%(ver2)s')")
+                      % {'mod': module.name,
+                         'ver': module.datastore_version_id,
+                         'ver2': datastore_version_id})
+            raise exception.ModuleInvalid(reason=reason)
+
+
 class BaseInstance(SimpleInstance):
     """Represents an instance.
     -----------
@@ -980,13 +1000,8 @@ class Instance(BuiltInstance):
         for aa_module in auto_apply_modules:
             if aa_module.id not in module_ids:
                 modules.append(aa_module)
-        module_list = []
+        validate_modules_for_apply(modules, datastore.id, datastore_version.id)
-        for module in modules:
+        module_list = module_views.get_module_list(modules)
-            module.contents = module_models.Module.deprocess_contents(
-                module.contents)
-            module_info = module_views.DetailedModuleView(module).data(
-                include_contents=True)
-            module_list.append(module_info)
 
         def _create_resources():
 

trove/instance/service.py

@@ -536,13 +536,9 @@ class InstanceController(wsgi.Controller):
         self.authorize_instance_action(context, 'module_apply', instance)
         module_ids = [mod['id'] for mod in body.get('modules', [])]
         modules = module_models.Modules.load_by_ids(context, module_ids)
-        module_list = []
+        models.validate_modules_for_apply(
-        for module in modules:
+            modules, instance.datastore.id, instance.datastore_version.id)
-            module.contents = module_models.Module.deprocess_contents(
+        module_list = module_views.get_module_list(modules)
-                module.contents)
-            module_info = module_views.DetailedModuleView(module).data(
-                include_contents=True)
-            module_list.append(module_info)
         client = create_guest_client(context, id)
         result_list = client.module_apply(module_list)
         models.Instance.add_instance_modules(context, id, modules)

trove/module/models.py

@@ -137,12 +137,14 @@ class Module(object):
     @staticmethod
     def create(context, name, module_type, contents,
                description, tenant_id, datastore,
-               datastore_version, auto_apply, visible, live_update):
+               datastore_version, auto_apply, visible, live_update,
+               priority_apply, apply_order, full_access):
         if module_type.lower() not in Modules.VALID_MODULE_TYPES:
             LOG.error(_("Valid module types: %s") % Modules.VALID_MODULE_TYPES)
             raise exception.ModuleTypeNotFound(module_type=module_type)
         Module.validate_action(
-            context, 'create', tenant_id, auto_apply, visible)
+            context, 'create', tenant_id, auto_apply, visible, priority_apply,
+            full_access)
         datastore_id, datastore_version_id = Module.validate_datastore(
             datastore, datastore_version)
         if Module.key_exists(
@@ -153,6 +155,9 @@ class Module(object):
             raise exception.ModuleAlreadyExists(
                 name=name, datastore=datastore_str, ds_version=ds_version_str)
         md5, processed_contents = Module.process_contents(contents)
+        is_admin = context.is_admin
+        if full_access:
+            is_admin = 0
         module = DBModule.create(
             name=name,
             type=module_type.lower(),
@@ -164,14 +169,17 @@ class Module(object):
             auto_apply=auto_apply,
             visible=visible,
             live_update=live_update,
+            priority_apply=priority_apply,
+            apply_order=apply_order,
+            is_admin=is_admin,
             md5=md5)
         return module
 
     # Certain fields require admin access to create/change/delete
     @staticmethod
-    def validate_action(context, action_str, tenant_id, auto_apply, visible):
+    def validate_action(context, action_str, tenant_id, auto_apply, visible,
-        error_str = None
+                        priority_apply, full_access):
-        if not context.is_admin:
+        admin_options_str = None
         option_strs = []
         if tenant_id is None:
             option_strs.append(_("Tenant: %s") % Modules.MATCH_ALL_NAME)
@@ -179,22 +187,35 @@ class Module(object):
             option_strs.append(_("Auto: %s") % auto_apply)
         if not visible:
             option_strs.append(_("Visible: %s") % visible)
+        if priority_apply:
+            option_strs.append(_("Priority: %s") % priority_apply)
+        if full_access is not None:
+            if full_access and option_strs:
+                admin_options_str = "(" + ", ".join(option_strs) + ")"
+                raise exception.InvalidModelError(
+                    errors=_('Cannot make module full access: %s') %
+                    admin_options_str)
+            option_strs.append(_("Full Access: %s") % full_access)
         if option_strs:
-                error_str = "(" + " ".join(option_strs) + ")"
+            admin_options_str = "(" + ", ".join(option_strs) + ")"
-        if error_str:
+        if not context.is_admin and admin_options_str:
             raise exception.ModuleAccessForbidden(
-                action=action_str, options=error_str)
+                action=action_str, options=admin_options_str)
+        return admin_options_str
 
     @staticmethod
     def validate_datastore(datastore, datastore_version):
         datastore_id = None
         datastore_version_id = None
         if datastore:
+            if datastore_version:
                 ds, ds_ver = datastore_models.get_datastore_version(
                     type=datastore, version=datastore_version)
                 datastore_id = ds.id
-            if datastore_version:
                 datastore_version_id = ds_ver.id
+            else:
+                ds = datastore_models.Datastore.load(datastore)
+                datastore_id = ds.id
         elif datastore_version:
             msg = _("Cannot specify version without datastore")
             raise exception.BadRequest(message=msg)
@@ -237,7 +258,8 @@ class Module(object):
     def delete(context, module):
         Module.validate_action(
             context, 'delete',
-            module.tenant_id, module.auto_apply, module.visible)
+            module.tenant_id, module.auto_apply, module.visible,
+            module.priority_apply, None)
         Module.enforce_live_update(module.id, module.live_update, module.md5)
         module.deleted = True
         module.deleted_at = datetime.utcnow()
@@ -282,28 +304,33 @@ class Module(object):
         return module
 
     @staticmethod
-    def update(context, module, original_module):
+    def update(context, module, original_module, full_access):
         Module.enforce_live_update(
             original_module.id, original_module.live_update,
             original_module.md5)
-        # we don't allow any changes to 'admin'-type modules, even if
+        # we don't allow any changes to 'is_admin' modules by non-admin
-        # the values changed aren't the admin ones.
+        if original_module.is_admin and not context.is_admin:
-        access_tenant_id = (None if (original_module.tenant_id is None or
+            raise exception.ModuleAccessForbidden(
-                                     module.tenant_id is None)
+                action='update', options='(Module is an admin module)')
-                            else module.tenant_id)
+        # we don't allow any changes to admin-only attributes by non-admin
-        access_auto_apply = original_module.auto_apply or module.auto_apply
+        admin_options = Module.validate_action(
-        access_visible = original_module.visible and module.visible
+            context, 'update', module.tenant_id, module.auto_apply,
-        Module.validate_action(
+            module.visible, module.priority_apply, full_access)
-            context, 'update',
+        # make sure we set the is_admin flag, but only if it was
-            access_tenant_id, access_auto_apply, access_visible)
+        # originally is_admin or we changed an admin option
+        module.is_admin = original_module.is_admin or (
+            1 if admin_options else 0)
+        # but we turn it on/off if full_access is specified
+        if full_access is not None:
+            module.is_admin = 0 if full_access else 1
         ds_id, ds_ver_id = Module.validate_datastore(
             module.datastore_id, module.datastore_version_id)
         if module.contents != original_module.contents:
             md5, processed_contents = Module.process_contents(module.contents)
             module.md5 = md5
             module.contents = processed_contents
-        else:
+        elif hasattr(original_module, 'encrypted_contents'):
-            # on load the contents were decrypted, so
+            # on load the contents may have been decrypted, so
             # we need to put the encrypted contents back before we update
             module.contents = original_module.encrypted_contents
         if module.datastore_id:
@@ -415,6 +442,7 @@ class DBModule(models.DatabaseModelBase):
         'id', 'name', 'type', 'contents', 'description',
         'tenant_id', 'datastore_id', 'datastore_version_id',
         'auto_apply', 'visible', 'live_update',
+        'priority_apply', 'apply_order', 'is_admin',
         'md5', 'created', 'updated', 'deleted', 'deleted_at']
 
 

trove/module/service.py

@@ -91,11 +91,15 @@ class ModuleController(wsgi.Controller):
         auto_apply = body['module'].get('auto_apply', 0)
         visible = body['module'].get('visible', 1)
         live_update = body['module'].get('live_update', 0)
+        priority_apply = body['module'].get('priority_apply', 0)
+        apply_order = body['module'].get('apply_order', 5)
+        full_access = body['module'].get('full_access', None)
 
         module = models.Module.create(
             context, name, module_type, contents,
             description, module_tenant_id, datastore, ds_version,
-            auto_apply, visible, live_update)
+            auto_apply, visible, live_update, priority_apply,
+            apply_order, full_access)
         view_data = views.DetailedModuleView(module)
         return wsgi.Result(view_data.data(), 200)
 
@@ -154,8 +158,15 @@ class ModuleController(wsgi.Controller):
             module.visible = body['module']['visible']
         if 'live_update' in body['module']:
             module.live_update = body['module']['live_update']
+        if 'priority_apply' in body['module']:
+            module.priority_apply = body['module']['priority_apply']
+        if 'apply_order' in body['module']:
+            module.apply_order = body['module']['apply_order']
+        full_access = None
+        if 'full_access' in body['module']:
+            full_access = body['module']['full_access']
 
-        models.Module.update(context, module, original_module)
+        models.Module.update(context, module, original_module, full_access)
         view_data = views.DetailedModuleView(module)
         return wsgi.Result(view_data.data(), 200)
 

trove/module/views.py

@@ -33,6 +33,9 @@ class ModuleView(object):
             datastore_id=self.module.datastore_id,
             datastore_version_id=self.module.datastore_version_id,
             auto_apply=self.module.auto_apply,
+            priority_apply=self.module.priority_apply,
+            apply_order=self.module.apply_order,
+            is_admin=self.module.is_admin,
             md5=self.module.md5,
             visible=self.module.visible,
             created=self.module.created,
@@ -48,13 +51,15 @@ class ModuleView(object):
         datastore = self.module.datastore_id
         datastore_version = self.module.datastore_version_id
         if datastore:
+            if datastore_version:
                 ds, ds_ver = (
                     datastore_models.get_datastore_version(
                         type=datastore, version=datastore_version))
                 datastore = ds.name
-            if datastore_version:
                 datastore_version = ds_ver.name
             else:
+                ds = datastore_models.Datastore.load(datastore)
+                datastore = ds.name
                 datastore_version = models.Modules.MATCH_ALL_NAME
         else:
             datastore = models.Modules.MATCH_ALL_NAME
@@ -95,5 +100,18 @@ class DetailedModuleView(ModuleView):
         if hasattr(self.module, 'instance_count'):
             module_dict["instance_count"] = self.module.instance_count
         if include_contents:
+            if not hasattr(self.module, 'encrypted_contents'):
+                self.module.encrypted_contents = self.module.contents
+                self.module.contents = models.Module.deprocess_contents(
+                    self.module.contents)
             module_dict['contents'] = self.module.contents
         return {"module": module_dict}
+
+
+def get_module_list(modules):
+    module_list = []
+    for module in modules:
+        module_info = DetailedModuleView(module).data(
+            include_contents=True)
+        module_list.append(module_info)
+    return module_list

trove/tests/scenario/groups/module_group.py

@@ -63,6 +63,21 @@ class ModuleCreateGroup(TestGroup):
         """Ensure create hidden module for non-admin fails."""
         self.test_runner.run_module_create_non_admin_hidden()
 
+    @test
+    def module_create_non_admin_priority(self):
+        """Ensure create priority module for non-admin fails."""
+        self.test_runner.run_module_create_non_admin_priority()
+
+    @test
+    def module_create_non_admin_no_full_access(self):
+        """Ensure create no full access module for non-admin fails."""
+        self.test_runner.run_module_create_non_admin_no_full_access()
+
+    @test
+    def module_create_full_access_with_admin_opt(self):
+        """Ensure create full access module with admin opts fails."""
+        self.test_runner.run_module_create_full_access_with_admin_opt()
+
     @test
     def module_create_bad_datastore(self):
         """Ensure create module with invalid datastore fails."""
@@ -154,12 +169,24 @@ class ModuleCreateGroup(TestGroup):
 
     @test(depends_on=[module_create, module_create_bin, module_create_bin2],
           runs_after=[module_create_admin_live_update])
+    def module_create_admin_priority_apply(self):
+        """Check that create module works with priority-apply option."""
+        self.test_runner.run_module_create_admin_priority_apply()
+
+    @test(depends_on=[module_create, module_create_bin, module_create_bin2],
+          runs_after=[module_create_admin_priority_apply])
     def module_create_datastore(self):
         """Check that create module with datastore works."""
         self.test_runner.run_module_create_datastore()
 
     @test(depends_on=[module_create, module_create_bin, module_create_bin2],
           runs_after=[module_create_datastore])
+    def module_create_different_datastore(self):
+        """Check that create module with different datastore works."""
+        self.test_runner.run_module_create_different_datastore()
+
+    @test(depends_on=[module_create, module_create_bin, module_create_bin2],
+          runs_after=[module_create_different_datastore])
     def module_create_ds_version(self):
         """Check that create module with ds version works."""
         self.test_runner.run_module_create_ds_version()
@@ -176,8 +203,20 @@ class ModuleCreateGroup(TestGroup):
         """Check that create with same name on different tenant works."""
         self.test_runner.run_module_create_different_tenant()
 
-    @test(depends_on=[module_create_all_tenant],
+    @test(depends_on=[module_create, module_create_bin, module_create_bin2],
           runs_after=[module_create_different_tenant])
+    def module_create_full_access(self):
+        """Check that create by admin with full access works."""
+        self.test_runner.run_module_create_full_access()
+
+    @test(depends_on=[module_create_all_tenant],
+          runs_after=[module_create_full_access])
+    def module_full_access_toggle(self):
+        """Check that toggling full access works."""
+        self.test_runner.run_module_full_access_toggle()
+
+    @test(depends_on=[module_create_all_tenant],
+          runs_after=[module_full_access_toggle])
     def module_list_again(self):
         """Check that list modules skips invisible modules."""
         self.test_runner.run_module_list_again()
@@ -236,60 +275,66 @@ class ModuleCreateGroup(TestGroup):
 
     @test(depends_on=[module_update],
           runs_after=[module_update_invisible_toggle])
+    def module_update_priority_toggle(self):
+        """Check that update module works for priority toggle."""
+        self.test_runner.run_module_update_priority_toggle()
+
+    @test(depends_on=[module_update],
+          runs_after=[module_update_priority_toggle])
     def module_update_unauth(self):
         """Ensure update module for unauth user fails."""
         self.test_runner.run_module_update_unauth()
 
     @test(depends_on=[module_update],
-          runs_after=[module_update_invisible_toggle])
+          runs_after=[module_update_priority_toggle])
     def module_update_non_admin_auto(self):
         """Ensure update module to auto_apply for non-admin fails."""
         self.test_runner.run_module_update_non_admin_auto()
 
     @test(depends_on=[module_update],
-          runs_after=[module_update_invisible_toggle])
+          runs_after=[module_update_priority_toggle])
     def module_update_non_admin_auto_off(self):
         """Ensure update module to auto_apply off for non-admin fails."""
         self.test_runner.run_module_update_non_admin_auto_off()
 
     @test(depends_on=[module_update],
-          runs_after=[module_update_invisible_toggle])
+          runs_after=[module_update_priority_toggle])
     def module_update_non_admin_auto_any(self):
         """Ensure any update module to auto_apply for non-admin fails."""
         self.test_runner.run_module_update_non_admin_auto_any()
 
     @test(depends_on=[module_update],
-          runs_after=[module_update_invisible_toggle])
+          runs_after=[module_update_priority_toggle])
     def module_update_non_admin_all_tenant(self):
         """Ensure update module to all tenant for non-admin fails."""
         self.test_runner.run_module_update_non_admin_all_tenant()
 
     @test(depends_on=[module_update],
-          runs_after=[module_update_invisible_toggle])
+          runs_after=[module_update_priority_toggle])
     def module_update_non_admin_all_tenant_off(self):
         """Ensure update module to all tenant off for non-admin fails."""
         self.test_runner.run_module_update_non_admin_all_tenant_off()
 
     @test(depends_on=[module_update],
-          runs_after=[module_update_invisible_toggle])
+          runs_after=[module_update_priority_toggle])
     def module_update_non_admin_all_tenant_any(self):
         """Ensure any update module to all tenant for non-admin fails."""
         self.test_runner.run_module_update_non_admin_all_tenant_any()
 
     @test(depends_on=[module_update],
-          runs_after=[module_update_invisible_toggle])
+          runs_after=[module_update_priority_toggle])
     def module_update_non_admin_invisible(self):
         """Ensure update module to invisible for non-admin fails."""
         self.test_runner.run_module_update_non_admin_invisible()
 
     @test(depends_on=[module_update],
-          runs_after=[module_update_invisible_toggle])
+          runs_after=[module_update_priority_toggle])
     def module_update_non_admin_invisible_off(self):
         """Ensure update module to invisible off for non-admin fails."""
         self.test_runner.run_module_update_non_admin_invisible_off()
 
     @test(depends_on=[module_update],
-          runs_after=[module_update_invisible_toggle])
+          runs_after=[module_update_priority_toggle])
     def module_update_non_admin_invisible_any(self):
         """Ensure any update module to invisible for non-admin fails."""
         self.test_runner.run_module_update_non_admin_invisible_any()
@@ -325,6 +370,11 @@ class ModuleInstCreateGroup(TestGroup):
         """Check that module-apply works."""
         self.test_runner.run_module_apply()
 
+    @test(runs_after=[module_query_empty])
+    def module_apply_wrong_module(self):
+        """Ensure that module-apply for wrong module fails."""
+        self.test_runner.run_module_apply_wrong_module()
+
     @test(depends_on=[module_apply])
     def module_list_instance_after_apply(self):
         """Check that the instance has one module associated."""
@@ -356,6 +406,11 @@ class ModuleInstCreateGroup(TestGroup):
         """Check that creating an instance with modules works."""
         self.test_runner.run_create_inst_with_mods()
 
+    @test(runs_after=[module_query_empty])
+    def create_inst_with_wrong_module(self):
+        """Ensure that creating an inst with wrong ds mod fails."""
+        self.test_runner.run_create_inst_with_wrong_module()
+
     @test(depends_on=[module_apply])
     def module_delete_applied(self):
         """Ensure that deleting an applied module fails."""

trove/tests/scenario/runners/module_runners.py

@@ -42,6 +42,28 @@ class ModuleRunner(TestRunner):
         self.MODULE_BINARY_CONTENTS = Crypto.Random.new().read(20)
         self.MODULE_BINARY_CONTENTS2 = '\x00\xFF\xea\x9c\x11\xfeok\xb1\x8ax'
 
+        self.module_name_order = [
+            {'suffix': self.MODULE_BINARY_SUFFIX,
+             'priority': True, 'order': 1},
+            {'suffix': self.MODULE_BINARY_SUFFIX2,
+             'priority': True, 'order': 2},
+            {'suffix': '_hidden_all_tenant_auto_priority',
+             'priority': True, 'order': 3},
+            {'suffix': '_hidden', 'priority': True, 'order': 4},
+            {'suffix': '_auto', 'priority': True, 'order': 5},
+            {'suffix': '_live', 'priority': True, 'order': 6},
+            {'suffix': '_priority', 'priority': True, 'order': 7},
+            {'suffix': '_ds', 'priority': False, 'order': 1},
+            {'suffix': '_ds_ver', 'priority': False, 'order': 2},
+            {'suffix': '_all_tenant_ds_ver', 'priority': False, 'order': 3},
+            {'suffix': '', 'priority': False, 'order': 4},
+            {'suffix': '_ds_diff', 'priority': False, 'order': 5},
+            {'suffix': '_diff_tenant', 'priority': False, 'order': 6},
+            {'suffix': '_full_access', 'priority': False, 'order': 7},
+            {'suffix': '_for_update', 'priority': False, 'order': 8},
+            {'suffix': '_updated', 'priority': False, 'order': 8},
+        ]
+
         self.mod_inst_id = None
         self.temp_module = None
         self._module_type = None
@@ -82,12 +104,19 @@ class ModuleRunner(TestRunner):
     def update_test_module(self):
         return self._get_test_module(1)
 
-    def build_module_args(self, extra=None):
+    def build_module_args(self, name_order=None):
-        extra = extra or ''
+        suffix = "_unknown"
-        name = self.MODULE_NAME + extra
+        priority = False
-        desc = self.MODULE_DESC + extra.replace('_', ' ')
+        order = 5
-        cont = self.get_module_contents(name)
+        if name_order is not None:
-        return name, desc, cont
+            name_rec = self.module_name_order[name_order]
+            suffix = name_rec['suffix']
+            priority = name_rec['priority']
+            order = name_rec['order']
+        name = self.MODULE_NAME + suffix
+        description = self.MODULE_DESC + suffix.replace('_', ' ')
+        contents = self.get_module_contents(name)
+        return name, description, contents, priority, order
 
     def get_module_contents(self, name=None):
         message = self.get_module_message(name=name)
@@ -102,7 +131,8 @@ class ModuleRunner(TestRunner):
             return not mod.visible and mod.tenant_id and not mod.auto_apply
         return self._find_module(_match, "Could not find invisible module")
 
-    def _find_module(self, match_fn, not_found_message, find_all=False):
+    def _find_module(self, match_fn, not_found_message, find_all=False,
+                     fail_on_not_found=True):
         found = [] if find_all else None
         for test_module in self.test_modules:
             if match_fn(test_module):
@@ -112,7 +142,10 @@ class ModuleRunner(TestRunner):
                     found = test_module
                     break
         if not found:
+            if fail_on_not_found:
                 self.fail(not_found_message)
+            else:
+                SkipTest(not_found_message)
         return found
 
     def _find_auto_apply_module(self):
@@ -125,6 +158,21 @@ class ModuleRunner(TestRunner):
             return mod.tenant_id is None and mod.visible
         return self._find_module(_match, "Could not find all tenant module")
 
+    def _find_priority_apply_module(self):
+        def _match(mod):
+            return mod.priority_apply and mod.tenant_id and mod.visible
+        return self._find_module(_match,
+                                 "Could not find priority-apply module")
+
+    def _find_diff_datastore_module(self):
+        def _match(mod):
+            return (mod.datastore and
+                    mod.datastore != models.Modules.MATCH_ALL_NAME and
+                    mod.datastore != self.instance_info.dbaas_datastore)
+        return self._find_module(_match,
+                                 "Could not find different datastore module",
+                                 fail_on_not_found=False)
+
     def _find_all_auto_apply_modules(self, visible=None):
         def _match(mod):
             return mod.auto_apply and (
@@ -132,6 +180,12 @@ class ModuleRunner(TestRunner):
         return self._find_module(
             _match, "Could not find all auto apply modules", find_all=True)
 
+    def _find_module_by_id(self, module_id):
+        def _match(mod):
+            return mod.id == module_id
+        return self._find_module(_match, "Could not find module with id %s" %
+                                 module_id)
+
     # Tests start here
     def run_module_delete_existing(self):
         modules = self.admin_client.modules.list()
@@ -178,6 +232,36 @@ class ModuleRunner(TestRunner):
             self.MODULE_NAME, self.module_type, self.MODULE_NEG_CONTENTS,
             visible=False)
 
+    def run_module_create_non_admin_priority(
+            self, expected_exception=exceptions.Forbidden,
+            expected_http_code=403):
+        client = self.auth_client
+        self.assert_raises(
+            expected_exception, expected_http_code,
+            client, client.modules.create,
+            self.MODULE_NAME, self.module_type, self.MODULE_NEG_CONTENTS,
+            priority_apply=True)
+
+    def run_module_create_non_admin_no_full_access(
+            self, expected_exception=exceptions.Forbidden,
+            expected_http_code=403):
+        client = self.auth_client
+        self.assert_raises(
+            expected_exception, expected_http_code,
+            client, client.modules.create,
+            self.MODULE_NAME, self.module_type, self.MODULE_NEG_CONTENTS,
+            full_access=False)
+
+    def run_module_create_full_access_with_admin_opt(
+            self, expected_exception=exceptions.BadRequest,
+            expected_http_code=400):
+        client = self.admin_client
+        self.assert_raises(
+            expected_exception, expected_http_code,
+            client, client.modules.create,
+            self.MODULE_NAME, self.module_type, self.MODULE_NEG_CONTENTS,
+            full_access=True, auto_apply=True)
+
     def run_module_create_bad_datastore(
             self, expected_exception=exceptions.NotFound,
             expected_http_code=404):
@@ -228,32 +312,44 @@ class ModuleRunner(TestRunner):
             self.admin_client.modules.list())
         self.module_other_count_prior_to_create = len(
             self.unauth_client.modules.list())
-        name, description, contents = self.build_module_args()
+        self.assert_module_create(self.auth_client, 10)
-        self.assert_module_create(
-            self.auth_client,
-            name=name,
-            module_type=self.module_type,
-            contents=contents,
-            description=description)
 
-    def assert_module_create(self, client, name=None, module_type=None,
+    def assert_module_create(self, client, name_order,
+                             name=None, module_type=None,
                              contents=None, description=None,
                              all_tenants=False,
                              datastore=None, datastore_version=None,
                              auto_apply=False,
-                             live_update=False, visible=True):
+                             live_update=False, visible=True,
+                             priority_apply=None,
+                             apply_order=None,
+                             full_access=None):
+        (temp_name, temp_description, temp_contents,
+         temp_priority, temp_order) = self.build_module_args(name_order)
+        name = name if name is not None else temp_name
+        description = (
+            description if description is not None else temp_description)
+        contents = contents if contents is not None else temp_contents
+        priority_apply = (
+            priority_apply if priority_apply is not None else temp_priority)
+        apply_order = apply_order if apply_order is not None else temp_order
+        module_type = module_type or self.module_type
         result = client.modules.create(
             name, module_type, contents,
             description=description,
             all_tenants=all_tenants,
             datastore=datastore, datastore_version=datastore_version,
             auto_apply=auto_apply,
-            live_update=live_update, visible=visible)
+            live_update=live_update, visible=visible,
+            priority_apply=priority_apply,
+            apply_order=apply_order,
+            full_access=full_access)
         username = client.real_client.client.username
         if (('alt' in username and 'admin' not in username) or
                 ('admin' in username and visible)):
             self.module_create_count += 1
             if datastore:
+                if datastore == self.instance_info.dbaas_datastore:
                     self.module_ds_create_count += 1
             else:
                 self.module_ds_all_create_count += 1
@@ -286,7 +382,8 @@ class ModuleRunner(TestRunner):
             expected_datastore=datastore,
             expected_datastore_version=datastore_version,
             expected_auto_apply=auto_apply,
-            expected_contents=contents)
+            expected_contents=contents,
+            expected_is_admin=('admin' in username and not full_access))
 
     def validate_module(self, module, validate_all=False,
                         expected_name=None,
@@ -304,7 +401,11 @@ class ModuleRunner(TestRunner):
                         expected_auto_apply=None,
                         expected_live_update=None,
                         expected_visible=None,
-                        expected_contents=None):
+                        expected_contents=None,
+                        expected_priority_apply=None,
+                        expected_apply_order=None,
+                        expected_is_admin=None,
+                        expected_full_access=None):
 
         if expected_all_tenants:
             expected_tenant = expected_tenant or models.Modules.MATCH_ALL_NAME
@@ -339,6 +440,18 @@ class ModuleRunner(TestRunner):
         if expected_auto_apply is not None:
             self.assert_equal(expected_auto_apply, module.auto_apply,
                               'Unexpected auto_apply')
+        if expected_priority_apply is not None:
+            self.assert_equal(expected_priority_apply, module.priority_apply,
+                              'Unexpected priority_apply')
+        if expected_apply_order is not None:
+            self.assert_equal(expected_apply_order, module.apply_order,
+                              'Unexpected apply_order')
+        if expected_is_admin is not None:
+            self.assert_equal(expected_is_admin, module.is_admin,
+                              'Unexpected is_admin')
+        if expected_full_access is not None:
+            self.assert_equal(expected_full_access, not module.is_admin,
+                              'Unexpected full_access')
         if validate_all:
             if expected_datastore_id:
                 self.assert_equal(expected_datastore_id, module.datastore_id,
@@ -355,13 +468,7 @@ class ModuleRunner(TestRunner):
                                   'Unexpected visible')
 
     def run_module_create_for_update(self):
-        name, description, contents = self.build_module_args('_for_update')
+        self.assert_module_create(self.auth_client, 14)
-        self.assert_module_create(
-            self.auth_client,
-            name=name,
-            module_type=self.module_type,
-            contents=contents,
-            description=description)
 
     def run_module_create_dupe(
             self, expected_exception=exceptions.BadRequest,
@@ -383,28 +490,16 @@ class ModuleRunner(TestRunner):
             datastore_version=self.instance_info.dbaas_datastore_version)
 
     def run_module_create_bin(self):
-        name, description, contents = self.build_module_args(
-            self.MODULE_BINARY_SUFFIX)
         self.assert_module_create(
-            self.admin_client,
+            self.admin_client, 0,
-            name=name,
-            module_type=self.module_type,
             contents=self.MODULE_BINARY_CONTENTS,
-            description=description,
+            auto_apply=True, visible=False)
-            auto_apply=True,
-            visible=False)
 
     def run_module_create_bin2(self):
-        name, description, contents = self.build_module_args(
-            self.MODULE_BINARY_SUFFIX2)
         self.assert_module_create(
-            self.admin_client,
+            self.admin_client, 1,
-            name=name,
-            module_type=self.module_type,
             contents=self.MODULE_BINARY_CONTENTS2,
-            description=description,
+            auto_apply=True, visible=False)
-            auto_apply=True,
-            visible=False)
 
     def run_module_show(self):
         test_module = self.main_test_module
@@ -419,7 +514,10 @@ class ModuleRunner(TestRunner):
             expected_datastore_version=test_module.datastore_version,
             expected_auto_apply=test_module.auto_apply,
             expected_live_update=False,
-            expected_visible=True)
+            expected_visible=True,
+            expected_priority_apply=test_module.priority_apply,
+            expected_apply_order=test_module.apply_order,
+            expected_is_admin=test_module.is_admin)
 
     def run_module_show_unauth_user(
             self, expected_exception=exceptions.NotFound,
@@ -434,28 +532,29 @@ class ModuleRunner(TestRunner):
             self.auth_client,
             self.module_count_prior_to_create + self.module_create_count)
 
-    def assert_module_list(self, client, expected_count, datastore=None,
+    def assert_module_list(self, client, expected_count, datastore=None):
-                           skip_validation=False):
         if datastore:
             module_list = client.modules.list(datastore=datastore)
         else:
             module_list = client.modules.list()
         self.assert_equal(expected_count, len(module_list),
                           "Wrong number of modules for list")
-        if not skip_validation:
         for module in module_list:
-                if module.name != self.MODULE_NAME:
+            # only validate the test modules
-                    continue
+            if module.name.startswith(self.MODULE_NAME):
-                test_module = self.main_test_module
+                test_module = self._find_module_by_id(module.id)
                 self.validate_module(
-                    module, validate_all=False,
+                    module, validate_all=True,
                     expected_name=test_module.name,
                     expected_module_type=test_module.type,
                     expected_description=test_module.description,
                     expected_tenant=test_module.tenant,
                     expected_datastore=test_module.datastore,
                     expected_datastore_version=test_module.datastore_version,
-                    expected_auto_apply=test_module.auto_apply)
+                    expected_auto_apply=test_module.auto_apply,
+                    expected_priority_apply=test_module.priority_apply,
+                    expected_apply_order=test_module.apply_order,
+                    expected_is_admin=test_module.is_admin)
 
     def run_module_list_unauth_user(self):
         self.assert_module_list(
@@ -465,95 +564,103 @@ class ModuleRunner(TestRunner):
              self.module_other_create_count))
 
     def run_module_create_admin_all(self):
-        name, description, contents = self.build_module_args(
-            '_hidden_all_tenant_auto')
         self.assert_module_create(
-            self.admin_client,
+            self.admin_client, 2,
-            name=name, module_type=self.module_type, contents=contents,
-            description=description,
             all_tenants=True,
             visible=False,
             auto_apply=True)
 
     def run_module_create_admin_hidden(self):
-        name, description, contents = self.build_module_args('_hidden')
         self.assert_module_create(
-            self.admin_client,
+            self.admin_client, 3,
-            name=name, module_type=self.module_type, contents=contents,
-            description=description,
             visible=False)
 
     def run_module_create_admin_auto(self):
-        name, description, contents = self.build_module_args('_auto')
         self.assert_module_create(
-            self.admin_client,
+            self.admin_client, 4,
-            name=name, module_type=self.module_type, contents=contents,
-            description=description,
             auto_apply=True)
 
     def run_module_create_admin_live_update(self):
-        name, description, contents = self.build_module_args('_live')
         self.assert_module_create(
-            self.admin_client,
+            self.admin_client, 5,
-            name=name, module_type=self.module_type, contents=contents,
-            description=description,
             live_update=True)
 
-    def run_module_create_datastore(self):
+    def run_module_create_admin_priority_apply(self):
-        name, description, contents = self.build_module_args('_ds')
         self.assert_module_create(
-            self.admin_client,
+            self.admin_client, 6)
-            name=name, module_type=self.module_type, contents=contents,
+
-            description=description,
+    def run_module_create_datastore(self):
+        self.assert_module_create(
+            self.admin_client, 7,
             datastore=self.instance_info.dbaas_datastore)
 
-    def run_module_create_ds_version(self):
+    def run_module_create_different_datastore(self):
-        name, description, contents = self.build_module_args('_ds_ver')
+        diff_datastore = self._get_different_datastore()
+        if not diff_datastore:
+            raise SkipTest("Could not find a different datastore")
         self.assert_module_create(
-            self.admin_client,
+            self.auth_client, 11,
-            name=name, module_type=self.module_type, contents=contents,
+            datastore=diff_datastore)
-            description=description,
+
+    def _get_different_datastore(self):
+        different_datastore = None
+        datastores = self.admin_client.datastores.list()
+        for datastore in datastores:
+            self.report.log("Found datastore: %s" % datastore.name)
+            if datastore.name != self.instance_info.dbaas_datastore:
+                different_datastore = datastore.name
+                break
+        return different_datastore
+
+    def run_module_create_ds_version(self):
+        self.assert_module_create(
+            self.admin_client, 8,
             datastore=self.instance_info.dbaas_datastore,
             datastore_version=self.instance_info.dbaas_datastore_version)
 
     def run_module_create_all_tenant(self):
-        name, description, contents = self.build_module_args(
-            '_all_tenant_ds_ver')
         self.assert_module_create(
-            self.admin_client,
+            self.admin_client, 9,
-            name=name, module_type=self.module_type, contents=contents,
-            description=description,
             all_tenants=True,
             datastore=self.instance_info.dbaas_datastore,
             datastore_version=self.instance_info.dbaas_datastore_version)
 
     def run_module_create_different_tenant(self):
-        name, description, contents = self.build_module_args()
         self.assert_module_create(
-            self.unauth_client,
+            self.unauth_client, 12)
-            name=name, module_type=self.module_type, contents=contents,
+
-            description=description)
+    def run_module_create_full_access(self):
+        self.assert_module_create(
+            self.admin_client, 13,
+            full_access=True)
+
+    def run_module_full_access_toggle(self):
+        self.assert_module_update(
+            self.admin_client,
+            self.main_test_module.id,
+            full_access=False)
+        self.assert_module_update(
+            self.admin_client,
+            self.main_test_module.id,
+            full_access=True)
 
     def run_module_list_again(self):
         self.assert_module_list(
             self.auth_client,
-            self.module_count_prior_to_create + self.module_create_count,
+            self.module_count_prior_to_create + self.module_create_count)
-            skip_validation=True)
 
     def run_module_list_ds(self):
         self.assert_module_list(
             self.auth_client,
             self.module_ds_count_prior_to_create + self.module_ds_create_count,
-            datastore=self.instance_info.dbaas_datastore,
+            datastore=self.instance_info.dbaas_datastore)
-            skip_validation=True)
 
     def run_module_list_ds_all(self):
         self.assert_module_list(
             self.auth_client,
             (self.module_ds_all_count_prior_to_create +
              self.module_ds_all_create_count),
-            datastore=models.Modules.MATCH_ALL_NAME,
+            datastore=models.Modules.MATCH_ALL_NAME)
-            skip_validation=True)
 
     def run_module_show_invisible(
             self, expected_exception=exceptions.NotFound,
@@ -570,8 +677,7 @@ class ModuleRunner(TestRunner):
             (self.module_admin_count_prior_to_create +
              self.module_create_count +
              self.module_admin_create_count +
-             self.module_other_create_count),
+             self.module_other_create_count))
-            skip_validation=True)
 
     def run_module_update(self):
         self.assert_module_update(
@@ -579,46 +685,6 @@ class ModuleRunner(TestRunner):
             self.main_test_module.id,
             description=self.MODULE_DESC + " modified")
 
-    def run_module_update_same_contents(self):
-        old_md5 = self.main_test_module.md5
-        self.assert_module_update(
-            self.auth_client,
-            self.main_test_module.id,
-            contents=self.get_module_contents(self.main_test_module.name))
-        self.assert_equal(old_md5, self.main_test_module.md5,
-                          "MD5 changed with same contents")
-
-    def run_module_update_auto_toggle(self):
-        module = self._find_auto_apply_module()
-        toggle_off_args = {'auto_apply': False}
-        toggle_on_args = {'auto_apply': True}
-        self.assert_module_toggle(module, toggle_off_args, toggle_on_args)
-
-    def assert_module_toggle(self, module, toggle_off_args, toggle_on_args):
-        # First try to update the module based on the change
-        # (this should toggle the state and allow non-admin access)
-        self.assert_module_update(
-            self.admin_client, module.id, **toggle_off_args)
-        # Now we can update using the non-admin client
-        self.assert_module_update(
-            self.auth_client, module.id, description='Updated by auth')
-        # Now set it back
-        self.assert_module_update(
-            self.admin_client, module.id, description=module.description,
-            **toggle_on_args)
-
-    def run_module_update_all_tenant_toggle(self):
-        module = self._find_all_tenant_module()
-        toggle_off_args = {'all_tenants': False}
-        toggle_on_args = {'all_tenants': True}
-        self.assert_module_toggle(module, toggle_off_args, toggle_on_args)
-
-    def run_module_update_invisible_toggle(self):
-        module = self._find_invisible_module()
-        toggle_off_args = {'visible': True}
-        toggle_on_args = {'visible': False}
-        self.assert_module_toggle(module, toggle_off_args, toggle_on_args)
-
     def assert_module_update(self, client, module_id, **kwargs):
         result = client.modules.update(module_id, **kwargs)
         found = False
@@ -638,6 +704,75 @@ class ModuleRunner(TestRunner):
             expected_args[new_key] = value
         self.validate_module(result, **expected_args)
 
+    def run_module_update_same_contents(self):
+        old_md5 = self.main_test_module.md5
+        self.assert_module_update(
+            self.auth_client,
+            self.main_test_module.id,
+            contents=self.get_module_contents(self.main_test_module.name))
+        self.assert_equal(old_md5, self.main_test_module.md5,
+                          "MD5 changed with same contents")
+
+    def run_module_update_auto_toggle(self,
+                                      expected_exception=exceptions.Forbidden,
+                                      expected_http_code=403):
+        module = self._find_auto_apply_module()
+        toggle_off_args = {'auto_apply': False}
+        toggle_on_args = {'auto_apply': True}
+        self.assert_module_toggle(module, toggle_off_args, toggle_on_args,
+                                  expected_exception=expected_exception,
+                                  expected_http_code=expected_http_code)
+
+    def assert_module_toggle(self, module, toggle_off_args, toggle_on_args,
+                             expected_exception, expected_http_code):
+        # First try to update the module based on the change
+        # (this should toggle the state but still not allow non-admin access)
+        client = self.admin_client
+        self.assert_module_update(client, module.id, **toggle_off_args)
+        # The non-admin client should fail to update
+        non_admin_client = self.auth_client
+        self.assert_raises(
+            expected_exception, expected_http_code,
+            non_admin_client, non_admin_client.modules.update, module.id,
+            description='Updated by non-admin')
+        # Make sure we can still update with the admin client
+        self.assert_module_update(
+            client, module.id, description='Updated by admin')
+        # Now set it back
+        self.assert_module_update(
+            client, module.id, description=module.description,
+            **toggle_on_args)
+
+    def run_module_update_all_tenant_toggle(
+            self, expected_exception=exceptions.Forbidden,
+            expected_http_code=403):
+        module = self._find_all_tenant_module()
+        toggle_off_args = {'all_tenants': False}
+        toggle_on_args = {'all_tenants': True}
+        self.assert_module_toggle(module, toggle_off_args, toggle_on_args,
+                                  expected_exception=expected_exception,
+                                  expected_http_code=expected_http_code)
+
+    def run_module_update_invisible_toggle(
+            self, expected_exception=exceptions.Forbidden,
+            expected_http_code=403):
+        module = self._find_invisible_module()
+        toggle_off_args = {'visible': True}
+        toggle_on_args = {'visible': False}
+        self.assert_module_toggle(module, toggle_off_args, toggle_on_args,
+                                  expected_exception=expected_exception,
+                                  expected_http_code=expected_http_code)
+
+    def run_module_update_priority_toggle(
+            self, expected_exception=exceptions.Forbidden,
+            expected_http_code=403):
+        module = self._find_priority_apply_module()
+        toggle_off_args = {'priority_apply': False}
+        toggle_on_args = {'priority_apply': True}
+        self.assert_module_toggle(module, toggle_off_args, toggle_on_args,
+                                  expected_exception=expected_exception,
+                                  expected_http_code=expected_http_code)
+
     def run_module_update_unauth(
             self, expected_exception=exceptions.NotFound,
             expected_http_code=404):
@@ -775,32 +910,47 @@ class ModuleRunner(TestRunner):
         self.assert_equal(expected_count, count,
                           "Wrong number of modules from query")
         expected_results = expected_results or {}
+        name_index = len(self.module_name_order)
         for modquery in modquery_list:
             if modquery.name in expected_results:
+                self.report.log("Validating module '%s'" % modquery.name)
                 expected = expected_results[modquery.name]
-                self.validate_module_info(
+                self.validate_module_apply_info(
                     modquery,
                     expected_status=expected['status'],
                     expected_message=expected['message'])
+                # make sure we're in the correct order
+                found = False
+                while name_index > 0:
+                    name_index -= 1
+                    name_order_rec = self.module_name_order[name_index]
+                    order_name = self.MODULE_NAME + name_order_rec['suffix']
+                    self.report.log("Next module order '%s'" % order_name)
+                    if order_name == modquery.name:
+                        self.report.log("Match found")
+                        found = True
+                        break
+                if name_index == 0 and not found:
+                    self.fail("Module '%s' was not found in the correct order"
+                              % modquery.name)
 
     def run_module_apply(self):
         self.assert_module_apply(self.auth_client, self.instance_info.id,
                                  self.main_test_module)
 
     def assert_module_apply(self, client, instance_id, module,
+                            expected_is_admin=False,
                             expected_status=None, expected_message=None,
                             expected_contents=None,
                             expected_http_code=200):
         module_apply_list = client.instances.module_apply(
             instance_id, [module.id])
         self.assert_client_code(client, expected_http_code)
-        admin_only = (not module.visible or module.auto_apply or
-                      not module.tenant_id)
         expected_status = expected_status or 'OK'
         expected_message = (expected_message or
                             self.get_module_message(module.name))
         for module_apply in module_apply_list:
-            self.validate_module_info(
+            self.validate_module_apply_info(
                 module_apply,
                 expected_name=module.name,
                 expected_module_type=module.type,
@@ -808,22 +958,22 @@ class ModuleRunner(TestRunner):
                 expected_datastore_version=module.datastore_version,
                 expected_auto_apply=module.auto_apply,
                 expected_visible=module.visible,
-                expected_admin_only=admin_only,
                 expected_contents=expected_contents,
                 expected_status=expected_status,
-                expected_message=expected_message)
+                expected_message=expected_message,
+                expected_is_admin=expected_is_admin)
 
-    def validate_module_info(self, module_apply,
+    def validate_module_apply_info(self, module_apply,
                                    expected_name=None,
                                    expected_module_type=None,
                                    expected_datastore=None,
                                    expected_datastore_version=None,
                                    expected_auto_apply=None,
                                    expected_visible=None,
-                             expected_admin_only=None,
                                    expected_contents=None,
                                    expected_message=None,
-                             expected_status=None):
+                                   expected_status=None,
+                                   expected_is_admin=None):
 
         prefix = "Module: %s -" % expected_name
         if expected_name:
@@ -845,9 +995,6 @@ class ModuleRunner(TestRunner):
         if expected_visible is not None:
             self.assert_equal(expected_visible, module_apply.visible,
                               '%s Unexpected visible' % prefix)
-        if expected_admin_only is not None:
-            self.assert_equal(expected_admin_only, module_apply.admin_only,
-                              '%s Unexpected admin_only' % prefix)
         if expected_contents is not None:
             self.assert_equal(expected_contents, module_apply.contents,
                               '%s Unexpected contents' % prefix)
@@ -859,6 +1006,20 @@ class ModuleRunner(TestRunner):
         if expected_status is not None:
             self.assert_equal(expected_status, module_apply.status,
                               '%s Unexpected status' % prefix)
+        if expected_is_admin is not None:
+            self.assert_equal(expected_is_admin, module_apply.is_admin,
+                              '%s Unexpected is_admin' % prefix)
+
+    def run_module_apply_wrong_module(
+            self, expected_exception=exceptions.BadRequest,
+            expected_http_code=400):
+        module = self._find_diff_datastore_module()
+        self.report.log("Found 'wrong' module: %s" % module.name)
+        client = self.auth_client
+        self.assert_raises(
+            expected_exception, expected_http_code,
+            client, client.instances.module_apply,
+            self.instance_info.id, [module.id])
 
     def run_module_list_instance_after_apply(self):
         self.assert_module_list_instance(
@@ -873,7 +1034,8 @@ class ModuleRunner(TestRunner):
             self.auth_client, self.instance_info.id, 2)
 
     def run_module_update_after_remove(self):
-        name, description, contents = self.build_module_args('_updated')
+        name, description, contents, priority, order = (
+            self.build_module_args(15))
         self.assert_module_update(
             self.auth_client,
             self.update_test_module.id,
@@ -951,6 +1113,24 @@ class ModuleRunner(TestRunner):
         self.assert_client_code(client, expected_http_code)
         return inst.id
 
+    def run_create_inst_with_wrong_module(
+            self, expected_exception=exceptions.BadRequest,
+            expected_http_code=400):
+        module = self._find_diff_datastore_module()
+        self.report.log("Found 'wrong' module: %s" % module.name)
+
+        client = self.auth_client
+        self.assert_raises(
+            expected_exception, expected_http_code,
+            client, client.instances.create,
+            self.instance_info.name + '_wrong_ds',
+            self.instance_info.dbaas_flavor_href,
+            self.instance_info.volume,
+            datastore=self.instance_info.dbaas_datastore,
+            datastore_version=self.instance_info.dbaas_datastore_version,
+            nics=self.instance_info.nics,
+            modules=[module.id])
+
     def run_module_delete_applied(
             self, expected_exception=exceptions.Forbidden,
             expected_http_code=403):

trove/tests/unittests/guestagent/test_manager.py

@@ -24,6 +24,7 @@ from mock import Mock
 from mock import patch
 from oslo_utils import encodeutils
 from proboscis.asserts import assert_equal
+from proboscis.asserts import assert_is_none
 from proboscis.asserts import assert_true
 
 from trove.common.context import TroveContext
@@ -31,6 +32,7 @@ from trove.common import exception
 from trove.guestagent.common import operating_system
 from trove.guestagent.datastore import manager
 from trove.guestagent import guest_log
+from trove.guestagent.module import module_manager
 from trove import rpc
 from trove.tests.unittests import trove_testtools
 
@@ -110,6 +112,12 @@ class ManagerTest(trove_testtools.TestCase):
         self.expected_details_sys['type'] = 'SYS'
         self.expected_details_sys['status'] = 'Enabled'
         self.expected_details_sys['name'] = self.log_name_sys
+        self.expected_module_details = {
+            'name': 'mymod',
+            'type': 'ping',
+            'contents': 'e262cfe36134'
+        }
+        self.manager.module_manager = Mock()
 
     def tearDown(self):
         super(ManagerTest, self).tearDown()
@@ -475,3 +483,36 @@ class ManagerTest(trove_testtools.TestCase):
             self.manager.status.end_install(
                 error_occurred=True,
                 post_processing=ANY)
+
+    def test_module_list(self):
+        with patch.object(module_manager.ModuleManager, 'read_module_results',
+                          return_value=[
+                              self.expected_module_details]) as mock_rmr:
+            module_list = self.manager.module_list(self.context)
+            expected = [self.expected_module_details]
+            assert_equal(self._flatten_list_of_dicts(expected),
+                         self._flatten_list_of_dicts(module_list),
+                         "Wrong list: %s (Expected: %s)" % (
+                             self._flatten_list_of_dicts(module_list),
+                             self._flatten_list_of_dicts(expected)))
+            assert_equal(1, mock_rmr.call_count)
+
+    def test_module_apply(self):
+        with patch.object(
+                module_manager.ModuleManager, 'apply_module',
+                return_value=[self.expected_module_details]) as mock_am:
+            module_details = self.manager.module_apply(
+                self.context,
+                [{'module': self.expected_module_details}])
+            assert_equal([[self.expected_module_details]], module_details)
+            assert_equal(1, mock_am.call_count)
+
+    def test_module_remove(self):
+        with patch.object(
+                module_manager.ModuleManager, 'remove_module',
+                return_value=[self.expected_module_details]) as mock_rm:
+            module_details = self.manager.module_remove(
+                self.context,
+                {'module': self.expected_module_details})
+            assert_is_none(module_details)
+            assert_equal(1, mock_rm.call_count)

trove/tests/unittests/instance/test_instance_models.py

@@ -17,6 +17,7 @@ from mock import Mock, patch
 
 from trove.backup import models as backup_models
 from trove.common import cfg
+from trove.common import crypto_utils
 from trove.common import exception
 from trove.common.instance import ServiceStatuses
 from trove.datastore import models as datastore_models
@@ -403,3 +404,68 @@ class TestReplication(trove_testtools.TestCase):
                           None, 'name', 2, "UUID", [], [], None,
                           self.datastore_version, 1,
                           None, slave_of_id=self.replica_info.id)
+
+
+class TestModules(trove_testtools.TestCase):
+
+    def setUp(self):
+        super(TestModules, self).setUp()
+
+    def tearDown(self):
+        super(TestModules, self).tearDown()
+
+    def _build_module(self, ds_id, ds_ver_id):
+        module = Mock()
+        module.datastore_id = ds_id
+        module.datastore_version_id = ds_ver_id
+        module.contents = crypto_utils.encode_data(
+            crypto_utils.encrypt_data(
+                'VGhpc2lzbXlkYXRhc3RyaW5n',
+                'thisismylongkeytouse'))
+        return module
+
+    def test_validate_modules_for_apply(self):
+        data = [
+            [[self._build_module('ds', 'ds_ver')], 'ds', 'ds_ver', True],
+            [[self._build_module('ds', None)], 'ds', 'ds_ver', True],
+            [[self._build_module(None, None)], 'ds', 'ds_ver', True],
+
+            [[self._build_module('ds', 'ds_ver')], 'ds', 'ds2_ver', False,
+             exception.TroveError],
+            [[self._build_module('ds', 'ds_ver')], 'ds2', 'ds_ver', False,
+             exception.TroveError],
+            [[self._build_module('ds', 'ds_ver')], 'ds2', 'ds2_ver', False,
+             exception.TroveError],
+            [[self._build_module('ds', None)], 'ds2', 'ds2_ver', False,
+             exception.TroveError],
+            [[self._build_module(None, None)], 'ds2', 'ds2_ver', True],
+
+            [[self._build_module(None, 'ds_ver')], 'ds2', 'ds_ver', True],
+        ]
+        for datum in data:
+            modules = datum[0]
+            ds_id = datum[1]
+            ds_ver_id = datum[2]
+            match = datum[3]
+            expected_exception = None
+            if not match:
+                expected_exception = datum[4]
+            ds = Mock()
+            ds.id = ds_id
+            ds.name = ds_id
+            ds_ver = Mock()
+            ds_ver.id = ds_ver_id
+            ds_ver.name = ds_ver_id
+            ds_ver.datastore_id = ds_id
+            with patch.object(datastore_models.Datastore, 'load',
+                              return_value=ds):
+                with patch.object(datastore_models.DatastoreVersion, 'load',
+                                  return_value=ds_ver):
+                    if match:
+                        models.validate_modules_for_apply(
+                            modules, ds_id, ds_ver_id)
+                    else:
+                        self.assertRaises(
+                            expected_exception,
+                            models.validate_modules_for_apply,
+                            modules, ds_id, ds_ver_id)

trove/tests/unittests/module/test_module_controller.py

@@ -30,6 +30,8 @@ class TestModuleController(trove_testtools.TestCase):
                 "name": 'test_module',
                 "module_type": 'test',
                 "contents": 'my_contents\n',
+                "priority_apply": 0,
+                "apply_order": 5
             }
         }
 
@@ -44,7 +46,7 @@ class TestModuleController(trove_testtools.TestCase):
         validator = jsonschema.Draft4Validator(schema)
         self.assertTrue(validator.is_valid(body))
 
-    def test_validate_create_blankname(self):
+    def test_validate_create_blank_name(self):
         body = self.module
         body['module']['name'] = "     "
         schema = self.controller.get_schema('create', body)
@@ -65,3 +67,14 @@ class TestModuleController(trove_testtools.TestCase):
         self.assertEqual(1, len(errors))
         self.assertIn("'$#$%^^' does not match '^.*[0-9a-zA-Z]+.*$'",
                       errors[0].message)
+
+    def test_validate_create_invalid_apply_order(self):
+        body = self.module
+        body['module']['apply_order'] = 12
+        schema = self.controller.get_schema('create', body)
+        validator = jsonschema.Draft4Validator(schema)
+        self.assertFalse(validator.is_valid(body))
+        errors = sorted(validator.iter_errors(body), key=lambda e: e.path)
+        self.assertEqual(1, len(errors))
+        self.assertIn("12 is greater than the maximum of 9",
+                      errors[0].message)

trove/tests/unittests/module/test_module_models.py

@@ -14,8 +14,11 @@
 #    under the License.
 #
 
+import copy
 from mock import Mock, patch
 
+from trove.common import exception
+from trove.datastore import models as datastore_models
 from trove.module import models
 from trove.taskmanager import api as task_api
 from trove.tests.unittests import trove_testtools
@@ -38,10 +41,101 @@ class CreateModuleTest(trove_testtools.TestCase):
     def tearDown(self):
         super(CreateModuleTest, self).tearDown()
 
-    def test_can_create_module(self):
+    def test_can_create_update_module(self):
         module = models.Module.create(
             self.context,
             self.name, self.module_type, self.contents,
-            'my desc', 'my_tenant', None, None, False, True, False)
+            'my desc', 'my_tenant', None, None, False, True, False,
+            False, 5, True)
         self.assertIsNotNone(module)
+        new_module = copy.copy(module)
+        models.Module.update(self.context, new_module, module, False)
         module.delete()
+
+    def test_validate_action(self):
+        # tenant_id, auto_apply, visible, priority_apply, full_access,
+        # valid, exception, works_for_admin
+        data = [
+            ['tenant', False, True, False, None,
+             True],
+
+            ['tenant', True, True, False, None,
+             False, exception.ModuleAccessForbidden],
+            ['tenant', False, False, False, None,
+             False, exception.ModuleAccessForbidden],
+            ['tenant', False, True, True, None,
+             False, exception.ModuleAccessForbidden],
+            ['tenant', False, True, False, True,
+             False, exception.ModuleAccessForbidden, False],
+            ['tenant', False, True, False, False,
+             False, exception.ModuleAccessForbidden],
+            ['tenant', True, False, True, False,
+             False, exception.ModuleAccessForbidden],
+
+            ['tenant', True, False, True, True,
+             False, exception.InvalidModelError, False],
+        ]
+        for datum in data:
+            tenant = datum[0]
+            auto_apply = datum[1]
+            visible = datum[2]
+            priority_apply = datum[3]
+            full_access = datum[4]
+            valid = datum[5]
+            expected_exception = None
+            if not valid:
+                expected_exception = datum[6]
+            context = Mock()
+            context.is_admin = False
+            works_for_admin = True
+            if len(datum) > 7:
+                works_for_admin = datum[7]
+            if valid:
+                models.Module.validate_action(
+                    context, 'action', tenant, auto_apply, visible,
+                    priority_apply, full_access)
+            else:
+                self.assertRaises(
+                    expected_exception,
+                    models.Module.validate_action, context, 'action', tenant,
+                    auto_apply, visible, priority_apply, full_access)
+                # also make sure that it works for admin
+                if works_for_admin:
+                    context.is_admin = True
+                    models.Module.validate_action(
+                        context, 'action', tenant, auto_apply, visible,
+                        priority_apply, full_access)
+
+    def test_validate_datastore(self):
+        # datastore, datastore_version, valid, exception
+        data = [
+            [None, None, True],
+            ['ds', None, True],
+            ['ds', 'ds_ver', True],
+            [None, 'ds_ver', False,
+             exception.BadRequest],
+        ]
+        for datum in data:
+            ds_id = datum[0]
+            ds_ver_id = datum[1]
+            valid = datum[2]
+            expected_exception = None
+            if not valid:
+                expected_exception = datum[3]
+            ds = Mock()
+            ds.id = ds_id
+            ds.name = ds_id
+            ds_ver = Mock()
+            ds_ver.id = ds_ver_id
+            ds_ver.name = ds_ver_id
+            ds_ver.datastore_id = ds_id
+            with patch.object(datastore_models.Datastore, 'load',
+                              return_value=ds):
+                with patch.object(datastore_models.DatastoreVersion, 'load',
+                                  return_value=ds_ver):
+                    if valid:
+                        models.Module.validate_datastore(ds_id, ds_ver_id)
+                    else:
+                        self.assertRaises(
+                            expected_exception,
+                            models.Module.validate_datastore, ds_id, ds_ver_id)

trove/tests/unittests/module/test_module_views.py

@@ -43,6 +43,9 @@ class DetailedModuleViewTest(trove_testtools.TestCase):
         self.module.datastore_version = '5.6'
         self.module.auto_apply = False
         self.module.tenant_id = 'my_tenant'
+        self.module.is_admin = False
+        self.module.priority_apply = False
+        self.module.apply_order = 5
 
     def tearDown(self):
         super(DetailedModuleViewTest, self).tearDown()
@@ -69,3 +72,9 @@ class DetailedModuleViewTest(trove_testtools.TestCase):
                              result['module']['auto_apply'])
             self.assertEqual(self.module.tenant_id,
                              result['module']['tenant_id'])
+            self.assertEqual(self.module.is_admin,
+                             result['module']['is_admin'])
+            self.assertEqual(self.module.priority_apply,
+                             result['module']['priority_apply'])
+            self.assertEqual(self.module.apply_order,
+                             result['module']['apply_order'])