From c0c272b3b874e00fce7e41f03779edf9b8fd0c15 Mon Sep 17 00:00:00 2001 From: Yikun Jiang Date: Fri, 11 Jan 2019 16:50:24 +0800 Subject: [PATCH] Refresh the Service.service_id after re-spawning children Currently, the oslo.service provider a mechanism that the main process will re-spawn the children as necessary. But if we kill the children service, the services (except the latest service) are always down after re-spawn. This reason of this problem is that the Service.service_id is inherited from the parent process [1] (the parent Service service id class attribute) and has been recorded as the last created service [2][3] when latest process is stared. But when re-spawning child process, only start() method would be called[1], so that the Service.service_id is not refreshed as expected. In order to refresh the Service class attribute service_id, we should store the service_id in instance attr origin_service_id, and set the class attr back using the instance attribute in start method. [1] https://github.com/openstack/oslo.service/blob/d987a4a/oslo_service/service.py#L648 [2] https://github.com/openstack/cinder/blob/099b141/cinder/service.py#L193 [3] https://github.com/openstack/cinder/blob/099b141/cinder/service.py#L344 Change-Id: Ibefda81215c5081634876a2064b15638388ae921 Closes-bug: #1811344 (cherry picked from commit 40127d95a9e83e97970dc388c18c99eea7715edc) --- cinder/service.py | 8 ++++++++ cinder/tests/unit/test_service.py | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/cinder/service.py b/cinder/service.py index 1d2144423fb..439d768e0b4 100644 --- a/cinder/service.py +++ b/cinder/service.py @@ -191,6 +191,7 @@ class Service(service.Service): service_ref.cluster_name = cluster service_ref.save() Service.service_id = service_ref.id + self.origin_service_id = service_ref.id except exception.NotFound: self._create_service_ref(ctxt, manager_class.RPC_API_VERSION) # Service entry Entry didn't exist because it was manually removed @@ -218,6 +219,12 @@ class Service(service.Service): if self.coordination: coordination.COORDINATOR.start() + # NOTE(yikun): When re-spawning child process, we should set the class + # attribute back using the origin service_id, otherwise, + # the Service.service_id will be inherited from the parent process, + # and will be recorded as the last started service id by mistaken. + Service.service_id = self.origin_service_id + self.manager.init_host(added_to_cluster=self.added_to_cluster, service_id=Service.service_id) @@ -342,6 +349,7 @@ class Service(service.Service): service_ref = objects.Service(context=context, **kwargs) service_ref.create() Service.service_id = service_ref.id + self.origin_service_id = service_ref.id self._ensure_cluster_exists(context, service_ref) # If we have updated the service_ref with replication data from # the cluster it will be saved. diff --git a/cinder/tests/unit/test_service.py b/cinder/tests/unit/test_service.py index c964718b054..9a69c4b95ec 100644 --- a/cinder/tests/unit/test_service.py +++ b/cinder/tests/unit/test_service.py @@ -99,6 +99,24 @@ class ServiceManagerTestCase(test.TestCase): self.assertEqual({}, rpc.LAST_OBJ_VERSIONS) self.assertEqual({}, rpc.LAST_RPC_VERSIONS) + def test_start_refresh_serivce_id(self): + serv = service.Service('test', + 'test', + 'test', + 'cinder.tests.unit.test_service.FakeManager') + # records the original service id + serv_id = serv.service_id + self.assertEqual(serv.origin_service_id, service.Service.service_id) + # update service id to other value + service.Service.service_id = serv_id + 1 + # make sure the class attr service_id have been changed + self.assertNotEqual(serv.origin_service_id, + service.Service.service_id) + # call start method + serv.start() + # After start, the service id is refreshed to original service_id + self.assertEqual(serv_id, service.Service.service_id) + class ServiceFlagsTestCase(test.TestCase): def test_service_enabled_on_create_based_on_flag(self):