From 33932023fb0f394359ae4a67cd5a508027cda7af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Szukie=C5=82oj=C4=87?= Date: Mon, 14 Mar 2016 13:30:45 +0100 Subject: [PATCH] Don't add public network to all nodes In default setup we should not assign public ip to all nodes. Just to ones with controller role. So we filter out public ip network for nodes that should not have it. Change-Id: I2a9ea4d06cc1ba15bad20b817659b7539827472a Closes-Bug: 1415552 --- nailgun/nailgun/api/v1/validators/network.py | 6 + .../extensions/network_manager/manager.py | 33 +--- nailgun/nailgun/objects/cluster.py | 50 +++++ nailgun/nailgun/objects/node.py | 79 +++++++- .../test_cluster_changes_handler.py | 8 +- .../test/integration/test_network_manager.py | 11 +- .../integration/test_node_nic_assignment.py | 179 ++++++++++++++++-- .../test_orchestrator_serializer_70.py | 2 +- 8 files changed, 311 insertions(+), 57 deletions(-) diff --git a/nailgun/nailgun/api/v1/validators/network.py b/nailgun/nailgun/api/v1/validators/network.py index bdff8c9477..64458548d5 100644 --- a/nailgun/nailgun/api/v1/validators/network.py +++ b/nailgun/nailgun/api/v1/validators/network.py @@ -815,6 +815,12 @@ class NetAssignmentValidator(BasicValidator): log_message=True ) else: + if not objects.Node.should_have_public(node_db): + public_id = next( + (n.id for n in node_group_db.networks + if n.name == consts.NETWORKS.public), None) + if public_id is not None: + net_group_ids.discard(public_id) unassigned_net_ids = net_group_ids - net_ids if unassigned_net_ids: raise errors.InvalidData( diff --git a/nailgun/nailgun/extensions/network_manager/manager.py b/nailgun/nailgun/extensions/network_manager/manager.py index 9a50acdeb2..b42b7e69f0 100644 --- a/nailgun/nailgun/extensions/network_manager/manager.py +++ b/nailgun/nailgun/extensions/network_manager/manager.py @@ -576,36 +576,9 @@ class NetworkManager(object): first avalable interface. Checks interface type and already assigned networks. """ - untagged = objects.NetworkGroup.is_untagged(ng) - dedicated = ng.meta.get('dedicated_nic') node_group = objects.NodeGroup.get_by_uid(ng.group_id) for node in node_group.nodes: - ifaces = set(node.interfaces) - for bond in node.bond_interfaces: - ifaces = ifaces ^ set(bond.slaves) - ifaces = sorted(ifaces, key=lambda i: i.name) - for iface in ifaces: - if any(six.moves.map(lambda ng: ng.meta.get('dedicated_nic'), - iface.assigned_networks_list)): - continue - if dedicated and iface.assigned_networks_list: - continue - if untagged and any(six.moves.map( - objects.NetworkGroup.is_untagged, - iface.assigned_networks_list)): - continue - assigned_nets = iface.assigned_networks_list + [ng] - objects.NIC.assign_networks(iface, assigned_nets) - break - else: - logger.warning( - "Cannot assign network %r appropriately for " - "node %r. Set unassigned network to the " - "interface %r", - ng.name, node.name, ifaces[0].name - ) - assigned_nets = ifaces[0].assigned_networks_list + [ng] - objects.NIC.assign_networks(ifaces[0], assigned_nets) + objects.Node.assign_network_to_interface(node, ng) @classmethod def get_default_interfaces_configuration(cls, node): @@ -631,11 +604,13 @@ class NetworkManager(object): ngs.add(admin_net) ngs_by_id = dict((ng.id, ng) for ng in ngs) + should_have_public = objects.Node.should_have_public(node) # sort Network Groups ids by map_priority to_assign_ids = list( next(six.moves.zip(*sorted( [[ng.id, ng.meta['map_priority']] - for ng in ngs], + for ng in ngs if (should_have_public or + ng.name != consts.NETWORKS.public)], key=lambda x: x[1]))) ) ng_ids = set(ng.id for ng in ngs) diff --git a/nailgun/nailgun/objects/cluster.py b/nailgun/nailgun/objects/cluster.py index 37624c1122..043a6bd60b 100644 --- a/nailgun/nailgun/objects/cluster.py +++ b/nailgun/nailgun/objects/cluster.py @@ -387,13 +387,59 @@ class Cluster(NailgunObject): cls.add_pending_changes(instance, "attributes") db().flush() + @classmethod + def _create_public_map(cls, instance, roles_metadata=None): + if instance.network_config.configuration_template is not None: + return + from nailgun import objects + public_map = {} + for node in instance.nodes: + public_map[node.id] = objects.Node.should_have_public( + node, roles_metadata) + return public_map + + @classmethod + def _update_public_network(cls, instance, public_map, roles_metadata): + """Applies changes to node's public_network checked using public_map. + + :param instance: Cluster object + :param public_map: dict of Node.id to should_have_public result. + :param roles_metadata: dict from objects.Cluster.get_roles + """ + + if instance.network_config.configuration_template is not None: + return + from nailgun import objects + for node in instance.nodes: + should_have_public = objects.Node.should_have_public( + node, roles_metadata) + if public_map.get(node.id) == should_have_public: + continue + if should_have_public: + objects.Node.assign_public_network(node) + else: + objects.Node.unassign_public_network(node) + @classmethod def patch_attributes(cls, instance, data): + """Applyes changes to Cluster attributes and updates networks. + + :param instance: Cluster object + :param data: dict + """ + + roles_metadata = Cluster.get_roles(instance) + # Note(kszukielojc): We need to create status map of public networks + # to avoid updating networks if there was no change to node public + # network after patching attributes + public_map = cls._create_public_map(instance, roles_metadata) + PluginManager.process_cluster_attributes(instance, data['editable']) instance.attributes.editable = dict_merge( instance.attributes.editable, data['editable']) cls.add_pending_changes(instance, "attributes") cls.get_network_manager(instance).update_restricted_networks(instance) + cls._update_public_network(instance, public_map, roles_metadata) db().flush() @classmethod @@ -623,6 +669,10 @@ class Cluster(NailgunObject): from nailgun.objects import OpenstackConfig OpenstackConfig.disable_by_nodes(nodes_to_remove) + map( + Node.assign_group, + nodes_to_add + ) map( net_manager.assign_networks_by_default, nodes_to_add diff --git a/nailgun/nailgun/objects/node.py b/nailgun/nailgun/objects/node.py index cc843568dc..f39044c636 100644 --- a/nailgun/nailgun/objects/node.py +++ b/nailgun/nailgun/objects/node.py @@ -51,6 +51,7 @@ from nailgun.objects import IPAddr from nailgun.objects import NailgunCollection from nailgun.objects import NailgunObject from nailgun.objects import NetworkGroup +from nailgun.objects import NetworkGroupCollection from nailgun.objects import NIC from nailgun.objects import Notification from nailgun.objects import Release @@ -349,7 +350,7 @@ class Node(NailgunObject): return False @classmethod - def should_have_public(cls, instance): + def should_have_public(cls, instance, roles_metadata=None): """Determine whether this node should be connected to Public network, no matter with or without an IP address assigned from that network @@ -360,13 +361,13 @@ class Node(NailgunObject): :param instance: Node DB instance :returns: True when node has Public network """ - if cls.should_have_public_with_ip(instance): + roles_metadata = roles_metadata or Cluster.get_roles(instance.cluster) + if cls.should_have_public_with_ip(instance, roles_metadata): return True dvr_enabled = Cluster.neutron_dvr_enabled(instance.cluster) if dvr_enabled: roles = itertools.chain(instance.roles, instance.pending_roles) - roles_metadata = Cluster.get_roles(instance.cluster) for role in roles: if roles_metadata.get(role, {}).get('public_for_dvr_required'): @@ -374,6 +375,69 @@ class Node(NailgunObject): return False + @classmethod + def assign_public_network(cls, node): + public_net = next(NetworkGroupCollection.filter_by( + node.nodegroup.networks, + name=consts.NETWORKS.public), None) + cls.assign_network_to_interface(node, public_net) + + @staticmethod + def get_interfaces_without_bonds_slaves(node): + ifaces = set(node.interfaces) + for bond in node.bond_interfaces: + ifaces ^= set(bond.slaves) + return sorted(ifaces, key=operator.attrgetter('name')) + + @classmethod + def assign_network_to_interface(cls, node, network): + """Assign network to interface by default for single node + + Assign given network to first available interface. + Checks interface type, if network is already assigned + and already assigned networks. + """ + if network is None: + return + untagged = NetworkGroup.is_untagged(network) + dedicated = network.meta.get('dedicated_nic') + ifaces = cls.get_interfaces_without_bonds_slaves(node) + for iface in ifaces: + if dedicated and iface.assigned_networks_list: + continue + for net in iface.assigned_networks_list: + if net.meta.get('dedicated_nic'): + break + if net == network: + return + if untagged and NetworkGroup.is_untagged(net): + break + else: + assigned_nets = iface.assigned_networks_list + [network] + NIC.assign_networks(iface, assigned_nets) + break + else: + logger.warning( + "Cannot assign network %r appropriately for " + "node %r. Set unassigned network to the " + "interface %r", + network.name, node.name, ifaces[0].name + ) + assigned_nets = ifaces[0].assigned_networks_list + [network] + NIC.assign_networks(ifaces[0], assigned_nets) + + @classmethod + def unassign_public_network(cls, node): + public_net = next(NetworkGroupCollection.filter_by( + node.nodegroup.networks, + name=consts.NETWORKS.public), None) + ifaces = cls.get_interfaces_without_bonds_slaves(node) + for iface in ifaces: + network_list = iface.assigned_networks_list + if public_net in network_list: + network_list.remove(public_net) + NIC.assign_networks(iface, network_list) + @classmethod def create(cls, data): """Create Node instance with specified parameters in DB. @@ -423,10 +487,9 @@ class Node(NailgunObject): cls.update_interfaces(new_node) cls.update_interfaces_offloading_modes(new_node) - # adding node into cluster + # role cannot be assigned if cluster_id is not set if new_node_cluster_id: - cls.add_into_cluster(new_node, new_node_cluster_id) - + new_node.cluster_id = new_node_cluster_id # updating roles if roles is not None: cls.update_roles(new_node, roles) @@ -435,6 +498,10 @@ class Node(NailgunObject): if primary_roles is not None: cls.update_primary_roles(new_node, primary_roles) + # adding node into cluster + if new_node_cluster_id: + cls.add_into_cluster(new_node, new_node_cluster_id) + # creating attributes cls.create_discover_notification(new_node) diff --git a/nailgun/nailgun/test/integration/test_cluster_changes_handler.py b/nailgun/nailgun/test/integration/test_cluster_changes_handler.py index 7085e22eba..f7553504b1 100644 --- a/nailgun/nailgun/test/integration/test_cluster_changes_handler.py +++ b/nailgun/nailgun/test/integration/test_cluster_changes_handler.py @@ -397,7 +397,9 @@ class TestHandlers(BaseIntegrationTest): }, cluster_kwargs={ 'net_provider': 'neutron', - 'net_segment_type': 'gre' + 'net_segment_type': 'gre', + 'editable_attributes': {'public_network_assignment': { + 'assign_to_all_nodes': {'value': True}}} }, nodes_kwargs=[ {'roles': ['controller'], 'pending_addition': True}, @@ -883,7 +885,9 @@ class TestHandlers(BaseIntegrationTest): }, cluster_kwargs={ 'net_provider': 'neutron', - 'net_segment_type': 'gre' + 'net_segment_type': 'gre', + 'editable_attributes': {'public_network_assignment': { + 'assign_to_all_nodes': {'value': True}}} }, nodes_kwargs=[ {'roles': ['controller'], 'pending_addition': True}, diff --git a/nailgun/nailgun/test/integration/test_network_manager.py b/nailgun/nailgun/test/integration/test_network_manager.py index 26bd97caf3..1bde21fbd5 100644 --- a/nailgun/nailgun/test/integration/test_network_manager.py +++ b/nailgun/nailgun/test/integration/test_network_manager.py @@ -882,7 +882,10 @@ class TestNetworkManager(BaseIntegrationTest): "name": "eth1", "mac": "00:00:00:00:00:03"}]) self.env.create( - cluster_kwargs={}, + cluster_kwargs={ + 'editable_attributes': {'public_network_assignment': { + 'assign_to_all_nodes': {'value': True}}} + }, nodes_kwargs=[ { "api": True, @@ -943,6 +946,7 @@ class TestNetworkManager(BaseIntegrationTest): def test_update_restricted_networks(self): restricted_net = { 'name': 'restricted_net', + 'map_priority': 5, 'restrictions': [ 'settings:additional_components.ironic.value == false' ] @@ -960,8 +964,9 @@ class TestNetworkManager(BaseIntegrationTest): rel.networks_metadata = netw_meta cluster = self.env.create_cluster( release_id=rel.id, - api=False - ) + api=False, + editable_attributes={'public_network_assignment': { + 'assign_to_all_nodes': {'value': True}}}) self.env.create_node(cluster_id=cluster.id) self.assertEqual(len(filter(lambda ng: ng.name == 'restricted_net', cluster.network_groups)), 0) diff --git a/nailgun/nailgun/test/integration/test_node_nic_assignment.py b/nailgun/nailgun/test/integration/test_node_nic_assignment.py index 17150ca502..1fa3ba8ada 100644 --- a/nailgun/nailgun/test/integration/test_node_nic_assignment.py +++ b/nailgun/nailgun/test/integration/test_node_nic_assignment.py @@ -39,7 +39,11 @@ class TestClusterHandlers(BaseIntegrationTest): {'name': 'eth1', 'mac': self.env.generate_random_mac()}]) node = self.env.create_node(api=True, meta=meta, mac=mac) - self.env.create_cluster(api=True, nodes=[node['id']]) + self.env.create_cluster( + api=True, + editable_attributes={'public_network_assignment': { + 'assign_to_all_nodes': {'value': True}}}, + nodes=[node['id']]) resp = self.app.get( reverse('NodeNICsHandler', kwargs={'node_id': node['id']}), @@ -99,7 +103,10 @@ class TestClusterHandlers(BaseIntegrationTest): class TestNodeHandlers(BaseIntegrationTest): def test_network_assignment_when_node_created_and_added(self): - cluster = self.env.create_cluster(api=True) + cluster = self.env.create_cluster( + api=True, + editable_attributes={'public_network_assignment': { + 'assign_to_all_nodes': {'value': True}}}) mac = self.env.generate_random_mac() meta = self.env.default_metadata() self.env.set_interfaces_in_meta( @@ -122,7 +129,10 @@ class TestNodeHandlers(BaseIntegrationTest): self.assertGreater(len(resp_nic['assigned_networks']), 0) def test_network_assignment_when_node_added(self): - cluster = self.env.create_cluster(api=True) + cluster = self.env.create_cluster( + api=True, + editable_attributes={'public_network_assignment': { + 'assign_to_all_nodes': {'value': True}}}) mac = self.env.generate_random_mac() meta = self.env.default_metadata() self.env.set_interfaces_in_meta( @@ -275,9 +285,12 @@ class TestNodeHandlers(BaseIntegrationTest): self.assertEqual(set(net_name_per_nic[i]), net_names) def test_neutron_assignment_when_network_cfg_changed_then_node_added(self): - cluster = self.env.create_cluster(api=True, - net_provider='neutron', - net_segment_type='vlan') + cluster = self.env.create_cluster( + api=True, + net_provider='neutron', + net_segment_type='vlan', + editable_attributes={'public_network_assignment': { + 'assign_to_all_nodes': {'value': True}}}) resp = self.env.neutron_networks_get(cluster['id']) nets = resp.json_body for net in nets['networks']: @@ -593,25 +606,36 @@ class TestNodePublicNetworkToNICAssignment(BaseIntegrationTest): def test_nova_net_public_network_assigned_to_second_nic_by_name(self): self.env.create_cluster( api=True, - net_provider=consts.CLUSTER_NET_PROVIDERS.nova_network) + net_provider=consts.CLUSTER_NET_PROVIDERS.nova_network, + editable_attributes={'public_network_assignment': { + 'assign_to_all_nodes': {'value': True}}}) self.create_node_and_check_assignment() def test_neutron_gre_public_network_assigned_to_second_nic_by_name(self): - self.env.create_cluster(api=True, - net_provider='neutron', - net_segment_type='gre') + self.env.create_cluster( + api=True, + net_provider='neutron', + net_segment_type='gre', + editable_attributes={'public_network_assignment': { + 'assign_to_all_nodes': {'value': True}}}) self.create_node_and_check_assignment() def test_neutron_tun_public_network_assigned_to_second_nic_by_name(self): - self.env.create_cluster(api=True, - net_provider='neutron', - net_segment_type='tun') + self.env.create_cluster( + api=True, + net_provider='neutron', + net_segment_type='tun', + editable_attributes={'public_network_assignment': { + 'assign_to_all_nodes': {'value': True}}}) self.create_node_and_check_assignment() def test_neutron_vlan_public_network_assigned_to_second_nic_by_name(self): - self.env.create_cluster(api=True, - net_provider='neutron', - net_segment_type='vlan') + self.env.create_cluster( + api=True, + net_provider='neutron', + net_segment_type='vlan', + editable_attributes={'public_network_assignment': { + 'assign_to_all_nodes': {'value': True}}}) self.create_node_and_check_assignment() @@ -716,3 +740,126 @@ class TestNodeNICsHandlersValidation(BaseIntegrationTest): self.assertEqual(resp.status_code, 400) self.assertIn("node is not added to any cluster", resp.json_body['message']) + + +class TestPublicNetworkAssigment(BaseIntegrationTest): + def setUp(self): + super(TestPublicNetworkAssigment, self).setUp() + self.default_networks = [u'fuelweb_admin', u'management', + u'storage', u'private'] + self.public_networks = ['public'] + + def set_assign_public_to_all_nodes(self, cluster, value): + attrs = { + 'public_network_assignment': { + 'assign_to_all_nodes': { + 'value': value + } + } + } + resp = self.app.patch( + reverse( + 'ClusterAttributesHandler', + kwargs={'cluster_id': cluster.id}), + params=jsonutils.dumps({'editable': attrs}), + headers=self.default_headers + ) + self.assertEqual(200, resp.status_code) + ejson = resp.json_body['editable'] + self.assertEqual( + ejson['public_network_assignment']['assign_to_all_nodes']['value'], + value + ) + + def check_nic_assigments(self, node, networks_by_mac): + resp = self.app.get( + reverse('NodeNICsHandler', kwargs={'node_id': node['id']}), + headers=self.default_headers) + + self.assertEqual(resp.status_code, 200) + + for resp_nic in resp.json_body: + net_names = [net['name'] for net in resp_nic['assigned_networks']] + self.assertListEqual(networks_by_mac[resp_nic['mac']], net_names) + + def check_node_public_ip_assigment(self, cluster, node, public_ip): + node_db = objects.Node.get_by_uid(node['id']) + nm = objects.Cluster.get_network_manager(cluster) + nets = nm.get_node_networks(node_db) + ng = nm.get_network_by_netname(consts.NETWORKS.public, nets) + self.assertEqual(public_ip, ng.get('ip')) + + def create_node_with_preset_macs(self, cluster, roles=None): + macs = [self.env.generate_random_mac(), self.env.generate_random_mac()] + meta = self.env.default_metadata() + self.env.set_interfaces_in_meta( + meta, + [{'name': 'eth0', 'mac': macs[0]}, + {'name': 'eth1', 'mac': macs[1]}]) + + node = self.env.create_node(api=True, meta=meta, mac=macs[0], + roles=roles, cluster_id=cluster.id) + return node, macs + + def test_only_controllers_changed_to_all_change(self): + self.env.create_cluster(api=True) + cluster = self.env.clusters[0] + + node1, macs1 = self.create_node_with_preset_macs(cluster, + ['controller']) + self.check_nic_assigments(node1, { + macs1[0]: self.default_networks, + macs1[1]: self.public_networks}) + + node2, macs2 = self.create_node_with_preset_macs(cluster, + roles=['cinder']) + self.check_nic_assigments(node2, { + macs2[0]: self.default_networks, + macs2[1]: []}) + + self.set_assign_public_to_all_nodes(cluster, True) + + self.check_nic_assigments(node1, { + macs1[0]: self.default_networks, + macs1[1]: self.public_networks}) + self.check_nic_assigments(node2, { + macs2[0]: self.default_networks, + macs2[1]: self.public_networks}) + + objects.Cluster.prepare_for_deployment(cluster) + self.check_node_public_ip_assigment(cluster, node1, '172.16.0.2/24') + self.check_node_public_ip_assigment(cluster, node2, '172.16.0.3/24') + + def test_all_to_only_controllers_change(self): + self.env.create_cluster( + api=True, + editable_attributes={'public_network_assignment': { + 'assign_to_all_nodes': {'value': True}}}) + + cluster = self.env.clusters[0] + + node1, macs1 = self.create_node_with_preset_macs(cluster, + ['controller']) + self.check_nic_assigments(node1, { + macs1[0]: self.default_networks, + macs1[1]: self.public_networks}) + + node2, macs2 = self.create_node_with_preset_macs(cluster, + ['cinder']) + self.check_nic_assigments(node2, { + macs2[0]: self.default_networks, + macs2[1]: self.public_networks}) + + self.set_assign_public_to_all_nodes(cluster, False) + + self.check_nic_assigments(node1, { + macs1[0]: self.default_networks, + macs1[1]: self.public_networks}) + self.check_nic_assigments(node2, { + macs2[0]: self.default_networks, + macs2[1]: []}) + + objects.Cluster.prepare_for_deployment(cluster) + self.check_node_public_ip_assigment(cluster, node1, '172.16.0.2/24') + with self.assertRaises(IndexError): + self.check_node_public_ip_assigment(cluster, node2, None) diff --git a/nailgun/nailgun/test/integration/test_orchestrator_serializer_70.py b/nailgun/nailgun/test/integration/test_orchestrator_serializer_70.py index 178af5f3b7..22ac1a59bf 100644 --- a/nailgun/nailgun/test/integration/test_orchestrator_serializer_70.py +++ b/nailgun/nailgun/test/integration/test_orchestrator_serializer_70.py @@ -1242,7 +1242,7 @@ class TestNetworkTemplateSerializer70(BaseDeploymentSerializer, return cluster - def create_more_nodes(self, iface_count=2): + def create_more_nodes(self, iface_count=3): self.env.create_nodes_w_interfaces_count( 1, iface_count, roles=['cinder'], cluster_id=self.cluster.id) self.env.create_nodes_w_interfaces_count(