From feb3205943d54e19fd5ba5ca56a3e89dcde4dc0c Mon Sep 17 00:00:00 2001 From: Andrew Bonney Date: Tue, 14 Jan 2025 10:04:38 +0000 Subject: [PATCH] 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 | 56 ++++++++- ironic/tests/unit/common/test_neutron.py | 114 +++++++++++++++--- ironic/tests/unit/stubs.py | 29 ++++- ...ysnet-identification-6f4e32fa3850de8b.yaml | 6 + 4 files changed, 188 insertions(+), 17 deletions(-) create mode 100644 releasenotes/notes/fix-physnet-identification-6f4e32fa3850de8b.yaml diff --git a/ironic/common/neutron.py b/ironic/common/neutron.py index 31ceefebd0..3708e2def9 100644 --- a/ironic/common/neutron.py +++ b/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 diff --git a/ironic/tests/unit/common/test_neutron.py b/ironic/tests/unit/common/test_neutron.py index dc6a07893d..b84e9ef54f 100644 --- a/ironic/tests/unit/common/test_neutron.py +++ b/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) diff --git a/ironic/tests/unit/stubs.py b/ironic/tests/unit/stubs.py index 6927f38d2a..45fd3c3860 100644 --- a/ironic/tests/unit/stubs.py +++ b/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', diff --git a/releasenotes/notes/fix-physnet-identification-6f4e32fa3850de8b.yaml b/releasenotes/notes/fix-physnet-identification-6f4e32fa3850de8b.yaml new file mode 100644 index 0000000000..471f9928d4 --- /dev/null +++ b/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.