Synchronise 2025.1 with upstream

doc/source/admin/scheduling.rst

@@ -1104,6 +1104,14 @@ provided in the option value.
 The resulted host weight would then be multiplied by the value of
 :oslo.config:option:`filter_scheduler.image_props_weight_multiplier`.
 
+
+.. note::
+
+    The weigher compares the values of the image properties as strings. If some
+    image properties are lists (eg. hw_numa_nodes), then if the values are
+    ordered differently, then the weigher will consider them as different
+    values.
+
 Utilization-aware scheduling
 ----------------------------
 

nova/api/openstack/compute/servers.py

@@ -512,12 +512,11 @@ def _process_bdms_for_create(
             create_kwargs['legacy_bdm'] = True
         elif block_device_mapping_v2:
             # Have to check whether --image is given, see bug 1433609
-            image_href = server_dict.get('imageRef')
-            image_uuid_specified = image_href is not None
+            image_ref = server_dict.get('imageRef')
             try:
                 block_device_mapping = [
                     block_device.BlockDeviceDict.from_api(bdm_dict,
-                        image_uuid_specified)
+                        image_uuid_specified=image_ref not in {None, ''})
                     for bdm_dict in block_device_mapping_v2]
             except exception.InvalidBDMFormat as e:
                 raise exc.HTTPBadRequest(explanation=e.format_message())

nova/objects/instance.py

@@ -1606,7 +1606,8 @@ def fill_metadata(self):
             for inst in [i for i in self if i.uuid in updated]:
                 # Patch up our instances with system_metadata from the fill
                 # operation
-                inst.system_metadata = utils.instance_sys_meta(updated)
+                inst.system_metadata = utils.instance_sys_meta(
+                    updated[inst.uuid])
 
     @base.remotable_classmethod
     def get_uuids_by_host(cls, context, host):

nova/scheduler/weights/image_props.py

@@ -74,10 +74,18 @@ def _weigh_object(self, host_state, request_spec):
         # context so we can access all of them, not only the ones from the
         # request.
         ctxt = nova_context.get_admin_context()
-        insts = objects.InstanceList(ctxt,
-                                     objects=host_state.instances.values())
         # system_metadata isn't loaded yet, let's do this.
-        insts.fill_metadata()
+        # Given all instances are in the same host, we can target the same
+        # cell.
+        try:
+            cell_mapping = objects.CellMapping.get_by_uuid(
+                ctxt, host_state.cell_uuid)
+        except exception.CellMappingNotFound:
+            return weight
+        with nova_context.target_cell(ctxt, cell_mapping) as cell_ctxt:
+            insts = objects.InstanceList(cell_ctxt,
+                objects=host_state.instances.values())
+            insts.fill_metadata()
 
         for inst in insts:
             try:

nova/tests/fixtures/libvirt.py

@@ -101,6 +101,7 @@ def _reset():
 VIR_DOMAIN_UNDEFINE_MANAGED_SAVE = 1
 VIR_DOMAIN_UNDEFINE_NVRAM = 4
 VIR_DOMAIN_UNDEFINE_KEEP_TPM = 64
+VIR_DOMAIN_UNDEFINE_KEEP_NVRAM = 8
 
 VIR_DOMAIN_AFFECT_CURRENT = 0
 VIR_DOMAIN_AFFECT_LIVE = 1

nova/tests/functional/regressions/test_bug_2077980.py

