Merge "update project/user for consumer in allocation"
This commit is contained in:
Zuul
Gerrit Code Review
commit
8a0e5fb536
nova
api/openstack/placement
tests/functional/api/openstack/placement
@ -217,6 +217,27 @@ class Consumer(base.VersionedObject, base.TimestampedObject):
|
||||
_create_in_db(self._context)
|
||||
self.obj_reset_changes()
|
||||
|
||||
def update(self):
|
||||
"""Used to update the consumer's project and user information without
|
||||
incrementing the consumer's generation.
|
||||
"""
|
||||
@db_api.placement_context_manager.writer
|
||||
def _update_in_db(ctx):
|
||||
upd_stmt = CONSUMER_TBL.update().values(
|
||||
project_id=self.project.id, user_id=self.user.id)
|
||||
# NOTE(jaypipes): We add the generation check to the WHERE clause
|
||||
# above just for safety. We don't need to check that the statement
|
||||
# actually updated a single row. If it did not, then the
|
||||
# consumer.increment_generation() call that happens in
|
||||
# AllocationList.create_all() will end up raising
|
||||
# ConcurrentUpdateDetected anyway
|
||||
upd_stmt = upd_stmt.where(sa.and_(
|
||||
CONSUMER_TBL.c.id == self.id,
|
||||
CONSUMER_TBL.c.generation == self.generation))
|
||||
ctx.session.execute(upd_stmt)
|
||||
_update_in_db(self._context)
|
||||
self.obj_reset_changes()
|
||||
|
||||
def increment_generation(self):
|
||||
"""Increments the consumer's generation.
|
||||
|
||||
|
||||
@ -16,6 +16,7 @@ import re
|
||||
|
||||
import jsonschema
|
||||
from oslo_config import cfg
|
||||
from oslo_log import log as logging
|
||||
from oslo_middleware import request_id
|
||||
from oslo_serialization import jsonutils
|
||||
from oslo_utils import timeutils
|
||||
@ -34,6 +35,7 @@ from nova.api.openstack.placement.objects import user as user_obj
|
||||
from nova.i18n import _
|
||||
|
||||
CONF = cfg.CONF
|
||||
LOG = logging.getLogger(__name__)
|
||||
|
||||
# Error code handling constants
|
||||
ENV_ERROR_CODE = 'placement.error_code'
|
||||
@ -586,6 +588,11 @@ def ensure_consumer(ctx, consumer_uuid, project_id, user_id,
|
||||
and User sub-objects and a boolean indicating whether a new Consumer object
|
||||
was created (as opposed to an existing consumer record retrieved)
|
||||
|
||||
:note: If the supplied project or user external identifiers do not match an
|
||||
existing consumer's project and user identifiers, the existing
|
||||
consumer's project and user IDs are updated to reflect the supplied
|
||||
ones.
|
||||
|
||||
:param ctx: The request context.
|
||||
:param consumer_uuid: The uuid of the consumer of the resources.
|
||||
:param project_id: The external ID of the project consuming the resources.
|
||||
@ -633,6 +640,37 @@ def ensure_consumer(ctx, consumer_uuid, project_id, user_id,
|
||||
'expected_gen': consumer.generation,
|
||||
'got_gen': consumer_generation,
|
||||
})
|
||||
# NOTE(jaypipes): The user may have specified a different project and
|
||||
# user external ID than the one that we had for the consumer. If this
|
||||
# is the case, go ahead and modify the consumer record with the
|
||||
# newly-supplied project/user information, but do not bump the consumer
|
||||
# generation (since it will be bumped in the
|
||||
# AllocationList.create_all() method).
|
||||
#
|
||||
# TODO(jaypipes): This means that there may be a partial update.
|
||||
# Imagine a scenario where a user calls POST /allocations, and the
|
||||
# payload references two consumers. The first consumer is a new
|
||||
# consumer and is auto-created. The second consumer is an existing
|
||||
# consumer, but contains a different project or user ID than the
|
||||
# existing consumer's record. If the eventual call to
|
||||
# AllocationList.create_all() fails for whatever reason (say, a
|
||||
# resource provider generation conflict or out of resources failure),
|
||||
# we will end up deleting the auto-created consumer but we MAY not undo
|
||||
# the changes to the second consumer's project and user ID. I say MAY
|
||||
# and not WILL NOT because I'm not sure that the exception that gets
|
||||
# raised from AllocationList.create_all() will cause the context
|
||||
# manager's transaction to rollback automatically. I believe that the
|
||||
# same transaction context is used for both util.ensure_consumer() and
|
||||
# AllocationList.create_all() within the same HTTP request, but need to
|
||||
# test this to be 100% certain...
|
||||
if (project_id != consumer.project.external_id or
|
||||
user_id != consumer.user.external_id):
|
||||
LOG.debug("Supplied project or user ID for consumer %s was "
|
||||
"different than existing record. Updating consumer "
|
||||
"record.", consumer_uuid)
|
||||
consumer.project = proj
|
||||
consumer.user = user
|
||||
consumer.update()
|
||||
except exception.NotFound:
|
||||
# If we are attempting to modify or create allocations after 1.26, we
|
||||
# need a consumer generation specified. The user must have specified
|
||||
|
||||
@ -55,6 +55,35 @@ class ConsumerTestCase(tb.PlacementDbBaseTestCase):
|
||||
self.assertEqual(2, c.user.id)
|
||||
self.assertRaises(exception.ConsumerExists, c.create)
|
||||
|
||||
def test_update(self):
|
||||
"""Tests the scenario where a user supplies a different project/user ID
|
||||
for an allocation's consumer and we call Consumer.update() to save that
|
||||
information to the consumers table.
|
||||
"""
|
||||
# First, create the consumer with the "fake-user" and "fake-project"
|
||||
# user/project in the base test class's setUp
|
||||
c = consumer_obj.Consumer(
|
||||
self.ctx, uuid=uuids.consumer, user=self.user_obj,
|
||||
project=self.project_obj)
|
||||
c.create()
|
||||
c = consumer_obj.Consumer.get_by_uuid(self.ctx, uuids.consumer)
|
||||
self.assertEqual(self.project_obj.id, c.project.id)
|
||||
self.assertEqual(self.user_obj.id, c.user.id)
|
||||
|
||||
# Now change the consumer's project and user to a different project
|
||||
another_user = user_obj.User(self.ctx, external_id='another-user')
|
||||
another_user.create()
|
||||
another_proj = project_obj.Project(
|
||||
self.ctx, external_id='another-project')
|
||||
another_proj.create()
|
||||
|
||||
c.project = another_proj
|
||||
c.user = another_user
|
||||
c.update()
|
||||
c = consumer_obj.Consumer.get_by_uuid(self.ctx, uuids.consumer)
|
||||
self.assertEqual(another_proj.id, c.project.id)
|
||||
self.assertEqual(another_user.id, c.user.id)
|
||||
|
||||
|
||||
@db_api.placement_context_manager.reader
|
||||
def _get_allocs_with_no_consumer_relationship(ctx):
|
||||
|
||||
@ -0,0 +1,102 @@
|
||||
# Test that it's possible to change the project or user identifier for a
|
||||
# consumer by specifying a different project_id or user_id value in the payload
|
||||
# of both a PUT /allocations/{consumer_uuid} or POST /allocations
|
||||
|
||||
fixtures:
|
||||
- APIFixture
|
||||
|
||||
defaults:
|
||||
request_headers:
|
||||
x-auth-token: admin
|
||||
accept: application/json
|
||||
content-type: application/json
|
||||
openstack-api-version: placement 1.28
|
||||
|
||||
tests:
|
||||
|
||||
- name: create cn1
|
||||
POST: /resource_providers
|
||||
data:
|
||||
name: cn1
|
||||
status: 200
|
||||
|
||||
- name: add inventory
|
||||
PUT: $HISTORY['create cn1'].$RESPONSE['links[?rel = "inventories"].href']
|
||||
data:
|
||||
resource_provider_generation: 0
|
||||
inventories:
|
||||
VCPU:
|
||||
total: 16
|
||||
MEMORY_MB:
|
||||
total: 2048
|
||||
|
||||
- name: create allocations for consumer1
|
||||
PUT: /allocations/11111111-1111-1111-1111-111111111111
|
||||
data:
|
||||
allocations:
|
||||
$HISTORY['create cn1'].$RESPONSE['uuid']:
|
||||
resources:
|
||||
MEMORY_MB: 1024
|
||||
VCPU: 2
|
||||
project_id: $ENVIRON['PROJECT_ID']
|
||||
user_id: $ENVIRON['USER_ID']
|
||||
consumer_generation: null
|
||||
status: 204
|
||||
|
||||
- name: get allocations for consumer1
|
||||
GET: /allocations/11111111-1111-1111-1111-111111111111
|
||||
status: 200
|
||||
response_json_paths:
|
||||
$.project_id: $ENVIRON['PROJECT_ID']
|
||||
$.user_id: $ENVIRON['USER_ID']
|
||||
|
||||
- name: change the project for consumer1
|
||||
PUT: /allocations/11111111-1111-1111-1111-111111111111
|
||||
data:
|
||||
allocations:
|
||||
$HISTORY['create cn1'].$RESPONSE['uuid']:
|
||||
resources:
|
||||
MEMORY_MB: 1024
|
||||
VCPU: 2
|
||||
project_id: $ENVIRON['PROJECT_ID_ALT']
|
||||
user_id: $ENVIRON['USER_ID']
|
||||
consumer_generation: 1
|
||||
status: 204
|
||||
|
||||
- name: check consumer1's project is now the other project
|
||||
GET: /allocations/11111111-1111-1111-1111-111111111111
|
||||
status: 200
|
||||
response_json_paths:
|
||||
$.project_id: $ENVIRON['PROJECT_ID_ALT']
|
||||
$.user_id: $ENVIRON['USER_ID']
|
||||
|
||||
- name: create allocations for two consumers
|
||||
POST: /allocations
|
||||
data:
|
||||
11111111-1111-1111-1111-111111111111:
|
||||
allocations:
|
||||
$HISTORY['create cn1'].$RESPONSE['uuid']:
|
||||
resources:
|
||||
MEMORY_MB: 1024
|
||||
VCPU: 1
|
||||
consumer_generation: 2
|
||||
# Change consumer1's project back to the original PROJECT_ID
|
||||
project_id: $ENVIRON['PROJECT_ID']
|
||||
user_id: $ENVIRON['USER_ID']
|
||||
22222222-2222-2222-2222-222222222222:
|
||||
allocations:
|
||||
$HISTORY['create cn1'].$RESPONSE['uuid']:
|
||||
resources:
|
||||
MEMORY_MB: 1024
|
||||
VCPU: 1
|
||||
consumer_generation: null
|
||||
project_id: $ENVIRON['PROJECT_ID_ALT']
|
||||
user_id: $ENVIRON['USER_ID_ALT']
|
||||
status: 204
|
||||
|
||||
- name: check consumer1's project is back to the original project
|
||||
GET: /allocations/11111111-1111-1111-1111-111111111111
|
||||
status: 200
|
||||
response_json_paths:
|
||||
$.project_id: $ENVIRON['PROJECT_ID']
|
||||
$.user_id: $ENVIRON['USER_ID']
|
||||
Loading…
x
Reference in New Issue
Block a user