Revert "Revert resize: wait for events according to hybrid plug"

This reverts commit 7a7a223.

That commit was added because - tl'dr - upon revert resize, Neutron
with the OVS backend and the iptables security group driver would send
us the network-vif-plugged event as soon as we updated the port
binding.

That behaviour has changed with commit 66c7f00. With that commit,
we started unplugging the vifs on the source compute host when doing a
resize. When reverting the resize, the vifs had to be re-plugged again,
regarldess of the networking backend in use. This renders commit
7a7a223. pointless, and it can be
reverted.

Backport is clean from master, and the TODO that was present in the
commit message on master is removed, as its a driver interface change
and can only be done on master.

Closes-bug: 1952003
Change-Id: I3cb39a9ec2c260f422b3c48122b9db512cdd799b
(cherry picked from commit 0b0f40d)

Author: notartom

.zuul.yaml

@@ -602,8 +602,7 @@
         - nova-lvm
         - nova-multi-cell
         - nova-next
-        - nova-ovs-hybrid-plug:
-            voting: false
+        - nova-ovs-hybrid-plug
         - nova-tox-validate-backport:
             voting: false
         - nova-tox-functional-centos8-py36

nova/compute/manager.py

@@ -4803,8 +4803,18 @@ def _finish_revert_snapshot_based_resize_at_source(
                   self.host, instance=instance)
         # TODO(mriedem): Calculate provider mappings when we support
         # cross-cell resize/migrate with ports having resource requests.
-        self._finish_revert_resize_network_migrate_finish(
-            ctxt, instance, migration, provider_mappings=None)
+        # NOTE(hanrong): we need to change migration.dest_compute to
+        # source host temporarily.
+        # "network_api.migrate_instance_finish" will setup the network
+        # for the instance on the destination host. For revert resize,
+        # the instance will back to the source host, the setup of the
+        # network for instance should be on the source host. So set
+        # the migration.dest_compute to source host at here.
+        with utils.temporary_mutation(
+            migration, dest_compute=migration.source_compute
+        ):
+            self.network_api.migrate_instance_finish(
+                ctxt, instance, migration, provider_mappings=None)
         network_info = self.network_api.get_instance_nw_info(ctxt, instance)
 
         # Remember that prep_snapshot_based_resize_at_source destroyed the
@@ -4896,50 +4906,6 @@ def revert_resize(self, context, instance, migration, request_spec):
             self.compute_rpcapi.finish_revert_resize(context, instance,
                     migration, migration.source_compute, request_spec)
 
-    def _finish_revert_resize_network_migrate_finish(
-            self, context, instance, migration, provider_mappings):
-        """Causes port binding to be updated. In some Neutron or port
-        configurations - see NetworkModel.get_bind_time_events() - we
-        expect the vif-plugged event from Neutron immediately and wait for it.
-        The rest of the time, the event is expected further along in the
-        virt driver, so we don't wait here.
-
-        :param context: The request context.
-        :param instance: The instance undergoing the revert resize.
-        :param migration: The Migration object of the resize being reverted.
-        :param provider_mappings: a dict of list of resource provider uuids
-            keyed by port uuid
-        :raises: eventlet.timeout.Timeout or
-                 exception.VirtualInterfacePlugException.
-        """
-        network_info = instance.get_network_info()
-        events = []
-        deadline = CONF.vif_plugging_timeout
-        if deadline and network_info:
-            events = network_info.get_bind_time_events(migration)
-            if events:
-                LOG.debug('Will wait for bind-time events: %s', events)
-        error_cb = self._neutron_failed_migration_callback
-        try:
-            with self.virtapi.wait_for_instance_event(instance, events,
-                                                      deadline=deadline,
-                                                      error_callback=error_cb):
-                # NOTE(hanrong): we need to change migration.dest_compute to
-                # source host temporarily.
-                # "network_api.migrate_instance_finish" will setup the network
-                # for the instance on the destination host. For revert resize,
-                # the instance will back to the source host, the setup of the
-                # network for instance should be on the source host. So set
-                # the migration.dest_compute to source host at here.
-                with utils.temporary_mutation(
-                        migration, dest_compute=migration.source_compute):
-                    self.network_api.migrate_instance_finish(
-                        context, instance, migration, provider_mappings)
-        except eventlet.timeout.Timeout:
-            with excutils.save_and_reraise_exception():
-                LOG.error('Timeout waiting for Neutron events: %s', events,
-                          instance=instance)
-
     @wrap_exception()
     @reverts_task_state
     @wrap_instance_event(prefix='compute')
