From d25991b41b5f476c293a6cf62361b115a160c64a Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Thu, 10 Jan 2013 01:18:33 +0000 Subject: Fix init_host checking moved instances Commit 6fc00d3465185 (https://review.openstack.org/#/c/18486/) introduced a couple of problems. 1) It introduced a method to try to deduce instance integer id from a name the driver reports and using instance_name_template. This fails badly when you're using something like 'instance-%(uuid)s' for instance_name_template. It's only used when the driver does not support the 'list_instance_uuids' method.. but XenAPI happens to not implement this method. 2) There's a call to _get_instance_volume_block_device_info() that is passing an instance_uuid when the method wants an instance. Fixes bug 1097987 (#1 above) by doing a brute force match against all instances' names. Fixes bug 1097978 (#2 above) Change-Id: I2da7658dd906fa06f0e867a2b22bf14e7f34cb12 --- nova/compute/manager.py | 87 +++++++++++++++++++++++++------------------------ nova/compute/utils.py | 60 ---------------------------------- 2 files changed, 44 insertions(+), 103 deletions(-) (limited to 'nova/compute') diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 682af2ce5..62cdaf03e 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -386,71 +386,72 @@ class ComputeManager(manager.SchedulerDependentManager): return self.conductor_api.instance_get_all_by_host(context, self.host) - def _destroy_evacuated_instances(self, context): - """Destroys evacuated instances. - - While the compute was down the instances running on it could be - evacuated to another host. Checking that instance host identical to - current host. Otherwise destroying it + def _get_instances_on_driver(self, context): + """Return a list of instance records that match the instances found + on the hypervisor. """ - - # getting all vms on this host local_instances = [] try: - # try to find all local instances by uuid + # Try to find all local instances by uuid. + # FIXME(comstud): Would be nice to consolidate this into + # a single query to nova-conductor. for uuid in self.driver.list_instance_uuids(): try: - local_instances.append(self.conductor_api. - instance_get_by_uuid(context, uuid)) + instance = self.conductor_api.instance_get_by_uuid( + context, uuid) + local_instances.append(instance) except exception.InstanceNotFound as e: LOG.error(_('Instance %(uuid)s found in the ' 'hypervisor, but not in the database'), locals()) continue + return local_instances except NotImplementedError: - # the driver doesn't support uuids listing, will do it in ugly way - for instance_name in self.driver.list_instances(): - try: - # couldn't find better way to find instance in db by it's - # name if i will run on the list of this host instances it - # will be hard to ignore instances that were created - # outside openstack. returns -1 if instance name doesn't - # match template - instance_id = compute_utils.parse_decimal_id(CONF - .instance_name_template, instance_name) - - if instance_id == -1: - continue - - local_instances.append(self.conductor_api. - instance_get(context, instance_id)) - except exception.InstanceNotFound as e: - LOG.error(_('Instance %(instance_name)s found in the ' - 'hypervisor, but not in the database'), - locals()) - continue + pass + # The driver doesn't support uuids listing, so we'll have + # to brute force. + driver_instances = self.driver.list_instances() + all_instances = self.conductor_api.instance_get_all(context) + name_map = dict([(instance['name'], instance) + for instance in all_instances]) + local_instances = [] + for driver_instance in driver_instances: + instance = name_map.get(driver_instance) + if not instance: + LOG.error(_('Instance %(driver_instance)s found in the ' + 'hypervisor, but not in the database'), + locals()) + continue + local_instances.append(instance) + return local_instances + + def _destroy_evacuated_instances(self, context): + """Destroys evacuated instances. + + While nova-compute was down, the instances running on it could be + evacuated to another host. Check that the instances reported + by the driver are still associated with this host. If they are + not, destroy them. + """ + our_host = self.host + local_instances = self._get_instances_on_driver(context) for instance in local_instances: instance_host = instance['host'] - host = self.host instance_name = instance['name'] - if instance['host'] != host: - LOG.info(_('instance host %(instance_host)s is not equal to ' - 'current host %(host)s. ' - 'Deleting zombie instance %(instance_name)s'), - locals()) - + if instance['host'] != our_host: + LOG.info(_('Deleting instance as its host (' + '%(instance_host)s) is not equal to our ' + 'host (%(our_host)s).'), + locals(), instance=instance) network_info = self._get_instance_nw_info(context, instance) bdi = self._get_instance_volume_block_device_info(context, - instance['uuid']) - + instance) self.driver.destroy(instance, self._legacy_nw_info(network_info), bdi, False) - LOG.info(_('zombie vm destroyed')) - def _init_instance(self, context, instance): '''Initialize this instance during service init.''' db_state = instance['power_state'] diff --git a/nova/compute/utils.py b/nova/compute/utils.py index 6d6b7cac9..8852cb820 100644 --- a/nova/compute/utils.py +++ b/nova/compute/utils.py @@ -253,63 +253,3 @@ def usage_volume_info(vol_usage): vol_usage['curr_write_bytes']) return usage_info - - -def parse_decimal_id(template, instance_name): - """Finds instance decimal id from instance name - - :param template: template e.g. instance-%03x-james - :param instance_name: instance name like instance-007-james - - :returns: parsed decimal id, e.g. 7 from the input above - """ - - # find pattern like %05x, %d..etc. - reg = re.search('(%\d*)([ioxds])', template) - format = reg.group(0) - - # split template to get prefix and suffix - tokens = template.split(format) - - if tokens[0]: - if not instance_name.startswith(tokens[0]): - # template prefix not match - return -1 - instance_name = instance_name[len(tokens[0]):] - - if tokens[1]: - if not instance_name.endswith(tokens[1]): - # template suffix not match - return -1 - instance_name = instance_name[:-len(tokens[1])] - - # validate that instance_id length matches - expected_length = format[1:-1] - - # if expected length is empty it means instance_id can be of any length - if expected_length: - if len(instance_name) < int(expected_length): - return -1 - # if instance_id has preciding zeroes it must be of expected length - if (instance_name[:1] == '0' and - len(instance_name) != int(expected_length)): - return -1 - - # if the minimal expected length empty, there should be no preceding zeros - elif instance_name[0] == '0': - return -1 - - # finding base of the template to convert to decimal - base_fmt = format[-1:] - base = 10 - if base_fmt == 'x': - base = 16 - elif base_fmt == 'o': - base = 8 - - try: - res = int(instance_name, base) - except ValueError: - res = -1 - - return res -- cgit