diff --git a/nova/compute/manager.py b/nova/compute/manager.py index b50b8a17045b..5af0e3369307 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -1864,12 +1864,21 @@ class ComputeManager(manager.Manager): retry['exc_reason'] = e.kwargs['reason'] # NOTE(comstud): Deallocate networks if the driver wants # us to do so. + # NOTE(mriedem): Always deallocate networking when using Neutron. + # This is to unbind any ports that the user supplied in the server + # create request, or delete any ports that nova created which were + # meant to be bound to this host. This check intentionally bypasses + # the result of deallocate_networks_on_reschedule because the + # default value in the driver is False, but that method was really + # only meant for Ironic and should be removed when nova-network is + # removed (since is_neutron() will then always be True). # NOTE(vladikr): SR-IOV ports should be deallocated to # allow new sriov pci devices to be allocated on a new host. # Otherwise, if devices with pci addresses are already allocated # on the destination host, the instance will fail to spawn. # info_cache.network_info should be present at this stage. if (self.driver.deallocate_networks_on_reschedule(instance) or + utils.is_neutron() or self.deallocate_sriov_ports_on_reschedule(instance)): self._cleanup_allocated_networks(context, instance, requested_networks) diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 4318be81ec31..2187d9475aee 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -4741,6 +4741,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): def test_rescheduled_exception(self, mock_hooks, mock_build_run, mock_build, mock_set, mock_nil, mock_save, mock_start, mock_finish): + self.flags(use_neutron=False) self._do_build_instance_update(mock_save, reschedule_update=True) mock_build_run.side_effect = exception.RescheduledException(reason='', instance_uuid=self.instance.uuid) @@ -4826,6 +4827,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): mock_macs_for_instance, mock_event_finish, mock_event_start, mock_ins_save, mock_build_ins, mock_build_and_run): + self.flags(use_neutron=False) instance = fake_instance.fake_instance_obj(self.context, vm_state=vm_states.ACTIVE, system_metadata={'network_allocated': 'True'}, @@ -4862,6 +4864,54 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): self.security_groups, self.block_device_mapping, request_spec={}, host_lists=[fake_host_list]) + @mock.patch.object(manager.ComputeManager, '_build_and_run_instance') + @mock.patch.object(manager.ComputeManager, '_cleanup_allocated_networks') + @mock.patch.object(conductor_api.ComputeTaskAPI, 'build_instances') + @mock.patch.object(objects.Instance, 'save') + @mock.patch.object(objects.InstanceActionEvent, 'event_start') + @mock.patch.object(objects.InstanceActionEvent, + 'event_finish_with_failure') + @mock.patch.object(virt_driver.ComputeDriver, 'macs_for_instance') + def test_rescheduled_exception_with_network_allocated_with_neutron(self, + mock_macs_for_instance, mock_event_finish, mock_event_start, + mock_ins_save, mock_build_ins, mock_cleanup_network, + mock_build_and_run): + """Tests that we always cleanup allocated networks for the instance + when using neutron and before we reschedule off the failed host. + """ + instance = fake_instance.fake_instance_obj(self.context, + vm_state=vm_states.ACTIVE, + system_metadata={'network_allocated': 'True'}, + expected_attrs=['metadata', 'system_metadata', 'info_cache']) + mock_ins_save.return_value = instance + mock_macs_for_instance.return_value = [] + mock_build_and_run.side_effect = exception.RescheduledException( + reason='', instance_uuid=self.instance.uuid) + + self.compute._do_build_and_run_instance(self.context, instance, + self.image, request_spec={}, + filter_properties=self.filter_properties, + injected_files=self.injected_files, + admin_password=self.admin_pass, + requested_networks=self.requested_networks, + security_groups=self.security_groups, + block_device_mapping=self.block_device_mapping, node=self.node, + limits=self.limits, host_list=fake_host_list) + + mock_build_and_run.assert_called_once_with(self.context, + instance, + self.image, self.injected_files, self.admin_pass, + self.requested_networks, self.security_groups, + self.block_device_mapping, self.node, self.limits, + self.filter_properties, {}) + mock_cleanup_network.assert_called_once_with( + self.context, instance, self.requested_networks) + mock_build_ins.assert_called_once_with(self.context, + [instance], self.image, self.filter_properties, + self.admin_pass, self.injected_files, self.requested_networks, + self.security_groups, self.block_device_mapping, + request_spec={}, host_lists=[fake_host_list]) + @mock.patch.object(manager.ComputeManager, '_build_and_run_instance') @mock.patch.object(conductor_api.ComputeTaskAPI, 'build_instances') @mock.patch.object(manager.ComputeManager, '_cleanup_allocated_networks') @@ -4982,6 +5032,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): mock_build_run, mock_build, mock_deallocate, mock_nil, mock_clean_net, mock_save, mock_start, mock_finish): + self.flags(use_neutron=False) self._do_build_instance_update(mock_save, reschedule_update=True) mock_build_run.side_effect = exception.RescheduledException(reason='', instance_uuid=self.instance.uuid) @@ -5425,6 +5476,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): def test_reschedule_on_resources_unavailable(self, mock_claim, mock_build, mock_nil, mock_save, mock_start, mock_finish, mock_notify): + self.flags(use_neutron=False) reason = 'resource unavailable' exc = exception.ComputeResourcesUnavailable(reason=reason) mock_claim.side_effect = exc