Plugin is able to extend existing network roles

The same network role from different plugins can be merged
according to policy of merge, that allows each plugin to extend the VIP for role.

TestCheckVIPsNames was deleted, because this patch makes it obsolete.

Change-Id: I81e773e53ab67b4e7d424805134c0d86dcc7a43a
Closes-Bug: #1487011

nailgun/nailgun/db/sqlalchemy/models/release.py

@@ -49,7 +49,7 @@ class Release(Base):
         nullable=False,
         default=consts.RELEASE_STATES.unavailable
     )
-    networks_metadata = Column(JSON, default=[])
+    networks_metadata = Column(JSON, default={})
     attributes_metadata = Column(JSON, default={})
     volumes_metadata = Column(JSON, default={})
     modes_metadata = Column(JSON, default={})

nailgun/nailgun/errors/__init__.py

@@ -109,7 +109,9 @@ default_messages = {
     "CannotFindExtension": "Cannot find extension",
 
     # unknown
-    "UnknownError": "Unknown error"
+    "UnknownError": "Unknown error",
+
+    "UnresolvableConflict": "Unresolvable conflict"
 }
 
 

nailgun/nailgun/objects/cluster.py

@@ -38,6 +38,7 @@ from nailgun.objects import NailgunObject
 from nailgun.objects import Release
 from nailgun.objects.serializers.cluster import ClusterSerializer
 from nailgun.plugins.manager import PluginManager
+from nailgun.plugins.merge_policies import NetworkRoleMergePolicy
 from nailgun.settings import settings
 from nailgun.utils import AttributesGenerator
 from nailgun.utils import dict_merge
@@ -191,9 +192,8 @@ class Cluster(NailgunObject):
 
     @classmethod
     def get_default_kernel_params(cls, instance):
-        kernel_params = instance.attributes.editable.get("kernel_params")
-        if kernel_params and kernel_params.get("kernel"):
-            return kernel_params.get("kernel").get("value")
+        kernel_params = instance.attributes.editable.get("kernel_params", {})
+        return kernel_params.get("kernel", {}).get("value")
 
     @classmethod
     def create_attributes(cls, instance):
@@ -787,8 +787,8 @@ class Cluster(NailgunObject):
 
     @classmethod
     def get_default_group(cls, instance):
-        return [g for g in instance.node_groups
-                if g.name == consts.NODE_GROUPS.default][0]
+        default = consts.NODE_GROUPS.default
+        return next(g for g in instance.node_groups if g.name == default)
 
     @classmethod
     def create_default_group(cls, instance):
@@ -967,14 +967,15 @@ class Cluster(NailgunObject):
         db().flush()
 
     @classmethod
-    def get_network_roles(cls, instance):
+    def get_network_roles(
+            cls, instance, merge_policy=NetworkRoleMergePolicy()):
         """Method for receiving network roles for particular cluster
 
         :param instance: nailgun.db.sqlalchemy.models.Cluster instance
+        :param merge_policy: the policy to merge same roles
         :returns: List of network roles' descriptions
         """
-        return (instance.release.network_roles_metadata +
-                PluginManager.get_network_roles(instance))
+        return PluginManager.get_network_roles(instance, merge_policy)
 
     @classmethod
     def set_network_template(cls, instance, template):

nailgun/nailgun/plugins/manager.py

@@ -85,19 +85,32 @@ class PluginManager(object):
         return cluster_plugins
 
     @classmethod
-    def get_network_roles(cls, cluster):
-        core_roles = cluster.release.network_roles_metadata
-        known_roles = set(role['id'] for role in core_roles)
+    def get_network_roles(cls, cluster, merge_policy):
+        """Returns the network roles from plugins.
+
+        The roles cluster and plugins will be mixed
+        according to merge policy.
+        """
+
+        instance_roles = cluster.release.network_roles_metadata
+        all_roles = dict((role['id'], role) for role in instance_roles)
+        conflict_roles = dict()
 
-        plugin_roles = []
-        conflict_roles = {}
         for plugin in cluster.plugins:
             for role in plugin.network_roles_metadata:
                 role_id = role['id']
