diff --git a/nova/network/neutron.py b/nova/network/neutron.py index aac8a7cc7b2..472bcb5b41b 100644 --- a/nova/network/neutron.py +++ b/nova/network/neutron.py @@ -1318,7 +1318,9 @@ def _update_ports_for_instance(self, context, instance, neutron, port_client = admin_client preexisting_port_ids = [] - created_port_ids = [] + created_port_ids = [ + port_id for _, port_id in requests_and_created_ports + if port_id is not None] ports_in_requested_order = [] nets_in_requested_order = [] created_vifs = [] # this list is for cleanups if we fail @@ -1361,7 +1363,6 @@ def _update_ports_for_instance(self, context, instance, neutron, if created_port_id: port_id = created_port_id - created_port_ids.append(port_id) else: port_id = request.port_id ports_in_requested_order.append(port_id) diff --git a/nova/tests/functional/regressions/test_bug_2134375.py b/nova/tests/functional/regressions/test_bug_2134375.py new file mode 100644 index 00000000000..6d69e9159d4 --- /dev/null +++ b/nova/tests/functional/regressions/test_bug_2134375.py @@ -0,0 +1,81 @@ +# 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 nova.conf + +from unittest import mock + +from nova import context +from nova import exception +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 + +CONF = nova.conf.CONF + + +class TestPortErrorDuringCreateServer( + test.TestCase, integrated_helpers.InstanceHelperMixin, +): + """Regression test for bug 2134375""" + + def setUp(self): + super().setUp() + self.neutron = self.useFixture(nova_fixtures.NeutronFixture(self)) + self.glance = self.useFixture(nova_fixtures.GlanceFixture(self)) + self.placement = self.useFixture(func_fixtures.PlacementFixture()).api + + self.api_fixture = self.useFixture(nova_fixtures.OSAPIFixture( + api_version='v2.1')) + + self.admin_api = self.api_fixture.admin_api + self.admin_api.microversion = 'latest' + self.api = self.admin_api + + self.start_service('conductor') + self.start_service('scheduler') + + self.compute = self._start_compute('host1') + + self.neutron._networks[ + self.neutron.network_1['id']] = self.neutron.network_1 + self.neutron._subnets[ + self.neutron.subnet_1['id']] = self.neutron.subnet_1 + + self.neutron._networks[ + self.neutron.network_2['id']] = self.neutron.network_2 + self.neutron._subnets[ + self.neutron.subnet_2['id']] = self.neutron.subnet_2 + + self.ctxt = context.get_admin_context() + + @mock.patch('nova.network.neutron.API._update_port', + # fails on the 1st call and triggers the cleanup + side_effect=exception.PortNotFound( + '00000000-0000-0000-0000-000000000000')) + def test_update_ports_for_instance_fails_delete_all_created_ports( + self, mock_update_port): + self.flags(max_attempts=1, group='scheduler') + + # There already two port + self.assertEqual(2, len(self.neutron.list_ports( + is_admin=True)['ports'])) + + self._create_server(name= 'test01', + networks=[{'uuid': self.neutron.network_1['id']}, + {'uuid': self.neutron.network_2['id']}], + expected_state='ERROR') + + # Created ports deleted and 2 ports remain + self.assertEqual(2, len(self.neutron.list_ports( + is_admin=True)['ports'])) diff --git a/nova/tests/functional/test_servers.py b/nova/tests/functional/test_servers.py index a25e24e6d95..aa79d802603 100644 --- a/nova/tests/functional/test_servers.py +++ b/nova/tests/functional/test_servers.py @@ -3561,8 +3561,12 @@ def fake_prep_resize(*args, **kwargs): usage = self._get_provider_usages(target_uuid) self.assertEqual(empty_usage, usage) else: + # Use a higher max_retries value to allow enough time for + # finish_resize to complete after multiple reschedules, + # especially under coverage (testenv:cover) where + # instrumentation overhead slows things down. server = self._wait_for_state_change(created_server, - "VERIFY_RESIZE") + "VERIFY_RESIZE", max_retries=30) # Verify that the selected host failed, and was rescheduled to # an alternate host. new_server_host = server.get("OS-EXT-SRV-ATTR:host") diff --git a/nova/tests/unit/network/test_neutron.py b/nova/tests/unit/network/test_neutron.py index 046b39c38c0..33d0aeca71e 100644 --- a/nova/tests/unit/network/test_neutron.py +++ b/nova/tests/unit/network/test_neutron.py @@ -6398,6 +6398,52 @@ def test_update_ports_for_instance_fails_rollback_ports_and_vifs(self, mock_delete_ports.assert_called_once_with( ntrn, instance, [uuids.created_port_id]) + @mock.patch('nova.network.neutron.API.has_dns_extension', + new=mock.Mock(return_value=False)) + @mock.patch('nova.network.neutron.API._populate_neutron_extension_values') + @mock.patch('nova.network.neutron.API._update_port', + # fails on the 1st call and triggers the cleanup + side_effect=exception.PortInUse( + port_id=uuids.created_port_id)) + @mock.patch('nova.network.neutron.API._unbind_ports') + @mock.patch('nova.network.neutron.API._delete_ports') + def test_update_ports_for_instance_fails_delete_all_created_ports(self, + mock_delete_ports, + mock_unbind_ports, + mock_update_port, + mock_populate_ext_values): + """Makes sure we delete created ports if we fail updating ports""" + ctxt = context.get_admin_context() + instance = fake_instance.fake_instance_obj(ctxt) + ntrn = mock.Mock(spec=client.Client) + # we have two requests, all ports where nova + # created the port (on the same network) + requests_and_created_ports = [ + (objects.NetworkRequest(network_id=uuids.network_id, + port_id=uuids.created_port_id), + uuids.created_port_id), + (objects.NetworkRequest(network_id=uuids.network_id, + port_id=uuids.created_port_id2), + uuids.created_port_id2), + ] + network = {'id': uuids.network_id} + nets = {uuids.network_id: network} + self.assertRaises(exception.PortInUse, + neutronapi.API()._update_ports_for_instance, + ctxt, instance, ntrn, ntrn, + requests_and_created_ports, nets, bind_host_id=None, + requested_ports_dict=None, + network_arqs=None) + # assert the calls + mock_update_port.assert_has_calls([ + mock.call(ntrn, instance, uuids.created_port_id, mock.ANY) + ]) + mock_unbind_ports.assert_called_once_with( + ctxt, [], ntrn, ntrn) + # should delete all ports + mock_delete_ports.assert_called_once_with( + ntrn, instance, [uuids.created_port_id, uuids.created_port_id2]) + @mock.patch('nova.network.neutron.API._get_floating_ip_by_address', return_value={"port_id": "1"}) @mock.patch('nova.network.neutron.API._show_port', diff --git a/releasenotes/notes/bug-2134375-580c10cfefc279fd.yaml b/releasenotes/notes/bug-2134375-580c10cfefc279fd.yaml new file mode 100644 index 00000000000..c33fa86c7a5 --- /dev/null +++ b/releasenotes/notes/bug-2134375-580c10cfefc279fd.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + `Bug #2134375`_: Fix ports remain during creating instance + with multiple network when error occurs updating first port. + + .. _bug #2134375: https://launchpad.net/bugs/2134375