Merge "Avoid inventory DELETE API (no conflict detection)"
This commit is contained in:
Zuul
@@ -939,66 +939,6 @@ class SchedulerReportClient(object):
|
||||
time.sleep(1)
|
||||
return False
|
||||
|
||||
@safe_connect
|
||||
def _delete_inventory(self, context, rp_uuid):
|
||||
"""Deletes all inventory records for a resource provider with the
|
||||
supplied UUID.
|
||||
"""
|
||||
if not self._provider_tree.has_inventory(rp_uuid):
|
||||
return None
|
||||
|
||||
curr = self._refresh_and_get_inventory(context, rp_uuid)
|
||||
|
||||
# Check to see if we need to update placement's view
|
||||
if not curr.get('inventories', {}):
|
||||
msg = "No inventory to delete from resource provider %s."
|
||||
LOG.debug(msg, rp_uuid)
|
||||
return
|
||||
|
||||
msg = ("Resource provider %s reported no inventory but previous "
|
||||
"inventory was detected. Deleting existing inventory records.")
|
||||
LOG.info(msg, rp_uuid)
|
||||
|
||||
cur_gen = curr['resource_provider_generation']
|
||||
url = '/resource_providers/%s/inventories' % rp_uuid
|
||||
r = self.delete(url, version="1.5",
|
||||
global_request_id=context.global_id)
|
||||
placement_req_id = get_placement_request_id(r)
|
||||
msg_args = {
|
||||
'rp_uuid': rp_uuid,
|
||||
'placement_req_id': placement_req_id,
|
||||
}
|
||||
|
||||
if r.status_code == 204:
|
||||
self._provider_tree.update_inventory(rp_uuid, {}, cur_gen + 1)
|
||||
LOG.info("[%(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._provider_tree.remove(rp_uuid)
|
||||
self.association_refresh_time.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 = ("[%(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['rc_str'] = rc_str
|
||||
LOG.warning(msg, msg_args)
|
||||
return
|
||||
|
||||
msg = ("[%(placement_req_id)s] Failed to delete inventory for "
|
||||
"resource provider %(rp_uuid)s. Got error response: %(err)s.")
|
||||
msg_args['err'] = r.text
|
||||
LOG.error(msg, msg_args)
|
||||
|
||||
def get_provider_tree_and_ensure_root(self, context, rp_uuid, name=None,
|
||||
parent_provider_uuid=None):
|
||||
"""Returns a fresh ProviderTree representing all providers which are in
|
||||
@@ -1059,10 +999,12 @@ class SchedulerReportClient(object):
|
||||
# Auto-create custom resource classes coming from a virt driver
|
||||
self._ensure_resource_classes(context, set(inv_data))
|
||||
|
||||
if inv_data:
|
||||
self._update_inventory(context, rp_uuid, inv_data)
|
||||
else:
|
||||
self._delete_inventory(context, rp_uuid)
|
||||
# NOTE(efried): Do not use the DELETE API introduced in microversion
|
||||
# 1.5, even if the new inventory is empty. It provides no way of
|
||||
# sending the generation down, so no way to trigger/detect a conflict
|
||||
# if an out-of-band update occurs between when we GET the latest and
|
||||
# when we invoke the DELETE. See bug #1746374.
|
||||
self._update_inventory(context, rp_uuid, inv_data)
|
||||
|
||||
@safe_connect
|
||||
def _ensure_traits(self, context, traits):
|
||||
@@ -1265,10 +1207,12 @@ class SchedulerReportClient(object):
|
||||
self._ensure_resource_provider(context, compute_node.uuid,
|
||||
compute_node.hypervisor_hostname)
|
||||
inv_data = _compute_node_to_inventory_dict(compute_node)
|
||||
if inv_data:
|
||||
self._update_inventory(context, compute_node.uuid, inv_data)
|
||||
else:
|
||||
self._delete_inventory(context, compute_node.uuid)
|
||||
# NOTE(efried): Do not use the DELETE API introduced in microversion
|
||||
# 1.5, even if the new inventory is empty. It provides no way of
|
||||
# sending the generation down, so no way to trigger/detect a conflict
|
||||
# if an out-of-band update occurs between when we GET the latest and
|
||||
# when we invoke the DELETE. See bug #1746374.
|
||||
self._update_inventory(context, compute_node.uuid, inv_data)
|
||||
|
||||
@safe_connect
|
||||
def get_allocations_for_consumer(self, context, consumer):
|
||||
|
||||
@@ -15,7 +15,6 @@ import time
|
||||
from keystoneauth1 import exceptions as ks_exc
|
||||
import mock
|
||||
import requests
|
||||
import six
|
||||
from six.moves.urllib import parse
|
||||
|
||||
import nova.conf
|
||||
@@ -2448,11 +2447,9 @@ class TestComputeNodeToInventoryDict(test.NoDBTestCase):
|
||||
class TestInventory(SchedulerReportClientTestCase):
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_ensure_resource_provider')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_delete_inventory')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_update_inventory')
|
||||
def test_update_compute_node(self, mock_ui, mock_delete, mock_erp):
|
||||
def test_update_compute_node(self, mock_ui, mock_erp):
|
||||
cn = self.compute_node
|
||||
self.client.update_compute_node(self.context, cn)
|
||||
mock_erp.assert_called_once_with(self.context, cn.uuid,
|
||||
@@ -2488,18 +2485,14 @@ class TestInventory(SchedulerReportClientTestCase):
|
||||
cn.uuid,
|
||||
expected_inv_data,
|
||||
)
|
||||
self.assertFalse(mock_delete.called)
|
||||
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_ensure_resource_provider')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_delete_inventory')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_update_inventory')
|
||||
def test_update_compute_node_no_inv(self, mock_ui, mock_delete,
|
||||
mock_erp):
|
||||
"""Ensure that if there are no inventory records, that we call
|
||||
_delete_inventory() instead of _update_inventory().
|
||||
def test_update_compute_node_no_inv(self, mock_ui, mock_erp):
|
||||
"""Ensure that if there are no inventory records, we still call
|
||||
_update_inventory().
|
||||
"""
|
||||
cn = self.compute_node
|
||||
cn.vcpus = 0
|
||||
@@ -2508,180 +2501,7 @@ class TestInventory(SchedulerReportClientTestCase):
|
||||
self.client.update_compute_node(self.context, cn)
|
||||
mock_erp.assert_called_once_with(self.context, cn.uuid,
|
||||
cn.hypervisor_hostname)
|
||||
mock_delete.assert_called_once_with(self.context, cn.uuid)
|
||||
self.assertFalse(mock_ui.called)
|
||||
|
||||
@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(self, mock_get, mock_delete, mock_info):
|
||||
cn = self.compute_node
|
||||
# Make sure the resource provider exists for preventing to call the API
|
||||
self._init_provider_tree()
|
||||
|
||||
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 = {'x-openstack-request-id':
|
||||
uuids.request_id}
|
||||
result = self.client._delete_inventory(self.context, cn.uuid)
|
||||
self.assertIsNone(result)
|
||||
self.assertTrue(mock_delete.called)
|
||||
self.assertEqual(uuids.request_id,
|
||||
mock_info.call_args[0][1]['placement_req_id'])
|
||||
mock_delete.assert_called_once_with(
|
||||
'/resource_providers/%s/inventories' % cn.uuid,
|
||||
version='1.5', global_request_id=self.context.global_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
|
||||
# Make sure the resource provider exists for preventing to call the API
|
||||
self._init_provider_tree()
|
||||
|
||||
mock_get.return_value.json.return_value = {
|
||||
'resource_provider_generation': 1,
|
||||
'inventories': {
|
||||
}
|
||||
}
|
||||
result = self.client._delete_inventory(self.context, cn.uuid)
|
||||
self.assertIsNone(result)
|
||||
self.assertFalse(mock_delete.called)
|
||||
self.assertFalse(mock_extract.called)
|
||||
self._validate_provider(cn.uuid, generation=1)
|
||||
|
||||
@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
|
||||
# Make sure the resource provider exists for preventing to call the API
|
||||
self._init_provider_tree()
|
||||
|
||||
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 = 409
|
||||
mock_delete.return_value.headers = {'x-openstack-request-id':
|
||||
uuids.request_id}
|
||||
rc_str = "VCPU, MEMORY_MB"
|
||||
in_use_exc = exception.InventoryInUse(
|
||||
resource_classes=rc_str,
|
||||
resource_provider=cn.uuid,
|
||||
)
|
||||
fault_text = """
|
||||
409 Conflict
|
||||
|
||||
There was a conflict when trying to complete your request.
|
||||
|
||||
update conflict: %s
|
||||
""" % six.text_type(in_use_exc)
|
||||
mock_delete.return_value.text = fault_text
|
||||
mock_delete.return_value.json.return_value = {
|
||||
'resource_provider_generation': 44,
|
||||
'inventories': {
|
||||
}
|
||||
}
|
||||
result = self.client._delete_inventory(self.context, cn.uuid)
|
||||
self.assertIsNone(result)
|
||||
self.assertTrue(mock_warn.called)
|
||||
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
|
||||
# Make sure the resource provider exists for preventing to call the API
|
||||
self._init_provider_tree()
|
||||
self.client.association_refresh_time[uuids.cn] = mock.Mock()
|
||||
|
||||
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 = {'x-openstack-request-id':
|
||||
uuids.request_id}
|
||||
result = self.client._delete_inventory(self.context, cn.uuid)
|
||||
self.assertIsNone(result)
|
||||
self.assertFalse(self.client._provider_tree.exists(cn.uuid))
|
||||
self.assertTrue(mock_debug.called)
|
||||
self.assertNotIn(cn.uuid, self.client.association_refresh_time)
|
||||
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.'
|
||||
'delete')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'get')
|
||||
def test_delete_inventory_inventory_error(self, mock_get, mock_delete,
|
||||
mock_warn, mock_error):
|
||||
cn = self.compute_node
|
||||
# Make sure the resource provider exists for preventing to call the API
|
||||
self._init_provider_tree()
|
||||
|
||||
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 = 409
|
||||
mock_delete.return_value.text = (
|
||||
'There was a failure'
|
||||
)
|
||||
mock_delete.return_value.json.return_value = {
|
||||
'resource_provider_generation': 44,
|
||||
'inventories': {
|
||||
}
|
||||
}
|
||||
mock_delete.return_value.headers = {'x-openstack-request-id':
|
||||
uuids.request_id}
|
||||
result = self.client._delete_inventory(self.context, cn.uuid)
|
||||
self.assertIsNone(result)
|
||||
self.assertFalse(mock_warn.called)
|
||||
self.assertTrue(mock_error.called)
|
||||
self.assertEqual(uuids.request_id,
|
||||
mock_error.call_args[0][1]['placement_req_id'])
|
||||
mock_ui.assert_called_once_with(self.context, cn.uuid, {})
|
||||
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'get')
|
||||
@@ -3116,14 +2936,12 @@ There was a conflict when trying to complete your request.
|
||||
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_update_inventory')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_delete_inventory')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_ensure_resource_classes')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_ensure_resource_provider')
|
||||
def test_set_inventory_for_provider_no_custom(self, mock_erp, mock_erc,
|
||||
mock_del, mock_upd):
|
||||
mock_upd):
|
||||
"""Tests that inventory records of all standard resource classes are
|
||||
passed to the report client's _update_inventory() method.
|
||||
"""
|
||||
@@ -3173,18 +2991,15 @@ There was a conflict when trying to complete your request.
|
||||
mock.sentinel.rp_uuid,
|
||||
inv_data,
|
||||
)
|
||||
self.assertFalse(mock_del.called)
|
||||
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_update_inventory')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_delete_inventory')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_ensure_resource_classes')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_ensure_resource_provider')
|
||||
def test_set_inventory_for_provider_no_inv(self, mock_erp, mock_erc,
|
||||
mock_del, mock_upd):
|
||||
mock_upd):
|
||||
"""Tests that passing empty set of inventory records triggers a delete
|
||||
of inventory for the provider.
|
||||
"""
|
||||
@@ -3202,19 +3017,17 @@ There was a conflict when trying to complete your request.
|
||||
parent_provider_uuid=None,
|
||||
)
|
||||
mock_erc.assert_called_once_with(self.context, set())
|
||||
self.assertFalse(mock_upd.called)
|
||||
mock_del.assert_called_once_with(self.context, mock.sentinel.rp_uuid)
|
||||
mock_upd.assert_called_once_with(
|
||||
self.context, mock.sentinel.rp_uuid, {})
|
||||
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_update_inventory')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_delete_inventory')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_ensure_resource_classes')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_ensure_resource_provider')
|
||||
def test_set_inventory_for_provider_with_custom(self, mock_erp,
|
||||
mock_erc, mock_del, mock_upd):
|
||||
def test_set_inventory_for_provider_with_custom(self, mock_erp, mock_erc,
|
||||
mock_upd):
|
||||
"""Tests that inventory records that include a custom resource class
|
||||
are passed to the report client's _update_inventory() method and that
|
||||
the custom resource class is auto-created.
|
||||
@@ -3274,10 +3087,7 @@ There was a conflict when trying to complete your request.
|
||||
mock.sentinel.rp_uuid,
|
||||
inv_data,
|
||||
)
|
||||
self.assertFalse(mock_del.called)
|
||||
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_delete_inventory', new=mock.Mock())
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_ensure_resource_classes', new=mock.Mock())
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
|
||||
Reference in New Issue
Block a user