-                if role_id in known_roles:
-                    conflict_roles[role_id] = plugin.name
-                known_roles.add(role_id)
-            plugin_roles.extend(plugin.network_roles_metadata)
+                if role_id in all_roles:
+                    try:
+                        merge_policy.apply_patch(
+                            all_roles[role_id],
+                            role
+                        )
+                    except errors.UnresolvableConflict as e:
+                        logger.error("cannot merge plugin {0}: {1}"
+                                     .format(plugin.name, e))
+                        conflict_roles[role_id] = plugin.name
+                else:
+                    all_roles[role_id] = role
 
         if conflict_roles:
             raise errors.NetworkRoleConflict(
@@ -106,7 +119,7 @@ class PluginManager(object):
                     ', '.join(conflict_roles),
                     ', '.join(set(conflict_roles.values()))))
 
-        return plugin_roles
+        return list(all_roles.values())
 
     @classmethod
     def get_plugins_deployment_tasks(cls, cluster):

nailgun/nailgun/plugins/merge_policies.py

@@ -0,0 +1,103 @@
+#    Copyright 2015 Mirantis, Inc.
+#
+#    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.
+
+import abc
+import six
+
+from nailgun.errors import errors
+
+
+@six.add_metaclass(abc.ABCMeta)
+class MergePolicy(object):
+    """Policy to merge attributes of plugins."""
+
+    @abc.abstractmethod
+    def apply_patch(self, target, patch):
+        """Applies patch to the target.
+
+        :param target: the origin object, the target can be modified.
+        :param patch: the modifications for merging with original
+        :return: the patched object.
+        """
+
+
+class NetworkRoleMergePolicy(MergePolicy):
+    """Policy for merging network roles."""
+
+    def __init__(self):
+        self.rules = {'vip': NetworkRoleMergePolicy._patch_vips}
+
+    @staticmethod
+    def _patch_vips(target, patch):
+        """Patches VIP attribute for network role.
+
+        :param: target: the origin VIPs.
+        :type target: list
+        :param patch: the VIPs, that will be added to origin
+        :type patch: list
+        :return: the patched VIPs
+        """
+        seen = dict((vip['name'], vip) for vip in target)
+        for vip in patch:
+            if vip['name'] in seen:
+                if vip != seen[vip['name']]:
+                    raise ValueError(
+                        "VIP '{0}' conflicts with existing one"
+                        .format(vip['name'])
+                    )
+            else:
+                seen[vip['name']] = vip
+                target.append(vip)
+
+        return target
+
+    def apply_patch(self, target, patch):
+        """Tries to apply patch to the target.
+
+        Conflicts will be resolved according to the
+        predefined rules.
+
+        :param target: the origin network role
+        :type target: dict
+        :param patch: the modifications for merging with origin
+        :type patch: dict
+        :raises: errors.UnresolvableConflict
+        """
+
+        target_props = target['properties']
+        patch_props = patch['properties']
+
+        conflict = set(target_props) & set(patch_props)
+        mergeable = set(self.rules)
+
+        # Exclude fields that has same value
+        for name in (conflict - mergeable):
+            if target_props[name] != patch_props[name]:
+                raise errors.UnresolvableConflict(
+                    "Cannot apply patch for attribute {0}: conflict"
+                    .format(name)
+                )
+            conflict.remove(name)
+
+        for name in conflict:
+            try:
+                target_props[name] = self.rules[name](
+                    target_props[name],
+                    patch_props[name]
+                )
+            except Exception as e:
+                raise errors.UnresolvableConflict(
+                    "Cannot apply patch for attribute {0}: {1}"
+                    .format(name, e)
+                )

nailgun/nailgun/task/manager.py

@@ -423,7 +423,7 @@ class ApplyChangesTaskManager(TaskManager, DeploymentCheckMixin):
             # example, enabled plugins contain conflicting network
             # configuration
             network_info = self.serialize_network_cfg(self.cluster)
-        except errors.DuplicatedVIPNames as e:
+        except (errors.DuplicatedVIPNames, errors.NetworkRoleConflict) as e:
             raise errors.CheckBeforeDeploymentError(e.message)
 
         logger.info(

nailgun/nailgun/test/unit/test_network_check.py

@@ -12,6 +12,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import copy
 from mock import MagicMock
 from mock import patch
 
@@ -600,25 +601,61 @@ class TestCheckVIPsNames(BaseIntegrationTest):
         )
 
         self.cluster = self.env.clusters[0]
