openstack
/
nova
Code Issues Proposed changes

Merged trunk.

This commit is contained in:
Brian Lamar 2011-06-20 14:55:29 -04:00
parent c178b3ce44 a62e0f3e10
commit 9d6f9b7a5d
13 changed files with 314 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
nova/api/openstack/ips.py
View File

@ -32,25 +32,24 @@ class Controller(object):
self.compute_api = nova.compute.API()
self.builder = nova.api.openstack.views.addresses.ViewBuilderV10()
def index(self, req, server_id):
def _get_instance(self, req, server_id):
try:
instance = self.compute_api.get(req.environ['nova.context'], id)
instance = self.compute_api.get(
req.environ['nova.context'], server_id)
except nova.exception.NotFound:
return faults.Fault(exc.HTTPNotFound())
return instance
def index(self, req, server_id):
instance = self._get_instance(req, server_id)
return {'addresses': self.builder.build(instance)}
def public(self, req, server_id):
try:
instance = self.compute_api.get(req.environ['nova.context'], id)
except nova.exception.NotFound:
return faults.Fault(exc.HTTPNotFound())
instance = self._get_instance(req, server_id)
return {'public': self.builder.build_public_parts(instance)}
def private(self, req, server_id):
try:
instance = self.compute_api.get(req.environ['nova.context'], id)
except nova.exception.NotFound:
return faults.Fault(exc.HTTPNotFound())
instance = self._get_instance(req, server_id)
return {'private': self.builder.build_private_parts(instance)}
def show(self, req, server_id, id):

3
nova/api/openstack/views/servers.py
View File

@ -75,7 +75,7 @@ class ViewBuilder(object):
}
inst_dict = {
'id': int(inst['id']),
'id': inst['id'],
'name': inst['display_name'],
'addresses': self.addresses_builder.build(inst),
'status': power_mapping[inst.get('state')]}
@ -99,6 +99,7 @@ class ViewBuilder(object):
self._build_image(inst_dict, inst)
self._build_flavor(inst_dict, inst)
inst_dict['uuid'] = inst['uuid']
return dict(server=inst_dict)
def _build_image(self, response, inst):

11
nova/compute/api.py
View File

@ -579,8 +579,15 @@ class API(base.Base):
def get(self, context, instance_id):
"""Get a single instance with the given instance_id."""
rv = self.db.instance_get(context, instance_id)
return dict(rv.iteritems())
# NOTE(sirp): id used to be exclusively integer IDs; now we're
# accepting both UUIDs and integer IDs. The handling of this
# is done in db/sqlalchemy/api/instance_get
if utils.is_uuid_like(instance_id):
uuid = instance_id
instance = self.db.instance_get_by_uuid(context, uuid)
else:
instance = self.db.instance_get(context, instance_id)
return dict(instance.iteritems())
@scheduler_api.reroute_compute("get")
def routing_get(self, context, instance_id):

1
nova/compute/manager.py
View File

@ -783,7 +783,6 @@ class ComputeManager(manager.SchedulerDependentManager):
def get_diagnostics(self, context, instance_id):
"""Retrieve diagnostics for an instance on this host."""
instance_ref = self.db.instance_get(context, instance_id)
if instance_ref["state"] == power_state.RUNNING:
LOG.audit(_("instance %s: retrieving diagnostics"), instance_id,
context=context)

5
nova/db/api.py
View File

@ -419,6 +419,11 @@ def instance_stop(context, instance_id):
return IMPL.instance_stop(context, instance_id)
def instance_get_by_uuid(context, uuid):
"""Get an instance or raise if it does not exist."""
return IMPL.instance_get_by_uuid(context, uuid)
def instance_get(context, instance_id):
"""Get an instance or raise if it does not exist."""
return IMPL.instance_get(context, instance_id)

65
nova/db/sqlalchemy/api.py
View File

