Merge "nova-manage: modify image properties in request_spec" into unmaintained/zed

Author: Zuul

nova/cmd/manage.py

@@ -3258,9 +3258,10 @@ def _validate_image_properties(self, image_properties):
         # Return the dict so we can update the instance system_metadata
         return image_properties
 
-    def _update_image_properties(self, instance, image_properties):
+    def _update_image_properties(self, ctxt, instance, image_properties):
         """Update instance image properties
 
+        :param ctxt: nova.context.RequestContext
         :param instance: The instance to update
         :param image_properties: List of image properties and values to update
         """
@@ -3284,8 +3285,13 @@ def _update_image_properties(self, instance, image_properties):
         for image_property, value in image_properties.items():
             instance.system_metadata[f'image_{image_property}'] = value
 
+        request_spec = objects.RequestSpec.get_by_instance_uuid(
+            ctxt, instance.uuid)
+        request_spec.image = instance.image_meta
+
         # Save and return 0
         instance.save()
+        request_spec.save()
         return 0
 
     @action_description(_(
@@ -3320,7 +3326,7 @@ def set(self, instance_uuid=None, image_properties=None):
                 instance = objects.Instance.get_by_uuid(
                     cctxt, instance_uuid, expected_attrs=['system_metadata'])
                 return self._update_image_properties(
-                    instance, image_properties)
+                    ctxt, instance, image_properties)
         except ValueError as e:
             print(str(e))
             return 6

nova/tests/unit/cmd/test_manage.py

@@ -4100,6 +4100,8 @@ def test_show_image_properties_unknown_failure(
             image_property='hw_disk_bus')
         self.assertEqual(1, ret, 'return code')
 
+    @mock.patch('nova.objects.RequestSpec.save')
+    @mock.patch('nova.objects.RequestSpec.get_by_instance_uuid')
     @mock.patch('nova.objects.Instance.get_by_uuid')
     @mock.patch('nova.context.target_cell')
     @mock.patch('nova.objects.Instance.save')
@@ -4108,7 +4110,8 @@ def test_show_image_properties_unknown_failure(
     @mock.patch('nova.context.get_admin_context',
                 new=mock.Mock(return_value=mock.sentinel.ctxt))
     def test_set_image_properties(
-        self, mock_instance_save, mock_target_cell, mock_get_instance
+        self, mock_instance_save, mock_target_cell, mock_get_instance,
+            mock_get_request_spec, mock_request_spec_save
     ):
         mock_target_cell.return_value.__enter__.return_value = \
             mock.sentinel.cctxt
@@ -4117,9 +4120,11 @@ def test_set_image_properties(
             vm_state=obj_fields.InstanceState.STOPPED,
             system_metadata={
                 'image_hw_disk_bus': 'virtio',
-            }
+            },
+            image_ref=''
         )
         mock_get_instance.return_value = instance
+        mock_get_request_spec.return_value = objects.RequestSpec()
         ret = self.commands.set(
             instance_uuid=uuidsentinel.instance,
             image_properties=['hw_cdrom_bus=sata']
@@ -4136,7 +4141,12 @@ def test_set_image_properties(
             instance.system_metadata.get('image_hw_disk_bus'),
             'image_hw_disk_bus'
         )
+        image_props = mock_get_request_spec.return_value.image.properties
+        self.assertEqual('sata', image_props.get('hw_cdrom_bus'))
+        self.assertEqual('virtio', image_props.get('hw_disk_bus'))
+
         mock_instance_save.assert_called_once()
+        mock_request_spec_save.assert_called_once()
 
     @mock.patch('nova.objects.Instance.get_by_uuid')
     @mock.patch('nova.objects.InstanceMapping.get_by_instance_uuid',

releasenotes/notes/nova-manage-image-property-bug-2078999-c493fc259d316c24.yaml

@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    Before the `Bug 2078999 <https://bugs.launchpad.net/nova/+bug/2078999>`_ was fixed,
+    the ``nova-manage image_property set`` command would update the image properties
+    embedded in the instance but would not update the ones in the request specs. This
+    led to an unexpected rollback of the image properties that were updated by the
+    command after an instance migration.