@@ -4997,8 +4963,18 @@ def _finish_revert_resize(
 
             self.network_api.setup_networks_on_host(context, instance,
                                                     migration.source_compute)
-            self._finish_revert_resize_network_migrate_finish(
-                context, instance, migration, provider_mappings)
+            # NOTE(hanrong): we need to change migration.dest_compute to
+            # source host temporarily. "network_api.migrate_instance_finish"
+            # will setup the network for the instance on the destination host.
+            # For revert resize, the instance will back to the source host, the
+            # setup of the network for instance should be on the source host.
+            # So set the migration.dest_compute to source host at here.
+            with utils.temporary_mutation(
+                    migration, dest_compute=migration.source_compute):
+                self.network_api.migrate_instance_finish(context,
+                                                         instance,
+                                                         migration,
+                                                         provider_mappings)
             network_info = self.network_api.get_instance_nw_info(context,
                                                                  instance)
 
@@ -5075,8 +5051,7 @@ def _fill_provider_mapping_based_on_allocs(
             # the provider mappings. If the instance has ports with
             # resource request then the port update will fail in
             # _update_port_binding_for_instance() called via
-            # _finish_revert_resize_network_migrate_finish() in
-            # finish_revert_resize.
+            # migrate_instance_finish() in finish_revert_resize.
             provider_mappings = None
         return provider_mappings
 
@@ -8287,8 +8262,8 @@ def pre_live_migration(self, context, instance, disk, migrate_data):
         return migrate_data
 
     @staticmethod
-    def _neutron_failed_migration_callback(event_name, instance):
-        msg = ('Neutron reported failure during migration '
+    def _neutron_failed_live_migration_callback(event_name, instance):
+        msg = ('Neutron reported failure during live migration '
                'with %(event)s for instance %(uuid)s')
         msg_args = {'event': event_name, 'uuid': instance.uuid}
         if CONF.vif_plugging_is_fatal:
@@ -8384,7 +8359,7 @@ class _BreakWaitForInstanceEvent(Exception):
                 disk = None
 
             deadline = CONF.vif_plugging_timeout
-            error_cb = self._neutron_failed_migration_callback
+            error_cb = self._neutron_failed_live_migration_callback
             # In order to avoid a race with the vif plugging that the virt
             # driver does on the destination host, we register our events
             # to wait for before calling pre_live_migration. Then if the

nova/network/model.py

@@ -478,17 +478,6 @@ def labeled_ips(self):
                     'ips': ips}
         return []
 
-    def has_bind_time_event(self, migration):
-        """Returns whether this VIF's network-vif-plugged external event will
-        be sent by Neutron at "bind-time" - in other words, as soon as the port
-        binding is updated. This is in the context of updating the port binding
-        to a host that already has the instance in a shutoff state - in
-        practice, this means reverting either a cold migration or a
-        non-same-host resize.
-        """
-        return (self.is_hybrid_plug_enabled() and not
-                migration.is_same_host())
-
     @property
     def has_live_migration_plug_time_event(self):
         """Returns whether this VIF's network-vif-plugged external event will
@@ -557,27 +546,13 @@ def wait(self, do_raise=True):
     def json(self):
         return jsonutils.dumps(self)
 
-    def get_bind_time_events(self, migration):
-        """Returns a list of external events for any VIFs that have
-        "bind-time" events during cold migration.
-        """
-        return [('network-vif-plugged', vif['id'])
-                for vif in self if vif.has_bind_time_event(migration)]
-
     def get_live_migration_plug_time_events(self):
         """Returns a list of external events for any VIFs that have
         "plug-time" events during live migration.
         """
         return [('network-vif-plugged', vif['id'])
                 for vif in self if vif.has_live_migration_plug_time_event]
 
-    def get_plug_time_events(self, migration):
-        """Returns a list of external events for any VIFs that have
-        "plug-time" events during cold migration.
-        """
-        return [('network-vif-plugged', vif['id'])
-                for vif in self if not vif.has_bind_time_event(migration)]
-
     def has_port_with_allocation(self):
         return any(vif.has_allocation() for vif in self)
 

nova/objects/migration.py

@@ -202,9 +202,6 @@ def instance(self):
     def instance(self, instance):
         self._cached_instance = instance
 
-    def is_same_host(self):
-        return self.source_compute == self.dest_compute
-
     @property
     def is_live_migration(self):
         return self.migration_type == fields.MigrationType.LIVE_MIGRATION

nova/tests/unit/compute/test_compute.py

@@ -5801,9 +5801,7 @@ def fake_finish_revert_migration_driver(*args, **kwargs):
             old_vm_state = vm_states.ACTIVE
         else:
             old_vm_state = vm_states.STOPPED
-        params = {'vm_state': old_vm_state,
-                  'info_cache': objects.InstanceInfoCache(
-                      network_info=network_model.NetworkInfo([]))}
+        params = {'vm_state': old_vm_state}
         instance = self._create_fake_instance_obj(params)
         request_spec = objects.RequestSpec()
 
@@ -5956,9 +5954,7 @@ def test_finish_revert_resize_validate_source_compute(self):
         def fake(*args, **kwargs):
             pass
 
-        params = {'info_cache': objects.InstanceInfoCache(
-                      network_info=network_model.NetworkInfo([]))}
-        instance = self._create_fake_instance_obj(params)
+        instance = self._create_fake_instance_obj()
         request_spec = objects.RequestSpec()
 
         self.stub_out('nova.virt.fake.FakeDriver.finish_migration', fake)