Synchronise zed with upstream

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',

nova/tests/unit/virt/libvirt/test_blockinfo.py

@@ -1289,6 +1289,7 @@ def test_get_root_info_no_bdm_empty_image_meta(self, mock_find_dev):
 
     @mock.patch('nova.virt.libvirt.blockinfo.get_info_from_bdm')
     def test_get_root_info_bdm(self, mock_get_info):
+        # call get_root_info() with DriverBlockDevice
         instance = objects.Instance(**self.test_instance)
         image_meta = objects.ImageMeta.from_dict(self.test_image_meta)
         root_bdm = {'mount_device': '/dev/vda',
@@ -1318,6 +1319,49 @@ def test_get_root_info_bdm(self, mock_get_info):
                                               {}, 'virtio')
         mock_get_info.reset_mock()
 
+    @mock.patch('nova.virt.libvirt.blockinfo.get_info_from_bdm')
+    def test_get_root_info_bdm_with_deepcopy(self, mock_get_info):
+        # call get_root_info() with BlockDeviceMapping
+        instance = objects.Instance(**self.test_instance)
+        image_meta = objects.ImageMeta.from_dict(self.test_image_meta)
+        root_bdm = objects.BlockDeviceMapping(self.context,
+            **fake_block_device.FakeDbBlockDeviceDict(
+                {'id': 3, 'instance_uuid': uuids.instance,
+                 'device_name': '/dev/sda',
+                 'source_type': 'blank',
+                 'destination_type': 'local',
+                 'device_type': 'cdrom',
+                 'disk_bus': 'virtio',
+                 'volume_id': 'fake-volume-id-1',
+                 'boot_index': 0}))
+        # No root_device_name
+        blockinfo.get_root_info(
+            instance, 'kvm', image_meta, root_bdm, 'virtio', 'ide')
+        mock_get_info.assert_called_once_with(
+            instance, 'kvm', image_meta, root_bdm, {}, 'virtio')
+        mock_get_info.reset_mock()
+        # Both device names
+        blockinfo.get_root_info(
+            instance, 'kvm', image_meta, root_bdm, 'virtio', 'scsi',
+            root_device_name='/dev/sda')
+        mock_get_info.assert_called_once_with(
+            instance, 'kvm', image_meta, root_bdm, {}, 'virtio')
+        mock_get_info.reset_mock()
+        # Missing device names
+        original_bdm = copy.deepcopy(root_bdm)
+        root_bdm.device_name = ''
+        blockinfo.get_root_info(
+            instance, 'kvm', image_meta, root_bdm, 'virtio', 'scsi',
+            root_device_name='/dev/sda')
+        mock_get_info.assert_called_with(
+            instance, 'kvm', image_meta, mock.ANY, {}, 'virtio')
+        actual_call = mock_get_info.call_args
+        _, _, _, actual_bdm, _, _ = actual_call[0]
+        self.assertEqual(
+            original_bdm.obj_to_primitive(),
+            actual_bdm.obj_to_primitive()
+        )
+
     def test_get_boot_order_simple(self):
         disk_info = {
             'disk_bus': 'virtio',

nova/virt/libvirt/blockinfo.py

@@ -69,6 +69,7 @@
 
 """
 
+import copy
 import itertools
 import operator
 
@@ -444,12 +445,13 @@ def get_root_info(instance, virt_type, image_meta, root_bdm,
                 'dev': block_device.strip_dev(root_device_name),
                 'boot_index': '1'}
 
+    root_bdm_copy = root_bdm
     if not get_device_name(root_bdm) and root_device_name:
-        root_bdm = root_bdm.copy()
-        root_bdm['device_name'] = root_device_name
+        root_bdm_copy = copy.deepcopy(root_bdm)
+        root_bdm_copy['device_name'] = root_device_name
 
     return get_info_from_bdm(
-        instance, virt_type, image_meta, root_bdm, {}, disk_bus,
+        instance, virt_type, image_meta, root_bdm_copy, {}, disk_bus,
     )
 
 

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.

tools/check-cherry-picks.sh

@@ -26,8 +26,11 @@ branches+=""
 for hash in $hashes; do
     branch=$(git branch -a --contains "$hash" 2>/dev/null| grep -oE '(master|stable/[a-z0-9.]+|unmaintained/[a-z0-9.]+)')
     if [ $? -ne 0 ]; then
-        echo "Cherry pick hash $hash not on any master, stable or unmaintained branches"
-        exit 1
+        branch=$(git tag --contains "$hash" 2>/dev/null| grep -oE '([0-9.]+-eol)')
+        if [ $? -ne 0 ]; then
+            echo "Cherry pick hash $hash not on any master, stable, unmaintained or EOL'd branches"
+            exit 1
+        fi
     fi
     branches+=" $branch"
     checked=$(($checked + 1))