diff --git a/nailgun/nailgun/extensions/network_manager/manager.py b/nailgun/nailgun/extensions/network_manager/manager.py index a33e08ac7a..d3337d06d5 100644 --- a/nailgun/nailgun/extensions/network_manager/manager.py +++ b/nailgun/nailgun/extensions/network_manager/manager.py @@ -328,20 +328,23 @@ class NetworkManager(object): return result @classmethod - def get_assigned_vips(cls, cluster): - """Return assigned VIPs mapped to names of network groups. + def get_assigned_vips(cls, cluster, network_names=None): + """Return assigned VIPs mapped to IDs of network groups. - :param cluster: Is an instance of :class:`objects.Cluster`. - :returns: A dict of VIPs mapped to names of network groups and - they are grouped by the type. + :param cluster: Is an instance of :class:`models.Cluster`. + :param network_names: Filter return VIPs by names of networks if it is + not None. + :returns: A dict of VIPs mapped to IDs of network groups and + they are grouped by the name. """ cluster_vips = \ objects.IPAddrCollection.get_vips_by_cluster_id(cluster.id) vips = defaultdict(dict) for vip in cluster_vips: - vips[vip.network_data.name][vip.vip_name] = objects.IPAddr.to_dict( - vip - ) + if network_names is not None and \ + vip.network_data.name not in network_names: + continue + vips[vip.network][vip.vip_name] = objects.IPAddr.to_dict(vip) return vips @@ -354,8 +357,8 @@ class NetworkManager(object): used for the upgrading procedure of clusters to copy VIPs from one cluster to the other. - :param cluster: Is an instance of :class:`objects.Cluster`. - :param vips: A dict of VIPs mapped to names of network groups + :param cluster: Is an instance of :class:`models.Cluster`. + :param vips: A dict of VIPs mapped to IDs of network groups that are grouped by the type. :raises: errors.AssignIPError """ @@ -363,43 +366,41 @@ class NetworkManager(object): objects.IPAddrCollection.get_vips_by_cluster_id(cluster.id) assigned_vips = defaultdict(dict) for vip in cluster_vips: - assigned_vips[vip.network_data.name][vip.vip_name] = vip + assigned_vips[vip.network][vip.vip_name] = vip for net_group in cluster.network_groups: - if net_group.name not in vips: + if net_group.id not in vips: continue - assigned_vips_by_type = assigned_vips.get(net_group.name, {}) - for vip_name, vip_dict in six.iteritems(vips[net_group.name]): - if not cls.check_ip_belongs_to_net( - vip_dict['ip_addr'], net_group): + assigned_vips_for_net = assigned_vips.get(net_group.id, {}) + for vip_name, vip in six.iteritems(vips[net_group.id]): + if not cls.check_ip_belongs_to_net(vip['ip_addr'], net_group): ranges = [(rng.first, rng.last) for rng in net_group.ip_ranges] raise errors.AssignIPError( - "Cannot assign VIP with the address '{ip_addr}' " - "because it does not belong to the network {net_id} - " - "'{net_name}' with ranges {net_ranges} of the cluster " - "'{cluster_id}'." - .format( - ip_addr=vip_dict['ip_addr'], + ("Cannot assign VIP with the address '{ip_addr}' " + "because it does not belong to the network {net_id} " + "- '{net_name}' with ranges {net_ranges} and CIDR " + "{net_cidr} of the cluster '{cluster_id}'.").format( + ip_addr=vip['ip_addr'], net_id=net_group.id, net_name=net_group.name, net_ranges=ranges, - cluster_id=cluster.id, - ) + net_cidr=net_group.cidr, + cluster_id=cluster.id) ) - if vip_name in assigned_vips_by_type: - assigned_vip = assigned_vips_by_type[vip_name] + if vip_name in assigned_vips_for_net: + assigned_vip = assigned_vips_for_net[vip_name] objects.IPAddr.update(assigned_vip, { - 'ip_addr': vip_dict['ip_addr'], - 'vip_namespace': vip_dict['vip_namespace'], - 'is_user_defined': vip_dict['is_user_defined'], + 'ip_addr': vip['ip_addr'], + 'vip_namespace': vip['vip_namespace'], + 'is_user_defined': vip['is_user_defined'], }) else: objects.IPAddr.create({ 'network': net_group.id, - 'ip_addr': vip_dict['ip_addr'], + 'ip_addr': vip['ip_addr'], 'vip_name': vip_name, - 'vip_namespace': vip_dict['vip_namespace'], - 'is_user_defined': vip_dict['is_user_defined'], + 'vip_namespace': vip['vip_namespace'], + 'is_user_defined': vip['is_user_defined'], }) @classmethod diff --git a/nailgun/nailgun/extensions/network_manager/tests/test_network_manager.py b/nailgun/nailgun/extensions/network_manager/tests/test_network_manager.py index 4cee8aef61..e2aab71233 100644 --- a/nailgun/nailgun/extensions/network_manager/tests/test_network_manager.py +++ b/nailgun/nailgun/extensions/network_manager/tests/test_network_manager.py @@ -70,6 +70,27 @@ def _convert_vips(vips): return vips +def _set_name(vips): + """Return VIPs mapped to names of network groups. + + :param vips: A dict of VIPs mapped to IDs of network groups. + :returns: A dict of VIPs mapped to names of network groups. + """ + return {objects.NetworkGroup.get_by_uid(id).name: vip + for id, vip in six.iteritems(vips)} + + +def _set_id(vips, cluster): + """Return VIPs mapped to IDs of network groups. + + :param vips: A dict of VIPs mapped to names of network groups. + :param cluster: Is an instance of :class:`models.Cluster`. + :returns: A dict of VIPs mapped to IDs of network groups. + """ + return {ng.id: vips[ng.name] + for ng in cluster.network_groups if ng.name in vips} + + def _make_vip(**kwargs): """Represents a VIP as a dict with some default values.""" data = { @@ -294,7 +315,7 @@ class TestNetworkManager(BaseIntegrationTest): needed_vip_ip = [ vip_info for network, vip_info in six.iteritems(vips_after) - if vip.network_data.name == network and vip.vip_name in vip_info + if vip.network_data.id == network and vip.vip_name in vip_info ][0][vip.vip_name]['ip_addr'] self.assertEqual(needed_vip_ip, ip_before) @@ -756,7 +777,7 @@ class TestNetworkManager(BaseIntegrationTest): cluster = self.create_env_w_controller() self.env.create_ip_addrs_by_rules(cluster, vips_to_create) vips = self.env.network_manager.get_assigned_vips(cluster) - self.assertEqual(vips_to_create, _convert_vips(vips)) + self.assertEqual(vips_to_create, _set_name(_convert_vips(vips))) def test_assign_given_vips_for_net_groups(self): haproxy = consts.NETWORK_VIP_NAMES_V6_1.haproxy @@ -772,6 +793,7 @@ class TestNetworkManager(BaseIntegrationTest): }, } cluster = self.env.create_cluster(api=False) + vips_to_assign = _set_id(vips_to_assign, cluster) self.env.network_manager.assign_given_vips_for_net_groups( cluster, vips_to_assign) vips = self.env.network_manager.get_assigned_vips(cluster) @@ -796,6 +818,7 @@ class TestNetworkManager(BaseIntegrationTest): } expected_msg_regexp = "^Cannot assign VIP with the address '10.10.0.1'" cluster = self.env.create_cluster(api=False) + vips_to_assign = _set_id(vips_to_assign, cluster) with self.assertRaisesRegexp(errors.AssignIPError, expected_msg_regexp): self.env.network_manager.assign_given_vips_for_net_groups( @@ -1577,7 +1600,7 @@ class TestNeutronManager70(BaseIntegrationTest): 'public': '172.16.0.3', }, } - self.assertEqual(expected_vips, _convert_vips(vips)) + self.assertEqual(expected_vips, _set_name(_convert_vips(vips))) def test_assign_given_vips_for_net_groups(self): # rewrite VIPs allocated on creation of cluster @@ -1591,6 +1614,7 @@ class TestNeutronManager70(BaseIntegrationTest): 'public': _make_vip(ip_addr='172.16.0.5'), }, } + vips_to_assign = _set_id(vips_to_assign, self.cluster) self.net_manager.assign_given_vips_for_net_groups( self.cluster, vips_to_assign) vips = self.net_manager.get_assigned_vips(self.cluster) diff --git a/nailgun/nailgun/objects/node.py b/nailgun/nailgun/objects/node.py index 81e489feef..b817abaee0 100644 --- a/nailgun/nailgun/objects/node.py +++ b/nailgun/nailgun/objects/node.py @@ -432,13 +432,19 @@ class Node(NailgunObject): @classmethod def assign_group(cls, instance): if instance.group_id is None and instance.ip: - admin_ngs = db().query(models.NetworkGroup).filter_by( - name="fuelweb_admin") + query = db().query(models.NetworkGroup.cidr, + models.NetworkGroup.group_id).join( + models.NodeGroup.networks + ).filter( + models.NetworkGroup.name == "fuelweb_admin", + models.NetworkGroup.group_id is not None, + # Only node group of the same cluster can be selected. + models.NodeGroup.cluster_id == instance.cluster_id + ) ip = IPAddress(instance.ip) - - for ng in admin_ngs: - if ip in IPNetwork(ng.cidr): - instance.group_id = ng.group_id + for cidr, group_id in query: + if ip in IPNetwork(cidr): + instance.group_id = group_id break if not instance.group_id: diff --git a/nailgun/nailgun/test/unit/test_objects.py b/nailgun/nailgun/test/unit/test_objects.py index 0b3694c47c..3fbcba9bba 100644 --- a/nailgun/nailgun/test/unit/test_objects.py +++ b/nailgun/nailgun/test/unit/test_objects.py @@ -21,6 +21,9 @@ import mock from itertools import cycle from itertools import ifilter + +from nailgun.db import db +from nailgun.db.sqlalchemy import models import uuid from sqlalchemy import inspect as sqlalchemy_inspect @@ -210,6 +213,31 @@ class TestNodeObject(BaseIntegrationTest): self.assertItemsEqual(roles, node.pending_roles) self.assertEqual(node.attributes, cluster.release.node_attributes) + def test_assign_group(self): + cluster = self.env.create( + cluster_kwargs={'api': False}, + nodes_kwargs=[{'role': 'controller'}] * 3) + new_cluster = self.env.create_cluster(api=False) + data = { + 'name': 'custom', + 'cluster_id': new_cluster.id + } + new_group = objects.NodeGroup.create(data) + admin_group_id = db().query( + models.NetworkGroup.id + ).join( + models.NetworkGroup.nodegroup + ).filter( + models.NodeGroup.cluster_id == new_group.cluster_id, + models.NetworkGroup.name == consts.NETWORKS.fuelweb_admin + ).first() + admin_group = objects.NetworkGroup.get_by_uid(admin_group_id) + admin_group.cidr = '10.20.0.0/24' + node = cluster.nodes[0] + roles = node.roles + objects.Node.update_cluster_assignment(node, new_cluster, [], roles) + self.assertEqual(new_group.id, node.group_id) + def test_update_cluster_assignment_with_templates_80(self): cluster = self.env.create( cluster_kwargs={'api': False},