From c5411d22f0d1da0cb15f5d7c8511a4caec53b265 Mon Sep 17 00:00:00 2001 From: Dheeraj Gupta Date: Tue, 7 Oct 2014 09:18:27 +0000 Subject: [PATCH] Extends use of ServiceProxy to more methods in HostAPI in cells Cells prepend full cell path to the service ID before returning any service related info. This means service ID is non numeric and can't be cast into Service objects. In cells, service_get_all method in HostAPI (which is used to display list of services) strips out the cell path from received IDs, creates Service objects using remaining numerical ID and uses a ServiceProxy to associate cell paths Service objects. However, other service related methods do not do so. They include: - service_update (Used for enabling/disabling services) - service_get_all_by_host (Used for evacuation) These functions try to cast received service info (with alphanumeric service IDs) into Service objects and fail with a ValueError. This leads to API cell throwing Error 500 for service-enable, service-disable and evacuate. This patch extends the ServiceProxy usage to both these methods. It also changes the corresponding HostAPI tests. Note: The required unit-tests are manually added to the below path, as new path for unit-tests is not present in stable/icehouse release. nova/tests/compute/test_host_api.py Conflicts: nova/compute/cells_api.py nova/tests/unit/compute/test_host_api.py Closes-Bug: 1361180 Change-Id: Iff2707602d5fabfbe8438150b5ad74b3c31bb011 (cherry picked from commit fcd24c6774af0add2bf20c604232e4db9747da7d) --- nova/compute/cells_api.py | 22 ++++++++++++++++------ nova/tests/compute/test_host_api.py | 16 +++++++++++----- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/nova/compute/cells_api.py b/nova/compute/cells_api.py index 812e52e13c66..6828a4fc3d29 100644 --- a/nova/compute/cells_api.py +++ b/nova/compute/cells_api.py @@ -554,10 +554,15 @@ class HostAPI(compute_api.HostAPI): # NOTE(danms): Currently cells does not support objects as # return values, so just convert the db-formatted service objects # to new-world objects here + + # NOTE(dheeraj): Use ServiceProxy here too. See johannes' + # note on service_get_all if db_service: - return service_obj.Service._from_db_object(context, - service_obj.Service(), - db_service) + cell_path, _id = cells_utils.split_cell_and_item(db_service['id']) + db_service['id'] = _id + ser_obj = service_obj.Service._from_db_object( + context, service_obj.Service(), db_service) + return ServiceProxy(ser_obj, cell_path) def service_update(self, context, host_name, binary, params_to_update): """Used to enable/disable a service. For compute services, setting to @@ -573,10 +578,15 @@ class HostAPI(compute_api.HostAPI): # NOTE(danms): Currently cells does not support objects as # return values, so just convert the db-formatted service objects # to new-world objects here + + # NOTE(dheeraj): Use ServiceProxy here too. See johannes' + # note on service_get_all if db_service: - return service_obj.Service._from_db_object(context, - service_obj.Service(), - db_service) + cell_path, _id = cells_utils.split_cell_and_item(db_service['id']) + db_service['id'] = _id + ser_obj = service_obj.Service._from_db_object( + context, service_obj.Service(), db_service) + return ServiceProxy(ser_obj, cell_path) def service_delete(self, context, service_id): """Deletes the specified service.""" diff --git a/nova/tests/compute/test_host_api.py b/nova/tests/compute/test_host_api.py index e4214cfb7f10..b6c45b84fa32 100644 --- a/nova/tests/compute/test_host_api.py +++ b/nova/tests/compute/test_host_api.py @@ -400,24 +400,30 @@ class ComputeHostAPICellsTestCase(ComputeHostAPITestCase): self.mox.StubOutWithMock(self.host_api.cells_rpcapi, 'service_get_by_compute_host') + # Cells return services with full cell_path prepended to IDs + fake_service = dict(test_service.fake_service, id='cell1@1') + exp_service = fake_service.copy() + self.host_api.cells_rpcapi.service_get_by_compute_host(self.ctxt, - 'fake-host').AndReturn(test_service.fake_service) + 'fake-host').AndReturn(fake_service) self.mox.ReplayAll() result = self.host_api.service_get_by_compute_host(self.ctxt, 'fake-host') - self._compare_obj(result, test_service.fake_service) + self._compare_obj(result, exp_service) def test_service_update(self): host_name = 'fake-host' binary = 'nova-compute' params_to_update = dict(disabled=True) - service_id = 42 - expected_result = dict(test_service.fake_service, id=service_id) + service_id = 'cell1@42' # Cells prepend full cell path to ID + + update_result = dict(test_service.fake_service, id=service_id) + expected_result = update_result.copy() self.mox.StubOutWithMock(self.host_api.cells_rpcapi, 'service_update') self.host_api.cells_rpcapi.service_update( self.ctxt, host_name, - binary, params_to_update).AndReturn(expected_result) + binary, params_to_update).AndReturn(update_result) self.mox.ReplayAll()