Filter physnets when ports are pre-allocated to a segment

If an Ironic Node is connected to multiple physnets, and wants to
connect to a Neutron VIF whose network has segments in multiple
physnets it may be unclear which one to connect to.

If the VIF is already allocated to a subnet with a segment mapping,
this can be used to resolve the process to a physnet.

When a network uses Neutron routed segments, this patch first
checks the port's fixed_ips to see if a segment/physnet can be
determined from the subnet it is attached to. If this cannot be
identified (or fixed_ips is unset), the behaviour falls back to
returning all physnets available in the segmented network.

A similar change in Neutron was made in
I56b22820d29b2d57faf28a2f9b685ab0b2c924b4

Change-Id: Ie693257cdb8c44eeaf49cec9678de047f35d5221

ironic/common/neutron.py

@@ -881,11 +881,51 @@ def _get_port_by_uuid(client, port_uuid):
     return port
 
 
+def _get_segment_by_subnet_uuid(client, subnet_uuid):
+    """Return a neutron segment from a subnet UUID.
+
+    :param client: A Neutron client object.
+    :param subnet_uuid: UUID of a Neutron subnet to query.
+    :returns: A dict describing the neutron segment, or None.
+    :raises: InvalidParameterValue if the subnet or segment does not exist.
+    :raises: NetworkError on failure to contact Neutron.
+    """
+    try:
+        subnet = client.get_subnet(subnet_uuid)
+    except openstack_exc.ResourceNotFound:
+        raise exception.InvalidParameterValue(
+            _('Neutron subnet %(subnet_uuid)s was not found') %
+            {'subnet_uuid': subnet_uuid})
+    except openstack_exc.OpenStackCloudException as exc:
+        raise exception.NetworkError(
+            _('Could not retrieve neutron subnet: %s') %
+            exc)
+
+    segment_id = subnet.get('segment_id')
+    if segment_id is None:
+        return None
+
+    try:
+        segment = client.get_segment(segment_id)
+    except openstack_exc.ResourceNotFound:
+        raise exception.InvalidParameterValue(
+            _('Neutron segment %(segment_id)s was not found') %
+            {'segment_id': segment_id})
+    except openstack_exc.OpenStackCloudException as exc:
+        raise exception.NetworkError(
+            _('Could not retrieve neutron segment: %s') %
+            exc)
+
+    return segment
+
+
 def get_physnets_by_port_uuid(client, port_uuid):
     """Return the set of physical networks associated with a neutron port.
 
     Query the network to which the port is attached and return the set of
-    physical networks associated with the segments in that network.
+    physical networks associated with the segments in that network. If a port
+    is assigned to a subnet with a direct segment mapping, return the physnet
+    associated with its segment instead.
 
     :param client: A Neutron client object.
     :param port_uuid: UUID of a Neutron port to query.
@@ -899,6 +939,20 @@ def get_physnets_by_port_uuid(client, port_uuid):
     network = _get_network_by_uuid_or_name(client, network_uuid)
 
     if network.segments is not None:
+        # If a port already has IP addresses assigned, this may indicate
+        # which segment it is connected to.
+        for fixed_ip in port.get('fixed_ips', []):
+            segment = _get_segment_by_subnet_uuid(client,
+                                                  fixed_ip.get('subnet_id'))
+            # When a network uses l2_adjancency, subnets don't map directly to
+            # segments
+            if segment is not None:
+                physnet = segment.get('physical_network')
+                # Return the segment's physnet if there is one for this network
+                # type
+                if physnet is not None:
+                    return set([physnet])
+
         # A network with multiple segments will have a 'segments' parameter
         # which will contain a list of segments. Each segment should have a
         # 'provider:physical_network' parameter which contains the physical

ironic/tests/unit/common/test_neutron.py

@@ -1304,22 +1304,65 @@ class TestGetNetworkByUUIDOrName(base.TestCase):
             network_name, ignore_missing=False)
 
 
+class TestGetSegmentBySubnetUUID(base.TestCase):
+
+    def setUp(self):
+        super(TestGetSegmentBySubnetUUID, self).setUp()
+        self.client = mock.MagicMock()
+
+    def test__get_segment_by_subnet_uuid(self):
+        segment_uuid = '9acb0256-2c1b-420a-b9d7-62bee90b6ed7'
+        subnet_uuid = 'ce9bd3d9-dfb2-40e0-8a96-91431af2759d'
+        subnet = stubs.FakeNeutronSubnet(segment_id=segment_uuid)
+        self.client.get_subnet.return_value = subnet
+        segment = stubs.FakeNeutronSegment(id=segment_uuid)
+        self.client.get_segment.return_value = segment
+        result = neutron._get_segment_by_subnet_uuid(
+            self.client, subnet_uuid)
+        self.client.get_subnet.assert_called_once_with(subnet_uuid)
+        self.client.get_segment.assert_called_once_with(segment_uuid)
+        self.assertEqual(segment, result)
+
+    def test__get_segment_by_subnet_uuid_failure(self):
+        subnet_uuid = 'ce9bd3d9-dfb2-40e0-8a96-91431af2759d'
+        self.client.get_subnet.side_effect = (
+            openstack_exc.OpenStackCloudException())
+        self.assertRaises(exception.NetworkError,
+                          neutron._get_segment_by_subnet_uuid,
+                          self.client, subnet_uuid)
+        self.client.get_subnet.assert_called_once_with(subnet_uuid)
+
+    def test__get_segment_by_subnet_uuid_missing_segment(self):
+        subnet_uuid = 'ce9bd3d9-dfb2-40e0-8a96-91431af2759d'
+        subnet = stubs.FakeNeutronSubnet(segment_id=None)
+        self.client.get_subnet.return_value = subnet
+        result = neutron._get_segment_by_subnet_uuid(
+            self.client, subnet_uuid)
+        self.client.get_subnet.assert_called_once_with(subnet_uuid)
+        self.assertIsNone(result)
+
+
+@mock.patch.object(neutron, '_get_segment_by_subnet_uuid', autospec=True)
 @mock.patch.object(neutron, '_get_network_by_uuid_or_name', autospec=True)
 @mock.patch.object(neutron, '_get_port_by_uuid', autospec=True)
 class TestGetPhysnetsByPortUUID(base.TestCase):
 
-    PORT_FIELDS = ['network_id']
+    PORT_FIELDS = ['network_id', 'fixed_ips']
     NETWORK_FIELDS = ['provider:physical_network', 'segments']
+    SUBNET_FIELDS = ['segment_id']
+    SEGMENT_FIELDS = ['physical_network']
 
     def setUp(self):
         super(TestGetPhysnetsByPortUUID, self).setUp()
         self.client = mock.MagicMock()
 
-    def test_get_physnets_by_port_uuid_single_segment(self, mock_gp, mock_gn):
+    def test_get_physnets_by_port_uuid_single_segment(
+            self, mock_gp, mock_gn, mock_gs):
         port_uuid = 'fake-port-uuid'
         network_uuid = 'fake-network-uuid'
         physnet = 'fake-physnet'
-        mock_gp.return_value = stubs.FakeNeutronPort(network_id=network_uuid)
+        mock_gp.return_value = stubs.FakeNeutronPort(network_id=network_uuid,
+                                                     fixed_ips=[])
         mock_gn.return_value = stubs.FakeNeutronNetwork(
             **{'segments': [
                 {'provider:physical_network': physnet}
@@ -1330,7 +1373,7 @@ class TestGetPhysnetsByPortUUID(base.TestCase):
         self.assertEqual({physnet}, result)
 
     def test_get_physnets_by_port_uuid_no_segment_no_physnet(
-            self, mock_gp, mock_gn):
+            self, mock_gp, mock_gn, mock_gs):
         port_uuid = 'fake-port-uuid'
         network_uuid = 'fake-network-uuid'
         mock_gp.return_value = stubs.FakeNeutronPort(network_id=network_uuid)
@@ -1343,7 +1386,7 @@ class TestGetPhysnetsByPortUUID(base.TestCase):
         self.assertEqual(set(), result)
 
     def test_get_physnets_by_port_uuid_no_segment(
-            self, mock_gp, mock_gn):
+            self, mock_gp, mock_gn, mock_gs):
         port_uuid = 'fake-port-uuid'
         network_uuid = 'fake-network-uuid'
         physnet = 'fake-physnet'
@@ -1356,13 +1399,14 @@ class TestGetPhysnetsByPortUUID(base.TestCase):
         mock_gn.assert_called_once_with(self.client, network_uuid)
         self.assertEqual({physnet}, result)
 
-    def test_get_physnets_by_port_uuid_multiple_segments(self, mock_gp,
-                                                         mock_gn):
+    def test_get_physnets_by_port_uuid_multiple_segments(
+            self, mock_gp, mock_gn, mock_gs):
         port_uuid = 'fake-port-uuid'
         network_uuid = 'fake-network-uuid'
         physnet1 = 'fake-physnet-1'
         physnet2 = 'fake-physnet-2'
-        mock_gp.return_value = stubs.FakeNeutronPort(network_id=network_uuid)
+        mock_gp.return_value = stubs.FakeNeutronPort(network_id=network_uuid,
+                                                     fixed_ips=[])
         mock_gn.return_value = stubs.FakeNeutronNetwork(
             **{'segments': [{'provider:physical_network': physnet1},
                             {'provider:physical_network': physnet2}]})
@@ -1371,11 +1415,49 @@ class TestGetPhysnetsByPortUUID(base.TestCase):
         mock_gn.assert_called_once_with(self.client, network_uuid)
         self.assertEqual({physnet1, physnet2}, result)
 
-    def test_get_physnets_by_port_uuid_multiple_segments_no_physnet(
-            self, mock_gp, mock_gn):
+    def test_get_physnets_by_port_uuid_multiple_segments_fixed_ip_l3(
+            self, mock_gp, mock_gn, mock_gs):
         port_uuid = 'fake-port-uuid'
         network_uuid = 'fake-network-uuid'
-        mock_gp.return_value = stubs.FakeNeutronPort(network_id=network_uuid)
+        subnet_uuid = 'fake-subnet-uuid'
+        physnet1 = 'fake-physnet-1'
+        mock_gp.return_value = stubs.FakeNeutronPort(
+            network_id=network_uuid,
+            fixed_ips=[{'subnet_id': subnet_uuid}])
+        mock_gs.return_value = stubs.FakeNeutronSegment(
+            physical_network=physnet1)
+        result = neutron.get_physnets_by_port_uuid(self.client, port_uuid)
+        mock_gs.assert_called_once_with(self.client, subnet_uuid)
+        mock_gp.assert_called_once_with(self.client, port_uuid)
+        self.assertEqual({physnet1}, result)
+
+    def test_get_physnets_by_port_uuid_multiple_segments_fixed_ip_l2(
+            self, mock_gp, mock_gn, mock_gs):
+        port_uuid = 'fake-port-uuid'
+        network_uuid = 'fake-network-uuid'
+        subnet_uuid = 'fake-subnet-uuid'
+        physnet1 = 'fake-physnet-1'
+        physnet2 = 'fake-physnet-2'
+        mock_gp.return_value = stubs.FakeNeutronPort(
+            network_id=network_uuid,
+            fixed_ips=[{'subnet_id': subnet_uuid}])
+        mock_gs.return_value = None
+        mock_gn.return_value = stubs.FakeNeutronNetwork(
+            **{'segments': [{'provider:physical_network': physnet1},
+                            {'provider:physical_network': physnet2}]})
+        result = neutron.get_physnets_by_port_uuid(self.client, port_uuid)
+        mock_gs.assert_called_once_with(self.client, subnet_uuid)
+        mock_gp.assert_called_once_with(self.client, port_uuid)
+        mock_gn.assert_called_once_with(self.client, network_uuid)
+        self.assertEqual({physnet1, physnet2}, result)
+
+    def test_get_physnets_by_port_uuid_multiple_segments_no_physnet(
+            self, mock_gp, mock_gn, mock_gs):
+        port_uuid = 'fake-port-uuid'
+        network_uuid = 'fake-network-uuid'
+        mock_gp.return_value = stubs.FakeNeutronPort(
+            network_id=network_uuid,
+            fixed_ips=[])
         mock_gn.return_value = stubs.FakeNeutronNetwork(
             **{'segments': [{'provider:physical_network': None},
                             {'provider:physical_network': None}]})
@@ -1384,7 +1466,8 @@ class TestGetPhysnetsByPortUUID(base.TestCase):
         mock_gn.assert_called_once_with(self.client, network_uuid)
         self.assertEqual(set(), result)
 
-    def test_get_physnets_by_port_uuid_port_missing(self, mock_gp, mock_gn):
+    def test_get_physnets_by_port_uuid_port_missing(
+            self, mock_gp, mock_gn, mock_gs):
         port_uuid = 'fake-port-uuid'
         mock_gp.side_effect = exception.InvalidParameterValue('error')
         self.assertRaises(exception.InvalidParameterValue,
@@ -1393,7 +1476,8 @@ class TestGetPhysnetsByPortUUID(base.TestCase):
         mock_gp.assert_called_once_with(self.client, port_uuid)
         self.assertFalse(mock_gn.called)
 
-    def test_get_physnets_by_port_uuid_port_failure(self, mock_gp, mock_gn):
+    def test_get_physnets_by_port_uuid_port_failure(
+            self, mock_gp, mock_gn, mock_gs):
         port_uuid = 'fake-port-uuid'
         mock_gp.side_effect = exception.NetworkError
         self.assertRaises(exception.NetworkError,
@@ -1403,7 +1487,7 @@ class TestGetPhysnetsByPortUUID(base.TestCase):
         self.assertFalse(mock_gn.called)
 
     def test_get_physnets_by_port_uuid_network_missing(
-            self, mock_gp, mock_gn):
+            self, mock_gp, mock_gn, mock_gs):
         port_uuid = 'fake-port-uuid'
         network_uuid = 'fake-network-uuid'
         mock_gp.return_value = stubs.FakeNeutronPort(network_id=network_uuid)
@@ -1415,7 +1499,7 @@ class TestGetPhysnetsByPortUUID(base.TestCase):
         mock_gn.assert_called_once_with(self.client, network_uuid)
 
     def test_get_physnets_by_port_uuid_network_failure(
-            self, mock_gp, mock_gn):
+            self, mock_gp, mock_gn, mock_gs):
         port_uuid = 'fake-port-uuid'
         network_uuid = 'fake-network-uuid'
         mock_gp.return_value = stubs.FakeNeutronPort(network_id=network_uuid)

ironic/tests/unit/stubs.py

@@ -140,7 +140,8 @@ class FakeNeutronSubnet(dict):
                         'gateway_ip',
                         'ipv6_address_mode',
                         'ipv6_ra_mode',
-                        'subnetpool_id']
+                        'subnetpool_id',
+                        'segment_id']
 
         raw = dict.fromkeys(SUBNET_ATTRS)
         raw.update(attrs)
@@ -159,6 +160,32 @@ class FakeNeutronSubnet(dict):
             raise AttributeError(key)
 
 
+class FakeNeutronSegment(dict):
+    def __init__(self, **attrs):
+        SEGMENT_ATTRS = ['id',
+                         'name',
+                         'network_id',
+                         'network_type',
+                         'physical_network',
+                         'segmentation_id']
+
+        raw = dict.fromkeys(SEGMENT_ATTRS)
+        raw.update(attrs)
+        super(FakeNeutronSegment, self).__init__(raw)
+
+    def __getattr__(self, key):
+        try:
+            return self[key]
+        except KeyError:
+            raise AttributeError(key)
+
+    def __setattr__(self, key, value):
+        if key in self:
+            self[key] = value
+        else:
+            raise AttributeError(key)
+
+
 class FakeNeutronNetwork(dict):
     def __init__(self, **attrs):
         NETWORK_ATTRS = ['id',

releasenotes/notes/fix-physnet-identification-6f4e32fa3850de8b.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Fixes the identification of physical network segment mapping so a virtual
+    interface (VIF) which has already been mapped to a physical network segment
+    can be identified.