diff options
| -rw-r--r-- | nova/compute/manager.py | 87 | ||||
| -rw-r--r-- | nova/compute/utils.py | 60 | ||||
| -rw-r--r-- | nova/tests/compute/test_compute.py | 230 | ||||
| -rw-r--r-- | nova/tests/compute/test_compute_utils.py | 94 |
4 files changed, 167 insertions, 304 deletions
diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 5627687fcb..347c4a3fbb 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 6d6b7cac9c..8852cb820e 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 diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py index 4337fdba9c..3387cb4421 100644 --- a/nova/tests/compute/test_compute.py +++ b/nova/tests/compute/test_compute.py @@ -53,6 +53,7 @@ from nova.openstack.common.notifier import test_notifier from nova.openstack.common import rpc from nova.openstack.common.rpc import common as rpc_common from nova.openstack.common import timeutils +from nova.openstack.common import uuidutils import nova.policy from nova import quota from nova import test @@ -3110,9 +3111,8 @@ class ComputeTestCase(BaseTestCase): self.assertEqual(timeouts.count(10), 10) self.assertTrue(None in timeouts) - def test_init_host_with_evacuated_instances_uuid_list(self): - # creating testdata - c = context.get_admin_context() + def test_destroy_evacuated_instances(self): + fake_context = context.get_admin_context() # instances in central db instances = [ @@ -3128,130 +3128,146 @@ class ComputeTestCase(BaseTestCase): # those are already been evacuated to other host evacuated_instance = self._create_fake_instance({'host': 'otherhost'}) - # creating mocks + instances.append(evacuated_instance) + + self.mox.StubOutWithMock(self.compute, + '_get_instances_on_driver') + self.mox.StubOutWithMock(self.compute, + '_get_instance_nw_info') + self.mox.StubOutWithMock(self.compute, + '_get_instance_volume_block_device_info') + self.mox.StubOutWithMock(self.compute, '_legacy_nw_info') + self.mox.StubOutWithMock(self.compute.driver, 'destroy') + + self.compute._get_instances_on_driver(fake_context).AndReturn( + instances) + self.compute._get_instance_nw_info(fake_context, + evacuated_instance).AndReturn( + 'fake_network_info') + self.compute._get_instance_volume_block_device_info( + fake_context, evacuated_instance).AndReturn('fake_bdi') + self.compute._legacy_nw_info('fake_network_info').AndReturn( + 'fake_legacy_network_info') + self.compute.driver.destroy(evacuated_instance, + 'fake_legacy_network_info', + 'fake_bdi', + False) + + self.mox.ReplayAll() + self.compute._destroy_evacuated_instances(fake_context) + + def test_init_host(self): + + our_host = self.compute.host + fake_context = 'fake-context' + startup_instances = ['inst1', 'inst2', 'inst3'] + + def _do_mock_calls(defer_iptables_apply): + self.compute.driver.init_host(host=our_host) + context.get_admin_context().AndReturn(fake_context) + self.compute._get_instances_at_startup(fake_context).AndReturn( + startup_instances) + if defer_iptables_apply: + self.compute.driver.filter_defer_apply_on() + self.compute._destroy_evacuated_instances(fake_context) + self.compute._init_instance(fake_context, startup_instances[0]) + self.compute._init_instance(fake_context, startup_instances[1]) + self.compute._init_instance(fake_context, startup_instances[2]) + if defer_iptables_apply: + self.compute.driver.filter_defer_apply_off() + self.compute._report_driver_status(fake_context) + self.compute.publish_service_capabilities(fake_context) + self.mox.StubOutWithMock(self.compute.driver, 'init_host') + self.mox.StubOutWithMock(self.compute.driver, + 'filter_defer_apply_on') + self.mox.StubOutWithMock(self.compute.driver, + 'filter_defer_apply_off') + self.mox.StubOutWithMock(self.compute, + '_get_instances_at_startup') + self.mox.StubOutWithMock(context, 'get_admin_context') + self.mox.StubOutWithMock(self.compute, + '_destroy_evacuated_instances') + self.mox.StubOutWithMock(self.compute, + '_init_instance') + self.mox.StubOutWithMock(self.compute, + '_report_driver_status') + self.mox.StubOutWithMock(self.compute, + 'publish_service_capabilities') - self.compute.driver.init_host(host=self.compute.host) + # Test with defer_iptables_apply + self.flags(defer_iptables_apply=True) + _do_mock_calls(True) - def fake_get_admin_context(): - return c + self.mox.ReplayAll() + self.compute.init_host() + self.mox.VerifyAll() - def fake_all(*args, **kwargs): - pass + # Test without defer_iptables_apply + self.mox.ResetAll() + self.flags(defer_iptables_apply=False) + _do_mock_calls(False) - def fake_list_instance_uuids(): - return [ - # those are still related to this host - instances[0]['uuid'], - instances[1]['uuid'], - instances[2]['uuid'], - # and this one already been evacuated to other host - evacuated_instance['uuid'] - ] - - def fake_destroy(instance, nw, bdi, destroyDisks): - self.assertFalse(destroyDisks) - self.assertEqual(instance['uuid'], evacuated_instance['uuid']) - - self.stubs.Set(nova.context, - 'get_admin_context', - fake_get_admin_context) - self.stubs.Set(self.compute.driver, 'filter_defer_apply_on', fake_all) - self.stubs.Set(self.compute.driver, - 'list_instance_uuids', - fake_list_instance_uuids) - self.stubs.Set(self.compute, '_get_instance_nw_info', fake_all) - self.stubs.Set(self.compute, '_get_instance_volume_block_device_info', - fake_all) - self.stubs.Set(self.compute.driver, 'destroy', fake_destroy) - self.stubs.Set(self.compute, '_legacy_nw_info', fake_all) - self.stubs.Set(self.compute, '_init_instance', fake_all) - - self.stubs.Set(self.compute.driver, 'filter_defer_apply_off', fake_all) - self.stubs.Set(self.compute, '_report_driver_status', fake_all) - self.stubs.Set(self.compute, 'publish_service_capabilities', fake_all) - # start test self.mox.ReplayAll() self.compute.init_host() + # VerifyCall done by tearDown - db.instance_destroy(c, evacuated_instance['uuid']) - for instance in instances: - db.instance_destroy(c, instance['uuid']) + def test_get_instances_on_driver(self): + fake_context = context.get_admin_context() - def test_init_host_with_evacuated_instances_names_list(self): - # creating testdata - c = context.get_admin_context() + driver_instances = [] + for x in xrange(10): + instance = dict(uuid=uuidutils.generate_uuid()) + driver_instances.append(instance) - # instances in central db - instances = [ - # those are still related to this host - jsonutils.to_primitive(self._create_fake_instance( - {'host': self.compute.host})), - jsonutils.to_primitive(self._create_fake_instance( - {'host': self.compute.host})), - jsonutils.to_primitive(self._create_fake_instance( - {'host': self.compute.host})) - ] + self.mox.StubOutWithMock(self.compute.driver, + 'list_instance_uuids') + self.mox.StubOutWithMock(self.compute.conductor_api, + 'instance_get_by_uuid') - # those are already been evacuated to other host - evacuated_instance = self._create_fake_instance({'host': 'otherhost'}) + self.compute.driver.list_instance_uuids().AndReturn( + [inst['uuid'] for inst in driver_instances]) + for x in xrange(len(driver_instances)): + self.compute.conductor_api.instance_get_by_uuid(fake_context, + driver_instances[x]['uuid']).AndReturn( + driver_instances[x]) - # creating mocks - self.mox.StubOutWithMock(self.compute.driver, 'init_host') + self.mox.ReplayAll() - self.compute.driver.init_host(host=self.compute.host) + result = self.compute._get_instances_on_driver(fake_context) + self.assertEqual(driver_instances, result) - def fake_get_admin_context(): - return c + def test_get_instances_on_driver_fallback(self): + # Test getting instances when driver doesn't support + # 'list_instance_uuids' + fake_context = context.get_admin_context() - def fake_all(*args, **kwargs): - pass + all_instances = [] + driver_instances = [] + for x in xrange(10): + instance = dict(name=uuidutils.generate_uuid()) + if x % 2: + driver_instances.append(instance) + all_instances.append(instance) - def fake_list_instances(): - return [ - # those are still related to this host - CONF.instance_name_template % instances[0]['id'], - CONF.instance_name_template % instances[1]['id'], - CONF.instance_name_template % instances[2]['id'], - # and this one already been evacuated to other host - CONF.instance_name_template % evacuated_instance['id'] - ] - - def fake_list_instance_uuids(): - raise NotImplementedError() + self.mox.StubOutWithMock(self.compute.driver, + 'list_instance_uuids') + self.mox.StubOutWithMock(self.compute.driver, + 'list_instances') + self.mox.StubOutWithMock(self.compute.conductor_api, + 'instance_get_all') - def fake_destroy(instance, nw, bdi, destroyDisks): - self.assertFalse(destroyDisks) - self.assertEqual(instance['uuid'], evacuated_instance['uuid']) + self.compute.driver.list_instance_uuids().AndRaise( + NotImplementedError()) + self.compute.driver.list_instances().AndReturn( + [inst['name'] for inst in driver_instances]) + self.compute.conductor_api.instance_get_all( + fake_context).AndReturn(all_instances) - self.stubs.Set(nova.context, - 'get_admin_context', - fake_get_admin_context) - self.stubs.Set(self.compute.driver, 'filter_defer_apply_on', fake_all) - self.stubs.Set(self.compute.driver, - 'list_instances', - fake_list_instances) - self.stubs.Set(self.compute.driver, - 'list_instance_uuids', - fake_list_instance_uuids) - - self.stubs.Set(self.compute, '_get_instance_nw_info', fake_all) - self.stubs.Set(self.compute, '_get_instance_volume_block_device_info', - fake_all) - self.stubs.Set(self.compute.driver, 'destroy', fake_destroy) - self.stubs.Set(self.compute, '_legacy_nw_info', fake_all) - self.stubs.Set(self.compute, '_init_instance', fake_all) - - self.stubs.Set(self.compute.driver, 'filter_defer_apply_off', fake_all) - self.stubs.Set(self.compute, '_report_driver_status', fake_all) - self.stubs.Set(self.compute, 'publish_service_capabilities', fake_all) - # start test self.mox.ReplayAll() - self.compute.init_host() - db.instance_destroy(c, evacuated_instance['uuid']) - for instance in instances: - db.instance_destroy(c, instance['uuid']) + result = self.compute._get_instances_on_driver(fake_context) + self.assertEqual(driver_instances, result) class ComputeAPITestCase(BaseTestCase): diff --git a/nova/tests/compute/test_compute_utils.py b/nova/tests/compute/test_compute_utils.py index bc2413a2c1..7414201323 100644 --- a/nova/tests/compute/test_compute_utils.py +++ b/nova/tests/compute/test_compute_utils.py @@ -390,97 +390,3 @@ class MetadataToDictTestCase(test.TestCase): def test_metadata_to_dict_empty(self): self.assertEqual(compute_utils.metadata_to_dict([]), {}) - - -class ParseDecimalIDTestCase(test.TestCase): - - def setUp(self): - super(ParseDecimalIDTestCase, self).setUp() - self.context = context.RequestContext('fake', 'fake') - - self.templates = [ - CONF.instance_name_template, - 'instance-%08x', - 'instance-%08o', - 'instance-%08d', - 'instance-%04x', - 'instance-%04o', - 'instance-%04d', - 'instance-%x', - 'instance-%o', - 'instance-%d', - 'james-%07x-bond', - 'james-%07o-bond', - 'james-%07d-bond', - 'xxxx%xxxx', - 'oooo%oooo', - 'dddd%dddd', - '%02x', - '%02o', - '%02d', - '%x', - '%o', - '%d', - '%07x-bond', - '%07o-bond', - '%07d-bond', - '123%xxxx', - '123%oooo', - '123%dddd', - '007%02x', - '007%02o', - '007%02d', - '42%x', - '42%o', - '42%d', - '700%07x007', - '700%07o007', - '700%07d007'] - - self.ids = [ - 1, - 5, - 10, - 42, - 90, - 100, - 256, - 500, - 1000, - 2500, - 19294, - 100500, - 21093404 - ] - - def _validate_id(self, template, name): - return compute_utils.parse_decimal_id(template, name) - - def test_name_template_based(self): - for template in self.templates: - for id in self.ids: - self.assertEqual(id, self._validate_id(template, - template % id)) - - def test_name_not_template_based(self): - - for template in self.templates: - for id in self.ids: - name = template % id - - self.assertEqual(-1, self._validate_id(template, - 'n%s' % name)) - self.assertEqual(-1, self._validate_id(template, - '%sw' % name)) - self.assertEqual(-1, self._validate_id(template, - 'reg%s' % name)) - self.assertEqual(-1, self._validate_id(template, - '%sex' % name)) - self.assertEqual(-1, self._validate_id(template, '%s%s%s' % ( - name[:1], - 'abr', - name[-1:]))) - self.assertEqual(-1, self._validate_id(template, '%s%s%s' % ( - name[:1], - 'qwer23456ert', - name[-1:]))) |