@ -797,6 +797,8 @@ def instance_create(context, values):
values['metadata'] = _metadata_refs(values.get('metadata'))
instance_ref = models.Instance()
instance_ref['uuid'] = str(utils.gen_uuid())
instance_ref.update(values)
session = get_session()
@ -859,39 +861,48 @@ def instance_stop(context, instance_id):
@require_context
def instance_get(context, instance_id, session=None):
if not session:
session = get_session()
result = None
def instance_get_by_uuid(context, uuid, session=None):
partial = _build_instance_get(context, session=session)
result = partial.filter_by(uuid=uuid)
result = result.first()
if not result:
# FIXME(sirp): it would be nice if InstanceNotFound would accept a
# uuid parameter as well
raise exception.InstanceNotFound(instance_id=uuid)
return result
if is_admin_context(context):
result = session.query(models.Instance).\
options(joinedload_all('fixed_ip.floating_ips')).\
options(joinedload_all('security_groups.rules')).\
options(joinedload('volumes')).\
options(joinedload_all('fixed_ip.network')).\
options(joinedload('metadata')).\
options(joinedload('instance_type')).\
filter_by(id=instance_id).\
filter_by(deleted=can_read_deleted(context)).\
first()
elif is_user_context(context):
result = session.query(models.Instance).\
options(joinedload_all('fixed_ip.floating_ips')).\
options(joinedload_all('security_groups.rules')).\
options(joinedload('volumes')).\
options(joinedload('metadata')).\
options(joinedload('instance_type')).\
filter_by(project_id=context.project_id).\
filter_by(id=instance_id).\
filter_by(deleted=False).\
first()
@require_context
def instance_get(context, instance_id, session=None):
partial = _build_instance_get(context, session=session)
result = partial.filter_by(id=instance_id)
result = result.first()
if not result:
raise exception.InstanceNotFound(instance_id=instance_id)
return result
@require_context
def _build_instance_get(context, session=None):
if not session:
session = get_session()
partial = session.query(models.Instance).\
options(joinedload_all('fixed_ip.floating_ips')).\
options(joinedload_all('security_groups.rules')).\
options(joinedload('volumes')).\
options(joinedload_all('fixed_ip.network')).\
options(joinedload('metadata')).\
options(joinedload('instance_type'))
if is_admin_context(context):
partial = partial.filter_by(deleted=can_read_deleted(context))
elif is_user_context(context):
partial = partial.filter_by(project_id=context.project_id).\
filter_by(deleted=False)
return partial
@require_admin_context
def instance_get_all(context):
session = get_session()

43
nova/db/sqlalchemy/migrate_repo/versions/025_add_uuid_to_instances.py Normal file
View File

@ -0,0 +1,43 @@
# vim: tabstop=4 shiftwidth=4 softtabstop=4
# Copyright 2011 OpenStack LLC.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
from sqlalchemy import Column, Integer, MetaData, String, Table
from nova import utils
meta = MetaData()
instances = Table("instances", meta,
Column("id", Integer(), primary_key=True, nullable=False))
uuid_column = Column("uuid", String(36))
def upgrade(migrate_engine):
meta.bind = migrate_engine
instances.create_column(uuid_column)
rows = migrate_engine.execute(instances.select())
for row in rows:
instance_uuid = str(utils.gen_uuid())
migrate_engine.execute(instances.update()\
.where(instances.c.id == row[0])\
.values(uuid=instance_uuid))
def downgrade(migrate_engine):
meta.bind = migrate_engine
instances.drop_column(uuid_column)

1
nova/db/sqlalchemy/models.py
View File

@ -233,6 +233,7 @@ class Instance(BASE, NovaBase):
os_type = Column(String(255))
vm_mode = Column(String(255))
uuid = Column(String(36))
# TODO(vish): see Ewan's email about state improvements, probably
# should be in a driver base class or some such

101
nova/scheduler/api.py
View File

