From 6dbe7b773b7a61380f9501ff6f0480baa87fed14 Mon Sep 17 00:00:00 2001 From: Andrew Bonney Date: Tue, 26 Apr 2022 11:35:38 +0100 Subject: [PATCH 1/2] Fix segment-aware scheduling permissions error Resolves a bug encountered when setting the Nova scheduler to be aware of Neutron routed provider network segments, by using 'query_placement_for_routed_network_aggregates'. Non-admin users attempting to access the 'segment_id' attribute of a subnet caused a traceback, resulting in instance creation failure. This patch ensures the Neutron client is initialised with an administrative context no matter what the requesting user's permissions are. Change-Id: Ic0f25e4d2395560fc2b68f3b469e266ac59abaa2 Closes-Bug: #1970383 (cherry picked from commit ee32934f34afd8e6df467361e9d71788cd36f6ee) (cherry picked from commit 60548e804219d91d8c68ab3d74dd0ae956cd33f3) (cherry picked from commit 28f94eb69a9b8267275460d0dbc18b3d7309cebd) --- nova/network/neutron.py | 4 ++-- nova/tests/unit/network/test_neutron.py | 6 ++++++ ...83-segment-scheduling-permissions-92ba907b10a9eb1c.yaml | 7 +++++++ 3 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/bug-1970383-segment-scheduling-permissions-92ba907b10a9eb1c.yaml diff --git a/nova/network/neutron.py b/nova/network/neutron.py index d35dcad16e2..e17e2533a9d 100644 --- a/nova/network/neutron.py +++ b/nova/network/neutron.py @@ -3567,7 +3567,7 @@ def get_segment_ids_for_network( if not self._has_segment_extension(context): return [] - client = get_client(context) + client = get_client(context, admin=True) try: # NOTE(sbauza): We can't use list_segments() directly because the # API is borked and returns both segments but also segmentation IDs @@ -3597,7 +3597,7 @@ def get_segment_id_for_subnet( if not self._has_segment_extension(context): return None - client = get_client(context) + client = get_client(context, admin=True) try: subnet = client.show_subnet(subnet_id)['subnet'] except neutron_client_exc.NeutronClientException as e: diff --git a/nova/tests/unit/network/test_neutron.py b/nova/tests/unit/network/test_neutron.py index 24e3ac3ed3b..49c780ffdcd 100644 --- a/nova/tests/unit/network/test_neutron.py +++ b/nova/tests/unit/network/test_neutron.py @@ -6323,6 +6323,7 @@ def test_get_segment_ids_for_network_passes(self, mock_client): res = self.api.get_segment_ids_for_network( self.context, uuids.network_id) self.assertEqual([uuids.segment_id], res) + mock_client.assert_called_once_with(self.context, admin=True) mocked_client.list_subnets.assert_called_once_with( network_id=uuids.network_id, fields='segment_id') @@ -6338,6 +6339,7 @@ def test_get_segment_ids_for_network_with_no_segments(self, mock_client): res = self.api.get_segment_ids_for_network( self.context, uuids.network_id) self.assertEqual([], res) + mock_client.assert_called_once_with(self.context, admin=True) mocked_client.list_subnets.assert_called_once_with( network_id=uuids.network_id, fields='segment_id') @@ -6353,6 +6355,7 @@ def test_get_segment_ids_for_network_fails(self, mock_client): self.assertRaises(exception.InvalidRoutedNetworkConfiguration, self.api.get_segment_ids_for_network, self.context, uuids.network_id) + mock_client.assert_called_once_with(self.context, admin=True) def test_get_segment_id_for_subnet_no_segment_ext(self): with mock.patch.object( @@ -6374,6 +6377,7 @@ def test_get_segment_id_for_subnet_passes(self, mock_client): res = self.api.get_segment_id_for_subnet( self.context, uuids.subnet_id) self.assertEqual(uuids.segment_id, res) + mock_client.assert_called_once_with(self.context, admin=True) mocked_client.show_subnet.assert_called_once_with(uuids.subnet_id) @mock.patch.object(neutronapi, 'get_client') @@ -6388,6 +6392,7 @@ def test_get_segment_id_for_subnet_with_no_segment(self, mock_client): self.assertIsNone( self.api.get_segment_id_for_subnet(self.context, uuids.subnet_id)) + mock_client.assert_called_once_with(self.context, admin=True) @mock.patch.object(neutronapi, 'get_client') def test_get_segment_id_for_subnet_fails(self, mock_client): @@ -6401,6 +6406,7 @@ def test_get_segment_id_for_subnet_fails(self, mock_client): self.assertRaises(exception.InvalidRoutedNetworkConfiguration, self.api.get_segment_id_for_subnet, self.context, uuids.subnet_id) + mock_client.assert_called_once_with(self.context, admin=True) @mock.patch.object(neutronapi.LOG, 'debug') def test_get_port_pci_slot(self, mock_debug): diff --git a/releasenotes/notes/bug-1970383-segment-scheduling-permissions-92ba907b10a9eb1c.yaml b/releasenotes/notes/bug-1970383-segment-scheduling-permissions-92ba907b10a9eb1c.yaml new file mode 100644 index 00000000000..88495079e75 --- /dev/null +++ b/releasenotes/notes/bug-1970383-segment-scheduling-permissions-92ba907b10a9eb1c.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + `Bug #1970383 `_: Fixes a + permissions error when using the + 'query_placement_for_routed_network_aggregates' scheduler variable, which + caused a traceback on instance creation for non-admin users. From e6c3819f931a4303e37a9556a8278691cab399bd Mon Sep 17 00:00:00 2001 From: Sylvain Bauza Date: Wed, 3 May 2023 17:00:14 +0200 Subject: [PATCH 2/2] Fix get_segments_id with subnets without segment_id Unfortunatly when we merged Ie166f3b51fddeaf916cda7c5ac34bbcdda0fd17a we forgot that subnets can have no segment_id field. Change-Id: Idb35b7e3c69fe8efe498abe4ebcc6cad8918c4ed Closes-Bug: #2018375 (cherry picked from commit 6d7bd6a03446d5227d515b2b4c0da632ef4aa4a1) (cherry picked from commit 6b8d9d419170fb0ec2c6df561a0874e6362382c1) (cherry picked from commit 77db64237b23050d94df113a38412c5333d23357) (cherry picked from commit 9a6a421c045a8031ff0923cca9aa7195fe987896) (cherry picked from commit d3f44cd0e77ddacc4c9c9ab57469e5c02fbce1ee) --- nova/network/neutron.py | 2 +- nova/tests/unit/network/test_neutron.py | 18 +++++++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/nova/network/neutron.py b/nova/network/neutron.py index e17e2533a9d..8a0f8292170 100644 --- a/nova/network/neutron.py +++ b/nova/network/neutron.py @@ -3579,7 +3579,7 @@ def get_segment_ids_for_network( 'Failed to get segment IDs for network %s' % network_id) from e # The segment field of an unconfigured subnet could be None return [subnet['segment_id'] for subnet in subnets - if subnet['segment_id'] is not None] + if subnet.get('segment_id') is not None] def get_segment_id_for_subnet( self, diff --git a/nova/tests/unit/network/test_neutron.py b/nova/tests/unit/network/test_neutron.py index 49c780ffdcd..22d36e00ccb 100644 --- a/nova/tests/unit/network/test_neutron.py +++ b/nova/tests/unit/network/test_neutron.py @@ -6328,7 +6328,7 @@ def test_get_segment_ids_for_network_passes(self, mock_client): network_id=uuids.network_id, fields='segment_id') @mock.patch.object(neutronapi, 'get_client') - def test_get_segment_ids_for_network_with_no_segments(self, mock_client): + def test_get_segment_ids_for_network_with_segments_none(self, mock_client): subnets = {'subnets': [{'segment_id': None}]} mocked_client = mock.create_autospec(client.Client) mock_client.return_value = mocked_client @@ -6343,6 +6343,22 @@ def test_get_segment_ids_for_network_with_no_segments(self, mock_client): mocked_client.list_subnets.assert_called_once_with( network_id=uuids.network_id, fields='segment_id') + @mock.patch.object(neutronapi, 'get_client') + def test_get_segment_ids_for_network_with_no_segments(self, mock_client): + subnets = {'subnets': [{}]} + mocked_client = mock.create_autospec(client.Client) + mock_client.return_value = mocked_client + mocked_client.list_subnets.return_value = subnets + with mock.patch.object( + self.api, '_has_segment_extension', return_value=True, + ): + res = self.api.get_segment_ids_for_network( + self.context, uuids.network_id) + self.assertEqual([], res) + mock_client.assert_called_once_with(self.context, admin=True) + mocked_client.list_subnets.assert_called_once_with( + network_id=uuids.network_id, fields='segment_id') + @mock.patch.object(neutronapi, 'get_client') def test_get_segment_ids_for_network_fails(self, mock_client): mocked_client = mock.create_autospec(client.Client)