diff --git a/nailgun/nailgun/db/sqlalchemy/models/release.py b/nailgun/nailgun/db/sqlalchemy/models/release.py index c5288b7046..6cefb973aa 100644 --- a/nailgun/nailgun/db/sqlalchemy/models/release.py +++ b/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={}) diff --git a/nailgun/nailgun/errors/__init__.py b/nailgun/nailgun/errors/__init__.py index 06d290d8d9..ba1b2fe3b9 100644 --- a/nailgun/nailgun/errors/__init__.py +++ b/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" } diff --git a/nailgun/nailgun/objects/cluster.py b/nailgun/nailgun/objects/cluster.py index b5af21c9f3..79f99984ec 100644 --- a/nailgun/nailgun/objects/cluster.py +++ b/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): diff --git a/nailgun/nailgun/plugins/manager.py b/nailgun/nailgun/plugins/manager.py index 02de05c28b..47e8453aa9 100644 --- a/nailgun/nailgun/plugins/manager.py +++ b/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): diff --git a/nailgun/nailgun/plugins/merge_policies.py b/nailgun/nailgun/plugins/merge_policies.py new file mode 100644 index 0000000000..f41240fb07 --- /dev/null +++ b/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) + ) diff --git a/nailgun/nailgun/task/manager.py b/nailgun/nailgun/task/manager.py index 14b4ce28e4..ad0a603e99 100644 --- a/nailgun/nailgun/task/manager.py +++ b/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( diff --git a/nailgun/nailgun/test/unit/test_network_check.py b/nailgun/nailgun/test/unit/test_network_check.py index 445139e1f9..c3a3a64ba3 100644 --- a/nailgun/nailgun/test/unit/test_network_check.py +++ b/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 + ) diff --git a/nailgun/nailgun/test/unit/test_objects.py b/nailgun/nailgun/test/unit/test_objects.py index ed669ec332..bbfdfac2ca 100644 --- a/nailgun/nailgun/test/unit/test_objects.py +++ b/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 = { diff --git a/nailgun/nailgun/test/unit/test_plugins_merge_policies.py b/nailgun/nailgun/test/unit/test_plugins_merge_policies.py new file mode 100644 index 0000000000..0c37dbe60d --- /dev/null +++ b/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}] + ) + )