From a7990e0263f2113e3814209118ecb2afc140826e Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Wed, 16 Mar 2011 15:01:10 -0700 Subject: [PATCH 01/11] Use _ trick to hide base test class, thereby avoiding mixins and helping PyLint --- nova/tests/test_auth.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/nova/tests/test_auth.py b/nova/tests/test_auth.py index 2a7817032a67..0d24e5db267a 100644 --- a/nova/tests/test_auth.py +++ b/nova/tests/test_auth.py @@ -80,10 +80,10 @@ class user_and_project_generator(object): self.manager.delete_project(self.project) -class AuthManagerTestCase(object): +class _AuthManagerBaseTestCase(test.TestCase): def setUp(self): FLAGS.auth_driver = self.auth_driver - super(AuthManagerTestCase, self).setUp() + super(_AuthManagerBaseTestCase, self).setUp() self.flags(connection_type='fake') self.manager = manager.AuthManager(new=True) @@ -324,11 +324,11 @@ class AuthManagerTestCase(object): self.assertTrue(user.is_admin()) -class AuthManagerLdapTestCase(AuthManagerTestCase, test.TestCase): +class AuthManagerLdapTestCase(_AuthManagerBaseTestCase): auth_driver = 'nova.auth.ldapdriver.FakeLdapDriver' -class AuthManagerDbTestCase(AuthManagerTestCase, test.TestCase): +class AuthManagerDbTestCase(_AuthManagerBaseTestCase): auth_driver = 'nova.auth.dbdriver.DbDriver' From 65b11b3b9c76db2440d480bbc41b72f24bee8afc Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Wed, 16 Mar 2011 15:11:39 -0700 Subject: [PATCH 02/11] Avoid mixins on image tests, keeping pylint much happier --- nova/tests/api/openstack/test_images.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/nova/tests/api/openstack/test_images.py b/nova/tests/api/openstack/test_images.py index 76f7589294e7..9a33236af110 100644 --- a/nova/tests/api/openstack/test_images.py +++ b/nova/tests/api/openstack/test_images.py @@ -41,10 +41,15 @@ from nova.tests.api.openstack import fakes FLAGS = flags.FLAGS -class BaseImageServiceTests(object): +class _BaseImageServiceTests(test.TestCase): """Tasks to test for all image services""" + def __init__(self): + super(_BaseImageServiceTests, self).__init__() + self.service = None + self.context = None + def test_create(self): fixture = {'name': 'test image', @@ -132,8 +137,7 @@ class BaseImageServiceTests(object): self.assertEquals(1, num_images) -class LocalImageServiceTest(test.TestCase, - BaseImageServiceTests): +class LocalImageServiceTest(_BaseImageServiceTests): """Tests the local image service""" @@ -152,8 +156,7 @@ class LocalImageServiceTest(test.TestCase, super(LocalImageServiceTest, self).tearDown() -class GlanceImageServiceTest(test.TestCase, - BaseImageServiceTests): +class GlanceImageServiceTest(_BaseImageServiceTests): """Tests the local image service""" From aecd4eb9d363875cd84be5aa6fdb9afeb500b4f4 Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Wed, 16 Mar 2011 15:44:45 -0700 Subject: [PATCH 03/11] Fix __init__ method on unit tests (they take a method_name kwarg) --- nova/tests/api/openstack/test_images.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nova/tests/api/openstack/test_images.py b/nova/tests/api/openstack/test_images.py index 9a33236af110..8814b9b1a160 100644 --- a/nova/tests/api/openstack/test_images.py +++ b/nova/tests/api/openstack/test_images.py @@ -45,8 +45,8 @@ class _BaseImageServiceTests(test.TestCase): """Tasks to test for all image services""" - def __init__(self): - super(_BaseImageServiceTests, self).__init__() + def __init__(self, *args, **kwargs): + super(_BaseImageServiceTests, self).__init__(*args, **kwargs) self.service = None self.context = None From b6824009d9767f951373fb1b92c7cb2de83b0d97 Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Mon, 28 Mar 2011 00:30:13 -0700 Subject: [PATCH 04/11] There were two periodic_tasks functions, due to parallel merges in compute.manager. --- nova/compute/manager.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 468771f46e19..eb8bddd0531d 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -141,12 +141,6 @@ class ComputeManager(manager.SchedulerDependentManager): """ self.driver.init_host(host=self.host) - def periodic_tasks(self, context=None): - """Tasks to be run at a periodic interval.""" - super(ComputeManager, self).periodic_tasks(context) - if FLAGS.rescue_timeout > 0: - self.driver.poll_rescued_instances(FLAGS.rescue_timeout) - def _update_state(self, context, instance_id): """Update the state of an instance from the driver info.""" # FIXME(ja): include other fields from state? @@ -1028,16 +1022,27 @@ class ComputeManager(manager.SchedulerDependentManager): def periodic_tasks(self, context=None): """Tasks to be run at a periodic interval.""" + + error_list = super(ComputeManager, self).periodic_tasks(context) if error_list is None: error_list = [] + try: + if FLAGS.rescue_timeout > 0: + self.driver.poll_rescued_instances(FLAGS.rescue_timeout) + except Exception as ex: + LOG.warning(_("Error during poll_rescued_instances: %s"), + unicode(ex)) + error_list.append(ex) + try: self._poll_instance_states(context) except Exception as ex: LOG.warning(_("Error during instance poll: %s"), unicode(ex)) error_list.append(ex) + return error_list def _poll_instance_states(self, context): From 684865d0b95a14f23e9187a6c3a404b5e8ed61ef Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Mon, 28 Mar 2011 00:31:28 -0700 Subject: [PATCH 05/11] Added poll_rescued_instances to virt driver base class --- nova/virt/driver.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/nova/virt/driver.py b/nova/virt/driver.py index f9cf1b8aa5a8..fcd31861d4fa 100644 --- a/nova/virt/driver.py +++ b/nova/virt/driver.py @@ -232,3 +232,7 @@ class ComputeDriver(object): def inject_network_info(self, instance): """inject network info for specified instance""" raise NotImplementedError() + + def poll_rescued_instances(self, timeout): + """Poll for rescued instances""" + raise NotImplementedError() From b2f7f3e05d18168b3310184aa9a3a6f11c57c154 Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Mon, 28 Mar 2011 00:32:48 -0700 Subject: [PATCH 06/11] pep8 fixes --- nova/compute/manager.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index eb8bddd0531d..f43397e365c8 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -1022,8 +1022,6 @@ class ComputeManager(manager.SchedulerDependentManager): def periodic_tasks(self, context=None): """Tasks to be run at a periodic interval.""" - - error_list = super(ComputeManager, self).periodic_tasks(context) if error_list is None: error_list = [] @@ -1042,7 +1040,7 @@ class ComputeManager(manager.SchedulerDependentManager): LOG.warning(_("Error during instance poll: %s"), unicode(ex)) error_list.append(ex) - + return error_list def _poll_instance_states(self, context): From c8969dbdd429c0b4c4f1211bd90311cabec8dd0d Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Mon, 28 Mar 2011 00:41:55 -0700 Subject: [PATCH 07/11] Assume that if we don't find a VM for an instance in the DB, and the DB state is NOSTATE, that the db instance is in the process of being spawned. Fix for bug744056 --- nova/compute/manager.py | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 468771f46e19..f396baa44f60 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -1051,16 +1051,31 @@ class ComputeManager(manager.SchedulerDependentManager): for db_instance in db_instances: name = db_instance['name'] + db_state = db_instance['state'] vm_instance = vm_instances.get(name) + if vm_instance is None: - LOG.info(_("Found instance '%(name)s' in DB but no VM. " - "Setting state to shutoff.") % locals()) - vm_state = power_state.SHUTOFF + #NOTE(justinsb): We have to be very careful here, because a + #concurrent operation could be in progress (e.g. a spawn) + if db_state == power_state.NOSTATE: + #Assume that NOSTATE => spawning + #TODO(justinsb): This does mean that if we crash during a + #spawn, the machine will never leave the spawning state. + #We could have a separate task to correct this error. + #TODO(justinsb): What happens during a live migration? + LOG.info(_("Found instance '%(name)s' in DB but no VM. " + "State=%(db_state), so assuming spawn is in " + "progress.") % locals()) + vm_state = db_state + else: + LOG.info(_("Found instance '%(name)s' in DB but no VM. " + "State=%(db_state), so setting state to " + "shutoff.") % locals()) + vm_state = power_state.SHUTOFF else: vm_state = vm_instance.state vms_not_found_in_db.remove(name) - db_state = db_instance['state'] if vm_state != db_state: LOG.info(_("DB/VM state mismatch. Changing state from " "'%(db_state)s' to '%(vm_state)s'") % locals()) From b648f3499626874327d8f1b087a578afe903d010 Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Mon, 28 Mar 2011 00:44:13 -0700 Subject: [PATCH 08/11] pep8 fixes --- nova/compute/manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index f396baa44f60..2b8494787aa6 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -1053,7 +1053,7 @@ class ComputeManager(manager.SchedulerDependentManager): name = db_instance['name'] db_state = db_instance['state'] vm_instance = vm_instances.get(name) - + if vm_instance is None: #NOTE(justinsb): We have to be very careful here, because a #concurrent operation could be in progress (e.g. a spawn) From 7ed45fe61416213a4fbfba7e45a765e43b933e16 Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Mon, 28 Mar 2011 01:05:20 -0700 Subject: [PATCH 09/11] Fixed some format strings --- nova/compute/manager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 2b8494787aa6..0e42a4bd28e3 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -1064,12 +1064,12 @@ class ComputeManager(manager.SchedulerDependentManager): #We could have a separate task to correct this error. #TODO(justinsb): What happens during a live migration? LOG.info(_("Found instance '%(name)s' in DB but no VM. " - "State=%(db_state), so assuming spawn is in " + "State=%(db_state)s, so assuming spawn is in " "progress.") % locals()) vm_state = db_state else: LOG.info(_("Found instance '%(name)s' in DB but no VM. " - "State=%(db_state), so setting state to " + "State=%(db_state)s, so setting state to " "shutoff.") % locals()) vm_state = power_state.SHUTOFF else: From 408de4bd5fe436e1829f4b916f0f20042e48eacc Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Mon, 28 Mar 2011 01:08:50 -0700 Subject: [PATCH 10/11] Clarified note about scope of the _poll_instance_states function --- nova/compute/manager.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 0e42a4bd28e3..93eca61fb928 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -1060,7 +1060,9 @@ class ComputeManager(manager.SchedulerDependentManager): if db_state == power_state.NOSTATE: #Assume that NOSTATE => spawning #TODO(justinsb): This does mean that if we crash during a - #spawn, the machine will never leave the spawning state. + #spawn, the machine will never leave the spawning state, + #but this is just the way nova is; this function isn't + #trying to correct that problem. #We could have a separate task to correct this error. #TODO(justinsb): What happens during a live migration? LOG.info(_("Found instance '%(name)s' in DB but no VM. " From 5a80f8b3d5ae3be774b0b3e1dbc89c9830273eaa Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Mon, 28 Mar 2011 10:00:33 -0700 Subject: [PATCH 11/11] Fix formatting of TODO and NOTE - should be a space after the # --- nova/compute/manager.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 93eca61fb928..eb42f054dee2 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -1055,16 +1055,16 @@ class ComputeManager(manager.SchedulerDependentManager): vm_instance = vm_instances.get(name) if vm_instance is None: - #NOTE(justinsb): We have to be very careful here, because a - #concurrent operation could be in progress (e.g. a spawn) + # NOTE(justinsb): We have to be very careful here, because a + # concurrent operation could be in progress (e.g. a spawn) if db_state == power_state.NOSTATE: - #Assume that NOSTATE => spawning - #TODO(justinsb): This does mean that if we crash during a - #spawn, the machine will never leave the spawning state, - #but this is just the way nova is; this function isn't - #trying to correct that problem. - #We could have a separate task to correct this error. - #TODO(justinsb): What happens during a live migration? + # Assume that NOSTATE => spawning + # TODO(justinsb): This does mean that if we crash during a + # spawn, the machine will never leave the spawning state, + # but this is just the way nova is; this function isn't + # trying to correct that problem. + # We could have a separate task to correct this error. + # TODO(justinsb): What happens during a live migration? LOG.info(_("Found instance '%(name)s' in DB but no VM. " "State=%(db_state)s, so assuming spawn is in " "progress.") % locals())