Update RequestSpec.flavor on resize_revert

Since I8abdf58a6537dd5e15a012ea37a7b48abd726579 in Newton
we update the RequestSpec.flavor to the new flavor during
a resize. However, if the resize is reverted, we didn't
revert the RequestSpec.flavor to the previous flavor
on the instance, which could cause issues later when
moving the instance since the scheduler will get a
RequestSpec with a flavor that doesn't match the actual
flavor on the instance.

This fixes the bug by updating the RequestSpec.flavor
with instance.old_flavor on resize_revert in the API.
Functional test wrinkles are added.

This fix was ported from the starlingx-staging/stx-nova
repo commit 71acfeae0.

Conflicts:
      nova/tests/functional/test_servers.py

NOTE(mriedem): The conflict is due to not having change
I40a244601e1612a87bd41cd6ad652202035a1ce4 in Pike, which
would be difficult to backport because of change
I89e2682c9210901cf1992dac2f9068b51f0373cd not being in
Pike, so it's easier to just update the existing tests
as they were in Pike.

Change-Id: Ic6e74702f2a5b57b437f4ffdfbc86c1e34cdac7d
Closes-Bug: #1785339
(cherry picked from commit ef3849e2da)
(cherry picked from commit 9125fc7caf)
(cherry picked from commit f2d2a9a7c5)

nova/compute/api.py

@@ -3187,6 +3187,21 @@ class API(base.Base):
         self._record_action_start(context, instance,
                                   instance_actions.REVERT_RESIZE)
 
+        # Conductor updated the RequestSpec.flavor during the initial resize
+        # operation to point at the new flavor, so we need to update the
+        # RequestSpec to point back at the original flavor, otherwise
+        # subsequent move operations through the scheduler will be using the
+        # wrong flavor.
+        try:
+            reqspec = objects.RequestSpec.get_by_instance_uuid(
+                context, instance.uuid)
+            reqspec.flavor = instance.old_flavor
+            reqspec.save()
+        except exception.RequestSpecNotFound:
+            # TODO(mriedem): Make this a failure in Stein when we drop
+            # compatibility for missing request specs.
+            pass
+
         # TODO(melwitt): We're not rechecking for strict quota here to guard
         # against going over quota during a race at this time because the
         # resource consumption for this operation is written to the database

nova/tests/functional/test_servers.py

@@ -1624,6 +1624,11 @@ class ServerMovingTests(ProviderUsageBaseTestCase):
         dest_allocation = allocations[dest_rp_uuid]['resources']
         self.assertFlavorMatchesAllocation(new_flavor, dest_allocation)
 
+        # Make sure the RequestSpec.flavor matches the new_flavor.
+        ctxt = context.get_admin_context()
+        reqspec = objects.RequestSpec.get_by_instance_uuid(ctxt, server['id'])
+        self.assertEqual(new_flavor['id'], reqspec.flavor.flavorid)
+
     def _test_resize_revert(self, dest_hostname):
         source_hostname = self._other_hostname(dest_hostname)
         source_rp_uuid = self._get_provider_uuid_by_host(source_hostname)
@@ -1640,6 +1645,11 @@ class ServerMovingTests(ProviderUsageBaseTestCase):
         self.api.post_server_action(server['id'], post)
         self._wait_for_state_change(self.api, server, 'ACTIVE')
 
+        # Make sure the RequestSpec.flavor matches the original flavor.
+        ctxt = context.get_admin_context()
+        reqspec = objects.RequestSpec.get_by_instance_uuid(ctxt, server['id'])
+        self.assertEqual(self.flavor1['id'], reqspec.flavor.flavorid)
+
         self._run_periodics()
 
         # the original host expected to have the old resource allocation

nova/tests/unit/compute/test_compute_api.py

@@ -1658,8 +1658,9 @@ class _ComputeAPIUnitTestMixIn(object):
     @mock.patch('nova.objects.Quotas.check_deltas')
     @mock.patch('nova.objects.Migration.get_by_instance_and_status')
     @mock.patch('nova.context.RequestContext.elevated')
-    def _test_revert_resize(self, mock_elevated, mock_get_migration,
-                            mock_check):
+    @mock.patch('nova.objects.RequestSpec.get_by_instance_uuid')
+    def _test_revert_resize(self, mock_get_reqspec, mock_elevated,
+                            mock_get_migration, mock_check):
         params = dict(vm_state=vm_states.RESIZED)
         fake_inst = self._create_instance_obj(params=params)
         fake_inst.old_flavor = fake_inst.flavor
@@ -1691,6 +1692,9 @@ class _ComputeAPIUnitTestMixIn(object):
                 self.context, fake_inst['uuid'], 'finished')
             mock_inst_save.assert_called_once_with(expected_task_state=[None])
             mock_mig_save.assert_called_once_with()
+            mock_get_reqspec.assert_called_once_with(
+                self.context, fake_inst.uuid)
+            mock_get_reqspec.return_value.save.assert_called_once_with()
             mock_record_action.assert_called_once_with(self.context, fake_inst,
                                                        'revertResize')
             mock_revert_resize.assert_called_once_with(