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

@@ -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(

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)

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

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)
 

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},

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)

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)

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(