From c58acf2c59420a78f6b7195e3c1ef25e84f12e20 Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Thu, 30 Sep 2010 21:19:35 -0400 Subject: Replace database instance 'ec2_id' with 'internal_id' throughout the nova.db package. internal_id is now an integer -- we need to figure out how to make this a bigint or something. --- nova/db/api.py | 4 ++-- nova/db/sqlalchemy/api.py | 20 +++++++++++--------- nova/db/sqlalchemy/models.py | 4 ++-- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/nova/db/api.py b/nova/db/api.py index b68a0fe8f..c3c29c2fc 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -280,9 +280,9 @@ def instance_get_floating_address(context, instance_id): return IMPL.instance_get_floating_address(context, instance_id) -def instance_get_by_ec2_id(context, ec2_id): +def instance_get_by_internal_id(context, internal_id): """Get an instance by ec2 id.""" - return IMPL.instance_get_by_ec2_id(context, ec2_id) + return IMPL.instance_get_by_internal_id(context, internal_id) def instance_is_vpn(context, instance_id): diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 9c3caf9af..6dd6b545a 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -384,10 +384,11 @@ def instance_create(_context, values): session = get_session() with session.begin(): - while instance_ref.ec2_id == None: - ec2_id = utils.generate_uid(instance_ref.__prefix__) - if not instance_ec2_id_exists(_context, ec2_id, session=session): - instance_ref.ec2_id = ec2_id + while instance_ref.internal_id == None: + internal_id = utils.generate_uid(instance_ref.__prefix__) + if not instance_internal_id_exists(_context, internal_id, + session=session): + instance_ref.internal_id = internal_id instance_ref.save(session=session) return instance_ref @@ -446,22 +447,23 @@ def instance_get_all_by_reservation(_context, reservation_id): ).all() -def instance_get_by_ec2_id(context, ec2_id): +def instance_get_by_internal_id(context, internal_id): session = get_session() instance_ref = session.query(models.Instance - ).filter_by(ec2_id=ec2_id + ).filter_by(internal_id=internal_id ).filter_by(deleted=_deleted(context) ).first() if not instance_ref: - raise exception.NotFound('Instance %s not found' % (ec2_id)) + raise exception.NotFound('Instance %s not found' % (internal_id)) return instance_ref -def instance_ec2_id_exists(context, ec2_id, session=None): +def instance_internal_id_exists(context, internal_id, session=None): if not session: session = get_session() - return session.query(exists().where(models.Instance.id==ec2_id)).one()[0] + return session.query(exists().where(models.Instance.id==internal_id) + ).one()[0] def instance_get_fixed_address(_context, instance_id): diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index 6cb377476..5c93c92a0 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -197,7 +197,7 @@ class Instance(BASE, NovaBase): __tablename__ = 'instances' __prefix__ = 'i' id = Column(Integer, primary_key=True) - ec2_id = Column(String(10), unique=True) + internal_id = Column(Integer, unique=True) admin_pass = Column(String(255)) @@ -214,7 +214,7 @@ class Instance(BASE, NovaBase): @property def name(self): - return self.ec2_id + return self.internal_id image_id = Column(String(255)) kernel_id = Column(String(255)) -- cgit From 58773e16ddd6f3aaa4aafefde55a3ae631e806dd Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Thu, 30 Sep 2010 21:59:52 -0400 Subject: Convert EC2 cloud.py from assuming that EC2 IDs are stored directly in the database, to assuming that EC2 IDs should be converted to internal IDs. The conversion between the internal ID and the EC2 ID is imperfect -- right now it turns internal IDs like 408 into EC2 IDs like i-408, and vice versa. Instead, EC2 IDs are supposed to be i-[base 36 of the integer]. --- nova/api/ec2/cloud.py | 58 ++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 41 insertions(+), 17 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 79c95788b..2fec49da8 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -113,6 +113,16 @@ class CloudController(object): result[key] = [line] return result + def ec2_id_to_internal_id(ec2_id): + """Convert an ec2 ID (i-[base 36 number]) to an internal id (int)""" + # TODO(gundlach): Maybe this should actually work? + return ec2_id[2:] + + def internal_id_to_ec2_id(internal_id): + """Convert an internal ID (int) to an ec2 ID (i-[base 36 number])""" + # TODO(gundlach): Yo maybe this should actually convert to base 36 + return "i-%d" % internal_id + def get_metadata(self, address): instance_ref = db.fixed_ip_get_instance(None, address) if instance_ref is None: @@ -144,7 +154,7 @@ class CloudController(object): }, 'hostname': hostname, 'instance-action': 'none', - 'instance-id': instance_ref['ec2_id'], + 'instance-id': internal_id_to_ec2_id(instance_ref['internal_id']), 'instance-type': instance_ref['instance_type'], 'local-hostname': hostname, 'local-ipv4': address, @@ -244,9 +254,11 @@ class CloudController(object): def delete_security_group(self, context, group_name, **kwargs): return True - def get_console_output(self, context, instance_id, **kwargs): - # instance_id is passed in as a list of instances - instance_ref = db.instance_get_by_ec2_id(context, instance_id[0]) + def get_console_output(self, context, ec2_id_list, **kwargs): + # ec2_id_list is passed in as a list of instances + ec2_id = ec2_id_list[0] + internal_id = ec2_id_to_internal_id(ec2_id) + instance_ref = db.instance_get_by_ec2_id(context, internal_id) return rpc.call('%s.%s' % (FLAGS.compute_topic, instance_ref['host']), {"method": "get_console_output", @@ -326,7 +338,8 @@ class CloudController(object): raise exception.ApiError("Volume status must be available") if volume_ref['attach_status'] == "attached": raise exception.ApiError("Volume is already attached") - instance_ref = db.instance_get_by_ec2_id(context, instance_id) + internal_id = ec2_id_to_internal_id(instance_id) + instance_ref = db.instance_get_by_internal_id(context, internal_id) host = instance_ref['host'] rpc.cast(db.queue_get_for(context, FLAGS.compute_topic, host), {"method": "attach_volume", @@ -360,9 +373,11 @@ class CloudController(object): # If the instance doesn't exist anymore, # then we need to call detach blind db.volume_detached(context) + internal_id = instance_ref['internal_id'] + ec2_id = internal_id_to_ec2_id(internal_id) return {'attachTime': volume_ref['attach_time'], 'device': volume_ref['mountpoint'], - 'instanceId': instance_ref['ec2_id'], + 'instanceId': internal_id, 'requestId': context.request_id, 'status': volume_ref['attach_status'], 'volumeId': volume_ref['id']} @@ -411,7 +426,9 @@ class CloudController(object): if instance['image_id'] == FLAGS.vpn_image_id: continue i = {} - i['instanceId'] = instance['ec2_id'] + internal_id = instance['internal_id'] + ec2_id = internal_id_to_ec2_id(internal_id) + i['instanceId'] = ec2_id i['imageId'] = instance['image_id'] i['instanceState'] = { 'code': instance['state'], @@ -464,9 +481,10 @@ class CloudController(object): instance_id = None if (floating_ip_ref['fixed_ip'] and floating_ip_ref['fixed_ip']['instance']): - instance_id = floating_ip_ref['fixed_ip']['instance']['ec2_id'] + internal_id = floating_ip_ref['fixed_ip']['instance']['ec2_id'] + ec2_id = internal_id_to_ec2_id(internal_id) address_rv = {'public_ip': address, - 'instance_id': instance_id} + 'instance_id': ec2_id} if context.user.is_admin(): details = "%s (%s)" % (address_rv['instance_id'], floating_ip_ref['project_id']) @@ -498,8 +516,9 @@ class CloudController(object): "floating_address": floating_ip_ref['address']}}) return {'releaseResponse': ["Address released."]} - def associate_address(self, context, instance_id, public_ip, **kwargs): - instance_ref = db.instance_get_by_ec2_id(context, instance_id) + def associate_address(self, context, ec2_id, public_ip, **kwargs): + internal_id = ec2_id_to_internal_id(ec2_id) + instance_ref = db.instance_get_by_internal_id(context, internal_id) fixed_address = db.instance_get_fixed_address(context, instance_ref['id']) floating_ip_ref = db.floating_ip_get_by_address(context, public_ip) @@ -610,7 +629,9 @@ class CloudController(object): inst = {} inst['mac_address'] = utils.generate_mac() inst['launch_index'] = num - inst['hostname'] = instance_ref['ec2_id'] + internal_id = instance_ref['internal_id'] + ec2_id = internal_id_to_ec2_id(internal_id) + inst['hostname'] = ec2_id db.instance_update(context, inst_id, inst) address = self.network_manager.allocate_fixed_ip(context, inst_id, @@ -634,12 +655,14 @@ class CloudController(object): return self._format_run_instances(context, reservation_id) - def terminate_instances(self, context, instance_id, **kwargs): + def terminate_instances(self, context, ec2_id_list, **kwargs): logging.debug("Going to start terminating instances") - for id_str in instance_id: + for id_str in ec2_id_list: + internal_id = ec2_id_to_internal_id(id_str) logging.debug("Going to try and terminate %s" % id_str) try: - instance_ref = db.instance_get_by_ec2_id(context, id_str) + instance_ref = db.instance_get_by_internal_id(context, + internal_id) except exception.NotFound: logging.warning("Instance %s was not found during terminate" % id_str) @@ -688,7 +711,7 @@ class CloudController(object): cloud.reboot(id_str, context=context) return True - def update_instance(self, context, instance_id, **kwargs): + def update_instance(self, context, ec2_id, **kwargs): updatable_fields = ['display_name', 'display_description'] changes = {} for field in updatable_fields: @@ -696,7 +719,8 @@ class CloudController(object): changes[field] = kwargs[field] if changes: db_context = {} - inst = db.instance_get_by_ec2_id(db_context, instance_id) + internal_id = ec2_id_to_internal_id(ec2_id) + inst = db.instance_get_by_internal_id(db_context, internal_id) db.instance_update(db_context, inst['id'], kwargs) return True -- cgit From 39080e5f5000e0f401ff19f3fd9dd8cfbffffe69 Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Thu, 30 Sep 2010 22:05:16 -0400 Subject: Find other places in the code that used ec2_id or get_instance_by_ec2_id and use internal_id as appropriate --- nova/compute/manager.py | 6 +++--- nova/tests/cloud_unittest.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index f370ede8b..131fac406 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -67,7 +67,7 @@ class ComputeManager(manager.Manager): def run_instance(self, context, instance_id, **_kwargs): """Launch a new instance with specified options.""" instance_ref = self.db.instance_get(context, instance_id) - if instance_ref['ec2_id'] in self.driver.list_instances(): + if instance_ref['internal_id'] in self.driver.list_instances(): raise exception.Error("Instance has already been created") logging.debug("instance %s: starting...", instance_id) project_id = instance_ref['project_id'] @@ -129,7 +129,7 @@ class ComputeManager(manager.Manager): raise exception.Error( 'trying to reboot a non-running' 'instance: %s (state: %s excepted: %s)' % - (instance_ref['ec2_id'], + (instance_ref['internal_id'], instance_ref['state'], power_state.RUNNING)) @@ -151,7 +151,7 @@ class ComputeManager(manager.Manager): if FLAGS.connection_type == 'libvirt': fname = os.path.abspath(os.path.join(FLAGS.instances_path, - instance_ref['ec2_id'], + instance_ref['internal_id'], 'console.log')) with open(fname, 'r') as f: output = f.read() diff --git a/nova/tests/cloud_unittest.py b/nova/tests/cloud_unittest.py index ae7dea1db..d316db153 100644 --- a/nova/tests/cloud_unittest.py +++ b/nova/tests/cloud_unittest.py @@ -236,7 +236,7 @@ class CloudTestCase(test.TrialTestCase): def test_update_of_instance_display_fields(self): inst = db.instance_create({}, {}) - self.cloud.update_instance(self.context, inst['ec2_id'], + self.cloud.update_instance(self.context, inst['internal_id'], display_name='c00l 1m4g3') inst = db.instance_get({}, inst['id']) self.assertEqual('c00l 1m4g3', inst['display_name']) -- cgit From 06cdef056b508e15869623da28ad18cc817e6848 Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Thu, 30 Sep 2010 22:09:46 -0400 Subject: First attempt at a uuid generator -- but we've lost a 'topic' input so i don't know what that did. --- nova/utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/nova/utils.py b/nova/utils.py index d18dd9843..86ff3d22e 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -126,7 +126,9 @@ def runthis(prompt, cmd, check_exit_code = True): def generate_uid(topic, size=8): - return '%s-%s' % (topic, ''.join([random.choice('01234567890abcdefghijklmnopqrstuvwxyz') for x in xrange(size)])) + #TODO(gundlach): we want internal ids to just be ints now. i just dropped + #off a topic prefix, so what have I broken? + return random.randint(0, 2**64-1) def generate_mac(): -- cgit From 4b11351aba7e71154f82a6a76590c786b4d7a53a Mon Sep 17 00:00:00 2001 From: Todd Willey Date: Fri, 1 Oct 2010 21:46:36 -0400 Subject: Keep handles to loggers open after daemonizing. --- nova/server.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/nova/server.py b/nova/server.py index d4563bfe0..c58a15041 100644 --- a/nova/server.py +++ b/nova/server.py @@ -106,6 +106,7 @@ def serve(name, main): def daemonize(args, name, main): """Does the work of daemonizing the process""" logging.getLogger('amqplib').setLevel(logging.WARN) + files_to_keep = [] if FLAGS.daemonize: logger = logging.getLogger() formatter = logging.Formatter( @@ -114,12 +115,14 @@ def daemonize(args, name, main): syslog = logging.handlers.SysLogHandler(address='/dev/log') syslog.setFormatter(formatter) logger.addHandler(syslog) + files_to_keep.append(syslog.socket) else: if not FLAGS.logfile: FLAGS.logfile = '%s.log' % name logfile = logging.FileHandler(FLAGS.logfile) logfile.setFormatter(formatter) logger.addHandler(logfile) + files_to_keep.append(logfile.stream) stdin, stdout, stderr = None, None, None else: stdin, stdout, stderr = sys.stdin, sys.stdout, sys.stderr @@ -139,6 +142,7 @@ def daemonize(args, name, main): stdout=stdout, stderr=stderr, uid=FLAGS.uid, - gid=FLAGS.gid + gid=FLAGS.gid, + files_preserve=files_to_keep ): main(args) -- cgit From 2a8e4a3e818f1d279a886e2e5f5ae49f3de26a4d Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Mon, 4 Oct 2010 14:26:55 -0400 Subject: Revert r312 --- nova/utils.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/nova/utils.py b/nova/utils.py index 86ff3d22e..5f64b13c4 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -126,9 +126,8 @@ def runthis(prompt, cmd, check_exit_code = True): def generate_uid(topic, size=8): - #TODO(gundlach): we want internal ids to just be ints now. i just dropped - #off a topic prefix, so what have I broken? - return random.randint(0, 2**64-1) + return '%s-%s' % (topic, ''.join([random.choice('01234567890abcdefghijklmnopqrstuvwxyz') for x in xrange(size)])) + def generate_mac(): -- cgit From dd0f365c98ae68afff9a0fbc75e7d5b88499b282 Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Mon, 4 Oct 2010 16:39:05 -0400 Subject: Fix broken unit tests --- nova/api/ec2/cloud.py | 24 ++++++++++++++---------- nova/db/sqlalchemy/api.py | 3 +++ nova/tests/cloud_unittest.py | 3 ++- nova/utils.py | 11 +++++++++-- 4 files changed, 28 insertions(+), 13 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 2fec49da8..7f5f4c4e9 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -72,6 +72,20 @@ def _gen_key(context, user_id, key_name): return {'private_key': private_key, 'fingerprint': fingerprint} +def ec2_id_to_internal_id(ec2_id): + """Convert an ec2 ID (i-[base 36 number]) to an internal id (int)""" + return int(ec2_id[2:], 36) + + +def internal_id_to_ec2_id(internal_id): + """Convert an internal ID (int) to an ec2 ID (i-[base 36 number])""" + digits = [] + while internal_id != 0: + internal_id, remainder = divmod(internal_id, 36) + digits.append('0123456789abcdefghijklmnopqrstuvwxyz'[remainder]) + return "i-%s" % ''.join(reversed(digits)) + + class CloudController(object): """ CloudController provides the critical dispatch between inbound API calls through the endpoint and messages @@ -113,16 +127,6 @@ class CloudController(object): result[key] = [line] return result - def ec2_id_to_internal_id(ec2_id): - """Convert an ec2 ID (i-[base 36 number]) to an internal id (int)""" - # TODO(gundlach): Maybe this should actually work? - return ec2_id[2:] - - def internal_id_to_ec2_id(internal_id): - """Convert an internal ID (int) to an ec2 ID (i-[base 36 number])""" - # TODO(gundlach): Yo maybe this should actually convert to base 36 - return "i-%d" % internal_id - def get_metadata(self, address): instance_ref = db.fixed_ip_get_instance(None, address) if instance_ref is None: diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 6dd6b545a..9d43da3ba 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -377,6 +377,9 @@ def fixed_ip_update(_context, address, values): ################### +#TODO(gundlach): instance_create and volume_create are nearly identical +#and should be refactored. I expect there are other copy-and-paste +#functions between the two of them as well. def instance_create(_context, values): instance_ref = models.Instance() for (key, value) in values.iteritems(): diff --git a/nova/tests/cloud_unittest.py b/nova/tests/cloud_unittest.py index d316db153..615e589cf 100644 --- a/nova/tests/cloud_unittest.py +++ b/nova/tests/cloud_unittest.py @@ -236,7 +236,8 @@ class CloudTestCase(test.TrialTestCase): def test_update_of_instance_display_fields(self): inst = db.instance_create({}, {}) - self.cloud.update_instance(self.context, inst['internal_id'], + ec2_id = cloud.internal_id_to_ec2_id(inst['internal_id']) + self.cloud.update_instance(self.context, ec2_id, display_name='c00l 1m4g3') inst = db.instance_get({}, inst['id']) self.assertEqual('c00l 1m4g3', inst['display_name']) diff --git a/nova/utils.py b/nova/utils.py index 5f64b13c4..b1699bda8 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -126,8 +126,15 @@ def runthis(prompt, cmd, check_exit_code = True): def generate_uid(topic, size=8): - return '%s-%s' % (topic, ''.join([random.choice('01234567890abcdefghijklmnopqrstuvwxyz') for x in xrange(size)])) - + if topic == "i": + # Instances have integer internal ids. + #TODO(gundlach): We should make this more than 32 bits, but we need to + #figure out how to make the DB happy with 64 bit integers. + return random.randint(0, 2**32-1) + else: + characters = '01234567890abcdefghijklmnopqrstuvwxyz' + choices = [random.choice(characters) for x in xrange(size)] + return '%s-%s' % (topic, ''.join(choices)) def generate_mac(): -- cgit From 8a2d7efa542e168fda81f703fa8e8c19467bf800 Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Tue, 5 Oct 2010 13:40:17 -0400 Subject: Fix clause comparing id to internal_id --- nova/db/sqlalchemy/api.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 61feed9d0..870e7b1a5 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -663,8 +663,9 @@ def instance_get_by_internal_id(context, internal_id): def instance_internal_id_exists(context, internal_id, session=None): if not session: session = get_session() - return session.query(exists().where(models.Instance.id==internal_id) - ).one()[0] + return session.query( + exists().where(models.Instance.internal_id==internal_id) + ).one()[0] @require_context -- cgit