openstack/cinder
Code Issues Proposed changes

Refresh the Service.service_id after re-spawning children

Browse Source 
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
This commit is contained in:
Yikun Jiang 2019-01-11 16:50:24 +08:00
parent 94202a2464
commit 40127d95a9
2 changed files with 26 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
cinder/service.py
View File

@ -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.

18
cinder/tests/unit/test_service.py
View File

@ -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):