From ad4f79836264ec79068f087008367e2d2ae8cbea Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Wed, 8 May 2019 14:39:22 +0200 Subject: [PATCH] Defaults missing group_policy to 'none' If more than one numbered request group is in the placement a_c query then the group_policy is mandatory. Based on the PTG discussion [1] 'none' seems to be a good default policy from nova perspective. So this patch makes sure that if the group_policy is not provided in the flavor extra_spec and there are more than one numbered group in the request and the flavor only provide one or zero groups (so groups are coming from other sources like neutron ports) then the group_policy is defaulted to 'none'. The reasoning behind this change: If more than one numbered request group is coming from the flavor extra_spec then the creator of the flavor is responsible to add a group_policy to the flavor. So in this nova only warns but let the request fail in placement to force the fixing of the flavor. However when numbered groups are coming from other sources (like neutron ports) then the creator of the flavor cannot know if additional group will be included so we don't want to force the flavor creator but simply default the group_policy. [1] http://lists.openstack.org/pipermail/openstack-discuss/2019-May/005807.html Change-Id: I0681de217ed9f5d77dae0d9555632b8d160bb179 --- doc/source/user/flavors.rst | 10 ++ nova/scheduler/utils.py | 58 ++++-- nova/tests/unit/scheduler/test_utils.py | 168 ++++++++++++++++-- ...aulting_group_policy-36f584cd3920818c.yaml | 19 ++ 4 files changed, 221 insertions(+), 34 deletions(-) create mode 100644 releasenotes/notes/defaulting_group_policy-36f584cd3920818c.yaml diff --git a/doc/source/user/flavors.rst b/doc/source/user/flavors.rst index 6c5bb29f836f..8a493f9a1020 100644 --- a/doc/source/user/flavors.rst +++ b/doc/source/user/flavors.rst @@ -799,6 +799,16 @@ Numbered groupings of resource classes and traits * ``none``: Different numbered request groups may be satisfied by different providers *or* common providers. + .. note:: + + If more than one group is specified then the ``group_policy`` is + mandatory in the request. However such groups might come from other + sources than flavor extra_spec (e.g. from Neutron ports with QoS + minimum bandwidth policy). If the flavor does not specify any groups + and ``group_policy`` but more than one group is coming from other + sources then nova will default the ``group_policy`` to ``none`` to + avoid scheduler failure. + For example, to create a server with the following VFs: * One SR-IOV virtual function (VF) on NET1 with bandwidth 10000 bytes/sec diff --git a/nova/scheduler/utils.py b/nova/scheduler/utils.py index 5ab0c50093f2..a2b5879fbaa9 100644 --- a/nova/scheduler/utils.py +++ b/nova/scheduler/utils.py @@ -66,6 +66,14 @@ class ResourceRequest(object): return ', '.join(sorted( list(str(rg) for rg in list(self._rg_by_id.values())))) + @property + def group_policy(self): + return self._group_policy + + @group_policy.setter + def group_policy(self, value): + self._group_policy = value + def get_request_group(self, ident): if ident not in self._rg_by_id: rq_grp = objects.RequestGroup(use_same_provider=bool(ident)) @@ -211,6 +219,10 @@ class ResourceRequest(object): for rg in self._rg_by_id.values(): yield rg.resources + def get_num_of_numbered_groups(self): + return len([ident for ident in self._rg_by_id.keys() + if ident is not None]) + def merged_resources(self, flavor_resources=None): """Returns a merge of {resource_class: amount} for all resource groups. @@ -300,31 +312,14 @@ class ResourceRequest(object): qparams = [] if self._group_policy is not None: qparams.append(('group_policy', self._group_policy)) - nr_of_numbered_groups = 0 + for ident, rg in self._rg_by_id.items(): # [('resourcesN', 'rclass:amount,rclass:amount,...'), # ('requiredN', 'trait_name,!trait_name,...'), # ('member_ofN', 'in:uuid,uuid,...'), # ('member_ofN', 'in:uuid,uuid,...')] qparams.extend(to_queryparams(rg, ident or '')) - if ident: - nr_of_numbered_groups += 1 - if nr_of_numbered_groups >= 2 and not self._group_policy: - # we know this will fail in placement so help the troubleshooting - LOG.warning( - "There is more than one numbered request group in the " - "allocation candidate query but the flavor did not specify " - "any group policy. This query will fail in placement due to " - "the missing group policy. If you specified more than one " - "numbered request group in the flavor extra_spec or booted " - "with more than one neutron port that has resource request " - "(i.e. the port has a QoS minimum bandwidth policy rule " - "attached) then you have to specify the group policy in the " - "flavor extra_spec. If it is OK to let these groups be " - "satisfied by overlapping resource providers then use " - "'group_policy': 'None'. If you want each group to be " - "satisfied from a separate resource provider then use " - "'group_policy': 'isolate'.") + return parse.urlencode(sorted(qparams)) @@ -468,9 +463,12 @@ def resources_from_request_spec(ctxt, spec_obj, host_manager): if rclass not in res_req.merged_resources()} # Now we don't need (or want) any remaining zero entries - remove them. res_req.strip_zeros() + + numbered_groups_from_flavor = res_req.get_num_of_numbered_groups() else: # Start with an empty one res_req = ResourceRequest() + numbered_groups_from_flavor = 0 # Process any image properties if 'image' in spec_obj and 'properties' in spec_obj.image: @@ -548,6 +546,28 @@ def resources_from_request_spec(ctxt, spec_obj, host_manager): for key in spec_obj.scheduler_hints)): res_req._limit = None + if res_req.get_num_of_numbered_groups() >= 2 and not res_req.group_policy: + LOG.warning( + "There is more than one numbered request group in the " + "allocation candidate query but the flavor did not specify " + "any group policy. This query would fail in placement due to " + "the missing group policy. If you specified more than one " + "numbered request group in the flavor extra_spec then you need to " + "specify the group policy in the flavor extra_spec. If it is OK " + "to let these groups be satisfied by overlapping resource " + "providers then use 'group_policy': 'none'. If you want each " + "group to be satisfied from a separate resource provider then " + "use 'group_policy': 'isolate'.") + + if numbered_groups_from_flavor <= 1: + LOG.info( + "At least one numbered request group is defined outside of " + "the flavor (e.g. in a port that has a QoS minimum bandwidth " + "policy rule attached) but the flavor did not specify any " + "group policy. To avoid the placement failure nova defaults " + "the group policy to 'none'.") + res_req.group_policy = 'none' + return res_req diff --git a/nova/tests/unit/scheduler/test_utils.py b/nova/tests/unit/scheduler/test_utils.py index a5a78fae5461..0a894f1849ce 100644 --- a/nova/tests/unit/scheduler/test_utils.py +++ b/nova/tests/unit/scheduler/test_utils.py @@ -362,7 +362,8 @@ class TestUtils(test.NoDBTestCase): self.context, reqspec, self.mock_host_manager) self.assertEqual([], req.get_request_group(None).aggregates) - @mock.patch("nova.scheduler.utils.ResourceRequest.from_extra_specs") + @mock.patch("nova.scheduler.utils.ResourceRequest.from_extra_specs", + return_value=utils.ResourceRequest()) def test_process_extra_specs_granular_called(self, mock_proc): flavor = objects.Flavor(vcpus=1, memory_mb=1024, @@ -794,20 +795,6 @@ class TestUtils(test.NoDBTestCase): ) self.assertEqual(expected_querystring, rr.to_querystring()) - def test_more_than_one_resource_request_without_group_policy_warns(self): - extra_specs = { - 'resources:VCPU': '2', - 'resources1:CUSTOM_FOO': '1' - } - rr = utils.ResourceRequest.from_extra_specs(extra_specs) - rr.add_request_group(objects.RequestGroup(resources={'CUSTOM_BAR': 5})) - - rr.to_querystring() - self.assertIn( - "There is more than one numbered request group in the allocation " - "candidate query but the flavor did not specify any group policy.", - self.stdlog.logger.output) - def test_resource_request_from_extra_specs_append_request(self): extra_specs = { 'resources:VCPU': '2', @@ -1138,3 +1125,154 @@ class TestUtils(test.NoDBTestCase): 1.8, utils.get_weight_multiplier(host1, 'cpu_weight_multiplier', 1.0) ) + + +class TestResourcesFromRequestGroupDefaultPolicy(test.NoDBTestCase): + """These test cases assert what happens when the group policy is missing + from the flavor but more than one numbered request group is requested from + various sources. Note that while image can provide required traits for the + resource request those traits are always added to the unnumbered group so + image cannot be a source of additional numbered groups. + """ + + def setUp(self): + super(TestResourcesFromRequestGroupDefaultPolicy, self).setUp() + self.context = nova_context.get_admin_context() + self.port_group1 = objects.RequestGroup.from_port_request( + self.context, uuids.port1, + port_resource_request={ + "resources": { + "NET_BW_IGR_KILOBIT_PER_SEC": 1000, + "NET_BW_EGR_KILOBIT_PER_SEC": 1000}, + "required": ["CUSTOM_PHYSNET_2", + "CUSTOM_VNIC_TYPE_NORMAL"] + }) + self.port_group2 = objects.RequestGroup.from_port_request( + self.context, uuids.port2, + port_resource_request={ + "resources": { + "NET_BW_IGR_KILOBIT_PER_SEC": 2000, + "NET_BW_EGR_KILOBIT_PER_SEC": 2000}, + "required": ["CUSTOM_PHYSNET_3", + "CUSTOM_VNIC_TYPE_DIRECT"] + }) + + def test_one_group_from_flavor_dont_warn(self): + flavor = objects.Flavor( + vcpus=1, memory_mb=1024, root_gb=10, ephemeral_gb=5, swap=0, + extra_specs={ + 'resources1:CUSTOM_BAR': '2', + }) + request_spec = objects.RequestSpec( + flavor=flavor, image=objects.ImageMeta(), requested_resources=[]) + + rr = utils.resources_from_request_spec( + self.context, request_spec, host_manager=mock.Mock()) + + log = self.stdlog.logger.output + self.assertNotIn( + "There is more than one numbered request group in the allocation " + "candidate query but the flavor did not specify any group policy.", + log) + self.assertNotIn( + "To avoid the placement failure nova defaults the group policy to " + "'none'.", + log) + self.assertIsNone(rr.group_policy) + self.assertNotIn('group_policy=none', rr.to_querystring()) + + def test_one_group_from_port_dont_warn(self): + flavor = objects.Flavor( + vcpus=1, memory_mb=1024, root_gb=10, ephemeral_gb=5, swap=0, + extra_specs={}) + request_spec = objects.RequestSpec( + flavor=flavor, image=objects.ImageMeta(), + requested_resources=[self.port_group1]) + + rr = utils.resources_from_request_spec( + self.context, request_spec, host_manager=mock.Mock()) + + log = self.stdlog.logger.output + self.assertNotIn( + "There is more than one numbered request group in the allocation " + "candidate query but the flavor did not specify any group policy.", + log) + self.assertNotIn( + "To avoid the placement failure nova defaults the group policy to " + "'none'.", + log) + self.assertIsNone(rr.group_policy) + self.assertNotIn('group_policy=none', rr.to_querystring()) + + def test_two_groups_from_flavor_only_warns(self): + flavor = objects.Flavor( + vcpus=1, memory_mb=1024, root_gb=10, ephemeral_gb=5, swap=0, + extra_specs={ + 'resources1:CUSTOM_BAR': '2', + 'resources2:CUSTOM_FOO': '1' + }) + request_spec = objects.RequestSpec( + flavor=flavor, image=objects.ImageMeta(), requested_resources=[]) + + rr = utils.resources_from_request_spec( + self.context, request_spec, host_manager=mock.Mock()) + + log = self.stdlog.logger.output + self.assertIn( + "There is more than one numbered request group in the allocation " + "candidate query but the flavor did not specify any group policy.", + log) + self.assertNotIn( + "To avoid the placement failure nova defaults the group policy to " + "'none'.", + log) + self.assertIsNone(rr.group_policy) + self.assertNotIn('group_policy', rr.to_querystring()) + + def test_one_group_from_flavor_one_from_port_policy_defaulted(self): + flavor = objects.Flavor( + vcpus=1, memory_mb=1024, root_gb=10, ephemeral_gb=5, swap=0, + extra_specs={ + 'resources1:CUSTOM_BAR': '2', + }) + request_spec = objects.RequestSpec( + flavor=flavor, image=objects.ImageMeta(), + requested_resources=[self.port_group1]) + + rr = utils.resources_from_request_spec( + self.context, request_spec, host_manager=mock.Mock()) + + log = self.stdlog.logger.output + self.assertIn( + "There is more than one numbered request group in the allocation " + "candidate query but the flavor did not specify any group policy.", + log) + self.assertIn( + "To avoid the placement failure nova defaults the group policy to " + "'none'.", + log) + self.assertEqual('none', rr.group_policy) + self.assertIn('group_policy=none', rr.to_querystring()) + + def test_two_groups_from_ports_policy_defaulted(self): + flavor = objects.Flavor( + vcpus=1, memory_mb=1024, root_gb=10, ephemeral_gb=5, swap=0, + extra_specs={}) + request_spec = objects.RequestSpec( + flavor=flavor, image=objects.ImageMeta(), + requested_resources=[self.port_group1, self.port_group2]) + + rr = utils.resources_from_request_spec( + self.context, request_spec, host_manager=mock.Mock()) + + log = self.stdlog.logger.output + self.assertIn( + "There is more than one numbered request group in the allocation " + "candidate query but the flavor did not specify any group policy.", + log) + self.assertIn( + "To avoid the placement failure nova defaults the group policy to " + "'none'.", + log) + self.assertEqual('none', rr.group_policy) + self.assertIn('group_policy=none', rr.to_querystring()) diff --git a/releasenotes/notes/defaulting_group_policy-36f584cd3920818c.yaml b/releasenotes/notes/defaulting_group_policy-36f584cd3920818c.yaml new file mode 100644 index 000000000000..bf251e6cf105 --- /dev/null +++ b/releasenotes/notes/defaulting_group_policy-36f584cd3920818c.yaml @@ -0,0 +1,19 @@ +--- +other: + - | + `Numbered request groups`_ can be defined in the flavor extra_spec + but they can come from other sources as well (e.g. neutron ports). + If there is more than one numbered request group in the + allocation candidate query and the flavor does not specify any + group policy then the query will fail in placement as group_policy + is mandatory in this case. Nova previously printed a warning to the + scheduler logs but let the request fail. However the creator of + the flavor cannot know if the flavor later on will be used in a boot + request that has other numbered request groups. So nova will start + defaulting the group_policy to 'none' which means that the resource + providers fulfilling the numbered request groups can overlap. + Nova will only default the group_policy if it is not provided in the flavor + extra_spec, and there is more than one numbered request group present in + the final request, and the flavor only provided one or zero of such groups. + + .. _`Numbered request groups`: https://docs.openstack.org/nova/latest/user/flavors.html#extra-specs-numbered-resource-groupings