placement: Do not save 0-valued inventory
Ironic nodes that are not available or operable have 0 values for vcpus, memory_mb, and local_gb in the returned dict from the Ironic virt driver's get_available_resource() call. Don't try to save these 0 values in the placement API inventory records, since the placement REST API will return an error. Instead, attempt to delete any inventory records for that Ironic node resource provider by PUT'ing an empty set of inventory records to the placement API. Closes-bug: #1651678 Change-Id: I10b22606f704abcb970939fb2cd77f026d4d6322
This commit is contained in:
parent
d442488ebe
commit
3c217acb9c
|
@ -14,6 +14,7 @@
|
|||
# under the License.
|
||||
|
||||
import functools
|
||||
import re
|
||||
import time
|
||||
|
||||
from keystoneauth1 import exceptions as ks_exc
|
||||
|
@ -32,6 +33,8 @@ LOG = logging.getLogger(__name__)
|
|||
VCPU = fields.ResourceClass.VCPU
|
||||
MEMORY_MB = fields.ResourceClass.MEMORY_MB
|
||||
DISK_GB = fields.ResourceClass.DISK_GB
|
||||
_RE_INV_IN_USE = re.compile("Inventory for (.+) on resource provider "
|
||||
"(.+) in use")
|
||||
|
||||
|
||||
def safe_connect(f):
|
||||
|
@ -65,32 +68,38 @@ def _compute_node_to_inventory_dict(compute_node):
|
|||
|
||||
:param compute_node: `objects.ComputeNode` object to translate
|
||||
"""
|
||||
return {
|
||||
VCPU: {
|
||||
result = {}
|
||||
|
||||
# NOTE(jaypipes): Ironic virt driver will return 0 values for vcpus,
|
||||
# memory_mb and disk_gb if the Ironic node is not available/operable
|
||||
if compute_node.vcpus > 0:
|
||||
result[VCPU] = {
|
||||
'total': compute_node.vcpus,
|
||||
'reserved': 0,
|
||||
'min_unit': 1,
|
||||
'max_unit': compute_node.vcpus,
|
||||
'step_size': 1,
|
||||
'allocation_ratio': compute_node.cpu_allocation_ratio,
|
||||
},
|
||||
MEMORY_MB: {
|
||||
}
|
||||
if compute_node.memory_mb > 0:
|
||||
result[MEMORY_MB] = {
|
||||
'total': compute_node.memory_mb,
|
||||
'reserved': CONF.reserved_host_memory_mb,
|
||||
'min_unit': 1,
|
||||
'max_unit': compute_node.memory_mb,
|
||||
'step_size': 1,
|
||||
'allocation_ratio': compute_node.ram_allocation_ratio,
|
||||
},
|
||||
DISK_GB: {
|
||||
}
|
||||
if compute_node.local_gb > 0:
|
||||
result[DISK_GB] = {
|
||||
'total': compute_node.local_gb,
|
||||
'reserved': CONF.reserved_host_disk_mb * 1024,
|
||||
'min_unit': 1,
|
||||
'max_unit': compute_node.local_gb,
|
||||
'step_size': 1,
|
||||
'allocation_ratio': compute_node.disk_allocation_ratio,
|
||||
},
|
||||
}
|
||||
}
|
||||
return result
|
||||
|
||||
|
||||
def _instance_to_allocations_dict(instance):
|
||||
|
@ -112,6 +121,19 @@ def _instance_to_allocations_dict(instance):
|
|||
}
|
||||
|
||||
|
||||
def _extract_inventory_in_use(body):
|
||||
"""Given an HTTP response body, extract the resource classes that were
|
||||
still in use when we tried to delete inventory.
|
||||
|
||||
:returns: String of resource classes or None if there was no InventoryInUse
|
||||
error in the response body.
|
||||
"""
|
||||
match = _RE_INV_IN_USE.search(body)
|
||||
if match:
|
||||
return match.group(1)
|
||||
return None
|
||||
|
||||
|
||||
class SchedulerReportClient(object):
|
||||
"""Client class for updating the scheduler."""
|
||||
|
||||
|
@ -271,13 +293,11 @@ class SchedulerReportClient(object):
|
|||
return {'inventories': {}}
|
||||
return result.json()
|
||||
|
||||
def _update_inventory_attempt(self, rp_uuid, inv_data):
|
||||
"""Update the inventory for this resource provider if needed.
|
||||
|
||||
:param rp_uuid: The resource provider UUID for the operation
|
||||
:param inv_data: The new inventory for the resource provider
|
||||
:returns: True if the inventory was updated (or did not need to be),
|
||||
False otherwise.
|
||||
def _get_inventory_and_update_provider_generation(self, rp_uuid):
|
||||
"""Helper method that retrieves the current inventory for the supplied
|
||||
resource provider according to the placement API. If the cached
|
||||
generation of the resource provider is not the same as the generation
|
||||
returned from the placement API, we update the cached generation.
|
||||
"""
|
||||
curr = self._get_inventory(rp_uuid)
|
||||
|
||||
|
@ -293,6 +313,17 @@ class SchedulerReportClient(object):
|
|||
{'old': my_rp.generation,
|
||||
'new': server_gen})
|
||||
my_rp.generation = server_gen
|
||||
return curr
|
||||
|
||||
def _update_inventory_attempt(self, rp_uuid, inv_data):
|
||||
"""Update the inventory for this resource provider if needed.
|
||||
|
||||
:param rp_uuid: The resource provider UUID for the operation
|
||||
:param inv_data: The new inventory for the resource provider
|
||||
:returns: True if the inventory was updated (or did not need to be),
|
||||
False otherwise.
|
||||
"""
|
||||
curr = self._get_inventory_and_update_provider_generation(rp_uuid)
|
||||
|
||||
# Check to see if we need to update placement's view
|
||||
if inv_data == curr.get('inventories', {}):
|
||||
|
@ -359,6 +390,66 @@ class SchedulerReportClient(object):
|
|||
time.sleep(1)
|
||||
return False
|
||||
|
||||
@safe_connect
|
||||
def _delete_inventory(self, rp_uuid):
|
||||
"""Deletes all inventory records for a resource provider with the
|
||||
supplied UUID.
|
||||
"""
|
||||
curr = self._get_inventory_and_update_provider_generation(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 = _LI("Compute node %s reported no inventory but previous "
|
||||
"inventory was detected. Deleting existing inventory "
|
||||
"records.")
|
||||
LOG.info(msg, rp_uuid)
|
||||
|
||||
url = '/resource_providers/%s/inventories' % rp_uuid
|
||||
cur_rp_gen = self._resource_providers[rp_uuid].generation
|
||||
payload = {
|
||||
'resource_provider_generation': cur_rp_gen,
|
||||
'inventories': {},
|
||||
}
|
||||
r = self.put(url, payload)
|
||||
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,
|
||||
}
|
||||
LOG.info(_LI('Deleted all inventory for resource provider '
|
||||
'%(rp_uuid)s at generation %(generation)i'),
|
||||
msg_args)
|
||||
return
|
||||
elif r.status_code == 409:
|
||||
rc_str = _extract_inventory_in_use(r.text)
|
||||
if rc_str is not None:
|
||||
msg = _LW("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,
|
||||
}
|
||||
LOG.warning(msg, msg_args)
|
||||
return
|
||||
|
||||
msg = _LE("Failed to delete inventory for resource provider "
|
||||
"%(rp_uuid)s. Got error response: %(err)s")
|
||||
msg_args = {
|
||||
'rp_uuid': rp_uuid,
|
||||
'err': r.text,
|
||||
}
|
||||
LOG.error(msg, msg_args)
|
||||
|
||||
def update_resource_stats(self, compute_node):
|
||||
"""Creates or updates stats for the supplied compute node.
|
||||
|
||||
|
@ -368,7 +459,10 @@ class SchedulerReportClient(object):
|
|||
self._ensure_resource_provider(compute_node.uuid,
|
||||
compute_node.hypervisor_hostname)
|
||||
inv_data = _compute_node_to_inventory_dict(compute_node)
|
||||
self._update_inventory(compute_node.uuid, inv_data)
|
||||
if inv_data:
|
||||
self._update_inventory(compute_node.uuid, inv_data)
|
||||
else:
|
||||
self._delete_inventory(compute_node.uuid)
|
||||
|
||||
def _get_allocations_for_instance(self, rp_uuid, instance):
|
||||
url = '/allocations/%s' % instance.uuid
|
||||
|
|
|
@ -140,3 +140,15 @@ class SchedulerReportClientTests(test.TestCase):
|
|||
usage_data = resp.json()['usages']
|
||||
vcpu_data = usage_data[res_class]
|
||||
self.assertEqual(0, vcpu_data)
|
||||
|
||||
# Trigger the reporting client deleting all inventory by setting
|
||||
# the compute node's CPU, RAM and disk amounts to 0.
|
||||
self.compute_node.vcpus = 0
|
||||
self.compute_node.memory_mb = 0
|
||||
self.compute_node.local_gb = 0
|
||||
self.client.update_resource_stats(self.compute_node)
|
||||
|
||||
# Check there's no more inventory records
|
||||
resp = self.client.get(inventory_url)
|
||||
inventory_data = resp.json()['inventories']
|
||||
self.assertEqual({}, inventory_data)
|
||||
|
|
|
@ -12,9 +12,11 @@
|
|||
|
||||
from keystoneauth1 import exceptions as ks_exc
|
||||
import mock
|
||||
import six
|
||||
|
||||
import nova.conf
|
||||
from nova import context
|
||||
from nova import exception
|
||||
from nova import objects
|
||||
from nova.objects import base as obj_base
|
||||
from nova.scheduler.client import report
|
||||
|
@ -410,20 +412,222 @@ class TestComputeNodeToInventoryDict(test.NoDBTestCase):
|
|||
}
|
||||
self.assertEqual(expected, result)
|
||||
|
||||
def test_compute_node_inventory_empty(self):
|
||||
uuid = uuids.compute_node
|
||||
name = 'computehost'
|
||||
compute_node = objects.ComputeNode(uuid=uuid,
|
||||
hypervisor_hostname=name,
|
||||
vcpus=0,
|
||||
cpu_allocation_ratio=16.0,
|
||||
memory_mb=0,
|
||||
ram_allocation_ratio=1.5,
|
||||
local_gb=0,
|
||||
disk_allocation_ratio=1.0)
|
||||
result = report._compute_node_to_inventory_dict(compute_node)
|
||||
self.assertEqual({}, result)
|
||||
|
||||
|
||||
class TestInventory(SchedulerReportClientTestCase):
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_ensure_resource_provider')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_update_inventory_attempt')
|
||||
'_delete_inventory')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_update_inventory')
|
||||
@mock.patch('nova.objects.ComputeNode.save')
|
||||
def test_update_resource_stats(self, mock_save, mock_ui, mock_erp):
|
||||
def test_update_resource_stats(self, mock_save, mock_ui, mock_delete,
|
||||
mock_erp):
|
||||
cn = self.compute_node
|
||||
self.client.update_resource_stats(cn)
|
||||
mock_save.assert_called_once_with()
|
||||
mock_erp.assert_called_once_with(cn.uuid, cn.hypervisor_hostname)
|
||||
expected_inv_data = {
|
||||
'VCPU': {
|
||||
'total': 8,
|
||||
'reserved': 0,
|
||||
'min_unit': 1,
|
||||
'max_unit': 8,
|
||||
'step_size': 1,
|
||||
'allocation_ratio': 16.0,
|
||||
},
|
||||
'MEMORY_MB': {
|
||||
'total': 1024,
|
||||
'reserved': 512,
|
||||
'min_unit': 1,
|
||||
'max_unit': 1024,
|
||||
'step_size': 1,
|
||||
'allocation_ratio': 1.5,
|
||||
},
|
||||
'DISK_GB': {
|
||||
'total': 10,
|
||||
'reserved': 0,
|
||||
'min_unit': 1,
|
||||
'max_unit': 10,
|
||||
'step_size': 1,
|
||||
'allocation_ratio': 1.0,
|
||||
},
|
||||
}
|
||||
mock_ui.assert_called_once_with(
|
||||
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')
|
||||
@mock.patch('nova.objects.ComputeNode.save')
|
||||
def test_update_resource_stats_no_inv(self, mock_save, mock_ui,
|
||||
mock_delete, mock_erp):
|
||||
"""Ensure that if there are no inventory records, that we call
|
||||
_delete_inventory() instead of _update_inventory().
|
||||
"""
|
||||
cn = self.compute_node
|
||||
cn.vcpus = 0
|
||||
cn.memory_mb = 0
|
||||
cn.local_gb = 0
|
||||
self.client.update_resource_stats(cn)
|
||||
mock_save.assert_called_once_with()
|
||||
mock_erp.assert_called_once_with(cn.uuid, cn.hypervisor_hostname)
|
||||
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('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'put')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'get')
|
||||
def test_delete_inventory_already_no_inventory(self, mock_get, mock_put,
|
||||
mock_extract):
|
||||
cn = self.compute_node
|
||||
rp = objects.ResourceProvider(uuid=cn.uuid, generation=42)
|
||||
# Make sure the ResourceProvider 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_put.called)
|
||||
self.assertFalse(mock_extract.called)
|
||||
new_gen = self.client._resource_providers[cn.uuid].generation
|
||||
self.assertEqual(1, new_gen)
|
||||
|
||||
@mock.patch('nova.scheduler.client.report._extract_inventory_in_use')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'put')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'get')
|
||||
def test_delete_inventory(self, mock_get, mock_put, mock_extract):
|
||||
cn = self.compute_node
|
||||
rp = objects.ResourceProvider(uuid=cn.uuid, generation=42)
|
||||
# Make sure the ResourceProvider 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_put.return_value.status_code = 200
|
||||
mock_put.return_value.json.return_value = {
|
||||
'resource_provider_generation': 44,
|
||||
'inventories': {
|
||||
}
|
||||
}
|
||||
result = self.client._delete_inventory(cn.uuid)
|
||||
self.assertIsNone(result)
|
||||
self.assertFalse(mock_extract.called)
|
||||
new_gen = self.client._resource_providers[cn.uuid].generation
|
||||
self.assertEqual(44, new_gen)
|
||||
|
||||
@mock.patch.object(report.LOG, 'warning')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'put')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'get')
|
||||
def test_delete_inventory_inventory_in_use(self, mock_get, mock_put,
|
||||
mock_warn):
|
||||
cn = self.compute_node
|
||||
rp = objects.ResourceProvider(uuid=cn.uuid, generation=42)
|
||||
# Make sure the ResourceProvider 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_put.return_value.status_code = 409
|
||||
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_put.return_value.text = fault_text
|
||||
mock_put.return_value.json.return_value = {
|
||||
'resource_provider_generation': 44,
|
||||
'inventories': {
|
||||
}
|
||||
}
|
||||
result = self.client._delete_inventory(cn.uuid)
|
||||
self.assertIsNone(result)
|
||||
self.assertTrue(mock_warn.called)
|
||||
|
||||
@mock.patch.object(report.LOG, 'error')
|
||||
@mock.patch.object(report.LOG, 'warning')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'put')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'get')
|
||||
def test_delete_inventory_inventory_error(self, mock_get, mock_put,
|
||||
mock_warn, mock_error):
|
||||
cn = self.compute_node
|
||||
rp = objects.ResourceProvider(uuid=cn.uuid, generation=42)
|
||||
# Make sure the ResourceProvider 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_put.return_value.status_code = 409
|
||||
mock_put.return_value.text = (
|
||||
'There was a failure'
|
||||
)
|
||||
mock_put.return_value.json.return_value = {
|
||||
'resource_provider_generation': 44,
|
||||
'inventories': {
|
||||
}
|
||||
}
|
||||
result = self.client._delete_inventory(cn.uuid)
|
||||
self.assertIsNone(result)
|
||||
self.assertFalse(mock_warn.called)
|
||||
self.assertTrue(mock_error.called)
|
||||
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'get')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
|
|
Loading…
Reference in New Issue