Merge "Implement cleanup_instance_network_on_host for neutron API"
This commit is contained in:
Zuul
+30
-33
@@ -4486,20 +4486,16 @@ class ComputeManager(manager.Manager):
|
||||
:param instance: Instance object that was resized/cold migrated
|
||||
:param migration: Migration object for the resize/cold migrate
|
||||
"""
|
||||
# setup_networks_on_host relies on the instance.host not being the same
|
||||
# as the host we pass in, so we have to mutate the instance to
|
||||
# effectively trick this code.
|
||||
with utils.temporary_mutation(instance, host=migration.dest_compute):
|
||||
LOG.debug('Deleting port bindings for source host.',
|
||||
instance=instance)
|
||||
try:
|
||||
self.network_api.setup_networks_on_host(
|
||||
ctxt, instance, host=self.host, teardown=True)
|
||||
except exception.PortBindingDeletionFailed as e:
|
||||
# Do not let this stop us from cleaning up since the guest
|
||||
# is already gone.
|
||||
LOG.error('Failed to delete port bindings from source host. '
|
||||
'Error: %s', six.text_type(e), instance=instance)
|
||||
LOG.debug('Deleting port bindings for source host.',
|
||||
instance=instance)
|
||||
try:
|
||||
self.network_api.cleanup_instance_network_on_host(
|
||||
ctxt, instance, self.host)
|
||||
except exception.PortBindingDeletionFailed as e:
|
||||
# Do not let this stop us from cleaning up since the guest
|
||||
# is already gone.
|
||||
LOG.error('Failed to delete port bindings from source host. '
|
||||
'Error: %s', six.text_type(e), instance=instance)
|
||||
|
||||
def _delete_volume_attachments(self, ctxt, bdms):
|
||||
"""Deletes volume attachment records for the given bdms.
|
||||
@@ -4600,22 +4596,19 @@ class ComputeManager(manager.Manager):
|
||||
self.network_api.migrate_instance_start(
|
||||
ctxt, instance, migration)
|
||||
|
||||
# Delete port bindings for the target host. This relies on the
|
||||
# instance.host not being the same as the host we pass in, so we
|
||||
# have to mutate the instance to effectively trick this code.
|
||||
with utils.temporary_mutation(instance, host=migration.source_compute):
|
||||
LOG.debug('Deleting port bindings for target host %s.',
|
||||
self.host, instance=instance)
|
||||
try:
|
||||
# Note that deleting the destination host port bindings does
|
||||
# not automatically activate the source host port bindings.
|
||||
self.network_api.setup_networks_on_host(
|
||||
ctxt, instance, host=self.host, teardown=True)
|
||||
except exception.PortBindingDeletionFailed as e:
|
||||
# Do not let this stop us from cleaning up since the guest
|
||||
# is already gone.
|
||||
LOG.error('Failed to delete port bindings from target host. '
|
||||
'Error: %s', six.text_type(e), instance=instance)
|
||||
# Delete port bindings for the target host.
|
||||
LOG.debug('Deleting port bindings for target host %s.',
|
||||
self.host, instance=instance)
|
||||
try:
|
||||
# Note that deleting the destination host port bindings does
|
||||
# not automatically activate the source host port bindings.
|
||||
self.network_api.cleanup_instance_network_on_host(
|
||||
ctxt, instance, self.host)
|
||||
except exception.PortBindingDeletionFailed as e:
|
||||
# Do not let this stop us from cleaning up since the guest
|
||||
# is already gone.
|
||||
LOG.error('Failed to delete port bindings from target host. '
|
||||
'Error: %s', six.text_type(e), instance=instance)
|
||||
|
||||
# Delete any volume attachments remaining for this target host.
|
||||
LOG.debug('Deleting volume attachments for target host.',
|
||||
@@ -6419,9 +6412,13 @@ class ComputeManager(manager.Manager):
|
||||
|
||||
self._power_off_instance(context, instance, clean_shutdown)
|
||||
current_power_state = self._get_power_state(context, instance)
|
||||
|
||||
self.network_api.cleanup_instance_network_on_host(context, instance,
|
||||
instance.host)
|
||||
# NOTE(mriedem): cleanup_instance_network_on_host assumes there are
|
||||
# multiple host bindings per port which is not the case with the
|
||||
# shelve/unshelve flow. Doing so will result in failures to update the
|
||||
# port binding on unshelve (since we would have deleted it here).
|
||||
if not utils.is_neutron():
|
||||
self.network_api.cleanup_instance_network_on_host(
|
||||
context, instance, instance.host)
|
||||
network_info = self.network_api.get_instance_nw_info(context, instance)
|
||||
|
||||
block_device_info = self._get_instance_block_device_info(context,
|
||||
|
||||
@@ -340,32 +340,41 @@ class API(base_api.NetworkAPI):
|
||||
# the current instance.host.
|
||||
has_binding_ext = self.supports_port_binding_extension(context)
|
||||
if port_migrating and has_binding_ext:
|
||||
# Attempt to delete all port bindings on the host and raise
|
||||
# any errors at the end.
|
||||
failed_port_ids = []
|
||||
for port in ports:
|
||||
# This call is safe in that 404s for non-existing
|
||||
# bindings are ignored.
|
||||
try:
|
||||
self.delete_port_binding(
|
||||
context, port['id'], host)
|
||||
except exception.PortBindingDeletionFailed:
|
||||
# delete_port_binding will log an error for each
|
||||
# failure but since we're iterating a list we want
|
||||
# to keep track of all failures to build a generic
|
||||
# exception to raise
|
||||
failed_port_ids.append(port['id'])
|
||||
if failed_port_ids:
|
||||
msg = (_("Failed to delete binding for port(s) "
|
||||
"%(port_ids)s and host %(host)s.") %
|
||||
{'port_ids': ','.join(failed_port_ids),
|
||||
'host': host})
|
||||
raise exception.PortBindingDeletionFailed(msg)
|
||||
self._delete_port_bindings(context, ports, host)
|
||||
elif port_migrating:
|
||||
# Setup the port profile
|
||||
self._setup_migration_port_profile(
|
||||
context, instance, host, admin_client, ports)
|
||||
|
||||
def _delete_port_bindings(self, context, ports, host):
|
||||
"""Attempt to delete all port bindings on the host.
|
||||
|
||||
:param context: The user request context.
|
||||
:param ports: list of port dicts to cleanup; the 'id' field is required
|
||||
per port dict in the list
|
||||
:param host: host from which to delete port bindings
|
||||
:raises: PortBindingDeletionFailed if port binding deletion fails.
|
||||
"""
|
||||
failed_port_ids = []
|
||||
for port in ports:
|
||||
# This call is safe in that 404s for non-existing
|
||||
# bindings are ignored.
|
||||
try:
|
||||
self.delete_port_binding(
|
||||
context, port['id'], host)
|
||||
except exception.PortBindingDeletionFailed:
|
||||
# delete_port_binding will log an error for each
|
||||
# failure but since we're iterating a list we want
|
||||
# to keep track of all failures to build a generic
|
||||
# exception to raise
|
||||
failed_port_ids.append(port['id'])
|
||||
if failed_port_ids:
|
||||
msg = (_("Failed to delete binding for port(s) "
|
||||
"%(port_ids)s and host %(host)s.") %
|
||||
{'port_ids': ','.join(failed_port_ids),
|
||||
'host': host})
|
||||
raise exception.PortBindingDeletionFailed(msg)
|
||||
|
||||
def _get_available_networks(self, context, project_id,
|
||||
net_ids=None, neutron=None,
|
||||
auto_allocate=False):
|
||||
@@ -3315,15 +3324,30 @@ class API(base_api.NetworkAPI):
|
||||
context, instance, host, migration, provider_mappings)
|
||||
|
||||
def cleanup_instance_network_on_host(self, context, instance, host):
|
||||
"""Cleanup network for specified instance on host."""
|
||||
# TODO(mriedem): This should likely be implemented at least for the
|
||||
# shelve offload operation because the instance is being removed from
|
||||
# a compute host, VIFs are unplugged, etc, so the ports should also
|
||||
# be unbound, albeit still logically attached to the instance (for the
|
||||
# shelve scenario). If _unbind_ports was going to be leveraged here, it
|
||||
# would have to be adjusted a bit since it currently clears the
|
||||
# device_id field on the port which is not what we'd want for shelve.
|
||||
pass
|
||||
"""Cleanup network for specified instance on host.
|
||||
|
||||
Port bindings for the given host are deleted. The ports associated
|
||||
with the instance via the port device_id field are left intact.
|
||||
|
||||
:param context: The user request context.
|
||||
:param instance: Instance object with the associated ports
|
||||
:param host: host from which to delete port bindings
|
||||
:raises: PortBindingDeletionFailed if port binding deletion fails.
|
||||
"""
|
||||
# First check to see if the port binding extension is supported.
|
||||
if not self.supports_port_binding_extension(context):
|
||||
LOG.info("Neutron extension '%s' is not supported; not cleaning "
|
||||
"up port bindings for host %s.",
|
||||
constants.PORT_BINDING_EXTENDED, host, instance=instance)
|
||||
return
|
||||
# Now get the ports associated with the instance. We go directly to
|
||||
# neutron rather than rely on the info cache just like
|
||||
# setup_networks_on_host.
|
||||
search_opts = {'device_id': instance.uuid,
|
||||
'tenant_id': instance.project_id,
|
||||
'fields': ['id']} # we only need the port id
|
||||
data = self.list_ports(context, **search_opts)
|
||||
self._delete_port_bindings(context, data['ports'], host)
|
||||
|
||||
def _get_pci_mapping_for_migration(self, instance, migration):
|
||||
if not instance.migration_context:
|
||||
|
||||
@@ -11033,19 +11033,13 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
|
||||
"""Happy path test for
|
||||
_confirm_snapshot_based_resize_delete_port_bindings.
|
||||
"""
|
||||
|
||||
def stub_setup_networks_on_host(ctxt, instance, *args, **kwargs):
|
||||
# Make sure the instance.host was mutated to point at the dest host
|
||||
self.assertEqual(self.migration.dest_compute, instance.host)
|
||||
|
||||
with mock.patch.object(
|
||||
self.compute.network_api, 'setup_networks_on_host',
|
||||
side_effect=stub_setup_networks_on_host) as setup_networks:
|
||||
self.compute.network_api,
|
||||
'cleanup_instance_network_on_host') as cleanup_networks:
|
||||
self.compute._confirm_snapshot_based_resize_delete_port_bindings(
|
||||
self.context, self.instance, self.migration)
|
||||
setup_networks.assert_called_once_with(
|
||||
self.context, self.instance, host=self.compute.host,
|
||||
teardown=True)
|
||||
cleanup_networks.assert_called_once_with(
|
||||
self.context, self.instance, self.compute.host)
|
||||
|
||||
def test_confirm_snapshot_based_resize_delete_port_bindings_errors(self):
|
||||
"""Tests error handling for
|
||||
@@ -11053,29 +11047,27 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
|
||||
"""
|
||||
# PortBindingDeletionFailed will be caught and logged.
|
||||
with mock.patch.object(
|
||||
self.compute.network_api, 'setup_networks_on_host',
|
||||
self.compute.network_api, 'cleanup_instance_network_on_host',
|
||||
side_effect=exception.PortBindingDeletionFailed(
|
||||
port_id=uuids.port_id, host=self.compute.host)
|
||||
) as setup_networks:
|
||||
) as cleanup_networks:
|
||||
self.compute._confirm_snapshot_based_resize_delete_port_bindings(
|
||||
self.context, self.instance, self.migration)
|
||||
setup_networks.assert_called_once_with(
|
||||
self.context, self.instance, host=self.compute.host,
|
||||
teardown=True)
|
||||
cleanup_networks.assert_called_once_with(
|
||||
self.context, self.instance, self.compute.host)
|
||||
self.assertIn('Failed to delete port bindings from source host',
|
||||
self.stdlog.logger.output)
|
||||
|
||||
# Anything else is re-raised.
|
||||
func = self.compute._confirm_snapshot_based_resize_delete_port_bindings
|
||||
with mock.patch.object(
|
||||
self.compute.network_api, 'setup_networks_on_host',
|
||||
self.compute.network_api, 'cleanup_instance_network_on_host',
|
||||
side_effect=test.TestingException('neutron down')
|
||||
) as setup_networks:
|
||||
) as cleanup_networks:
|
||||
self.assertRaises(test.TestingException, func,
|
||||
self.context, self.instance, self.migration)
|
||||
setup_networks.assert_called_once_with(
|
||||
self.context, self.instance, host=self.compute.host,
|
||||
teardown=True)
|
||||
cleanup_networks.assert_called_once_with(
|
||||
self.context, self.instance, self.compute.host)
|
||||
|
||||
def test_delete_volume_attachments(self):
|
||||
"""Happy path test for _delete_volume_attachments."""
|
||||
@@ -11206,15 +11198,6 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
|
||||
# at the source compute.
|
||||
self.assertEqual(migration.source_compute, migration.dest_compute)
|
||||
|
||||
def stub_setup_networks_on_host(ctxt, instance, *args, **kwargs):
|
||||
# The instance.host should have been mutated to point at the
|
||||
# source compute.
|
||||
self.assertEqual(self.migration.source_compute, instance.host)
|
||||
# Raise PortBindingDeletionFailed to make sure it's caught and
|
||||
# logged but not fatal.
|
||||
raise exception.PortBindingDeletionFailed(port_id=uuids.port_id,
|
||||
host=self.compute.host)
|
||||
|
||||
with test.nested(
|
||||
mock.patch.object(self.compute, 'network_api'),
|
||||
mock.patch.object(self.compute, '_get_instance_block_device_info'),
|
||||
@@ -11227,8 +11210,11 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
|
||||
):
|
||||
mock_network_api.migrate_instance_start.side_effect = \
|
||||
stub_migrate_instance_start
|
||||
mock_network_api.setup_networks_on_host.side_effect = \
|
||||
stub_setup_networks_on_host
|
||||
# Raise PortBindingDeletionFailed to make sure it's caught and
|
||||
# logged but not fatal.
|
||||
mock_network_api.cleanup_instance_network_on_host.side_effect = \
|
||||
exception.PortBindingDeletionFailed(port_id=uuids.port_id,
|
||||
host=self.compute.host)
|
||||
# Run the code.
|
||||
self.compute._revert_snapshot_based_resize_at_dest(
|
||||
self.context, self.instance, self.migration)
|
||||
@@ -11243,9 +11229,9 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
|
||||
block_device_info=mock_get_bdi.return_value)
|
||||
mock_network_api.migrate_instance_start.assert_called_once_with(
|
||||
self.context, self.instance, self.migration)
|
||||
mock_network_api.setup_networks_on_host.assert_called_once_with(
|
||||
self.context, self.instance, host=self.compute.host,
|
||||
teardown=True)
|
||||
mock_network_api.cleanup_instance_network_on_host.\
|
||||
assert_called_once_with(
|
||||
self.context, self.instance, self.compute.host)
|
||||
# Assert that even though setup_networks_on_host raised
|
||||
# PortBindingDeletionFailed it was handled and logged.
|
||||
self.assertIn('Failed to delete port bindings from target host.',
|
||||
|
||||
@@ -170,7 +170,7 @@ class ShelveComputeManagerTestCase(test_compute.BaseTestCase):
|
||||
mock_notify_instance_usage.assert_has_calls(
|
||||
mock_notify_instance_usage_call_list)
|
||||
mock_power_off.assert_has_calls(mock_power_off_call_list)
|
||||
mock_cleanup.assert_has_calls(mock_cleanup_call_list)
|
||||
mock_cleanup.assert_not_called()
|
||||
mock_snapshot.assert_called_once_with(self.context, instance,
|
||||
'fake_image_id', mock.ANY)
|
||||
mock_get_power_state.assert_has_calls(mock_get_power_state_call_list)
|
||||
|
||||
Reference in New Issue
Block a user