+        self.plugin = self.env.create_plugin(cluster=self.cluster)
         self.task = Task(cluster_id=self.cluster.id)
         self.db.add(self.task)
         self.db.flush()
 
-    def test_check_vip_names(self):
-        # in order VIPAssigningConflict error to be raised within
-        # 'check_before_deployment' VIP names introduced by plugins
-        # for the cluster must overlap with those in network configuration
-        # of the cluster itself, so we make here such overlapping
+    def test_fail_if_vips_for_cluster_are_not_unique(self):
         cluster_net_roles = self.cluster.release.network_roles_metadata
-
-        with patch(
-                'nailgun.objects.cluster.PluginManager.get_network_roles',
-                new=MagicMock(return_value=cluster_net_roles)
-        ):
-
-            with self.assertRaises(errors.CheckBeforeDeploymentError) as exc:
-                ApplyChangesTaskManager(self.cluster.id)\
-                    .check_before_deployment(self.task)
+        cluster_net_roles[1]["properties"]["vip"].append({
+            "name": "vip1"
+        })
+        cluster_net_roles[0]["properties"]["vip"].append({
+            "name": "vip1"
+        })
+        self.db.flush()
+        with self.assertRaises(errors.CheckBeforeDeploymentError) as exc:
+            ApplyChangesTaskManager(self.cluster.id)\
+                .check_before_deployment(self.task)
 
             self.assertIn('Duplicate VIP names found in network configuration',
                           exc.exception.message)
+
+    def test_check_vip_names_is_merged(self):
+        cluster_net_roles = self.cluster.release.network_roles_metadata
+        cluster_net_roles[0]["properties"]["vip"].append({
+            "name": "vip1", "namespace": "vip_ns1"
+        })
+        plugin_net_roles = [copy.deepcopy(cluster_net_roles[0])]
+        plugin_net_roles[0]["properties"]["vip"] = [{
+            "name": "vip1", "namespace": "vip_ns1"
+        }]
+        self.plugin.network_roles_metadata = plugin_net_roles
+        self.db.flush()
+        self.assertNotRaises(
+            errors.CheckBeforeDeploymentError,
+            ApplyChangesTaskManager(self.cluster.id)
+            .check_before_deployment,
+            self.task
+        )
+
+    def test_fail_if_vips_cannot_be_merged(self):
+        cluster_net_roles = self.cluster.release.network_roles_metadata
+        cluster_net_roles[0]["properties"]["vip"].append({
+            "name": "vip1", "namespace": "vip_ns1"
+        })
+        plugin_net_roles = [copy.deepcopy(cluster_net_roles[0])]
+        plugin_net_roles[0]["properties"]["vip"] = [{
+            "name": "vip1", "namespace": "vip_ns2"
+        }]
+        self.plugin.network_roles_metadata = plugin_net_roles
+        self.db.flush()
+        with self.assertRaises(errors.CheckBeforeDeploymentError) as exc:
+            ApplyChangesTaskManager(self.cluster.id)\
+                .check_before_deployment(self.task)
+
+            self.assertIn(
+                'Cannot override existing network roles',
+                exc.exception.message
+            )

nailgun/nailgun/test/unit/test_objects.py

@@ -53,6 +53,7 @@ from nailgun.network.neutron import NeutronManager70
 from nailgun import objects
 from nailgun.plugins.manager import PluginManager
 from nailgun.test import base
+from nailgun.utils import dict_merge
 
 
 class TestObjects(BaseIntegrationTest):
@@ -955,8 +956,8 @@ class TestClusterObject(BaseTestCase):
                 ]
             }
         }
-        network_role.update(kwargs)
-        return network_role
+
+        return dict_merge(network_role, kwargs)
 
     def test_network_defaults(self):
         cluster = objects.Cluster.get_by_uid(self.env.create(api=True)['id'])
@@ -1081,7 +1082,7 @@ class TestClusterObject(BaseTestCase):
 
     def test_get_network_roles(self):
         cluster = self.env.clusters[0]
-        self.assertEqual(
+        self.assertItemsEqual(
             objects.Cluster.get_network_roles(cluster),
             cluster.release.network_roles_metadata)
 
@@ -1135,6 +1136,23 @@ class TestClusterObject(BaseTestCase):
             cluster.release.network_roles_metadata + network_roles)
 
     def test_get_plugin_network_roles_fail(self):