@ -24,6 +24,7 @@ from nova import exception
from nova import flags
from nova import log as logging
from nova import rpc
from nova import utils
from eventlet import greenpool
@ -201,38 +202,78 @@ class RedirectResult(exception.Error):
class reroute_compute(object):
"""Decorator used to indicate that the method should
delegate the call the child zones if the db query
can't find anything."""
"""
reroute_compute is responsible for trying to lookup a resource in the
current zone and if it's not found there, delegating the call to the
child zones.
Since reroute_compute will be making 'cross-zone' calls, the ID for the
object must come in as a UUID-- if we receive an integer ID, we bail.
The steps involved are:
1. Validate that item_id is UUID like
2. Lookup item by UUID in the zone local database
3. If the item was found, then extract integer ID, and pass that to
the wrapped method. (This ensures that zone-local code can
continue to use integer IDs).
4. If the item was not found, we delgate the call to a child zone
using the UUID.
"""
def __init__(self, method_name):
self.method_name = method_name
def _route_to_child_zones(self, context, collection, item_uuid):
if not FLAGS.enable_zone_routing:
raise exception.InstanceNotFound(instance_id=item_uuid)
zones = db.zone_get_all(context)
if not zones:
raise exception.InstanceNotFound(instance_id=item_uuid)
# Ask the children to provide an answer ...
LOG.debug(_("Asking child zones ..."))
result = self._call_child_zones(zones,
wrap_novaclient_function(_issue_novaclient_command,
collection, self.method_name, item_uuid))
# Scrub the results and raise another exception
# so the API layers can bail out gracefully ...
raise RedirectResult(self.unmarshall_result(result))
def __call__(self, f):
def wrapped_f(*args, **kwargs):
collection, context, item_id = \
collection, context, item_id_or_uuid = \
self.get_collection_context_and_id(args, kwargs)
try:
# Call the original function ...
attempt_reroute = False
if utils.is_uuid_like(item_id_or_uuid):
item_uuid = item_id_or_uuid
try:
instance = db.instance_get_by_uuid(context, item_uuid)
except exception.InstanceNotFound, e:
# NOTE(sirp): since a UUID was passed in, we can attempt
# to reroute to a child zone
attempt_reroute = True
LOG.debug(_("Instance %(item_uuid)s not found "
"locally: '%(e)s'" % locals()))
else:
# NOTE(sirp): since we're not re-routing in this case, and
# we we were passed a UUID, we need to replace that UUID
# with an integer ID in the argument list so that the
# zone-local code can continue to use integer IDs.
item_id = instance['id']
args = list(args) # needs to be mutable to replace
self.replace_uuid_with_id(args, kwargs, item_id)
if attempt_reroute:
return self._route_to_child_zones(context, collection,
item_uuid)
else:
return f(*args, **kwargs)
except exception.InstanceNotFound, e:
LOG.debug(_("Instance %(item_id)s not found "
"locally: '%(e)s'" % locals()))
if not FLAGS.enable_zone_routing:
raise
zones = db.zone_get_all(context)
if not zones:
raise
# Ask the children to provide an answer ...
LOG.debug(_("Asking child zones ..."))
result = self._call_child_zones(zones,
wrap_novaclient_function(_issue_novaclient_command,
collection, self.method_name, item_id))
# Scrub the results and raise another exception
# so the API layers can bail out gracefully ...
raise RedirectResult(self.unmarshall_result(result))
return wrapped_f
def _call_child_zones(self, zones, function):
@ -251,6 +292,18 @@ class reroute_compute(object):
instance_id = args[2]
return ("servers", context, instance_id)
@staticmethod
def replace_uuid_with_id(args, kwargs, replacement_id):
"""
Extracts the UUID parameter from the arg or kwarg list and replaces
it with an integer ID.
"""
if 'instance_id' in kwargs:
kwargs['instance_id'] = replacement_id
elif len(args) > 1:
args.pop(2)
args.insert(2, replacement_id)
def unmarshall_result(self, zone_responses):
"""Result is a list of responses from each child zone.
Each decorator derivation is responsible to turning this

74
nova/tests/api/openstack/test_servers.py
View File

@ -49,10 +49,22 @@ FLAGS = flags.FLAGS
FLAGS.verbose = True
def return_server(context, id):
FAKE_UUID = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa'
def fake_gen_uuid():
return FAKE_UUID
def return_server_by_id(context, id):
return stub_instance(id)
def return_server_by_uuid(context, uuid):
id = 1
return stub_instance(id, uuid=uuid)
def return_server_with_addresses(private, public):
def _return_server(context, id):
return stub_instance(id, private_address=private,
@ -111,7 +123,8 @@ def instance_address(context, instance_id):
def stub_instance(id, user_id=1, private_address=None, public_addresses=None,
host=None, power_state=0, reservation_id=""):
host=None, power_state=0, reservation_id="",
uuid=FAKE_UUID):
metadata = []
metadata.append(InstanceMetadata(key='seq', value=id))
@ -129,7 +142,7 @@ def stub_instance(id, user_id=1, private_address=None, public_addresses=None,
server_name = "reservation_%s" % (reservation_id, )
instance = {
"id": id,
"id": int(id),
"admin_pass": "",
"user_id": user_id,
"project_id": "",
@ -157,7 +170,8 @@ def stub_instance(id, user_id=1, private_address=None, public_addresses=None,
"display_name": server_name,
"display_description": "",
"locked": False,
"metadata": metadata}
"metadata": metadata,
"uuid": uuid}
instance["fixed_ip"] = {
"address": private_address,
@ -196,8 +210,11 @@ class ServersTest(test.TestCase):
fakes.stub_out_auth(self.stubs)
fakes.stub_out_key_pair_funcs(self.stubs)
fakes.stub_out_image_service(self.stubs)
self.stubs.Set(utils, 'gen_uuid', fake_gen_uuid)
self.stubs.Set(nova.db.api, 'instance_get_all', return_servers)
self.stubs.Set(nova.db.api, 'instance_get', return_server)
self.stubs.Set(nova.db.api, 'instance_get', return_server_by_id)
self.stubs.Set(nova.db, 'instance_get_by_uuid',
return_server_by_uuid)
self.stubs.Set(nova.db.api, 'instance_get_all_by_user',
return_servers)
self.stubs.Set(nova.db.api, 'instance_add_security_group',
@ -229,6 +246,36 @@ class ServersTest(test.TestCase):
self.assertEqual(res_dict['server']['id'], 1)
self.assertEqual(res_dict['server']['name'], 'server1')
def test_get_server_by_uuid(self):
"""
The steps involved with resolving a UUID are pretty complicated;
here's what's happening in this scenario:
1. Show is calling `routing_get`
2. `routing_get` is wrapped by `reroute_compute` which does the work
of resolving requests to child zones.
3. `reroute_compute` looks up the UUID by hitting the stub
(returns_server_by_uuid)
4. Since the stub return that the record exists, `reroute_compute`
considers the request to be 'zone local', so it replaces the UUID
in the argument list with an integer ID and then calls the inner
function ('get').
5. The call to `get` hits the other stub 'returns_server_by_id` which
has the UUID set to FAKE_UUID
So, counterintuitively, we call `get` twice on the `show` command.
"""
req = webob.Request.blank('/v1.0/servers/%s' % FAKE_UUID)
res = req.get_response(fakes.wsgi_app())
res_dict = json.loads(res.body)
self.assertEqual(res_dict['server']['id'], 1)
self.assertEqual(res_dict['server']['uuid'], FAKE_UUID)
self.assertEqual(res_dict['server']['name'], 'server1')
def test_get_server_by_id_v1_1(self):
req = webob.Request.blank('/v1.1/servers/1')
res = req.get_response(fakes.wsgi_app())
@ -540,7 +587,8 @@ class ServersTest(test.TestCase):
def _setup_for_create_instance(self):
"""Shared implementation for tests below that create instance"""
def instance_create(context, inst):
return {'id': '1', 'display_name': 'server_test'}
return {'id': 1, 'display_name': 'server_test',
'uuid': FAKE_UUID}
def server_update(context, id, params):
return instance_create(context, id)
@ -594,11 +642,22 @@ class ServersTest(test.TestCase):
self.assertEqual(1, server['id'])
self.assertEqual(2, server['flavorId'])
self.assertEqual(3, server['imageId'])
self.assertEqual(FAKE_UUID, server['uuid'])
self.assertEqual(res.status_int, 200)
def test_create_instance(self):
self._test_create_instance_helper()
def test_create_instance_has_uuid(self):
"""Tests at the db-layer instead of API layer since that's where the
UUID is generated
"""
ctxt = context.RequestContext(1, 1)
values = {}
instance = nova.db.api.instance_create(ctxt, values)
expected = FAKE_UUID
self.assertEqual(instance['uuid'], expected)
def test_create_instance_via_zones(self):
"""Server generated ReservationID"""
self._setup_for_create_instance()
@ -1850,7 +1909,8 @@ class TestServerInstanceCreation(test.TestCase):
self.injected_files = kwargs['injected_files']
else:
self.injected_files = None
return [{'id': '1234', 'display_name': 'fakeinstance'}]
return [{'id': '1234', 'display_name': 'fakeinstance',
'uuid': FAKE_UUID}]
def set_admin_password(self, *args, **kwargs):
pass

32
nova/tests/scheduler/test_scheduler.py
View File

@ -48,6 +48,10 @@ flags.DECLARE('stub_network', 'nova.compute.manager')
flags.DECLARE('instances_path', 'nova.compute.manager')
FAKE_UUID_NOT_FOUND = 'ffffffff-ffff-ffff-ffff-ffffffffffff'
FAKE_UUID = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa'
class TestDriver(driver.Scheduler):
"""Scheduler Driver for Tests"""
def schedule(context, topic, *args, **kwargs):
@ -926,12 +930,23 @@ def zone_get_all(context):
]
def fake_instance_get_by_uuid(context, uuid):
if FAKE_UUID_NOT_FOUND:
raise exception.InstanceNotFound(instance_id=uuid)
else:
return {'id': 1}
class FakeRerouteCompute(api.reroute_compute):
def __init__(self, method_name, id_to_return=1):
super(FakeRerouteCompute, self).__init__(method_name)
self.id_to_return = id_to_return
def _call_child_zones(self, zones, function):
return []
def get_collection_context_and_id(self, args, kwargs):
return ("servers", None, 1)
return ("servers", None, self.id_to_return)
def unmarshall_result(self, zone_responses):
return dict(magic="found me")
@ -960,6 +975,8 @@ class ZoneRedirectTest(test.TestCase):
self.stubs = stubout.StubOutForTesting()
self.stubs.Set(db, 'zone_get_all', zone_get_all)
self.stubs.Set(db, 'instance_get_by_uuid',
fake_instance_get_by_uuid)
self.enable_zone_routing = FLAGS.enable_zone_routing
FLAGS.enable_zone_routing = True
@ -976,8 +993,19 @@ class ZoneRedirectTest(test.TestCase):
except api.RedirectResult, e:
self.fail(_("Successful database hit should succeed"))
def test_trap_not_found_locally(self):
def test_trap_not_found_locally_id_passed(self):
"""When an integer ID is not found locally, we cannot reroute to
another zone, so just return InstanceNotFound exception
"""
decorator = FakeRerouteCompute("foo")
self.assertRaises(exception.InstanceNotFound,
decorator(go_boom), None, None, 1)
def test_trap_not_found_locally_uuid_passed(self):
"""When a UUID is found, if the item isn't found locally, we should
try to reroute to a child zone to see if they have it
"""
decorator = FakeRerouteCompute("foo", id_to_return=FAKE_UUID_NOT_FOUND)
try:
result = decorator(go_boom)(None, None, 1)
self.assertFail(_("Should have rerouted."))

18
nova/tests/test_utils.py
View File

@ -275,3 +275,21 @@ class GenericUtilsTestCase(test.TestCase):
# error case
result = utils.parse_server_string('www.exa:mple.com:8443')
self.assertEqual(('', ''), result)
class IsUUIDLikeTestCase(test.TestCase):
def assertUUIDLike(self, val, expected):
result = utils.is_uuid_like(val)
self.assertEqual(result, expected)
def test_good_uuid(self):
val = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa'
self.assertUUIDLike(val, True)
def test_integer_passed(self):
val = 1
self.assertUUIDLike(val, False)
def test_non_uuid_string_passed(self):
val = 'foo-fooo'
self.assertUUIDLike(val, False)

15
nova/utils.py
View File

@ -35,6 +35,7 @@ import struct
import sys
import time
import types
import uuid
from xml.sax import saxutils
from eventlet import event
@ -726,3 +727,17 @@ def parse_server_string(server_str):
except:
LOG.debug(_('Invalid server_string: %s' % server_str))
return ('', '')
def gen_uuid():
return uuid.uuid4()
def is_uuid_like(val):
"""For our purposes, a UUID is a string in canoical form:
aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa
"""
if not isinstance(val, basestring):
return False
return (len(val) == 36) and (val.count('-') == 4)