@@ -0,0 +1,49 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+from oslo_utils.fixture import uuidsentinel as uuids
+
+from nova.tests.functional.api import client
+from nova.tests.functional import integrated_helpers
+
+
+class TestImageToLocalBDM(integrated_helpers._IntegratedTestBase):
+    """Regression test for bug 2077980
+
+    This regression test asserts the behaviour of server creation requests when
+    using a BDM with source_type=image, destination_type=local, and
+    boot_index=0, which should result in a failure.
+    """
+    microversion = 'latest'
+
+    # TODO(stephenfin): We should eventually fail regardless of the value of
+    # imageRef, but that requires an API microversion
+    def test_image_to_local_bdm__empty_image(self):
+        """Assert behaviour when booting with imageRef set to empty string"""
+        server = {
+            'name': 'test_image_to_local_bdm__null_image',
+            'imageRef': '',
+            'flavorRef': '1',   # m1.tiny from DefaultFlavorsFixture,
+            'networks': 'none',
+            'block_device_mapping_v2': [{
+                'boot_index': 0,
+                'uuid': uuids.image,
+                'source_type': 'image',
+                'destination_type': 'local'}],
+        }
+        # NOTE(stephenfin): This should always fail as we don't allow users to
+        # set this as it would conflict with the flavor definition.
+        ex = self.assertRaises(
+            client.OpenStackApiException, self.api.post_server,
+            {'server': server})
+        self.assertEqual(400, ex.response.status_code)
+        self.assertIn("Mapping image to local is not supported.", str(ex))

nova/tests/functional/regressions/test_bug_2125935.py

@@ -0,0 +1,107 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from unittest import mock
+
+from nova.scheduler import weights
+from nova.tests.functional import integrated_helpers
+
+
+class HostNameWeigher(weights.BaseHostWeigher):
+    # We want to predictabilly have host1 first
+    _weights = {'host1': 1, 'host2': 0}
+
+    def _weigh_object(self, host_state, weight_properties):
+        # Any undefined host gets no weight.
+        return self._weights.get(host_state.host, 0)
+
+
+class TestImagePropsWeigher(integrated_helpers._IntegratedTestBase):
+    """Tests for image props weigher """
+
+    compute_driver = 'fake.MediumFakeDriver'
+    microversion = 'latest'
+    ADMIN_API = True
+
+    def _setup_compute_service(self):
+        # Override to prevent the default 'compute' service from starting
+        pass
+
+    def setUp(self):
+        weight_classes = [
+            __name__ + '.HostNameWeigher',
+            'nova.scheduler.weights.image_props.ImagePropertiesWeigher'
+        ]
+        self.flags(weight_classes=weight_classes,
+                   group='filter_scheduler')
+        self.flags(image_props_weight_multiplier=2.0, group='filter_scheduler')
+        super(TestImagePropsWeigher, self).setUp()
+
+        # Start only the compute services we want for testing
+        self.compute1 = self._start_compute('host1')
+        self.compute2 = self._start_compute('host2')
+
+    @mock.patch('nova.weights.LOG.debug')
+    def test_boot(self, mock_debug):
+        server1 = self._create_server(
+            name='inst1',
+            networks='none',
+            )
+
+        # the weigher sees that there are no existing instances on both hosts
+        # with the same image props
+        mock_debug.assert_any_call(
+            "%s: raw weights %s",
+            "ImagePropertiesWeigher",
+            {('host2', 'host2'): 0.0, ('host1', 'host1'): 0.0})
+        # because of the HostNameWeigher, we're sure that the instance lands
+        # on host1
+        self.assertEqual('host1', server1['OS-EXT-SRV-ATTR:host'])
+        # let's make sure that we don't assert the calls from the previous
+        # schedules.
+        mock_debug.reset_mock()
+
+        server2 = self._create_server(
+            name='inst2',
+            host='host2',
+            networks='none',
+            )
+        # Since we force to a host, the weigher will not be called
+        self.assertEqual('host2', server2['OS-EXT-SRV-ATTR:host'])
+
+        server3 = self._create_server(
+            name='inst3',
+            networks='none',
+            )
+
+        # now the weigher sees that host1 has an instance with the same image
+        # props
+        mock_debug.assert_any_call(
+            "%s: raw weights %s",
+            "ImagePropertiesWeigher",
+            {('host2', 'host2'): 1.0, ('host1', 'host1'): 1.0})
+        mock_debug.reset_mock()
+        # server3 is now on the same host than host1 as the weigh multiplier
+        # makes the scheduler to pack instances sharing the same image props.
+        self.assertEqual('host1', server3['OS-EXT-SRV-ATTR:host'])
+
+        server4 = self._create_server(
+            name='inst4',
+            networks='none',
+            )
+        # eventually, the weigher sees the two existing instances on host1
+        mock_debug.assert_any_call(
+            "%s: raw weights %s",
+            "ImagePropertiesWeigher",
+            {('host2', 'host2'): 1.0, ('host1', 'host1'): 2.0})
+        # server4 is now packed with server1 and server3.
+        self.assertEqual('host1', server4['OS-EXT-SRV-ATTR:host'])