+        plugins_kw_list = [
+            self.env.get_default_plugin_metadata(
+                name='test_plugin_{0}'.format(idx),
+                network_roles_metadata=[
+                    self._get_network_role_metadata(
+                        properties={'gateway': bool(idx)}
+                    )
+                ]
+            ) for idx in six.moves.range(2)
+        ]
+
+        cluster = self._create_cluster_with_plugins(plugins_kw_list)
+        self.assertRaises(
+            errors.NetworkRoleConflict,
+            objects.Cluster.get_network_roles, cluster)
+
+    def test_merge_network_roles(self):
         network_roles = [self._get_network_role_metadata()]
         plugins_kw_list = [
             self.env.get_default_plugin_metadata(
@@ -1144,9 +1162,10 @@ class TestClusterObject(BaseTestCase):
         ]
 
         cluster = self._create_cluster_with_plugins(plugins_kw_list)
-        self.assertRaises(
-            errors.NetworkRoleConflict,
-            objects.Cluster.get_network_roles, cluster)
+        self.assertItemsEqual(
+            cluster.release.network_roles_metadata + network_roles,
+            objects.Cluster.get_network_roles(cluster)
+        )
 
     def test_get_volumes_metadata_when_plugins_are_enabled(self):
         plugin_volumes_metadata = {

nailgun/nailgun/test/unit/test_plugins_merge_policies.py

@@ -0,0 +1,90 @@
+# -*- coding: utf-8 -*-
+
+#    Copyright 2015 Mirantis, Inc.
+#
+#    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 nailgun import consts
+from nailgun.errors import errors
+from nailgun.plugins.merge_policies import NetworkRoleMergePolicy
+from nailgun.test.base import BaseTestCase
+
+
+class TestNetworkRoleMergePolicy(BaseTestCase):
+    def setUp(self):
+        super(TestNetworkRoleMergePolicy, self).setUp()
+        self.policy = NetworkRoleMergePolicy()
+
+    @staticmethod
+    def _make_plugin_network_role(**kwargs):
+        properties = {
+            'subnet': True,
+            'gateway': False,
+            'vip': []
+        }
+
+        properties.update(kwargs)
+
+        return {
+            'id': 'test_network_role',
+            'default_mapping': consts.NETWORKS.public,
+            'properties': properties
+        }
+
+    def test_apply_path(self):
+        target = self._make_plugin_network_role(vip=[{'name': 'test_vip_a'}])
+        patch = self._make_plugin_network_role(vip=[{'name': 'test_vip_b'}])
+        self.policy.apply_patch(target, patch)
+        expected = self._make_plugin_network_role(
+            vip=[{'name': 'test_vip_a'}, {'name': 'test_vip_b'}]
+        )
+
+        self.assertDictEqual(expected, target)
+
+    def test_apply_patch_vips_without_duplicates(self):
+        target = self._make_plugin_network_role(
+            vip=[{'name': 'test_vip_a'}, {'name': 'test_vip_b'}]
+        )
+        patch = self._make_plugin_network_role(vip=[{'name': 'test_vip_a'}])
+        self.policy.apply_patch(target, patch)
+        self.assertItemsEqual(
+            [{'name': 'test_vip_a'}, {'name': 'test_vip_b'}],
+            target['properties']['vip']
+        )
+
+    def test_apply_patch_fail_if_conflict(self):
+        with self.assertRaisesRegexp(errors.UnresolvableConflict, 'subnet'):
+            self.policy.apply_patch(
+                self._make_plugin_network_role(subnet=True),
+                self._make_plugin_network_role(subnet=False)
+            )
+
+        with self.assertRaisesRegexp(errors.UnresolvableConflict, 'prop1'):
+            self.policy.apply_patch(
+                self._make_plugin_network_role(prop1=0.1),
+                self._make_plugin_network_role(prop1=1)
+            )
+
+    def test_apply_path_fail_if_vip_conflict(self):
+        with self.assertRaisesRegexp(
+                errors.UnresolvableConflict,
+                "VIP 'test' conflicts with existing one"):
+            self.policy.apply_patch(
+                self._make_plugin_network_role(
+                    vip=[{"name": "test", "value": 1}]
+                ),
+                self._make_plugin_network_role(
+                    vip=[{"name": "test", "value": 2}]
+                )
+            )