From fcd24c6774af0add2bf20c604232e4db9747da7d 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. Change-Id: Iff2707602d5fabfbe8438150b5ad74b3c31bb011 Closes-Bug: 1361180 --- nova/compute/cells_api.py | 24 ++++++++++++++++++------ nova/tests/unit/compute/test_host_api.py | 16 +++++++++++----- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/nova/compute/cells_api.py b/nova/compute/cells_api.py index c7dab6d3afff..13d81b9bbe0c 100644 --- a/nova/compute/cells_api.py +++ b/nova/compute/cells_api.py @@ -552,10 +552,16 @@ 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 objects.Service._from_db_object(context, - objects.Service(), - db_service) + cell_path, _id = cells_utils.split_cell_and_item(db_service['id']) + db_service['id'] = _id + ser_obj = objects.Service._from_db_object(context, + objects.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 @@ -571,10 +577,16 @@ 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 objects.Service._from_db_object(context, - objects.Service(), - db_service) + cell_path, _id = cells_utils.split_cell_and_item(db_service['id']) + db_service['id'] = _id + ser_obj = objects.Service._from_db_object(context, + objects.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/unit/compute/test_host_api.py b/nova/tests/unit/compute/test_host_api.py index 348d2dea3d51..470fe09fbd41 100644 --- a/nova/tests/unit/compute/test_host_api.py +++ b/nova/tests/unit/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()