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