Add --aggregate option to 'resource provider inventory set'
This adds a new '--aggregate' option which can set resource provider inventory for all resource providers that are members of the specified aggregate. The main motivation for this option is to give operators a tool to make setting overcommit allocation ratios on a per-aggregate basis easier, after the functionality from the old Aggregate[Core|Ram|Disk]Filter scheduler filters was removed back in Ocata [1]. [1] http://lists.openstack.org/pipermail/openstack-dev/2018-January/126283.html Story: 2006318 Task: 36189 Change-Id: Ib0cbb58d0adbbcfe83ee48d2ff6c9af1a516a7ae
This commit is contained in:
parent
51c0064f51
commit
f08cb39813
|
@ -11,10 +11,15 @@
|
||||||
# under the License.
|
# under the License.
|
||||||
|
|
||||||
from collections import defaultdict
|
from collections import defaultdict
|
||||||
|
import itertools
|
||||||
|
|
||||||
from osc_lib.command import command
|
from osc_lib.command import command
|
||||||
|
from osc_lib import exceptions
|
||||||
|
from osc_lib.i18n import _
|
||||||
from osc_lib import utils
|
from osc_lib import utils
|
||||||
|
from oslo_utils import excutils
|
||||||
|
|
||||||
|
from osc_placement.resources import common
|
||||||
from osc_placement import version
|
from osc_placement import version
|
||||||
|
|
||||||
|
|
||||||
|
@ -92,7 +97,7 @@ def parse_resource_argument(resource):
|
||||||
return name, field, value
|
return name, field, value
|
||||||
|
|
||||||
|
|
||||||
class SetInventory(command.Lister):
|
class SetInventory(command.Lister, version.CheckerMixin):
|
||||||
|
|
||||||
"""Replaces the set of inventory records for the resource provider.
|
"""Replaces the set of inventory records for the resource provider.
|
||||||
|
|
||||||
|
@ -117,7 +122,8 @@ class SetInventory(command.Lister):
|
||||||
parser.add_argument(
|
parser.add_argument(
|
||||||
'uuid',
|
'uuid',
|
||||||
metavar='<uuid>',
|
metavar='<uuid>',
|
||||||
help='UUID of the resource provider'
|
help='UUID of the resource provider or UUID of the aggregate, '
|
||||||
|
'if --aggregate is specified'
|
||||||
)
|
)
|
||||||
fields_help = '\n'.join(
|
fields_help = '\n'.join(
|
||||||
'{} - {}'.format(f, INVENTORY_FIELDS[f]['help'].lower())
|
'{} - {}'.format(f, INVENTORY_FIELDS[f]['help'].lower())
|
||||||
|
@ -130,34 +136,91 @@ class SetInventory(command.Lister):
|
||||||
default=[],
|
default=[],
|
||||||
action='append'
|
action='append'
|
||||||
)
|
)
|
||||||
|
parser.add_argument(
|
||||||
|
'--aggregate',
|
||||||
|
action='store_true',
|
||||||
|
help='If this option is specified, the inventories for all '
|
||||||
|
'resource providers that are members of the aggregate will '
|
||||||
|
'be set. This option requires at least '
|
||||||
|
'``--os-placement-api-version 1.3``'
|
||||||
|
)
|
||||||
|
|
||||||
return parser
|
return parser
|
||||||
|
|
||||||
def take_action(self, parsed_args):
|
def take_action(self, parsed_args):
|
||||||
|
|
||||||
|
http = self.app.client_manager.placement
|
||||||
|
|
||||||
|
if parsed_args.aggregate:
|
||||||
|
self.check_version(version.ge('1.3'))
|
||||||
|
filters = {'member_of': parsed_args.uuid}
|
||||||
|
url = common.url_with_filters(RP_BASE_URL, filters)
|
||||||
|
rps = http.request('GET', url).json()['resource_providers']
|
||||||
|
if not rps:
|
||||||
|
raise exceptions.CommandError(
|
||||||
|
'No resource providers found in aggregate with uuid %s.' %
|
||||||
|
parsed_args.uuid)
|
||||||
|
else:
|
||||||
|
url = RP_BASE_URL + '/' + parsed_args.uuid
|
||||||
|
rps = [http.request('GET', url).json()]
|
||||||
|
|
||||||
|
resources_list = []
|
||||||
|
ret = 0
|
||||||
|
for rp in rps:
|
||||||
inventories = defaultdict(dict)
|
inventories = defaultdict(dict)
|
||||||
|
url = BASE_URL.format(uuid=rp['uuid'])
|
||||||
|
payload = {'inventories': inventories,
|
||||||
|
'resource_provider_generation': rp['generation']}
|
||||||
|
|
||||||
|
# Apply resource values to inventories
|
||||||
for r in parsed_args.resource:
|
for r in parsed_args.resource:
|
||||||
name, field, value = parse_resource_argument(r)
|
name, field, value = parse_resource_argument(r)
|
||||||
inventories[name][field] = value
|
inventories[name][field] = value
|
||||||
|
|
||||||
http = self.app.client_manager.placement
|
try:
|
||||||
|
|
||||||
url = RP_BASE_URL + '/' + parsed_args.uuid
|
|
||||||
rp = http.request('GET', url).json()
|
|
||||||
|
|
||||||
payload = {'inventories': inventories,
|
|
||||||
'resource_provider_generation': rp['generation']}
|
|
||||||
url = BASE_URL.format(uuid=parsed_args.uuid)
|
|
||||||
resources = http.request('PUT', url, json=payload).json()
|
resources = http.request('PUT', url, json=payload).json()
|
||||||
|
except Exception as exp:
|
||||||
|
with excutils.save_and_reraise_exception() as err_ctx:
|
||||||
|
if parsed_args.aggregate:
|
||||||
|
self.log.error(_('Failed to set inventory for '
|
||||||
|
'resource provider %(rp)s: %(exp)s.'),
|
||||||
|
{'rp': rp['uuid'], 'exp': exp})
|
||||||
|
err_ctx.reraise = False
|
||||||
|
ret += 1
|
||||||
|
continue
|
||||||
|
resources_list.append((rp['uuid'], resources))
|
||||||
|
|
||||||
|
if ret > 0:
|
||||||
|
msg = _('Failed to set inventory for %(ret)s of %(total)s '
|
||||||
|
'resource providers.') % {'ret': ret, 'total': len(rps)}
|
||||||
|
raise exceptions.CommandError(msg)
|
||||||
|
|
||||||
|
def get_rows(fields, resources, rp_uuid=None):
|
||||||
inventories = [
|
inventories = [
|
||||||
dict(resource_class=k, **v)
|
dict(resource_class=k, **v)
|
||||||
for k, v in resources['inventories'].items()
|
for k, v in resources['inventories'].items()
|
||||||
]
|
]
|
||||||
|
prepend = (rp_uuid, ) if rp_uuid else ()
|
||||||
|
# This is a generator expression
|
||||||
|
rows = (prepend + utils.get_dict_properties(i, fields)
|
||||||
|
for i in inventories)
|
||||||
|
return rows
|
||||||
|
|
||||||
fields = ('resource_class', ) + FIELDS
|
fields = ('resource_class', ) + FIELDS
|
||||||
rows = (utils.get_dict_properties(i, fields) for i in inventories)
|
if parsed_args.aggregate:
|
||||||
|
# If this is an aggregate batch, create output that will include
|
||||||
|
# resource provider as the first field to differentiate the values
|
||||||
|
rows = ()
|
||||||
|
for rp_uuid, resources in resources_list:
|
||||||
|
subrows = get_rows(fields, resources, rp_uuid=rp_uuid)
|
||||||
|
rows = itertools.chain(rows, subrows)
|
||||||
|
fields = ('resource_provider', ) + fields
|
||||||
return fields, rows
|
return fields, rows
|
||||||
|
else:
|
||||||
|
# If this was not an aggregate batch, show output for the one
|
||||||
|
# resource provider (show payload of the first item in the list),
|
||||||
|
# keeping the behavior prior to the addition of --aggregate option
|
||||||
|
return fields, get_rows(fields, resources_list[0][1])
|
||||||
|
|
||||||
|
|
||||||
class SetClassInventory(command.ShowOne):
|
class SetClassInventory(command.ShowOne):
|
||||||
|
|
|
@ -262,10 +262,15 @@ class BaseTestCase(base.BaseTestCase):
|
||||||
cmd += ' --resource-class ' + resource_class
|
cmd += ' --resource-class ' + resource_class
|
||||||
self.openstack(cmd)
|
self.openstack(cmd)
|
||||||
|
|
||||||
def resource_inventory_set(self, uuid, *resources):
|
def resource_inventory_set(self, uuid, *resources, **kwargs):
|
||||||
cmd = 'resource provider inventory set {uuid} {resources}'.format(
|
opts = []
|
||||||
uuid=uuid, resources=' '.join(
|
if kwargs.get('aggregate'):
|
||||||
['--resource %s' % r for r in resources]))
|
opts.append('--aggregate')
|
||||||
|
fmt = 'resource provider inventory set {uuid} {resources} {opts}'
|
||||||
|
cmd = fmt.format(
|
||||||
|
uuid=uuid,
|
||||||
|
resources=' '.join(['--resource %s' % r for r in resources]),
|
||||||
|
opts=' '.join(opts))
|
||||||
return self.openstack(cmd, use_json=True)
|
return self.openstack(cmd, use_json=True)
|
||||||
|
|
||||||
def resource_inventory_class_set(self, uuid, resource_class, **kwargs):
|
def resource_inventory_class_set(self, uuid, resource_class, **kwargs):
|
||||||
|
|
|
@ -10,6 +10,10 @@
|
||||||
# License for the specific language governing permissions and limitations
|
# License for the specific language governing permissions and limitations
|
||||||
# under the License.
|
# under the License.
|
||||||
|
|
||||||
|
from collections import defaultdict
|
||||||
|
import copy
|
||||||
|
import uuid
|
||||||
|
|
||||||
import six
|
import six
|
||||||
|
|
||||||
from osc_placement.tests.functional import base
|
from osc_placement.tests.functional import base
|
||||||
|
@ -203,6 +207,14 @@ class TestSetInventory(base.BaseTestCase):
|
||||||
self.assertEqual(16, inv['MEMORY_MB']['step_size'])
|
self.assertEqual(16, inv['MEMORY_MB']['step_size'])
|
||||||
self.assertEqual(32, inv['VCPU']['total'])
|
self.assertEqual(32, inv['VCPU']['total'])
|
||||||
|
|
||||||
|
def test_fail_aggregate_arg_version_handling(self):
|
||||||
|
agg = str(uuid.uuid4())
|
||||||
|
self.assertCommandFailed(
|
||||||
|
'Operation or argument is not supported with version 1.0; '
|
||||||
|
'requires at least version 1.3',
|
||||||
|
self.resource_inventory_set,
|
||||||
|
agg, 'MEMORY_MB=16', aggregate=True)
|
||||||
|
|
||||||
|
|
||||||
class TestInventory15(TestInventory):
|
class TestInventory15(TestInventory):
|
||||||
VERSION = '1.5'
|
VERSION = '1.5'
|
||||||
|
@ -212,3 +224,145 @@ class TestInventory15(TestInventory):
|
||||||
self.resource_inventory_set(rp['uuid'], 'MEMORY_MB=16', 'VCPU=32')
|
self.resource_inventory_set(rp['uuid'], 'MEMORY_MB=16', 'VCPU=32')
|
||||||
self.resource_inventory_delete(rp['uuid'])
|
self.resource_inventory_delete(rp['uuid'])
|
||||||
self.assertEqual([], self.resource_inventory_list(rp['uuid']))
|
self.assertEqual([], self.resource_inventory_list(rp['uuid']))
|
||||||
|
|
||||||
|
|
||||||
|
class TestAggregateInventory(base.BaseTestCase):
|
||||||
|
VERSION = '1.3'
|
||||||
|
|
||||||
|
def _get_expected_inventories(self, old_inventories, resources):
|
||||||
|
new_inventories = []
|
||||||
|
for old_inventory in old_inventories:
|
||||||
|
new_inventory = defaultdict(dict)
|
||||||
|
new_inventory.update(copy.deepcopy(old_inventory))
|
||||||
|
for resource in resources:
|
||||||
|
rc, keyval = resource.split(':')
|
||||||
|
key, val = keyval.split('=')
|
||||||
|
# Handle allocation ratio which is a float
|
||||||
|
val = float(val) if '.' in val else int(val)
|
||||||
|
new_inventory[rc][key] = val
|
||||||
|
# The resource_class field is added by the osc_placement CLI,
|
||||||
|
# so add it to our expected inventories
|
||||||
|
if 'resource_class' not in new_inventory[rc]:
|
||||||
|
new_inventory[rc]['resource_class'] = rc
|
||||||
|
new_inventories.append(new_inventory)
|
||||||
|
return new_inventories
|
||||||
|
|
||||||
|
def _setup_two_resource_providers_in_aggregate(self):
|
||||||
|
rps = []
|
||||||
|
inventory2 = ['VCPU=8',
|
||||||
|
'VCPU:max_unit=4',
|
||||||
|
'VCPU:allocation_ratio=16.0',
|
||||||
|
'MEMORY_MB=1024',
|
||||||
|
'MEMORY_MB:reserved=256',
|
||||||
|
'MEMORY_MB:allocation_ratio=2.5',
|
||||||
|
'DISK_GB=16',
|
||||||
|
'DISK_GB:allocation_ratio=1.5',
|
||||||
|
'DISK_GB:min_unit=2',
|
||||||
|
'DISK_GB:step_size=2']
|
||||||
|
inventory1 = inventory2 + ['VGPU=8',
|
||||||
|
'VGPU:allocation_ratio=1.0',
|
||||||
|
'VGPU:min_unit=2',
|
||||||
|
'VGPU:step_size=2']
|
||||||
|
for i, inventory in enumerate([inventory1, inventory2]):
|
||||||
|
rps.append(self.resource_provider_create())
|
||||||
|
resp = self.resource_inventory_set(rps[i]['uuid'], *inventory)
|
||||||
|
# Verify the resource_provider column is not present without
|
||||||
|
# --aggregate
|
||||||
|
self.assertNotIn('resource_provider', resp)
|
||||||
|
# Put both resource providers in the same aggregate
|
||||||
|
agg = str(uuid.uuid4())
|
||||||
|
for rp in rps:
|
||||||
|
self.resource_provider_aggregate_set(rp['uuid'], agg)
|
||||||
|
return rps, agg
|
||||||
|
|
||||||
|
def test_fail_if_no_rps_in_aggregate(self):
|
||||||
|
nonexistent_agg = str(uuid.uuid4())
|
||||||
|
exc = self.assertRaises(base.CommandException,
|
||||||
|
self.resource_inventory_set,
|
||||||
|
nonexistent_agg,
|
||||||
|
'VCPU=8',
|
||||||
|
aggregate=True)
|
||||||
|
self.assertIn('No resource providers found in aggregate with uuid {}'
|
||||||
|
.format(nonexistent_agg), six.text_type(exc))
|
||||||
|
|
||||||
|
def test_with_aggregate_one_fails(self):
|
||||||
|
# Set up some existing inventories with two resource providers
|
||||||
|
rps, agg = self._setup_two_resource_providers_in_aggregate()
|
||||||
|
# Set a custom resource class inventory on the first resource provider
|
||||||
|
self.resource_class_create('CUSTOM_FOO')
|
||||||
|
rp1_uuid = rps[0]['uuid']
|
||||||
|
rp1_inv = self.resource_inventory_set(rp1_uuid, 'CUSTOM_FOO=1')
|
||||||
|
# Create an allocation for custom resource class on first provider
|
||||||
|
consumer = str(uuid.uuid4())
|
||||||
|
alloc = 'rp=%s,CUSTOM_FOO=1' % rp1_uuid
|
||||||
|
self.resource_allocation_set(consumer, [alloc])
|
||||||
|
# Try to set allocation ratio for an aggregate. The first set should
|
||||||
|
# fail because we're not going to set the custom resource class (which
|
||||||
|
# is equivalent to trying to remove it) and removal isn't allowed if
|
||||||
|
# there is an allocation of it present. The second set should succeed
|
||||||
|
new_resources = ['VCPU:allocation_ratio=5.0', 'VCPU:total=8']
|
||||||
|
exc = self.assertRaises(base.CommandException,
|
||||||
|
self.resource_inventory_set,
|
||||||
|
agg, *new_resources, aggregate=True)
|
||||||
|
self.assertIn('Failed to set inventory for 1 of 2 resource providers.',
|
||||||
|
six.text_type(exc))
|
||||||
|
output = self.output.getvalue() + self.error.getvalue()
|
||||||
|
err_txt = ("update conflict: Inventory for 'CUSTOM_FOO' on resource "
|
||||||
|
"provider '%s' in use. (HTTP 409)." % rp1_uuid)
|
||||||
|
self.assertIn('Failed to set inventory for resource provider %s: %s' %
|
||||||
|
(rp1_uuid, err_txt), output)
|
||||||
|
# Placement will default the following internally
|
||||||
|
placement_defaults = ['VCPU:max_unit=2147483647',
|
||||||
|
'VCPU:min_unit=1',
|
||||||
|
'VCPU:reserved=0',
|
||||||
|
'VCPU:step_size=1']
|
||||||
|
# Get expected inventory for the second resource provider (succeeded)
|
||||||
|
new_inventories = self._get_expected_inventories(
|
||||||
|
# Since inventories are expected to be fully replaced,
|
||||||
|
# use empty dict for old inventory
|
||||||
|
[{}],
|
||||||
|
new_resources + placement_defaults)
|
||||||
|
resp = self.resource_inventory_list(rps[1]['uuid'])
|
||||||
|
self.assertDictEqual(new_inventories[0],
|
||||||
|
{r['resource_class']: r for r in resp})
|
||||||
|
# First resource provider should have remained the same (failed)
|
||||||
|
resp = self.resource_inventory_list(rp1_uuid)
|
||||||
|
self.assertDictEqual({r['resource_class']: r for r in rp1_inv},
|
||||||
|
{r['resource_class']: r for r in resp})
|
||||||
|
|
||||||
|
def test_with_aggregate(self):
|
||||||
|
# Set up some existing inventories with two resource providers
|
||||||
|
rps, agg = self._setup_two_resource_providers_in_aggregate()
|
||||||
|
# Now, go ahead and update the allocation ratios and verify
|
||||||
|
new_resources = ['VCPU:allocation_ratio=5.0',
|
||||||
|
'VCPU:total=8',
|
||||||
|
'MEMORY_MB:allocation_ratio=6.0',
|
||||||
|
'MEMORY_MB:total=1024',
|
||||||
|
'DISK_GB:allocation_ratio=7.0',
|
||||||
|
'DISK_GB:total=16']
|
||||||
|
resp = self.resource_inventory_set(agg, *new_resources, aggregate=True)
|
||||||
|
# Verify the resource_provider column is present with --aggregate
|
||||||
|
for rp in resp:
|
||||||
|
self.assertIn('resource_provider', rp)
|
||||||
|
# Placement will default the following internally
|
||||||
|
placement_defaults = ['VCPU:max_unit=2147483647',
|
||||||
|
'VCPU:min_unit=1',
|
||||||
|
'VCPU:reserved=0',
|
||||||
|
'VCPU:step_size=1',
|
||||||
|
'MEMORY_MB:max_unit=2147483647',
|
||||||
|
'MEMORY_MB:min_unit=1',
|
||||||
|
'MEMORY_MB:reserved=0',
|
||||||
|
'MEMORY_MB:step_size=1',
|
||||||
|
'DISK_GB:max_unit=2147483647',
|
||||||
|
'DISK_GB:min_unit=1',
|
||||||
|
'DISK_GB:reserved=0',
|
||||||
|
'DISK_GB:step_size=1']
|
||||||
|
new_inventories = self._get_expected_inventories(
|
||||||
|
# Since inventories are expected to be fully replaced,
|
||||||
|
# use empty dicts for old inventories
|
||||||
|
[{}, {}],
|
||||||
|
new_resources + placement_defaults)
|
||||||
|
for i in range(2):
|
||||||
|
resp = self.resource_inventory_list(rps[i]['uuid'])
|
||||||
|
self.assertDictEqual(new_inventories[i],
|
||||||
|
{r['resource_class']: r for r in resp})
|
||||||
|
|
|
@ -0,0 +1,10 @@
|
||||||
|
---
|
||||||
|
features:
|
||||||
|
- |
|
||||||
|
A new ``--aggregate`` option has been added to the ``resource provider
|
||||||
|
inventory set`` command which can set resource provider inventory for
|
||||||
|
all resource providers that are members of the specified aggregate.
|
||||||
|
For example, VCPU, MEMORY_MB, and/or DISK_GB allocation ratios can be
|
||||||
|
managed in aggregate to resolve `bug 1804125`_.
|
||||||
|
|
||||||
|
.. _bug 1804125: https://docs.openstack.org/nova/latest/admin/configuration/schedulers.html#bug-1804125
|
|
@ -7,3 +7,4 @@ six>=1.10.0 # MIT
|
||||||
keystoneauth1>=3.3.0 # Apache-2.0
|
keystoneauth1>=3.3.0 # Apache-2.0
|
||||||
simplejson>=3.16.0 # MIT
|
simplejson>=3.16.0 # MIT
|
||||||
osc-lib>=1.2.0 # Apache-2.0
|
osc-lib>=1.2.0 # Apache-2.0
|
||||||
|
oslo.utils>=3.37.0 # Apache-2.0
|
||||||
|
|
Loading…
Reference in New Issue