Merge "Add support for RP re-parenting and orphaning"

This commit is contained in:
Zuul 2021-07-23 20:22:36 +00:00 committed by Gerrit Code Review
commit c4d785cc35
9 changed files with 368 additions and 55 deletions

View File

@ -671,9 +671,18 @@ resource_provider_parent_provider_uuid_request:
type: string
in: body
required: false
description: >
The UUID of the immediate parent of the resource provider. Once set, the
parent of a resource provider cannot be changed.
description: |
The UUID of the immediate parent of the resource provider.
* Before version ``1.37``, once set, the parent of a resource provider
cannot be changed.
* Since version ``1.37``, it can be set to any existing provider UUID
excepts to providers that would cause a loop. Also it can be set to null
to transform the provider to a new root provider. This operation needs
to be used carefully. Moving providers can mean that the original rules
used to create the existing resource allocations may be invalidated
by that move.
min_version: 1.14
resource_provider_parent_provider_uuid_required_no_min:
type: string

View File

@ -293,6 +293,8 @@ def update_resource_provider(req):
if want_version.matches((1, 14)):
schema = rp_schema.PUT_RP_SCHEMA_V1_14
allow_reparenting = want_version.matches((1, 37))
data = util.extract_json(req.body, schema)
for field in rp_obj.ResourceProvider.SETTABLE_FIELDS:
@ -300,7 +302,7 @@ def update_resource_provider(req):
setattr(resource_provider, field, data[field])
try:
resource_provider.save()
resource_provider.save(allow_reparenting=allow_reparenting)
except db_exc.DBDuplicateEntry:
raise webob.exc.HTTPConflict(
'Conflicting resource provider %(name)s already exists.' %

View File

@ -88,6 +88,7 @@ VERSIONS = [
'1.35', # Add a `root_required` queryparam on `GET /allocation_candidates`
'1.36', # Add a `same_subtree` parameter on GET /allocation_candidates
# and allow resourceless requests for groups in `same_subtree`.
'1.37', # Allow re-parenting and un-parenting resource providers
]

View File

@ -10,6 +10,7 @@
# License for the specific language governing permissions and limitations
# under the License.
import collections
import copy
# NOTE(cdent): The resource provider objects are designed to never be
@ -581,14 +582,21 @@ class ResourceProvider(object):
def destroy(self):
self._delete(self._context, self.id)
def save(self):
def save(self, allow_reparenting=False):
"""Save the changes to the database
:param allow_reparenting: If True then it allows changing the parent RP
to a different RP as well as changing it to None (un-parenting).
If False, then only changing the parent from None to an RP is allowed
the rest is rejected with ObjectActionError.
"""
# These are the only fields we are willing to save with.
# If there are others, ignore them.
updates = {
'name': self.name,
'parent_provider_uuid': self.parent_provider_uuid,
}
self._update_in_db(self._context, self.id, updates)
self._update_in_db(self._context, self.id, updates, allow_reparenting)
@classmethod
def get_by_uuid(cls, context, uuid):
@ -769,21 +777,14 @@ class ResourceProvider(object):
raise exception.NotFound()
@db_api.placement_context_manager.writer
def _update_in_db(self, context, id, updates):
# A list of resource providers in the same tree with the
# resource provider to update
same_tree = []
def _update_in_db(self, context, id, updates, allow_reparenting):
# A list of resource providers in the subtree of resource provider to
# update
subtree_rps = []
# The new root RP if changed
new_root_id = None
new_root_uuid = None
if 'parent_provider_uuid' in updates:
# TODO(jaypipes): For now, "re-parenting" and "un-parenting" are
# not possible. If the provider already had a parent, we don't
# allow changing that parent due to various issues, including:
#
# * if the new parent is a descendant of this resource provider, we
# introduce the possibility of a loop in the graph, which would
# be very bad
# * potentially orphaning heretofore-descendants
#
# So, for now, let's just prevent re-parenting...
my_ids = res_ctx.provider_ids_from_uuid(context, self.uuid)
parent_uuid = updates.pop('parent_provider_uuid')
if parent_uuid is not None:
@ -795,54 +796,69 @@ class ResourceProvider(object):
action='create',
reason='parent provider UUID does not exist.')
if (my_ids.parent_id is not None and
my_ids.parent_id != parent_ids.id):
my_ids.parent_id != parent_ids.id and
not allow_reparenting):
raise exception.ObjectActionError(
action='update',
reason='re-parenting a provider is not currently '
'allowed.')
if my_ids.parent_uuid is None:
# So the user specifies a parent for an RP that doesn't
# have one. We have to check that by this new parent we
# don't create a loop in the tree. Basically the new parent
# cannot be the RP itself or one of its descendants.
# However as the RP's current parent is None the above
# condition is the same as "the new parent cannot be any RP
# from the current RP tree".
same_tree = get_all_by_filters(
context, filters={'in_tree': self.uuid})
rp_uuids_in_the_same_tree = [rp.uuid for rp in same_tree]
if parent_uuid in rp_uuids_in_the_same_tree:
raise exception.ObjectActionError(
action='update',
reason='creating loop in the provider tree is '
'not allowed.')
# So the user specified a new parent. We have to make sure
# that the new parent is not a descendant of the
# current RP to avoid a loop in the graph. It could be
# easily checked by traversing the tree from the new parent
# up to the root and see if we ever hit the current RP
# along the way. However later we need to update every
# descendant of the current RP with a possibly new root
# so we go with the more expensive way and gather every
# descendant for the current RP and check if the new
# parent is part of that set.
subtree_rps = self.get_subtree(context)
subtree_rp_uuids = {rp.uuid for rp in subtree_rps}
if parent_uuid in subtree_rp_uuids:
raise exception.ObjectActionError(
action='update',
reason='creating loop in the provider tree is '
'not allowed.')
updates['root_provider_id'] = parent_ids.root_id
updates['parent_provider_id'] = parent_ids.id
self.root_provider_uuid = parent_ids.root_uuid
new_root_id = parent_ids.root_id
new_root_uuid = parent_ids.root_uuid
else:
if my_ids.parent_id is not None:
raise exception.ObjectActionError(
action='update',
reason='un-parenting a provider is not currently '
'allowed.')
if not allow_reparenting:
raise exception.ObjectActionError(
action='update',
reason='un-parenting a provider is not currently '
'allowed.')
# we don't need to do loop detection but we still need to
# collect the RPs from the subtree so that the new root
# value is updated in the whole subtree below.
subtree_rps = self.get_subtree(context)
# this RP becomes a new root RP
updates['root_provider_id'] = my_ids.id
updates['parent_provider_id'] = None
self.root_provider_uuid = my_ids.uuid
new_root_id = my_ids.id
new_root_uuid = my_ids.uuid
db_rp = context.session.query(models.ResourceProvider).filter_by(
id=id).first()
db_rp.update(updates)
context.session.add(db_rp)
# We should also update the root providers of resource providers
# originally in the same tree. If re-parenting is supported,
# this logic should be changed to update only descendents of the
# re-parented resource providers, not all the providers in the tree.
for rp in same_tree:
# We should also update the root providers of the resource providers
# that are in our subtree
for rp in subtree_rps:
# If the parent is not updated, this clause is skipped since the
# `same_tree` has no element.
rp.root_provider_uuid = parent_ids.root_uuid
# `subtree_rps` has no element.
rp.root_provider_uuid = new_root_uuid
db_rp = context.session.query(
models.ResourceProvider).filter_by(id=rp.id).first()
data = {'root_provider_id': parent_ids.root_id}
data = {'root_provider_id': new_root_id}
db_rp.update(data)
context.session.add(db_rp)
@ -865,6 +881,31 @@ class ResourceProvider(object):
setattr(resource_provider, field, db_resource_provider[field])
return resource_provider
def get_subtree(self, context, rp_uuid_to_child_rps=None):
"""Return every RP from the same tree that is part of the subtree
rooted at the current RP.
:param context: the request context
:param rp_uuid_to_child_rps: a dict of list of children
ResourceProviders keyed by the UUID of their parent RP. If it is
None then this dict is calculated locally.
:return: a list of ResourceProvider objects
"""
# if we are at a start of a recursion then prepare some data structure
if rp_uuid_to_child_rps is None:
same_tree = get_all_by_filters(
context, filters={'in_tree': self.uuid})
rp_uuid_to_child_rps = collections.defaultdict(set)
for rp in same_tree:
if rp.parent_provider_uuid:
rp_uuid_to_child_rps[rp.parent_provider_uuid].add(rp)
subtree = [self]
for child_rp in rp_uuid_to_child_rps[self.uuid]:
subtree.extend(
child_rp.get_subtree(context, rp_uuid_to_child_rps))
return subtree
@db_api.placement_context_manager.reader
def _get_all_by_filters_from_db(context, filters):

