Prevent metadata updates until instance is active.
This avoids a race condition where setting metadata too early in the build process can cause a build failure due to the VM not existing in Xen. bug 1100866 Change-Id: I702128799997d91d5f0c0ac189f619bfdf011988
This commit is contained in:
parent
7d20f1fe55
commit
6e3fcd5e3f
|
@ -136,6 +136,10 @@ class Controller(object):
|
|||
raise exc.HTTPRequestEntityTooLarge(explanation=unicode(error),
|
||||
headers={'Retry-After': 0})
|
||||
|
||||
except exception.InstanceInvalidState as state_error:
|
||||
common.raise_http_conflict_for_instance_invalid_state(state_error,
|
||||
'update metadata')
|
||||
|
||||
@wsgi.serializers(xml=common.MetaItemTemplate)
|
||||
def show(self, req, server_id, id):
|
||||
"""Return a single metadata item."""
|
||||
|
@ -162,10 +166,15 @@ class Controller(object):
|
|||
try:
|
||||
server = self.compute_api.get(context, server_id)
|
||||
self.compute_api.delete_instance_metadata(context, server, id)
|
||||
|
||||
except exception.InstanceNotFound:
|
||||
msg = _('Server does not exist')
|
||||
raise exc.HTTPNotFound(explanation=msg)
|
||||
|
||||
except exception.InstanceInvalidState as state_error:
|
||||
common.raise_http_conflict_for_instance_invalid_state(state_error,
|
||||
'delete metadata')
|
||||
|
||||
|
||||
def create_resource():
|
||||
return wsgi.Resource(Controller())
|
||||
|
|
|
@ -2160,6 +2160,9 @@ class API(base.Base):
|
|||
|
||||
@wrap_check_policy
|
||||
@check_instance_lock
|
||||
@check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.PAUSED,
|
||||
vm_states.SUSPENDED, vm_states.STOPPED],
|
||||
task_state=None)
|
||||
def delete_instance_metadata(self, context, instance, key):
|
||||
"""Delete the given metadata item from an instance."""
|
||||
self.db.instance_metadata_delete(context, instance['uuid'], key)
|
||||
|
@ -2171,6 +2174,9 @@ class API(base.Base):
|
|||
|
||||
@wrap_check_policy
|
||||
@check_instance_lock
|
||||
@check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.PAUSED,
|
||||
vm_states.SUSPENDED, vm_states.STOPPED],
|
||||
task_state=None)
|
||||
def update_instance_metadata(self, context, instance,
|
||||
metadata, delete=False):
|
||||
"""Updates or creates instance metadata.
|
||||
|
|
|
@ -21,6 +21,7 @@ import webob
|
|||
|
||||
from nova.api.openstack.compute import server_metadata
|
||||
from nova.compute import rpcapi as compute_rpcapi
|
||||
from nova.compute import vm_states
|
||||
import nova.db
|
||||
from nova import exception
|
||||
from nova.openstack.common import cfg
|
||||
|
@ -75,14 +76,16 @@ def return_server(context, server_id):
|
|||
return {'id': server_id,
|
||||
'uuid': '0cc3346e-9fef-4445-abe6-5d2b2690ec64',
|
||||
'name': 'fake',
|
||||
'locked': False}
|
||||
'locked': False,
|
||||
'vm_state': vm_states.ACTIVE}
|
||||
|
||||
|
||||
def return_server_by_uuid(context, server_uuid):
|
||||
return {'id': 1,
|
||||
'uuid': '0cc3346e-9fef-4445-abe6-5d2b2690ec64',
|
||||
'name': 'fake',
|
||||
'locked': False}
|
||||
'locked': False,
|
||||
'vm_state': vm_states.ACTIVE}
|
||||
|
||||
|
||||
def return_server_nonexistent(context, server_id):
|
||||
|
@ -93,10 +96,9 @@ def fake_change_instance_metadata(self, context, instance, diff):
|
|||
pass
|
||||
|
||||
|
||||
class ServerMetaDataTest(test.TestCase):
|
||||
|
||||
class BaseTest(test.TestCase):
|
||||
def setUp(self):
|
||||
super(ServerMetaDataTest, self).setUp()
|
||||
super(BaseTest, self).setUp()
|
||||
fakes.stub_out_key_pair_funcs(self.stubs)
|
||||
self.stubs.Set(nova.db, 'instance_get', return_server)
|
||||
self.stubs.Set(nova.db, 'instance_get_by_uuid',
|
||||
|
@ -112,6 +114,9 @@ class ServerMetaDataTest(test.TestCase):
|
|||
self.uuid = str(uuid.uuid4())
|
||||
self.url = '/v1.1/fake/servers/%s/metadata' % self.uuid
|
||||
|
||||
|
||||
class ServerMetaDataTest(BaseTest):
|
||||
|
||||
def test_index(self):
|
||||
req = fakes.HTTPRequest.blank(self.url)
|
||||
res_dict = self.controller.index(req, self.uuid)
|
||||
|
@ -510,3 +515,50 @@ class ServerMetaDataTest(test.TestCase):
|
|||
req.body = jsonutils.dumps(data)
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.controller.update_all, req, self.uuid, data)
|
||||
|
||||
|
||||
class BadStateServerMetaDataTest(BaseTest):
|
||||
|
||||
def setUp(self):
|
||||
super(BadStateServerMetaDataTest, self).setUp()
|
||||
self.stubs.Set(nova.db, 'instance_get', self._return_server_in_build)
|
||||
self.stubs.Set(nova.db, 'instance_get_by_uuid',
|
||||
self._return_server_in_build_by_uuid)
|
||||
self.stubs.Set(nova.db, 'instance_metadata_delete',
|
||||
delete_server_metadata)
|
||||
|
||||
def test_invalid_state_on_delete(self):
|
||||
req = fakes.HTTPRequest.blank(self.url + '/key2')
|
||||
req.method = 'DELETE'
|
||||
self.assertRaises(webob.exc.HTTPConflict, self.controller.delete,
|
||||
req, self.uuid, 'key2')
|
||||
|
||||
def test_invalid_state_on_update_metadata(self):
|
||||
self.stubs.Set(nova.db, 'instance_metadata_update',
|
||||
return_create_instance_metadata)
|
||||
req = fakes.HTTPRequest.blank(self.url)
|
||||
req.method = 'POST'
|
||||
req.content_type = 'application/json'
|
||||
expected = {
|
||||
'metadata': {
|
||||
'key1': 'updatedvalue',
|
||||
'key29': 'newkey',
|
||||
}
|
||||
}
|
||||
req.body = jsonutils.dumps(expected)
|
||||
self.assertRaises(webob.exc.HTTPConflict, self.controller.update_all,
|
||||
req, self.uuid, expected)
|
||||
|
||||
def _return_server_in_build(self, context, server_id):
|
||||
return {'id': server_id,
|
||||
'uuid': '0cc3346e-9fef-4445-abe6-5d2b2690ec64',
|
||||
'name': 'fake',
|
||||
'locked': False,
|
||||
'vm_state': vm_states.BUILDING}
|
||||
|
||||
def _return_server_in_build_by_uuid(self, context, server_uuid):
|
||||
return {'id': 1,
|
||||
'uuid': '0cc3346e-9fef-4445-abe6-5d2b2690ec64',
|
||||
'name': 'fake',
|
||||
'locked': False,
|
||||
'vm_state': vm_states.BUILDING}
|
||||
|
|
|
@ -5302,6 +5302,24 @@ class ComputeAPITestCase(BaseTestCase):
|
|||
|
||||
db.instance_destroy(_context, instance['uuid'])
|
||||
|
||||
def test_disallow_metadata_changes_during_building(self):
|
||||
def fake_change_instance_metadata(inst, ctxt, diff, instance=None,
|
||||
instance_uuid=None):
|
||||
pass
|
||||
self.stubs.Set(compute_rpcapi.ComputeAPI, 'change_instance_metadata',
|
||||
fake_change_instance_metadata)
|
||||
|
||||
instance = self._create_fake_instance({'vm_state': vm_states.BUILDING})
|
||||
instance = dict(instance)
|
||||
|
||||
self.assertRaises(exception.InstanceInvalidState,
|
||||
self.compute_api.delete_instance_metadata, self.context,
|
||||
instance, "key")
|
||||
|
||||
self.assertRaises(exception.InstanceInvalidState,
|
||||
self.compute_api.update_instance_metadata, self.context,
|
||||
instance, "key")
|
||||
|
||||
def test_get_instance_faults(self):
|
||||
# Get an instances latest fault.
|
||||
instance = self._create_fake_instance()
|
||||
|
|
Loading…
Reference in New Issue