From 36b110dfc5f8f04234b189a50e96de8dde5f3471 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Wed, 13 Nov 2019 18:27:36 -0500 Subject: [PATCH] Do not reschedule on ExternalNetworkAttachForbidden When creating a server, if network setup fails with ExternalNetworkAttachForbidden then we should abort the build rather than reschedule because it's not going to work on another host either. A functional test is added for this since it's hard to reason about whether or not it works properly due to the async nature of network setup during server create and the _build_resources context manager. It's probably not a bad idea to have a functional test for a scenario that uses external networks either (and we can build on this). Change-Id: I6dd5b3f75e2746f53f9758f1a186794a98dba9bf Closes-Bug: #1852465 --- nova/compute/manager.py | 3 +- .../functional/test_external_networks.py | 80 +++++++++++++++++++ 2 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 nova/tests/functional/test_external_networks.py diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 6767f1b1c2f7..4a400afef181 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -2546,7 +2546,8 @@ class ComputeManager(manager.Manager): 'not rescheduling.') % e.format_message() raise exception.BuildAbortException(instance_uuid=instance.uuid, reason=msg) - except (exception.VirtualInterfaceCreateException, + except (exception.ExternalNetworkAttachForbidden, + exception.VirtualInterfaceCreateException, exception.VirtualInterfaceMacAddressException, exception.FixedIpInvalidOnHost, exception.UnableToAutoAllocateNetwork, diff --git a/nova/tests/functional/test_external_networks.py b/nova/tests/functional/test_external_networks.py new file mode 100644 index 000000000000..0d86a2930016 --- /dev/null +++ b/nova/tests/functional/test_external_networks.py @@ -0,0 +1,80 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import copy + +from oslo_utils.fixture import uuidsentinel as uuids + +from nova import test +from nova.tests import fixtures as nova_fixtures +from nova.tests.functional import fixtures as func_fixtures +from nova.tests.functional import integrated_helpers +from nova.tests.unit.image import fake as fake_image +from nova.tests.unit import policy_fixture + + +class TestNeutronExternalNetworks(test.TestCase, + integrated_helpers.InstanceHelperMixin): + """Tests for creating a server on a neutron network with + router:external=True. + """ + def setUp(self): + super(TestNeutronExternalNetworks, self).setUp() + # Use the standard fixtures. + self.useFixture(policy_fixture.RealPolicyFixture()) + self.useFixture(func_fixtures.PlacementFixture()) + fake_image.stub_out_image_service(self) + self.addCleanup(fake_image.FakeImageService_reset) + neutron = self.useFixture(nova_fixtures.NeutronFixture(self)) + self._setup_external_network(neutron) + # Start nova controller services. + api_fixture = self.useFixture(nova_fixtures.OSAPIFixture( + api_version='v2.1')) + self.api = api_fixture.api + self.start_service('conductor') + self.start_service('scheduler') + self.start_service('compute', host='host1') + + @staticmethod + def _setup_external_network(neutron): + # Add related network to the neutron fixture specifically for these + # tests. It cannot be added globally in the fixture init as it adds a + # second network that makes auto allocation based tests fail due to + # ambiguous networks. + external_network = copy.deepcopy(neutron.network_2) # has shared=False + external_network['name'] = 'external' + external_network['router:external'] = True + external_network['id'] = uuids.external_network + neutron._networks[external_network['id']] = external_network + + def test_non_admin_create_server_on_external_network(self): + """By default policy non-admin users are not allowed to create + servers on external networks that are not shared. Doing so should + result in the server failing to build with an + ExternalNetworkAttachForbidden error. Note that we do not use + SpawnIsSynchronousFixture to make _allocate_network_async synchronous + in this test since that would change the behavior of the + ComputeManager._build_resources method and abort the build which is + not how ExternalNetworkAttachForbidden is really handled in reality. + """ + server = self._build_minimal_create_server_request( + 'test_non_admin_create_server_on_external_network', + image_uuid=fake_image.get_valid_image_id(), + networks=[{'uuid': uuids.external_network}]) + server = self.api.post_server({'server': server}) + server = self._wait_for_state_change(server, 'ERROR') + # The BuildAbortException should be in the server fault message. + self.assertIn('aborted: Failed to allocate the network(s), not ' + 'rescheduling.', server['fault']['message']) + # And ExternalNetworkAttachForbidden should be in the logs. + self.assertIn('ExternalNetworkAttachForbidden', + self.stdlog.logger.output)