Merge "Fix accumulated nits from port binding for live migration series"
This commit is contained in:
Zuul
Gerrit Code Review
commit
4d19b1779c
|
|
@ -1059,10 +1059,6 @@ class ComputeManager(manager.Manager):
|
|||
{'state': event.get_name()},
|
||||
instance_uuid=event.get_instance_uuid())
|
||||
context = nova.context.get_admin_context(read_deleted='yes')
|
||||
# Join on info_cache since that's needed in migrate_instance_start.
|
||||
instance = objects.Instance.get_by_uuid(context,
|
||||
event.get_instance_uuid(),
|
||||
expected_attrs=['info_cache'])
|
||||
vm_power_state = None
|
||||
event_transition = event.get_transition()
|
||||
if event_transition == virtevent.EVENT_LIFECYCLE_STOPPED:
|
||||
|
|
@ -1081,6 +1077,22 @@ class ComputeManager(manager.Manager):
|
|||
else:
|
||||
LOG.warning("Unexpected lifecycle event: %d", event_transition)
|
||||
|
||||
migrate_finish_statuses = {
|
||||
# This happens on the source node and indicates live migration
|
||||
# entered post-copy mode.
|
||||
virtevent.EVENT_LIFECYCLE_POSTCOPY_STARTED: 'running (post-copy)',
|
||||
# Suspended for offline migration.
|
||||
virtevent.EVENT_LIFECYCLE_MIGRATION_COMPLETED: 'running'
|
||||
}
|
||||
|
||||
expected_attrs = []
|
||||
if event_transition in migrate_finish_statuses:
|
||||
# Join on info_cache since that's needed in migrate_instance_start.
|
||||
expected_attrs.append('info_cache')
|
||||
instance = objects.Instance.get_by_uuid(context,
|
||||
event.get_instance_uuid(),
|
||||
expected_attrs=expected_attrs)
|
||||
|
||||
# Note(lpetrut): The event may be delayed, thus not reflecting
|
||||
# the current instance power state. In that case, ignore the event.
|
||||
current_power_state = self._get_power_state(context, instance)
|
||||
|
|
@ -1105,13 +1117,9 @@ class ComputeManager(manager.Manager):
|
|||
# is resumed on the destination host in order to reduce network
|
||||
# downtime. Otherwise the ports are bound to the destination host
|
||||
# in post_live_migration_at_destination.
|
||||
migrate_finish_statuses = {
|
||||
# This happens on the source node and indicates live migration
|
||||
# entered post-copy mode.
|
||||
virtevent.EVENT_LIFECYCLE_POSTCOPY_STARTED: 'running (post-copy)',
|
||||
# Suspended for offline migration.
|
||||
virtevent.EVENT_LIFECYCLE_MIGRATION_COMPLETED: 'running'
|
||||
}
|
||||
# TODO(danms): Explore options for using a different live migration
|
||||
# specific callback for this instead of piggy-backing on the
|
||||
# handle_lifecycle_event callback.
|
||||
if (instance.task_state == task_states.MIGRATING and
|
||||
event_transition in migrate_finish_statuses):
|
||||
status = migrate_finish_statuses[event_transition]
|
||||
|
|
@ -6130,6 +6138,9 @@ class ComputeManager(manager.Manager):
|
|||
action=fields.NotificationAction.LIVE_MIGRATION_PRE,
|
||||
phase=fields.NotificationPhase.START)
|
||||
|
||||
# The driver pre_live_migration will plug vifs on the host.
|
||||
# We call plug_vifs before calling ensure_filtering_rules_for_instance,
|
||||
# to ensure bridge is set up.
|
||||
migrate_data = self.driver.pre_live_migration(context,
|
||||
instance,
|
||||
block_device_info,
|
||||
|
|
|
|||
|
|
@ -242,16 +242,11 @@ class LiveMigrationTask(base.TaskBase):
|
|||
"%s") % destination
|
||||
raise exception.MigrationPreCheckError(msg)
|
||||
|
||||
if not self.network_api.supports_port_binding_extension(self.context):
|
||||
LOG.debug('Extended port binding is not available.',
|
||||
instance=self.instance)
|
||||
# Neutron doesn't support the binding-extended API so we can't
|
||||
# attempt port binding on the destination host.
|
||||
return
|
||||
|
||||
# Check to see that both the source and destination compute hosts
|
||||
# Check to see that neutron supports the binding-extended API and
|
||||
# check to see that both the source and destination compute hosts
|
||||
# are new enough to support the new port binding flow.
|
||||
if (supports_extended_port_binding(self.context, self.source) and
|
||||
if (self.network_api.supports_port_binding_extension(self.context) and
|
||||
supports_extended_port_binding(self.context, self.source) and
|
||||
supports_extended_port_binding(self.context, destination)):
|
||||
self.migrate_data.vifs = (
|
||||
self._bind_ports_on_destination(destination))
|
||||
|
|
|
|||
|
|
@ -407,11 +407,27 @@ 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.
|
||||
self.delete_port_binding(
|
||||
context, port['id'], host)
|
||||
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)
|
||||
elif port_migrating:
|
||||
# Setup the port profile
|
||||
self._setup_migration_port_profile(
|
||||
|
|
@ -1334,20 +1350,19 @@ class API(base_api.NetworkAPI):
|
|||
if resp:
|
||||
LOG.debug('Activated binding for port %s and host %s.',
|
||||
port_id, host)
|
||||
# A 409 means the port binding is already active, which shouldn't
|
||||
# happen if the caller is doing things in the correct order.
|
||||
elif resp.status_code == 409:
|
||||
LOG.warning('Binding for port %s and host %s is already '
|
||||
'active.', port_id, host)
|
||||
else:
|
||||
# A 409 means the port binding is already active, which shouldn't
|
||||
# happen if the caller is doing things in the correct order.
|
||||
if resp.status_code == 409:
|
||||
LOG.warning('Binding for port %s and host %s is already '
|
||||
'active.', port_id, host)
|
||||
else:
|
||||
# Log the details, raise an exception.
|
||||
LOG.error('Unexpected error trying to activate binding '
|
||||
'for port %s and host %s. Code: %s. '
|
||||
'Error: %s', port_id, host, resp.status_code,
|
||||
resp.text)
|
||||
raise exception.PortBindingActivationFailed(
|
||||
port_id=port_id, host=host)
|
||||
# Log the details, raise an exception.
|
||||
LOG.error('Unexpected error trying to activate binding '
|
||||
'for port %s and host %s. Code: %s. '
|
||||
'Error: %s', port_id, host, resp.status_code,
|
||||
resp.text)
|
||||
raise exception.PortBindingActivationFailed(
|
||||
port_id=port_id, host=host)
|
||||
|
||||
def _get_pci_device_profile(self, pci_dev):
|
||||
dev_spec = self.pci_whitelist.get_devspec(pci_dev)
|
||||
|
|
@ -2555,7 +2570,7 @@ class API(base_api.NetworkAPI):
|
|||
# port bindings will be updated correctly when
|
||||
# migrate_instance_finish runs.
|
||||
LOG.error('Unexpected error trying to get binding info '
|
||||
'for port %s and destination host %s. Code: %s. '
|
||||
'for port %s and destination host %s. Code: %i. '
|
||||
'Error: %s', vif['id'], dest_host, resp.status_code,
|
||||
resp.text)
|
||||
|
||||
|
|
|
|||
|
|
@ -55,6 +55,7 @@ class VIFMigrateData(obj_base.NovaObject):
|
|||
# NOTE(mriedem): This might not be sufficient based on how the
|
||||
# destination host is configured for all vif types. See the note in
|
||||
# the libvirt driver here: https://review.openstack.org/#/c/551370/
|
||||
# 29/nova/virt/libvirt/driver.py@7036
|
||||
'source_vif': fields.Field(fields.NetworkVIFModel()),
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -107,10 +107,16 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
|
|||
mock_get_power_state.return_value = current_pwr_state
|
||||
|
||||
self.compute.handle_lifecycle_event(event)
|
||||
|
||||
expected_attrs = []
|
||||
if transition in [virtevent.EVENT_LIFECYCLE_POSTCOPY_STARTED,
|
||||
virtevent.EVENT_LIFECYCLE_MIGRATION_COMPLETED]:
|
||||
expected_attrs.append('info_cache')
|
||||
|
||||
mock_get.assert_called_once_with(
|
||||
test.MatchType(context.RequestContext),
|
||||
event.get_instance_uuid.return_value,
|
||||
expected_attrs=['info_cache'])
|
||||
expected_attrs=expected_attrs)
|
||||
|
||||
if event_pwr_state == current_pwr_state:
|
||||
mock_sync.assert_called_with(mock.ANY, mock_get.return_value,
|
||||
|
|
@ -179,11 +185,11 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
|
|||
with mock.patch.object(self.compute, '_get_power_state',
|
||||
return_value=power_state.PAUSED):
|
||||
with mock.patch.object(self.compute.network_api,
|
||||
'migrate_instance_finish') as mig_finish:
|
||||
'migrate_instance_start') as mig_start:
|
||||
self.compute.handle_lifecycle_event(event)
|
||||
# Since we failed to find the migration record, we shouldn't call
|
||||
# migrate_instance_finish.
|
||||
mig_finish.assert_not_called()
|
||||
# migrate_instance_start.
|
||||
mig_start.assert_not_called()
|
||||
mock_get_migration.assert_called_once_with(
|
||||
test.MatchType(context.RequestContext), uuids.instance,
|
||||
'running (post-copy)')
|
||||
|
|
|
|||
|
|
@ -4450,15 +4450,19 @@ class TestNeutronv2WithMock(_TestNeutronv2Common):
|
|||
port_id=uuids.port1, host='new-host')) as del_binding:
|
||||
with mock.patch.object(self.api, 'supports_port_binding_extension',
|
||||
return_value=True):
|
||||
self.assertRaises(exception.PortBindingDeletionFailed,
|
||||
self.api.setup_networks_on_host,
|
||||
self.context, instance, host='new-host',
|
||||
teardown=True)
|
||||
ex = self.assertRaises(
|
||||
exception.PortBindingDeletionFailed,
|
||||
self.api.setup_networks_on_host,
|
||||
self.context, instance, host='new-host', teardown=True)
|
||||
# Make sure both ports show up in the exception message.
|
||||
self.assertIn(uuids.port1, six.text_type(ex))
|
||||
self.assertIn(uuids.port2, six.text_type(ex))
|
||||
self.api._clear_migration_port_profile.assert_called_once_with(
|
||||
self.context, instance, get_client_mock.return_value,
|
||||
get_ports['ports'])
|
||||
del_binding.assert_called_once_with(
|
||||
self.context, uuids.port1, 'new-host')
|
||||
del_binding.assert_has_calls([
|
||||
mock.call(self.context, uuids.port1, 'new-host'),
|
||||
mock.call(self.context, uuids.port2, 'new-host')])
|
||||
|
||||
@mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock())
|
||||
def test_update_port_profile_for_migration_teardown_true_no_profile(
|
||||
|
|
|
|||
|
|
@ -232,6 +232,28 @@ class HostTestCase(test.NoDBTestCase):
|
|||
self.assertEqual(event.EVENT_LIFECYCLE_POSTCOPY_STARTED,
|
||||
expected_event.transition)
|
||||
|
||||
def test_event_lifecycle_callback_suspended_migrated(self):
|
||||
"""Tests the suspended lifecycle event with libvirt with migrated"""
|
||||
hostimpl = mock.MagicMock()
|
||||
conn = mock.MagicMock()
|
||||
fake_dom_xml = """
|
||||
<domain type='kvm'>
|
||||
<uuid>cef19ce0-0ca2-11df-855d-b19fbce37686</uuid>
|
||||
</domain>
|
||||
"""
|
||||
dom = fakelibvirt.Domain(conn, fake_dom_xml, running=True)
|
||||
# See https://libvirt.org/html/libvirt-libvirt-domain.html for values.
|
||||
VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED = 1
|
||||
with mock.patch.object(host.libvirt,
|
||||
'VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED', new=1,
|
||||
create=True):
|
||||
host.Host._event_lifecycle_callback(
|
||||
conn, dom, fakelibvirt.VIR_DOMAIN_EVENT_SUSPENDED,
|
||||
detail=VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED, opaque=hostimpl)
|
||||
expected_event = hostimpl._queue_event.call_args[0][0]
|
||||
self.assertEqual(event.EVENT_LIFECYCLE_MIGRATION_COMPLETED,
|
||||
expected_event.transition)
|
||||
|
||||
def test_event_emit_delayed_call_delayed(self):
|
||||
ev = event.LifecycleEvent(
|
||||
"cef19ce0-0ca2-11df-855d-b19fbce37686",
|
||||
|
|
|
|||
|
|
@ -7649,14 +7649,9 @@ class LibvirtDriver(driver.ComputeDriver):
|
|||
|
||||
def _pre_live_migration_plug_vifs(self, instance, network_info,
|
||||
migrate_data):
|
||||
# We call plug_vifs before the compute manager calls
|
||||
# ensure_filtering_rules_for_instance, to ensure bridge is set up
|
||||
# Retry operation is necessary because continuously request comes,
|
||||
# concurrent request occurs to iptables, then it complains.
|
||||
if 'vifs' in migrate_data and migrate_data.vifs:
|
||||
LOG.debug('Plugging VIFs using destination host port bindings '
|
||||
'before live migration.', instance=instance)
|
||||
# Plug VIFs using destination host port binding information.
|
||||
vif_plug_nw_info = network_model.NetworkInfo([])
|
||||
for migrate_vif in migrate_data.vifs:
|
||||
vif_plug_nw_info.append(migrate_vif.get_dest_vif())
|
||||
|
|
@ -7664,6 +7659,9 @@ class LibvirtDriver(driver.ComputeDriver):
|
|||
LOG.debug('Plugging VIFs before live migration.',
|
||||
instance=instance)
|
||||
vif_plug_nw_info = network_info
|
||||
# Retry operation is necessary because continuous live migration
|
||||
# requests to the same host cause concurrent requests to iptables,
|
||||
# then it complains.
|
||||
max_retry = CONF.live_migration_retry_count
|
||||
for cnt in range(max_retry):
|
||||
try:
|
||||
|
|
|
|||
|
|
@ -173,7 +173,7 @@ class Host(object):
|
|||
# NOTE(siva_krishnan): We have to check if
|
||||
# VIR_DOMAIN_EVENT_SUSPENDED_POSTCOPY and
|
||||
# VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED exist since the current
|
||||
# minimum version of libvirt (1.2.9) don't have those attributes.
|
||||
# minimum version of libvirt (1.3.1) don't have those attributes.
|
||||
# This check can be removed once MIN_LIBVIRT_VERSION is bumped to
|
||||
# at least 1.3.3.
|
||||
if (hasattr(libvirt, 'VIR_DOMAIN_EVENT_SUSPENDED_POSTCOPY') and
|
||||
|
|
|
|||
Loading…
Reference in New Issue