From 5fc7ee5f62903082bb86a4206ae6efce0e28a425 Mon Sep 17 00:00:00 2001 From: Shawn Hartsock Date: Thu, 20 Feb 2014 19:34:41 -0500 Subject: [PATCH] add support for host driver cleanup during shutdown Provides a place for any drivers to potentially setup graceful-shutdown code. VMware driver's proper driver-lifecycle code included. This is critical in environments where the the vmware driver is setup and torn down at high frequency. Prevents run-away vSphere by closing stateless HTTP management sessions gracefully. Change-Id: I67a91613643540243ab1210b333ed8e121f05802 related to bug: 1262288 Closes-bug: 1292583 --- nova/compute/manager.py | 3 ++ nova/manager.py | 7 ++++ nova/service.py | 6 ++++ nova/tests/compute/test_compute_mgr.py | 15 +++++++++ nova/tests/test_service.py | 23 +++++++++++++ nova/tests/virt/vmwareapi/test_driver_api.py | 35 ++++++++++++++++++++ nova/virt/driver.py | 6 ++++ nova/virt/vmwareapi/driver.py | 19 +++++++++-- 8 files changed, 111 insertions(+), 3 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 48c9d5d12a..61078e3000 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -826,6 +826,9 @@ class ComputeManager(manager.Manager): if CONF.defer_iptables_apply: self.driver.filter_defer_apply_off() + def cleanup_host(self): + self.driver.cleanup_host(host=self.host) + def pre_start_hook(self): """After the service is initialized, but before we fully bring the service up by listening on RPC queues, make sure to update diff --git a/nova/manager.py b/nova/manager.py index ed51613c46..8ff807dfee 100644 --- a/nova/manager.py +++ b/nova/manager.py @@ -89,6 +89,13 @@ class Manager(base.Base, periodic_task.PeriodicTasks): """ pass + def cleanup_host(self): + """Hook to do cleanup work when the service shuts down. + + Child classes should override this method. + """ + pass + def pre_start_hook(self): """Hook to provide the manager the ability to do additional start-up work before any RPC queues/consumers are created. This is diff --git a/nova/service.py b/nova/service.py index 0c8a765eda..443009044d 100644 --- a/nova/service.py +++ b/nova/service.py @@ -318,6 +318,12 @@ class Service(service.Service): except Exception: pass + try: + self.manager.cleanup_host() + except Exception: + LOG.exception(_('Service error occurred during cleanup_host')) + pass + super(Service, self).stop() def periodic_tasks(self, raise_on_error=False): diff --git a/nova/tests/compute/test_compute_mgr.py b/nova/tests/compute/test_compute_mgr.py index c02643a366..a9f761d115 100644 --- a/nova/tests/compute/test_compute_mgr.py +++ b/nova/tests/compute/test_compute_mgr.py @@ -207,6 +207,21 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): self.mox.VerifyAll() self.mox.UnsetStubs() + @mock.patch('nova.objects.instance.InstanceList') + def test_cleanup_host(self, mock_instance_list): + # just testing whether the cleanup_host method + # when fired will invoke the underlying driver's + # equivalent method. + + mock_instance_list.get_by_host.return_value = [] + + with mock.patch.object(self.compute, 'driver') as mock_driver: + self.compute.init_host() + mock_driver.init_host.assert_called_once() + + self.compute.cleanup_host() + mock_driver.cleanup_host.assert_called_once() + def test_init_host_with_deleted_migration(self): our_host = self.compute.host not_our_host = 'not-' + our_host diff --git a/nova/tests/test_service.py b/nova/tests/test_service.py index d8bf03124f..823198410f 100644 --- a/nova/tests/test_service.py +++ b/nova/tests/test_service.py @@ -21,6 +21,7 @@ Unit Tests for remote procedure calls using queue import sys import testtools +import mock import mox from oslo.config import cfg @@ -253,6 +254,28 @@ class ServiceTestCase(test.TestCase): serv.stop() + @mock.patch('nova.servicegroup.API') + @mock.patch('nova.conductor.api.LocalAPI.service_get_by_args') + def test_parent_graceful_shutdown_with_cleanup_host(self, + mock_svc_get_by_args, + mock_API): + mock_svc_get_by_args.return_value = {'id': 'some_value'} + mock_manager = mock.Mock() + + serv = service.Service(self.host, + self.binary, + self.topic, + 'nova.tests.test_service.FakeManager') + + serv.manager = mock_manager + serv.manager.additional_endpoints = [] + + serv.start() + serv.manager.init_host.assert_called_with() + + serv.stop() + serv.manager.cleanup_host.assert_called_with() + class TestWSGIService(test.TestCase): diff --git a/nova/tests/virt/vmwareapi/test_driver_api.py b/nova/tests/virt/vmwareapi/test_driver_api.py index 92b67e8c81..2c80fed566 100644 --- a/nova/tests/virt/vmwareapi/test_driver_api.py +++ b/nova/tests/virt/vmwareapi/test_driver_api.py @@ -1522,6 +1522,41 @@ class VMwareAPIVCDriverTestCase(VMwareAPIVMTestCase): super(VMwareAPIVCDriverTestCase, self).tearDown() vmwareapi_fake.cleanup() + def _setup_mocks_for_session(self, mock_init): + mock_init.return_value = None + + vcdriver = driver.VMwareVCDriver(None, False) + vcdriver._session = mock.Mock() + return vcdriver + + @mock.patch('nova.virt.vmwareapi.driver.VMwareVCDriver.__init__') + def test_init_host_and_cleanup_host(self, mock_init): + vcdriver = self._setup_mocks_for_session(mock_init) + vcdriver.init_host("foo") + vcdriver._session._create_session.assert_called_once() + + vcdriver.cleanup_host("foo") + vcdriver._session.vim.client.service.Logout.assert_called_once() + + @mock.patch('nova.virt.vmwareapi.driver.LOG') + @mock.patch('nova.virt.vmwareapi.driver.VMwareVCDriver.__init__') + def test_cleanup_host_with_no_login(self, mock_init, mock_logger): + vcdriver = self._setup_mocks_for_session(mock_init) + vcdriver.init_host("foo") + vcdriver._session._create_session.assert_called_once() + + # Not logged in... + # observe that no exceptions were thrown + mock_sc = mock.Mock() + vcdriver._session.vim.retrieve_service_content.return_value = mock_sc + web_fault = suds.WebFault(mock.Mock(), mock.Mock()) + vcdriver._session.vim.client.service.Logout.side_effect = web_fault + vcdriver.cleanup_host("foo") + + # assert that the mock Logout method was never called + vcdriver._session.vim.client.service.Logout.assert_called_once() + mock_logger.debug.assert_called_once() + def test_datastore_regex_configured(self): for node in self.conn._resources.keys(): self.assertEqual(self.conn._datastore_regex, diff --git a/nova/virt/driver.py b/nova/virt/driver.py index dbdea2542f..4d0719d010 100644 --- a/nova/virt/driver.py +++ b/nova/virt/driver.py @@ -139,6 +139,12 @@ class ComputeDriver(object): # TODO(Vek): Need to pass context in for access to auth_token raise NotImplementedError() + def cleanup_host(self, host): + """Clean up anything that is necessary for the driver gracefully stop, + including ending remote sessions. This is optional. + """ + pass + def get_info(self, instance): """Get the current status of an instance, by name (not ID!) diff --git a/nova/virt/vmwareapi/driver.py b/nova/virt/vmwareapi/driver.py index e2dd02d774..72b87a560d 100644 --- a/nova/virt/vmwareapi/driver.py +++ b/nova/virt/vmwareapi/driver.py @@ -24,6 +24,7 @@ import time from eventlet import event from oslo.config import cfg +import suds from nova import exception from nova.openstack.common.gettextutils import _ @@ -138,9 +139,21 @@ class VMwareESXDriver(driver.ComputeDriver): return self._host_state def init_host(self, host): - """Do the initialization that needs to be done.""" - # FIXME(sateesh): implement this - pass + vim = self._session.vim + if vim is None: + self._session._create_session() + + def cleanup_host(self, host): + # NOTE(hartsocks): we lean on the init_host to force the vim object + # to not be None. + vim = self._session.vim + service_content = vim.get_service_content() + session_manager = service_content.sessionManager + try: + vim.client.service.Logout(session_manager) + except suds.WebFault: + LOG.debug(_("No vSphere session was open during cleanup_host.")) + pass def list_instances(self): """List VM instances."""