From 5f2b88b1aafb9a35638e54f796685f633428b6a5 Mon Sep 17 00:00:00 2001 From: Arvind Nadendla Date: Mon, 2 Apr 2018 15:38:33 -0700 Subject: [PATCH] update scheduler to use image-traits Update the scheduler to take into account the traits specified as part of the image properties. The required traits from the image are added to the resource request along with the traits and resources from the flavor. This change does not handle rebuilding of the instance, this will come as part of a follow on patch. Added unit and functional tests Change-Id: I3e00080a6b557c6a27634b1dc04fc589f0da0bc2 Implements: blueprint glance-image-traits --- nova/scheduler/utils.py | 40 ++++- nova/tests/fixtures.py | 22 ++- nova/tests/functional/test_servers.py | 219 ++++++++++++++++++++++-- nova/tests/unit/scheduler/test_utils.py | 143 ++++++++++++++-- 4 files changed, 391 insertions(+), 33 deletions(-) diff --git a/nova/scheduler/utils.py b/nova/scheduler/utils.py index 602e3310bc..5793f4e207 100644 --- a/nova/scheduler/utils.py +++ b/nova/scheduler/utils.py @@ -101,7 +101,7 @@ class ResourceRequest(object): return @classmethod - def from_extra_specs(cls, extra_specs): + def from_extra_specs(cls, extra_specs, req=None): """Processes resources and traits in numbered groupings in extra_specs. Examines extra_specs for items of the following forms: @@ -111,10 +111,16 @@ class ResourceRequest(object): "trait$N:$TRAIT_NAME": "required" :param extra_specs: The flavor extra_specs dict. + :param req: the ResourceRequest object to add the requirements to or + None to create a new ResourceRequest :return: A ResourceRequest object representing the resources and required traits in the extra_specs. """ - ret = cls() + if req is not None: + ret = req + else: + ret = cls() + for key, val in extra_specs.items(): match = cls.XS_KEYPAT.match(key) if not match: @@ -135,6 +141,31 @@ class ResourceRequest(object): return ret + @classmethod + def from_image_props(cls, image_meta_props, req=None): + """Processes image properties and adds trait requirements to the + ResourceRequest + + :param image_meta_props: The ImageMetaProps object. + :param req: the ResourceRequest object to add the requirements to or + None to create a new ResourceRequest + :return: A ResourceRequest object representing the required traits on + the image. + """ + if req is not None: + ret = req + else: + ret = cls() + + if 'traits_required' in image_meta_props: + for trait in image_meta_props.traits_required: + # required traits from the image are always added to the + # unnumbered request group, granular request groups are not + # supported in image traits + ret._add_trait(None, trait, "required") + + return ret + def resource_groups(self): for rg in self._rg_by_id.values(): yield rg.resources @@ -332,6 +363,11 @@ def resources_from_request_spec(spec_obj): # Start with an empty one res_req = ResourceRequest() + # Process any image properties + if 'image' in spec_obj and 'properties' in spec_obj.image: + res_req = ResourceRequest.from_image_props(spec_obj.image.properties, + req=res_req) + # Add the (remaining) items from the spec_resources to the sharing group for rclass, amount in spec_resources.items(): res_req.get_request_group(None).resources[rclass] = amount diff --git a/nova/tests/fixtures.py b/nova/tests/fixtures.py index bc88558cad..2080c8272d 100644 --- a/nova/tests/fixtures.py +++ b/nova/tests/fixtures.py @@ -1511,6 +1511,7 @@ class CinderFixtureNewAttachFlow(fixtures.Fixture): # This represents a bootable image-backed volume to test # boot-from-volume scenarios. IMAGE_BACKED_VOL = '6ca404f3-d844-4169-bb96-bc792f37de98' + IMAGE_WITH_TRAITS_BACKED_VOL = '6194fc02-c60e-4a01-a8e5-600798208b5f' def __init__(self, test): super(CinderFixtureNewAttachFlow, self).__init__() @@ -1589,14 +1590,23 @@ class CinderFixtureNewAttachFlow(fixtures.Fixture): } # Check for our special image-backed volume. - if volume_id == self.IMAGE_BACKED_VOL: + if (volume_id == self.IMAGE_BACKED_VOL or + volume_id == self.IMAGE_WITH_TRAITS_BACKED_VOL): # Make it a bootable volume. volume['bootable'] = True - # Add the image_id metadata. - volume['volume_image_metadata'] = { - # There would normally be more image metadata in here... - 'image_id': '155d900f-4e14-4e4c-a73d-069cbf4541e6' - } + if volume_id == self.IMAGE_BACKED_VOL: + # Add the image_id metadata. + volume['volume_image_metadata'] = { + # There would normally be more image metadata in here. + 'image_id': '155d900f-4e14-4e4c-a73d-069cbf4541e6' + } + elif volume_id == self.IMAGE_WITH_TRAITS_BACKED_VOL: + # Add the image_id metadata with traits. + volume['volume_image_metadata'] = { + 'image_id': '155d900f-4e14-4e4c-a73d-069cbf4541e6', + "container_format": "ami", + "trait:HW_CPU_X86_SGX": "required", + } return volume diff --git a/nova/tests/functional/test_servers.py b/nova/tests/functional/test_servers.py index c92f1c9b6c..a9a7887fce 100644 --- a/nova/tests/functional/test_servers.py +++ b/nova/tests/functional/test_servers.py @@ -3706,36 +3706,225 @@ class TraitsBasedSchedulingTest(ProviderUsageBaseTestCase): self.admin_api.post_extra_spec( self.flavor_with_trait['id'], {'extra_specs': {'trait:HW_CPU_X86_VMX': 'required'}}) + self.flavor_without_trait = flavors[1] + + # Note that we're using v2.35 explicitly as the api returns 404 + # starting with 2.36 + with nova.utils.temporary_mutation(self.api, microversion='2.35'): + images = self.api.get_images() + self.image_id_with_trait = images[0]['id'] + self.api.api_put('/images/%s/metadata' % self.image_id_with_trait, + {'metadata': { + 'trait:HW_CPU_X86_SGX': 'required'}}) + self.image_id_without_trait = images[1]['id'] + + def _create_server_with_traits(self, flavor_id, image_id): + """Create a server with given flavor and image id's + :param flavor_id: the flavor id + :param image_id: the image id + :return: create server response + """ - def _create_server(self): - # Create the server using the flavor with the required trait. server_req = self._build_minimal_create_server_request( self.api, 'trait-based-server', - image_uuid='76fa36fc-c930-4bf3-8c8a-ea2a2420deb6', - flavor_id=self.flavor_with_trait['id'], networks='none') + image_uuid=image_id, + flavor_id=flavor_id, networks='none') return self.api.post_server({'server': server_req}) - def test_traits_based_scheduling(self): - """Tests that a server create request using a required trait ends - up on the single compute node resource provider that also has that + def _create_server_with_volume(self, flavor_id, volume_id): + """Create a server with block device mapping(volume) with the given + flavor and volume id's + :param flavor_id: the flavor id + :param volume_id: the volume id + :return: create server response + """ + + server_req_body = { + # There is no imageRef because this is boot from volume. + 'server': { + 'flavorRef': flavor_id, + 'name': 'test_image_trait_on_volume_backed', + # We don't care about networking for this test. This + # requires + # microversion >= 2.37. + 'networks': 'none', + 'block_device_mapping_v2': [{ + 'boot_index': 0, + 'uuid': volume_id, + 'source_type': 'volume', + 'destination_type': 'volume' + }] + } + } + server = self.api.post_server(server_req_body) + return server + + def test_flavor_traits_based_scheduling(self): + """Tests that a server create request using a required trait on flavor + ends up on the single compute node resource provider that also has that trait in Placement. """ - # Decorate the compute_with_trait resource provider with that same - # trait. + + # Decorate compute1 resource provider with that same trait. rp_uuid = self._get_provider_uuid_by_host(self.compute1.host) self._set_provider_traits(rp_uuid, ['HW_CPU_X86_VMX']) - server = self._create_server() + + # Create server using only flavor trait + server = self._create_server_with_traits(self.flavor_with_trait['id'], + self.image_id_without_trait) server = self._wait_for_state_change(self.admin_api, server, 'ACTIVE') # Assert the server ended up on the expected compute host that has # the required trait. self.assertEqual(self.compute1.host, server['OS-EXT-SRV-ATTR:host']) - def test_traits_based_scheduling_no_valid_host(self): - """Tests that a server create request using a required trait - fails to find a valid host since no compute node resource providers - have the trait. + def test_image_traits_based_scheduling(self): + """Tests that a server create request using a required trait on image + ends up on the single compute node resource provider that also has that + trait in Placement. """ - server = self._create_server() + + # Decorate compute2 resource provider with image trait. + rp_uuid = self._get_provider_uuid_by_host(self.compute2.host) + self._set_provider_traits(rp_uuid, ['HW_CPU_X86_SGX']) + + # Create server using only image trait + server = self._create_server_with_traits( + self.flavor_without_trait['id'], self.image_id_with_trait) + server = self._wait_for_state_change(self.admin_api, server, 'ACTIVE') + # Assert the server ended up on the expected compute host that has + # the required trait. + self.assertEqual(self.compute2.host, server['OS-EXT-SRV-ATTR:host']) + + def test_flavor_image_traits_based_scheduling(self): + """Tests that a server create request using a required trait on flavor + AND a required trait on the image ends up on the single compute node + resource provider that also has that trait in Placement. + """ + + # Decorate compute2 resource provider with both flavor and image trait. + rp_uuid = self._get_provider_uuid_by_host(self.compute2.host) + self._set_provider_traits(rp_uuid, ['HW_CPU_X86_VMX', + 'HW_CPU_X86_SGX']) + + # Create server using flavor and image trait + server = self._create_server_with_traits( + self.flavor_with_trait['id'], self.image_id_with_trait) + server = self._wait_for_state_change(self.admin_api, server, 'ACTIVE') + # Assert the server ended up on the expected compute host that has + # the required trait. + self.assertEqual(self.compute2.host, server['OS-EXT-SRV-ATTR:host']) + + def test_image_trait_on_volume_backed_instance(self): + """Tests that when trying to launch a volume-backed instance with a + required trait on the image, the instance ends up on the single compute + node resource provider that also has that trait in Placement. + """ + # Decorate compute2 resource provider with volume image metadata trait. + rp_uuid = self._get_provider_uuid_by_host(self.compute2.host) + self._set_provider_traits(rp_uuid, ['HW_CPU_X86_SGX']) + + self.useFixture(nova_fixtures.CinderFixtureNewAttachFlow(self)) + # Create our server with a volume containing the image meta data with a + # required trait + server = self._create_server_with_volume( + self.flavor_without_trait['id'], + nova_fixtures.CinderFixtureNewAttachFlow. + IMAGE_WITH_TRAITS_BACKED_VOL) + + server = self._wait_for_state_change(self.admin_api, server, 'ACTIVE') + # Assert the server ended up on the expected compute host that has + # the required trait. + self.assertEqual(self.compute2.host, server['OS-EXT-SRV-ATTR:host']) + + def test_flavor_image_trait_on_volume_backed_instance(self): + """Tests that when trying to launch a volume-backed instance with a + required trait on flavor AND a required trait on the image, + the instance ends up on the single compute node resource provider that + also has those traits in Placement. + """ + # Decorate compute2 resource provider with volume image metadatz trait. + rp_uuid = self._get_provider_uuid_by_host(self.compute2.host) + self._set_provider_traits(rp_uuid, ['HW_CPU_X86_VMX', + 'HW_CPU_X86_SGX']) + + self.useFixture(nova_fixtures.CinderFixtureNewAttachFlow(self)) + # Create our server with a flavor trait and a volume containing the + # image meta data with a required trait + server = self._create_server_with_volume( + self.flavor_with_trait['id'], + nova_fixtures.CinderFixtureNewAttachFlow. + IMAGE_WITH_TRAITS_BACKED_VOL) + + server = self._wait_for_state_change(self.admin_api, server, 'ACTIVE') + # Assert the server ended up on the expected compute host that has + # the required trait. + self.assertEqual(self.compute2.host, server['OS-EXT-SRV-ATTR:host']) + + def test_flavor_traits_based_scheduling_no_valid_host(self): + """Tests that a server create request using a required trait expressed + in flavor fails to find a valid host since no compute node resource + providers have the trait. + """ + + # Decorate compute1 resource provider with the image trait. + rp_uuid = self._get_provider_uuid_by_host(self.compute1.host) + self._set_provider_traits(rp_uuid, ['HW_CPU_X86_SGX']) + + server = self._create_server_with_traits(self.flavor_with_trait['id'], + self.image_id_without_trait) + # The server should go to ERROR state because there is no valid host. + server = self._wait_for_state_change(self.admin_api, server, 'ERROR') + self.assertIsNone(server['OS-EXT-SRV-ATTR:host']) + # Make sure the failure was due to NoValidHost by checking the fault. + self.assertIn('fault', server) + self.assertIn('No valid host', server['fault']['message']) + + def test_image_traits_based_scheduling_no_valid_host(self): + """Tests that a server create request using a required trait expressed + in image fails to find a valid host since no compute node resource + providers have the trait. + """ + + # Decorate compute1 resource provider with that flavor trait. + rp_uuid = self._get_provider_uuid_by_host(self.compute1.host) + self._set_provider_traits(rp_uuid, ['HW_CPU_X86_VMX']) + + server = self._create_server_with_traits( + self.flavor_without_trait['id'], self.image_id_with_trait) + # The server should go to ERROR state because there is no valid host. + server = self._wait_for_state_change(self.admin_api, server, 'ERROR') + self.assertIsNone(server['OS-EXT-SRV-ATTR:host']) + # Make sure the failure was due to NoValidHost by checking the fault. + self.assertIn('fault', server) + self.assertIn('No valid host', server['fault']['message']) + + def test_flavor_image_traits_based_scheduling_no_valid_host(self): + """Tests that a server create request using a required trait expressed + in flavor AND a required trait expressed in the image fails to find a + valid host since no compute node resource providers have the trait. + """ + + server = self._create_server_with_traits( + self.flavor_with_trait['id'], self.image_id_with_trait) + # The server should go to ERROR state because there is no valid host. + server = self._wait_for_state_change(self.admin_api, server, 'ERROR') + self.assertIsNone(server['OS-EXT-SRV-ATTR:host']) + # Make sure the failure was due to NoValidHost by checking the fault. + self.assertIn('fault', server) + self.assertIn('No valid host', server['fault']['message']) + + def test_image_trait_on_volume_backed_instance_no_valid_host(self): + """Tests that when trying to launch a volume-backed instance with a + required trait on the image fails to find a valid host since no compute + node resource providers have the trait. + """ + self.useFixture(nova_fixtures.CinderFixtureNewAttachFlow(self)) + # Create our server with a volume + server = self._create_server_with_volume( + self.flavor_without_trait['id'], + nova_fixtures.CinderFixtureNewAttachFlow. + IMAGE_WITH_TRAITS_BACKED_VOL) + # The server should go to ERROR state because there is no valid host. server = self._wait_for_state_change(self.admin_api, server, 'ERROR') self.assertIsNone(server['OS-EXT-SRV-ATTR:host']) diff --git a/nova/tests/unit/scheduler/test_utils.py b/nova/tests/unit/scheduler/test_utils.py index ff3b425fca..6b22aa810b 100644 --- a/nova/tests/unit/scheduler/test_utils.py +++ b/nova/tests/unit/scheduler/test_utils.py @@ -36,12 +36,25 @@ class TestUtils(test.NoDBTestCase): for ident in ex_by_id: self.assertEqual(vars(ex_by_id[ident]), vars(ob_by_id[ident])) - def _test_resources_from_request_spec(self, flavor, expected): - fake_spec = objects.RequestSpec(flavor=flavor) + @staticmethod + def _get_image_with_traits(): + image_prop = { + 'properties': { + 'trait:CUSTOM_IMAGE_TRAIT1': 'required', + 'trait:CUSTOM_IMAGE_TRAIT2': 'required', + }, + 'id': 'c8b1790e-a07d-4971-b137-44f2432936cd' + } + image = objects.ImageMeta.from_dict(image_prop) + return image + + def _test_resources_from_request_spec(self, expected, flavor, + image=objects.ImageMeta()): + fake_spec = objects.RequestSpec(flavor=flavor, image=image) resources = utils.resources_from_request_spec(fake_spec) self.assertResourceRequestsEqual(expected, resources) - def test_resources_from_request_spec(self): + def test_resources_from_request_spec_flavor_only(self): flavor = objects.Flavor(vcpus=1, memory_mb=1024, root_gb=10, @@ -56,7 +69,36 @@ class TestUtils(test.NoDBTestCase): 'DISK_GB': 15, } ) - self._test_resources_from_request_spec(flavor, expected_resources) + self._test_resources_from_request_spec(expected_resources, flavor) + + def test_resources_from_request_spec_flavor_and_image_traits(self): + image = self._get_image_with_traits() + flavor = objects.Flavor(vcpus=1, + memory_mb=1024, + root_gb=10, + ephemeral_gb=5, + swap=0, + extra_specs={ + 'trait:CUSTOM_FLAVOR_TRAIT': 'required', + 'trait:CUSTOM_IMAGE_TRAIT2': 'required'}) + expected_resources = utils.ResourceRequest() + expected_resources._rg_by_id[None] = plib.RequestGroup( + use_same_provider=False, + resources={ + 'VCPU': 1, + 'MEMORY_MB': 1024, + 'DISK_GB': 15, + }, + required_traits={ + # trait:CUSTOM_IMAGE_TRAIT2 is defined in both extra_specs and + # image metadata. We get a union of both. + 'CUSTOM_IMAGE_TRAIT1', + 'CUSTOM_IMAGE_TRAIT2', + 'CUSTOM_FLAVOR_TRAIT', + } + ) + self._test_resources_from_request_spec(expected_resources, flavor, + image) def test_resources_from_request_spec_with_no_disk(self): flavor = objects.Flavor(vcpus=1, @@ -72,7 +114,7 @@ class TestUtils(test.NoDBTestCase): 'MEMORY_MB': 1024, } ) - self._test_resources_from_request_spec(flavor, expected_resources) + self._test_resources_from_request_spec(expected_resources, flavor) def test_get_resources_from_request_spec_custom_resource_class(self): flavor = objects.Flavor(vcpus=1, @@ -91,7 +133,7 @@ class TestUtils(test.NoDBTestCase): "CUSTOM_TEST_CLASS": 1, } ) - self._test_resources_from_request_spec(flavor, expected_resources) + self._test_resources_from_request_spec(expected_resources, flavor) def test_get_resources_from_request_spec_override_flavor_amounts(self): flavor = objects.Flavor(vcpus=1, @@ -112,7 +154,7 @@ class TestUtils(test.NoDBTestCase): "DISK_GB": 99, } ) - self._test_resources_from_request_spec(flavor, expected_resources) + self._test_resources_from_request_spec(expected_resources, flavor) def test_get_resources_from_request_spec_remove_flavor_amounts(self): flavor = objects.Flavor(vcpus=1, @@ -130,7 +172,7 @@ class TestUtils(test.NoDBTestCase): "MEMORY_MB": 1024, } ) - self._test_resources_from_request_spec(flavor, expected_resources) + self._test_resources_from_request_spec(expected_resources, flavor) def test_get_resources_from_request_spec_vgpu(self): flavor = objects.Flavor(vcpus=1, @@ -152,7 +194,7 @@ class TestUtils(test.NoDBTestCase): "VGPU_DISPLAY_HEAD": 1, } ) - self._test_resources_from_request_spec(flavor, expected_resources) + self._test_resources_from_request_spec(expected_resources, flavor) def test_get_resources_from_request_spec_bad_std_resource_class(self): flavor = objects.Flavor(vcpus=1, @@ -228,7 +270,7 @@ class TestUtils(test.NoDBTestCase): 'SRIOV_NET_VF': 1, } ) - self._test_resources_from_request_spec(flavor, expected_resources) + self._test_resources_from_request_spec(expected_resources, flavor) def test_resources_from_request_spec_aggregates(self): destination = objects.Destination() @@ -442,6 +484,87 @@ class TestUtils(test.NoDBTestCase): self.assertResourceRequestsEqual( expected, utils.ResourceRequest.from_extra_specs(extra_specs)) + def test_resource_request_from_extra_specs_append_request(self): + extra_specs = { + 'resources:VCPU': '2', + 'resources:MEMORY_MB': '2048', + 'trait:HW_CPU_X86_AVX': 'required', + } + existing_req = utils.ResourceRequest() + existing_req._rg_by_id[None] = plib.RequestGroup( + use_same_provider=False, + required_traits={ + 'CUSTOM_MAGIC', + } + ) + # Build up a ResourceRequest from the inside to compare against. + expected = utils.ResourceRequest() + expected._rg_by_id[None] = plib.RequestGroup( + use_same_provider=False, + resources={ + 'VCPU': 2, + 'MEMORY_MB': 2048, + }, + required_traits={ + # In addition to traits from extra spec, we get traits from a + # previous existing resource request + 'HW_CPU_X86_AVX', + 'CUSTOM_MAGIC', + } + ) + self.assertResourceRequestsEqual( + expected, utils.ResourceRequest.from_extra_specs(extra_specs, + req=existing_req)) + + def test_resource_request_from_image_props(self): + props = {'trait:CUSTOM_TRUSTED': 'required'} + image_meta_props = objects.ImageMetaProps.from_dict(props) + + # Build up a ResourceRequest from the inside to compare against. + expected = utils.ResourceRequest() + expected._rg_by_id[None] = plib.RequestGroup( + use_same_provider=False, + required_traits={ + 'CUSTOM_TRUSTED', + } + ) + self.assertResourceRequestsEqual( + expected, utils.ResourceRequest.from_image_props(image_meta_props)) + + def test_resource_request_from_image_props_append_request(self): + props = {'trait:CUSTOM_MAGIC': 'required'} + image_meta_props = objects.ImageMetaProps.from_dict(props) + + existing_req = utils.ResourceRequest() + existing_req._rg_by_id[None] = plib.RequestGroup( + use_same_provider=False, + resources={ + 'VCPU': 2, + 'MEMORY_MB': 2048, + }, + required_traits={ + 'HW_CPU_X86_AVX', + } + ) + # Build up a ResourceRequest from the inside to compare against. + expected = utils.ResourceRequest() + expected._rg_by_id[None] = plib.RequestGroup( + use_same_provider=False, + resources={ + 'VCPU': 2, + 'MEMORY_MB': 2048, + }, + required_traits={ + # In addition to information already contained in the existing + # resource request, we add the traits from image properties + 'HW_CPU_X86_AVX', + 'CUSTOM_MAGIC', + } + ) + self.assertResourceRequestsEqual( + expected, utils.ResourceRequest.from_image_props(image_meta_props, + req=existing_req)) + def test_merge_resources(self): resources = { 'VCPU': 1, 'MEMORY_MB': 1024,