nova/tests/unit/objects/test_instance.py

@@ -2066,6 +2066,8 @@ def test_fill_metadata(self):
                   'host': 'foo'}
 
         for i in range(5):
+            # persist some metadata directly in the DB
+            values['system_metadata'] = {'BTTF2': '2015'}
             db.instance_create(self.context,
                                dict(values))
 
@@ -2089,12 +2091,17 @@ def test_fill_metadata(self):
         self.assertEqual(0, len([i for i in insts
                                  if 'system_metadata' not in i]))
 
+        #  Inst 1 is now updated with the new metadata
+        self.assertEqual({'BTTF1': '1955'}, insts[1].system_metadata)
+
         # Inst 2 should have not had its in-memory copy clobbered
         self.assertEqual({'bttf3': '1885'}, insts[2].system_metadata)
 
-        # Inst 1 should have system_metadata loaded, but empty
-        self.assertIn('system_metadata', insts[0])
-        self.assertEqual({}, insts[0].system_metadata)
+        # Other instances should have system_metadata loaded directly from the
+        # DB
+        for i in [0, 3, 4]:
+            self.assertIn('system_metadata', insts[i])
+            self.assertEqual({'BTTF2': '2015'}, insts[i].system_metadata)
 
     def test_fill_metadata_nop(self):
         insts = objects.InstanceList([objects.Instance(uuid=uuids.inst,

nova/tests/unit/scheduler/weights/test_weights_image_props.py

@@ -16,6 +16,8 @@
 
 from oslo_utils.fixture import uuidsentinel as uuids
 
+from nova import context as nova_context
+from nova import exception
 from nova import objects
 from nova.scheduler import weights
 from nova.scheduler.weights import image_props
@@ -36,7 +38,7 @@
     properties=objects.ImageMetaProps(os_distro='linux', hw_machine_type='pc'))
 
 
-class ImagePropertiesWeigherTestCase(test.NoDBTestCase):
+class _ImagePropertiesWeigherBase(test.NoDBTestCase):
 
     def setUp(self):
         super().setUp()
@@ -90,6 +92,9 @@ def _get_all_hosts(self):
         return [fakes.FakeHostState(host, node, values)
                 for host, node, values in host_values]
 
+
+class ImagePropertiesWeigherTestCase(_ImagePropertiesWeigherBase):
+
     @mock.patch('nova.objects.InstanceList.fill_metadata')
     def test_multiplier_default(self, mock_fm):
         hostinfo_list = self._get_all_hosts()
@@ -194,3 +199,71 @@ def test_multiplier_per_property(self, mock_fm):
         self.assertEqual('host3', weights[0].obj.host)
         mock_fm.assert_has_calls([mock.call(), mock.call(),
                                   mock.call(), mock.call()])
+
+
+class TestTargetCellCalled(_ImagePropertiesWeigherBase):
+    # Using real cell infrastructure instead of SingleCellSimple fixture
+    # as we need to verify set_target_cell calls are made
+    USES_DB_SELF = True
+
+    @mock.patch('nova.objects.InstanceList.fill_metadata')
+    @mock.patch.object(nova_context, 'set_target_cell')
+    @mock.patch('nova.objects.CellMapping.get_by_uuid')
+    @mock.patch.object(nova_context.RequestContext, 'from_dict')
+    @mock.patch.object(nova_context, 'get_admin_context')
+    def test_target_cell_called(self, mock_get_admin_context, mock_from_dict,
+                                mock_get_by_uuid, mock_target_cell, mock_fm):
+        fake_context = nova_context.RequestContext(is_admin=True)
+        fake_cell_ctx = nova_context.RequestContext(is_admin=True)
+        # let's simulate that we set a cell context as target_cell calls
+        # from_dict internally
+        mock_from_dict.return_value = fake_cell_ctx
+        mock_get_admin_context.return_value = fake_context
+        self.flags(image_props_weight_multiplier=1.0, group='filter_scheduler')
+
+        fake_cell_mapping = objects.CellMapping(uuid=uuids.cell1)
+        mock_get_by_uuid.side_effect = [fake_cell_mapping, fake_cell_mapping,
+                                        # host3 won't have a cell mapping
+                                        exception.CellMappingNotFound(
+                                            uuid=uuids.cell2)]
+
+        # Just create three hosts with only one instance, we just want to test
+        # the calls to target_cell and fill_metadata.
+        hostinfo_list = [
+            fakes.FakeHostState('host1', 'node1',
+                               {'instances': {uuids.inst1: objects.Instance(
+                                               uuid=uuids.inst1)},
+                                'cell_uuid': uuids.cell1}),
+            fakes.FakeHostState('host2', 'node2',
+                               {'instances': {}, 'cell_uuid': uuids.cell1}),
+            # host3 is in a different cell
+            fakes.FakeHostState('host3', 'node3',
+                                {'instances': {}, 'cell_uuid': uuids.cell2}),
+        ]
+        request_spec = objects.RequestSpec(image=PROP_WIN_PC)
+
+        weights = self.weight_handler.get_weighed_objects(
+            self.weighers, hostinfo_list, weighing_properties=request_spec)
+
+        mock_get_by_uuid.assert_has_calls([
+            mock.call(fake_context, uuids.cell1),
+            mock.call(fake_context, uuids.cell1),
+            mock.call(fake_context, uuids.cell2)])
+
+        # Now we see that set_target_cell is called with the cell context only
+        # twice given host3 does not have a cell mapping
+        mock_target_cell.assert_has_calls(
+            [mock.call(fake_cell_ctx, fake_cell_mapping),
+             mock.call(fake_cell_ctx, fake_cell_mapping)])
+
+        # Ensure that fill_metadata was called with the correct context
+        def _fake_fill_metadata(_self):
+            self.assertEqual(fake_cell_ctx, _self._context)
+        mock_fm.side_effect = _fake_fill_metadata
+
+        # Double check that we called it only twice
+        self.assertEqual(2, mock_fm.call_count)
+
+        # host1 wins but we return all three hosts.
+        self.assertEqual(3, len(weights))
+        self.assertEqual('host1', weights[0].obj.host)

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

@@ -2826,6 +2826,7 @@ def _test_config_uefi(self):
         obj.os_mach_type = "pc-q35-5.1"
         obj.os_loader = '/tmp/OVMF_CODE.secboot.fd'
         obj.os_loader_type = 'pflash'
+        obj.os_nvram = '/foo/bar/instance-00000012_VARS.fd'
         obj.os_loader_secure = True
         obj.os_loader_stateless = True
         xml = obj.to_xml()
@@ -2840,6 +2841,7 @@ def _test_config_uefi(self):
               <os>
                 <type machine="pc-q35-5.1">hvm</type>
                 <loader stateless='yes' secure='yes' readonly='yes' type='pflash'>/tmp/OVMF_CODE.secboot.fd</loader>
+                 <nvram>/foo/bar/instance-00000012_VARS.fd</nvram>
               </os>
             </domain>""",  # noqa: E501
             xml,