View File

@ -665,3 +665,16 @@ not have a resources$S. If this is provided, at least one of the resource
providers satisfying a specified request group must be an ancestor of the
rest. The ``same_subtree`` query parameter can be repeated and each repeat
group is treated independently.
Xena
----
1.37 - Allow re-parenting and un-parenting via PUT /resource_providers/{uuid}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
.. versionadded:: Xena
Add support for re-parenting and un-parenting a resource provider via ``PUT
/resource_providers/{uuid}`` API by allowing changing the
``parent_provider_uuid`` to any existing provider, except providers in same
subtree. Un-parenting can be achieved by setting the ``parent_provider_uuid``
to ``null``. This means that the provider becomes a new root provider.

View File

@ -133,9 +133,34 @@ class ResourceProviderTestCase(tb.PlacementDbBaseTestCase):
)
self.assertEqual('new-name', retrieved_resource_provider.name)
def test_save_reparenting_fail(self):
def test_get_subtree(self):
root1 = self._create_provider('root1')
child1 = self._create_provider('child1', parent=root1.uuid)
child2 = self._create_provider('child2', parent=root1.uuid)
grandchild1 = self._create_provider('grandchild1', parent=child1.uuid)
grandchild2 = self._create_provider('grandchild2', parent=child1.uuid)
grandchild3 = self._create_provider('grandchild3', parent=child2.uuid)
grandchild4 = self._create_provider('grandchild4', parent=child2.uuid)
self.assertEqual(
{grandchild1.uuid},
{rp.uuid for rp in grandchild1.get_subtree(self.context)})
self.assertEqual(
{child1.uuid, grandchild1.uuid, grandchild2.uuid},
{rp.uuid for rp in child1.get_subtree(self.context)})
self.assertEqual(
{child2.uuid, grandchild3.uuid, grandchild4.uuid},
{rp.uuid for rp in child2.get_subtree(self.context)})
self.assertEqual(
{root1.uuid, child1.uuid, child2.uuid,
grandchild1.uuid, grandchild2.uuid, grandchild3.uuid,
grandchild4.uuid},
{rp.uuid for rp in root1.get_subtree(self.context)})
def test_save_reparenting_not_allowed(self):
"""Tests that we prevent a resource provider's parent provider UUID
from being changed from a non-NULL value to another non-NULL value.
from being changed from a non-NULL value to another non-NULL value if
not explicitly requested.
"""
cn1 = self._create_provider('cn1')
self._create_provider('cn2')
@ -156,6 +181,161 @@ class ResourceProviderTestCase(tb.PlacementDbBaseTestCase):
exc = self.assertRaises(exception.ObjectActionError, cn1.save)
self.assertIn('un-parenting a provider is not currently', str(exc))
def test_save_reparent_same_tree(self):
root1 = self._create_provider('root1')
child1 = self._create_provider('child1', parent=root1.uuid)
child2 = self._create_provider('child2', parent=root1.uuid)
self._create_provider('grandchild1', parent=child1.uuid)
self._create_provider('grandchild2', parent=child1.uuid)
self._create_provider('grandchild3', parent=child2.uuid)
self._create_provider('grandchild4', parent=child2.uuid)
test_rp = self._create_provider('test_rp', parent=child1.uuid)
test_rp_child = self._create_provider(
'test_rp_child', parent=test_rp.uuid)
# move test_rp RP upwards
test_rp.parent_provider_uuid = root1.uuid
test_rp.save(allow_reparenting=True)
# to make sure that this re-parenting does not effect the child test RP
# in the db we need to reload it before we assert any change
test_rp_child = rp_obj.ResourceProvider.get_by_uuid(
self.ctx, test_rp_child.uuid)
self.assertEqual(root1.uuid, test_rp.parent_provider_uuid)
self.assertEqual(root1.uuid, test_rp.root_provider_uuid)
self.assertEqual(test_rp.uuid, test_rp_child.parent_provider_uuid)
self.assertEqual(root1.uuid, test_rp_child.root_provider_uuid)
# move downwards
test_rp.parent_provider_uuid = child1.uuid
test_rp.save(allow_reparenting=True)
# to make sure that this re-parenting does not effect the child test RP
# in the db we need to reload it before we assert any change
test_rp_child = rp_obj.ResourceProvider.get_by_uuid(
self.ctx, test_rp_child.uuid)
self.assertEqual(child1.uuid, test_rp.parent_provider_uuid)
self.assertEqual(root1.uuid, test_rp.root_provider_uuid)
self.assertEqual(test_rp.uuid, test_rp_child.parent_provider_uuid)
self.assertEqual(root1.uuid, test_rp_child.root_provider_uuid)
# move sideways
test_rp.parent_provider_uuid = child2.uuid
test_rp.save(allow_reparenting=True)
# to make sure that this re-parenting does not effect the child test RP
# in the db we need to reload it before we assert any change
test_rp_child = rp_obj.ResourceProvider.get_by_uuid(
self.ctx, test_rp_child.uuid)
self.assertEqual(child2.uuid, test_rp.parent_provider_uuid)
self.assertEqual(root1.uuid, test_rp.root_provider_uuid)
self.assertEqual(test_rp.uuid, test_rp_child.parent_provider_uuid)
self.assertEqual(root1.uuid, test_rp_child.root_provider_uuid)
def test_save_reparent_another_tree(self):
root1 = self._create_provider('root1')
child1 = self._create_provider('child1', parent=root1.uuid)
self._create_provider('child2', parent=root1.uuid)
root2 = self._create_provider('root2')
self._create_provider('child3', parent=root2.uuid)
child4 = self._create_provider('child4', parent=root2.uuid)
test_rp = self._create_provider('test_rp', parent=child1.uuid)
test_rp_child = self._create_provider(
'test_rp_child', parent=test_rp.uuid)
test_rp.parent_provider_uuid = child4.uuid
test_rp.save(allow_reparenting=True)
# the re-parenting affected the the child test RP in the db so we
# have to reload it and assert the change
test_rp_child = rp_obj.ResourceProvider.get_by_uuid(
self.ctx, test_rp_child.uuid)
self.assertEqual(child4.uuid, test_rp.parent_provider_uuid)
self.assertEqual(root2.uuid, test_rp.root_provider_uuid)
self.assertEqual(test_rp.uuid, test_rp_child.parent_provider_uuid)
self.assertEqual(root2.uuid, test_rp_child.root_provider_uuid)
def test_save_reparent_to_new_root(self):
root1 = self._create_provider('root1')
child1 = self._create_provider('child1', parent=root1.uuid)
test_rp = self._create_provider('test_rp', parent=child1.uuid)
test_rp_child = self._create_provider(
'test_rp_child', parent=test_rp.uuid)
# we are creating a new root from a subtree, a.k.a un-parenting
test_rp.parent_provider_uuid = None
test_rp.save(allow_reparenting=True)
# the un-parenting affected the the child test RP in the db so we
# have to reload it and assert the change
test_rp_child = rp_obj.ResourceProvider.get_by_uuid(
self.ctx, test_rp_child.uuid)
self.assertIsNone(test_rp.parent_provider_uuid)
self.assertEqual(test_rp.uuid, test_rp.root_provider_uuid)
self.assertEqual(test_rp.uuid, test_rp_child.parent_provider_uuid)
self.assertEqual(test_rp.uuid, test_rp_child.root_provider_uuid)
def test_save_reparent_the_root(self):
root1 = self._create_provider('root1')
child1 = self._create_provider('child1', parent=root1.uuid)
# now the test_rp is also a root RP
test_rp = self._create_provider('test_rp')
test_rp_child = self._create_provider(
'test_rp_child', parent=test_rp.uuid)
test_rp.parent_provider_uuid = child1.uuid
test_rp.save(allow_reparenting=True)
# the re-parenting affected the the child test RP in the db so we
# have to reload it and assert the change
test_rp_child = rp_obj.ResourceProvider.get_by_uuid(
self.ctx, test_rp_child.uuid)
self.assertEqual(child1.uuid, test_rp.parent_provider_uuid)
self.assertEqual(root1.uuid, test_rp.root_provider_uuid)
self.assertEqual(test_rp.uuid, test_rp_child.parent_provider_uuid)
self.assertEqual(root1.uuid, test_rp_child.root_provider_uuid)
def test_save_reparent_loop_fail(self):
root1 = self._create_provider('root1')
test_rp = self._create_provider('test_rp', parent=root1.uuid)
test_rp_child = self._create_provider(
'test_rp_child', parent=test_rp.uuid)
test_rp_grandchild = self._create_provider(
'test_rp_grandchild', parent=test_rp_child.uuid)
# self loop, i.e. we are our parents
test_rp.parent_provider_uuid = test_rp.uuid
exc = self.assertRaises(
exception.ObjectActionError, test_rp.save, allow_reparenting=True)
self.assertIn(
'creating loop in the provider tree is not allowed.', str(exc))
# direct loop, i.e. our child is our parent
test_rp.parent_provider_uuid = test_rp_child.uuid
exc = self.assertRaises(
exception.ObjectActionError, test_rp.save, allow_reparenting=True)
self.assertIn(
'creating loop in the provider tree is not allowed.', str(exc))
# indirect loop, i.e. our grandchild is our parent
test_rp.parent_provider_uuid = test_rp_grandchild.uuid
exc = self.assertRaises(
exception.ObjectActionError, test_rp.save, allow_reparenting=True)
self.assertIn(
'creating loop in the provider tree is not allowed.', str(exc))
def test_nested_providers(self):
"""Create a hierarchy of resource providers and run through a series of
tests that ensure one cannot delete a resource provider that has no

View File

@ -41,13 +41,13 @@ tests:
response_json_paths:
$.errors[0].title: Not Acceptable
- name: latest microversion is 1.36
- name: latest microversion is 1.37
GET: /
request_headers:
openstack-api-version: placement latest
response_headers:
vary: /openstack-api-version/
openstack-api-version: placement 1.36
openstack-api-version: placement 1.37
- name: other accept header bad version
GET: /

View File

@ -378,10 +378,11 @@ tests:
parent_provider_uuid: $ENVIRON['PARENT_PROVIDER_UUID']
status: 200
- name: fail trying to un-parent
- name: fail trying to un-parent with old microversion
PUT: /resource_providers/$ENVIRON['RP_UUID']
request_headers:
content-type: application/json
openstack-api-version: placement 1.36
data:
name: child
parent_provider_uuid: null
@ -389,6 +390,36 @@ tests:
response_strings:
- 'un-parenting a provider is not currently allowed'
- name: un-parent provider
PUT: /resource_providers/$ENVIRON['RP_UUID']
request_headers:
content-type: application/json
openstack-api-version: placement 1.37
data:
name: child
parent_provider_uuid: null
status: 200
response_json_paths:
$.uuid: $ENVIRON['RP_UUID']
$.name: 'child'
$.parent_provider_uuid: null
$.root_provider_uuid: $ENVIRON['RP_UUID']
- name: re-parent back to its original parent after un-parent
PUT: /resource_providers/$ENVIRON['RP_UUID']
request_headers:
content-type: application/json
openstack-api-version: placement 1.37
data:
name: child
parent_provider_uuid: $ENVIRON['PARENT_PROVIDER_UUID']
status: 200
response_json_paths:
$.uuid: $ENVIRON['RP_UUID']
$.name: child
$.parent_provider_uuid: $ENVIRON['PARENT_PROVIDER_UUID']
$.root_provider_uuid: $ENVIRON['PARENT_PROVIDER_UUID']
- name: 409 conflict while trying to delete parent with existing child
DELETE: /resource_providers/$ENVIRON['PARENT_PROVIDER_UUID']
status: 409
@ -595,10 +626,11 @@ tests:
response_json_paths:
$.errors[0].title: Bad Request
- name: fail trying to re-parent to a different provider
- name: fail trying to re-parent to a different provider with old microversion
PUT: /resource_providers/$ENVIRON['RP_UUID']
request_headers:
content-type: application/json
openstack-api-version: placement 1.36
data:
name: child
parent_provider_uuid: $ENVIRON['ALT_PARENT_PROVIDER_UUID']
@ -606,6 +638,36 @@ tests:
response_strings:
- 're-parenting a provider is not currently allowed'
- name: re-parent to a different provider
PUT: /resource_providers/$ENVIRON['RP_UUID']
request_headers:
content-type: application/json
openstack-api-version: placement 1.37
data:
name: child
parent_provider_uuid: $ENVIRON['ALT_PARENT_PROVIDER_UUID']
status: 200
response_json_paths:
$.uuid: $ENVIRON['RP_UUID']
$.name: 'child'
$.parent_provider_uuid: $ENVIRON['ALT_PARENT_PROVIDER_UUID']
$.root_provider_uuid: $ENVIRON['ALT_PARENT_PROVIDER_UUID']
- name: re-parent back to its original parent
PUT: /resource_providers/$ENVIRON['RP_UUID']
request_headers:
content-type: application/json
openstack-api-version: placement 1.37
data:
name: child
parent_provider_uuid: $ENVIRON['PARENT_PROVIDER_UUID']
status: 200
response_json_paths:
$.uuid: $ENVIRON['RP_UUID']
$.name: child
$.parent_provider_uuid: $ENVIRON['PARENT_PROVIDER_UUID']
$.root_provider_uuid: $ENVIRON['PARENT_PROVIDER_UUID']
- name: create a new provider
POST: /resource_providers
request_headers:

View File

@ -0,0 +1,5 @@
---
features:
- |
With the new microversion ``1.37`` placement now supports re-parenting and
un-parenting resource providers via ``PUT /resource_providers/{uuid}`` API.