diff --git a/nova/scheduler/client/report.py b/nova/scheduler/client/report.py index e62eb794d43..4dbb37d24f5 100644 --- a/nova/scheduler/client/report.py +++ b/nova/scheduler/client/report.py @@ -251,10 +251,18 @@ def put(self, url, data, version=None): url, endpoint_filter=self.ks_filter, raise_exc=False, **kwargs) - def delete(self, url): + def delete(self, url, version=None): + kwargs = {} + if version is not None: + # TODO(mriedem): Perform some version discovery at some point. + kwargs = { + 'headers': { + 'OpenStack-API-Version': 'placement %s' % version + }, + } return self._client.delete( url, - endpoint_filter=self.ks_filter, raise_exc=False) + endpoint_filter=self.ks_filter, raise_exc=False, **kwargs) # TODO(sbauza): Change that poor interface into passing a rich versioned # object that would provide the ResourceProvider requirements. @@ -612,6 +620,9 @@ def _update_inventory(self, rp_uuid, inv_data): def _delete_inventory(self, rp_uuid): """Deletes all inventory records for a resource provider with the supplied UUID. + + First attempt to DELETE the inventory using microversion 1.5. If + this results in a 406, fail over to a PUT. """ curr = self._get_inventory_and_update_provider_generation(rp_uuid) @@ -627,50 +638,70 @@ def _delete_inventory(self, rp_uuid): LOG.info(msg, rp_uuid) url = '/resource_providers/%s/inventories' % rp_uuid + r = self.delete(url, version="1.5") + placement_req_id = get_placement_request_id(r) cur_rp_gen = self._resource_providers[rp_uuid]['generation'] - payload = { - 'resource_provider_generation': cur_rp_gen, - 'inventories': {}, + msg_args = { + 'rp_uuid': rp_uuid, + 'placement_req_id': placement_req_id, } - r = self.put(url, payload) - placement_req_id = get_placement_request_id(r) - if r.status_code == 200: - # Update our view of the generation for next time - updated_inv = r.json() - new_gen = updated_inv['resource_provider_generation'] - - self._resource_providers[rp_uuid]['generation'] = new_gen - msg_args = { - 'rp_uuid': rp_uuid, - 'generation': new_gen, - 'placement_req_id': placement_req_id, + if r.status_code == 406: + # microversion 1.5 not available so try the earlier way + # TODO(cdent): When we're happy that all placement + # servers support microversion 1.5 we can remove this + # call and the associated code. + LOG.debug('Falling back to placement API microversion 1.0 ' + 'for deleting all inventory for a resource provider.') + payload = { + 'resource_provider_generation': cur_rp_gen, + 'inventories': {}, } - LOG.info(_LI('[%(placement_req_id)s] Deleted all inventory for ' - 'resource provider %(rp_uuid)s at generation ' - '%(generation)i'), + r = self.put(url, payload) + placement_req_id = get_placement_request_id(r) + msg_args['placement_req_id'] = placement_req_id + if r.status_code == 200: + # Update our view of the generation for next time + updated_inv = r.json() + new_gen = updated_inv['resource_provider_generation'] + + self._resource_providers[rp_uuid]['generation'] = new_gen + msg_args['generation'] = new_gen + LOG.info(_LI("[%(placement_req_id)s] Deleted all inventory " + "for resource provider %(rp_uuid)s at generation " + "%(generation)i."), + msg_args) + return + + if r.status_code == 204: + self._resource_providers[rp_uuid]['generation'] = cur_rp_gen + 1 + LOG.info(_LI("[%(placement_req_id)s] Deleted all inventory for " + "resource provider %(rp_uuid)s."), msg_args) return + elif r.status_code == 404: + # This can occur if another thread deleted the inventory and the + # resource provider already + LOG.debug("[%(placement_req_id)s] Resource provider %(rp_uuid)s " + "deleted by another thread when trying to delete " + "inventory. Ignoring.", + msg_args) + self._resource_providers.pop(rp_uuid, None) + self._provider_aggregate_map.pop(rp_uuid, None) + return elif r.status_code == 409: rc_str = _extract_inventory_in_use(r.text) if rc_str is not None: msg = _LW("[%(placement_req_id)s] We cannot delete inventory " "%(rc_str)s for resource provider %(rp_uuid)s " "because the inventory is in use.") - msg_args = { - 'rp_uuid': rp_uuid, - 'rc_str': rc_str, - 'placement_req_id': placement_req_id, - } + msg_args['rc_str'] = rc_str LOG.warning(msg, msg_args) return msg = _LE("[%(placement_req_id)s] Failed to delete inventory for " - "resource provider %(rp_uuid)s. Got error response: %(err)s") - msg_args = { - 'rp_uuid': rp_uuid, - 'err': r.text, - 'placement_req_id': placement_req_id, - } + "resource provider %(rp_uuid)s. Got error response: " + "%(err)s.") + msg_args['err'] = r.text LOG.error(msg, msg_args) def set_inventory_for_provider(self, rp_uuid, rp_name, inv_data): diff --git a/nova/tests/unit/scheduler/client/test_report.py b/nova/tests/unit/scheduler/client/test_report.py index 50d684f529a..c588a38837d 100644 --- a/nova/tests/unit/scheduler/client/test_report.py +++ b/nova/tests/unit/scheduler/client/test_report.py @@ -758,13 +758,16 @@ def test_update_compute_node_no_inv(self, mock_ui, mock_delete, mock_delete.assert_called_once_with(cn.uuid) self.assertFalse(mock_ui.called) - @mock.patch('nova.scheduler.client.report._extract_inventory_in_use') + @mock.patch.object(report.LOG, 'info') @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' 'put') + @mock.patch.object(report.LOG, 'info') + @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' + 'delete') @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' 'get') - def test_delete_inventory_already_no_inventory(self, mock_get, mock_put, - mock_extract): + def test_delete_inventory(self, mock_get, mock_delete, mock_debug, + mock_put, mock_info): cn = self.compute_node rp = dict(uuid=cn.uuid, generation=42) # Make sure the resource provider exists for preventing to call the API @@ -773,23 +776,52 @@ def test_delete_inventory_already_no_inventory(self, mock_get, mock_put, mock_get.return_value.json.return_value = { 'resource_provider_generation': 1, 'inventories': { + 'VCPU': {'total': 16}, } } + mock_delete.return_value.status_code = 204 + mock_delete.return_value.headers = {'openstack-request-id': + uuids.request_id} result = self.client._delete_inventory(cn.uuid) self.assertIsNone(result) self.assertFalse(mock_put.called) + self.assertEqual(uuids.request_id, + mock_info.call_args[0][1]['placement_req_id']) + + @mock.patch('nova.scheduler.client.report._extract_inventory_in_use') + @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' + 'delete') + @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' + 'get') + def test_delete_inventory_already_no_inventory(self, mock_get, mock_delete, + mock_extract): + cn = self.compute_node + rp = dict(uuid=cn.uuid, generation=42) + # Make sure the resource provider exists for preventing to call the API + self.client._resource_providers[cn.uuid] = rp + + mock_get.return_value.json.return_value = { + 'resource_provider_generation': 1, + 'inventories': { + } + } + result = self.client._delete_inventory(cn.uuid) + self.assertIsNone(result) + self.assertFalse(mock_delete.called) self.assertFalse(mock_extract.called) new_gen = self.client._resource_providers[cn.uuid]['generation'] self.assertEqual(1, new_gen) @mock.patch.object(report.LOG, 'info') - @mock.patch('nova.scheduler.client.report._extract_inventory_in_use') @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' 'put') + @mock.patch.object(report.LOG, 'debug') + @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' + 'delete') @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' 'get') - def test_delete_inventory(self, mock_get, mock_put, mock_extract, - mock_info): + def test_delete_inventory_put(self, mock_get, mock_delete, mock_debug, + mock_put, mock_info): cn = self.compute_node rp = dict(uuid=cn.uuid, generation=42) # Make sure the resource provider exists for preventing to call the API @@ -798,11 +830,10 @@ def test_delete_inventory(self, mock_get, mock_put, mock_extract, mock_get.return_value.json.return_value = { 'resource_provider_generation': 1, 'inventories': { - 'VCPU': {'total': 16}, - 'MEMORY_MB': {'total': 1024}, 'DISK_GB': {'total': 10}, } } + mock_delete.return_value.status_code = 406 mock_put.return_value.status_code = 200 mock_put.return_value.json.return_value = { 'resource_provider_generation': 44, @@ -813,19 +844,88 @@ def test_delete_inventory(self, mock_get, mock_put, mock_extract, uuids.request_id} result = self.client._delete_inventory(cn.uuid) self.assertIsNone(result) - self.assertFalse(mock_extract.called) + self.assertTrue(mock_debug.called) + self.assertTrue(mock_put.called) new_gen = self.client._resource_providers[cn.uuid]['generation'] self.assertEqual(44, new_gen) self.assertTrue(mock_info.called) self.assertEqual(uuids.request_id, mock_info.call_args[0][1]['placement_req_id']) - @mock.patch.object(report.LOG, 'warning') @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' 'put') + @mock.patch.object(report.LOG, 'debug') + @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' + 'delete') @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' 'get') - def test_delete_inventory_inventory_in_use(self, mock_get, mock_put, + def test_delete_inventory_put_failover(self, mock_get, mock_delete, + mock_debug, mock_put): + cn = self.compute_node + rp = dict(uuid=cn.uuid, generation=42) + # Make sure the resource provider exists for preventing to call the API + self.client._resource_providers[cn.uuid] = rp + + mock_get.return_value.json.return_value = { + 'resource_provider_generation': 42, + 'inventories': { + 'DISK_GB': {'total': 10}, + } + } + mock_delete.return_value.status_code = 406 + mock_put.return_value.status_code = 200 + self.client._delete_inventory(cn.uuid) + self.assertTrue(mock_debug.called) + exp_url = '/resource_providers/%s/inventories' % rp['uuid'] + payload = { + 'resource_provider_generation': 42, + 'inventories': {}, + } + mock_put.assert_called_once_with(exp_url, payload) + + @mock.patch.object(report.LOG, 'error') + @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' + 'put') + @mock.patch.object(report.LOG, 'debug') + @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' + 'delete') + def test_delete_inventory_put_failover_in_use(self, mock_delete, + mock_debug, mock_put, + mock_error): + cn = self.compute_node + rp = dict(uuid=cn.uuid, generation=42) + # Make sure the resource provider exists for preventing to call the API + self.client._resource_providers[cn.uuid] = rp + mock_delete.return_value.status_code = 406 + mock_put.return_value.status_code = 409 + mock_put.return_value.text = ( + 'There was a *fake* failure: inventory in use' + ) + mock_put.return_value.json.return_value = { + 'resource_provider_generation': 44, + 'inventories': { + } + } + mock_put.return_value.headers = {'openstack-request-id': + uuids.request_id} + self.client._delete_inventory(cn.uuid) + self.assertTrue(mock_debug.called) + exp_url = '/resource_providers/%s/inventories' % cn.uuid + payload = { + 'resource_provider_generation': rp['generation'], + 'inventories': {}, + } + mock_put.assert_called_once_with(exp_url, payload) + self.assertTrue(mock_error.called) + self.assertEqual(uuids.request_id, + mock_error.call_args[0][1]['placement_req_id']) + + @mock.patch.object(report.LOG, 'warning') + @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' + 'delete') + @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' + 'get') + def test_delete_inventory_inventory_in_use(self, mock_get, mock_delete, mock_warn): cn = self.compute_node rp = dict(uuid=cn.uuid, generation=42) @@ -840,8 +940,8 @@ def test_delete_inventory_inventory_in_use(self, mock_get, mock_put, 'DISK_GB': {'total': 10}, } } - mock_put.return_value.status_code = 409 - mock_put.return_value.headers = {'openstack-request-id': + mock_delete.return_value.status_code = 409 + mock_delete.return_value.headers = {'openstack-request-id': uuids.request_id} rc_str = "VCPU, MEMORY_MB" in_use_exc = exception.InventoryInUse( @@ -855,8 +955,8 @@ def test_delete_inventory_inventory_in_use(self, mock_get, mock_put, update conflict: %s """ % six.text_type(in_use_exc) - mock_put.return_value.text = fault_text - mock_put.return_value.json.return_value = { + mock_delete.return_value.text = fault_text + mock_delete.return_value.json.return_value = { 'resource_provider_generation': 44, 'inventories': { } @@ -867,13 +967,50 @@ def test_delete_inventory_inventory_in_use(self, mock_get, mock_put, self.assertEqual(uuids.request_id, mock_warn.call_args[0][1]['placement_req_id']) + @mock.patch.object(report.LOG, 'debug') + @mock.patch.object(report.LOG, 'info') + @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' + 'delete') + @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' + 'get') + def test_delete_inventory_inventory_404(self, mock_get, mock_delete, + mock_info, mock_debug): + """Test that when we attempt to delete all the inventory for a resource + provider but another thread has already deleted that resource provider, + that we simply remove the resource provider from our local cache and + return. + """ + cn = self.compute_node + rp = dict(uuid=cn.uuid, generation=42) + # Make sure the resource provider exists for preventing to call the API + self.client._resource_providers[cn.uuid] = rp + + mock_get.return_value.json.return_value = { + 'resource_provider_generation': 1, + 'inventories': { + 'VCPU': {'total': 16}, + 'MEMORY_MB': {'total': 1024}, + 'DISK_GB': {'total': 10}, + } + } + mock_delete.return_value.status_code = 404 + mock_delete.return_value.headers = {'openstack-request-id': + uuids.request_id} + result = self.client._delete_inventory(cn.uuid) + self.assertIsNone(result) + self.assertNotIn(cn.uuid, self.client._resource_providers) + self.assertTrue(mock_debug.called) + self.assertIn('deleted by another thread', mock_debug.call_args[0][0]) + self.assertEqual(uuids.request_id, + mock_debug.call_args[0][1]['placement_req_id']) + @mock.patch.object(report.LOG, 'error') @mock.patch.object(report.LOG, 'warning') @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' - 'put') + 'delete') @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' 'get') - def test_delete_inventory_inventory_error(self, mock_get, mock_put, + def test_delete_inventory_inventory_error(self, mock_get, mock_delete, mock_warn, mock_error): cn = self.compute_node rp = dict(uuid=cn.uuid, generation=42) @@ -888,16 +1025,16 @@ def test_delete_inventory_inventory_error(self, mock_get, mock_put, 'DISK_GB': {'total': 10}, } } - mock_put.return_value.status_code = 409 - mock_put.return_value.text = ( + mock_delete.return_value.status_code = 409 + mock_delete.return_value.text = ( 'There was a failure' ) - mock_put.return_value.json.return_value = { + mock_delete.return_value.json.return_value = { 'resource_provider_generation': 44, 'inventories': { } } - mock_put.return_value.headers = {'openstack-request-id': + mock_delete.return_value.headers = {'openstack-request-id': uuids.request_id} result = self.client._delete_inventory(cn.uuid) self.assertIsNone(result)