Sort configurations to merge roles in a correct order

Multiple configurations for node roles are merged
into one single configuration during serialization process.
It is required to keep the same order of configurations
to have a predictable behavior.

Change-Id: I649cccc73ae406a1f6953ad8139910bb18acc95f
Closes-Bug: #1524284

nailgun/nailgun/api/v1/handlers/openstack_config.py

@@ -42,7 +42,7 @@ class OpenstackConfigCollectionHandler(BaseHandler):
         data = self.checked_data(
             self.validator.validate_query, data=web.input())
         return objects.OpenstackConfigCollection.to_json(
-            objects.OpenstackConfig.find_configs(**data))
+            objects.OpenstackConfigCollection.filter_by(None, **data))
 
     @content
     def POST(self):

nailgun/nailgun/objects/openstack_config.py

@@ -12,8 +12,6 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import six
-
 from nailgun import consts
 from nailgun.db import db
 from nailgun.db.sqlalchemy import models
@@ -33,7 +31,7 @@ class OpenstackConfig(NailgunObject):
     def create(cls, data):
         data['config_type'] = cls._get_config_type(data)
         data['is_active'] = True
-        config = cls.find_config(**data)
+        config = OpenstackConfigCollection.filter_by(None, **data).first()
         if config:
             cls.disable(config)
         return super(OpenstackConfig, cls).create(data)
@@ -62,70 +60,6 @@ class OpenstackConfig(NailgunObject):
             return consts.OPENSTACK_CONFIG_TYPES.role
         return consts.OPENSTACK_CONFIG_TYPES.cluster
 
-    @classmethod
-    def _find_configs_query(cls, filters):
-        """Build query to filter configurations.
-
-        Filters are applied like AND condition.
-        """
-        query = db().query(cls.model).order_by(cls.model.id.desc())
-        for key, value in six.iteritems(filters):
-            # TODO(asaprykin): There should be a better way to check
-            # presence of column in the model.
-            field = getattr(cls.model, key, None)
-            if field:
-                query = query.filter(field == value)
-
-        return query
-
-    @classmethod
-    def find_config(cls, **filters):
-        """Returns a single configuration for specified filters.
-
-        Example:
-            OpenstackConfig.find_config(cluster_id=10, node_id=12)
-        """
-        query = cls._find_configs_query(filters)
-        return query.first()
-
-    @classmethod
-    def find_configs(cls, **filters):
-        """Returns list of configurations for specified filters.
-
-        Example:
-            OpenstackConfig.find_configs(cluster_id=10, node_id=12)
-        """
-        return cls._find_configs_query(filters)
-
-    @classmethod
-    def find_configs_for_nodes(cls, cluster, nodes):
-        """Returns list of configurations that should be applied.
-
-        Returns list of configurations for specified nodes that will be
-        applied.
-        """
-        all_configs = cls.find_configs(cluster_id=cluster.id, is_active=True)
-        node_ids = set(n.id for n in nodes)
-        node_roles = set()
-
-        for node in nodes:
-            node_roles.update(node.roles)
-
-        configs = []
-
-        for config in all_configs:
-            if config.config_type == consts.OPENSTACK_CONFIG_TYPES.cluster:
-                configs.append(config)
-            elif (config.config_type == consts.OPENSTACK_CONFIG_TYPES.node and
-                    config.node_id in node_ids):
-                configs.append(config)
-            elif (config.config_type ==
-                    consts.OPENSTACK_CONFIG_TYPES.role and
-                    config.node_role in node_roles):
-                configs.append(config)
-
-        return configs
-
     @classmethod
     def disable_by_nodes(cls, nodes):
         """Disactivate all active configurations for specified nodes."""
@@ -142,3 +76,35 @@ class OpenstackConfig(NailgunObject):
 class OpenstackConfigCollection(NailgunCollection):
 
     single = OpenstackConfig
+
+    @classmethod
+    def find_configs_for_nodes(cls, cluster, nodes):
+        """Returns list of configurations that should be applied.
+
+        Returns list of configurations for specified nodes that will be
+        applied. List is sorted by the config_type and node_role fields.
+        """
+        configs_query = cls.filter_by(
+            None, cluster_id=cluster.id, is_active=True)
+        configs_query = configs_query.order_by(cls.single.model.node_role)
+
+        node_ids = set(n.id for n in nodes)
+        node_roles = set()
+
+        for node in nodes:
+            node_roles.update(node.roles)
+
+        configs = []
+
+        for config in configs_query:
+            if config.config_type == consts.OPENSTACK_CONFIG_TYPES.cluster:
+                configs.append(config)
+            elif (config.config_type == consts.OPENSTACK_CONFIG_TYPES.node and
+                    config.node_id in node_ids):
+                configs.append(config)
+            elif (config.config_type ==
+                    consts.OPENSTACK_CONFIG_TYPES.role and
+                    config.node_role in node_roles):
+                configs.append(config)
+
+        return configs

nailgun/nailgun/orchestrator/tasks_serializer.py

@@ -407,7 +407,7 @@ class UploadConfiguration(GenericRolesHook):
     def serialize(self):
         configs = self.configs
         if configs is None:
-            configs = objects.OpenstackConfig.find_configs_for_nodes(
+            configs = objects.OpenstackConfigCollection.find_configs_for_nodes(
                 self.cluster, self.nodes)
 
         node_configs = defaultdict(lambda: defaultdict(dict))

nailgun/nailgun/task/task.py

@@ -1863,7 +1863,7 @@ class UpdateOpenstackConfigTask(object):
 
     @classmethod
     def message(cls, task, cluster, nodes):
-        configs = objects.OpenstackConfig.find_configs_for_nodes(
+        configs = objects.OpenstackConfigCollection.find_configs_for_nodes(
             cluster, nodes)
 
         refresh_on = set()

nailgun/nailgun/test/unit/test_stages_task_serialization.py

@@ -260,6 +260,36 @@ class TestHooksSerializers(BaseTaskSerializationTest):
         self.assertItemsEqual([self.nodes[2].uid], role_uids)
         self.assertItemsEqual([self.nodes[0].uid], node_uids)
 
+    def test_upload_configuration_merge_roles(self):
+        task_config = {
+            'id': 'upload_configuration',
+            'type': 'upload_file',
+            'role': '*',
+        }
+
+        self.env.create_openstack_config(
+            cluster_id=self.cluster.id,
+            config_type=consts.OPENSTACK_CONFIG_TYPES.role,
+            node_role='compute',
+            configuration={'value_a': 'compute', 'value_b': 'compute'}),
+        self.env.create_openstack_config(
+            cluster_id=self.cluster.id,
+            config_type=consts.OPENSTACK_CONFIG_TYPES.role,
+            node_role='cinder',
+            configuration={'value_a': 'cinder', 'value_c': 'cinder'})
+
+        task = tasks_serializer.UploadConfiguration(
+            task_config, self.cluster, self.nodes)
+        serialized_task = next(task.serialize())
+        config = yaml.safe_load(
+            serialized_task['parameters']['data'])
+        self.assertEqual(config, {
+            'configuration': {
+                'value_a': 'compute',
+                'value_b': 'compute',
+                'value_c': 'cinder'
+            }})
+
     def test_update_hosts(self):
         # mark one node as ready so we can test for duplicates
         self.env.nodes[0].status = consts.NODE_STATUSES.ready