Drop compat for non-update_provider_tree code paths

In Train [1] we deprecated support for compute drivers
that did not implement the update_provider_tree method.
That compat code is now removed along with the get_inventory
method definition and (most) references to it.

As a result there are more things we can remove but those
will come in separate changes.

[1] I1eae47bce08f6292d38e893a2122289bcd6f4b58

Change-Id: Ib62ac0b692eb92a2ed364ec9f486ded05def39ad
This commit is contained in:
Matt Riedemann
2019-11-07 17:15:50 -05:00
parent 6479d492c0
commit c80912866f
7 changed files with 57 additions and 125 deletions

View File

@@ -23,13 +23,13 @@ Background
---------- ----------
In the movement towards using placement for scheduling and resource management, In the movement towards using placement for scheduling and resource management,
the virt driver method ``get_available_resource`` was initially superseded by the virt driver method ``get_available_resource`` was initially superseded by
``get_inventory``, whereby the driver could specify its inventory in terms ``get_inventory`` (now gone), whereby the driver could specify its inventory in
understood by placement. In Queens, a ``get_traits`` driver method was added. terms understood by placement. In Queens, a ``get_traits`` driver method was
But ``get_inventory`` is limited to expressing only inventory (not traits or added. But ``get_inventory`` is limited to expressing only inventory (not
aggregates). And both of these methods are limited to the resource provider traits or aggregates). And both of these methods are limited to the resource
corresponding to the compute node. provider corresponding to the compute node.
Recent developments such as Nested Resource Providers necessitate the ability Developments such as Nested Resource Providers necessitate the ability
for the virt driver to have deeper control over what the resource tracker for the virt driver to have deeper control over what the resource tracker
configures in placement on behalf of the compute node. This need is filled by configures in placement on behalf of the compute node. This need is filled by
the virt driver method ``update_provider_tree`` and its consumption by the the virt driver method ``update_provider_tree`` and its consumption by the
@@ -130,9 +130,6 @@ aggregates, and traits associated with those resource providers.
PF1 PF2 PF3 PF4------BW1 (root) PF1 PF2 PF3 PF4------BW1 (root)
agg2 agg2
This method supersedes ``get_inventory`` and ``get_traits``: if this method is
implemented, neither ``get_inventory`` nor ``get_traits`` is used.
Driver implementations of ``update_provider_tree`` are expected to use public Driver implementations of ``update_provider_tree`` are expected to use public
``ProviderTree`` methods to effect changes to the provider tree passed in. ``ProviderTree`` methods to effect changes to the provider tree passed in.
Some of the methods which may be useful are as follows: Some of the methods which may be useful are as follows:

View File

@@ -82,6 +82,7 @@ def _instance_is_live_migrating(instance):
return False return False
# TODO(mriedem): Remove this now that get_inventory isn't being used anymore
def _normalize_inventory_from_cn_obj(inv_data, cn): def _normalize_inventory_from_cn_obj(inv_data, cn):
"""Helper function that injects various information from a compute node """Helper function that injects various information from a compute node
object into the inventory dict returned from the virt driver's object into the inventory dict returned from the virt driver's
@@ -1153,59 +1154,38 @@ class ResourceTracker(object):
# the inventory, traits, and aggregates throughout. # the inventory, traits, and aggregates throughout.
allocs = None allocs = None
try: try:
try: self.driver.update_provider_tree(prov_tree, nodename)
self.driver.update_provider_tree(prov_tree, nodename) except exception.ReshapeNeeded:
except exception.ReshapeNeeded: if not startup:
if not startup: # This isn't supposed to happen during periodic, so raise
# This isn't supposed to happen during periodic, so raise # it up; the compute manager will treat it specially.
# it up; the compute manager will treat it specially. raise
raise LOG.info("Performing resource provider inventory and "
LOG.info("Performing resource provider inventory and " "allocation data migration during compute service "
"allocation data migration during compute service " "startup or fast-forward upgrade.")
"startup or fast-forward upgrade.") allocs = self.reportclient.get_allocations_for_provider_tree(
allocs = self.reportclient.get_allocations_for_provider_tree( context, nodename)
context, nodename) self.driver.update_provider_tree(prov_tree, nodename,
self.driver.update_provider_tree(prov_tree, nodename, allocations=allocs)
allocations=allocs)
# Inject driver capabilities traits into the provider # Inject driver capabilities traits into the provider
# tree. We need to determine the traits that the virt # tree. We need to determine the traits that the virt
# driver owns - so those that come from the tree itself # driver owns - so those that come from the tree itself
# (via the virt driver) plus the compute capabilities # (via the virt driver) plus the compute capabilities
# traits, and then merge those with the traits set # traits, and then merge those with the traits set
# externally that the driver does not own - and remove any # externally that the driver does not own - and remove any
# set on the provider externally that the virt owns but # set on the provider externally that the virt owns but
# aren't in the current list of supported traits. For # aren't in the current list of supported traits. For
# example, let's say we reported multiattach support as a # example, let's say we reported multiattach support as a
# trait at t1 and then at t2 it's not, so we need to # trait at t1 and then at t2 it's not, so we need to
# remove it. But at both t1 and t2 there is a # remove it. But at both t1 and t2 there is a
# CUSTOM_VENDOR_TRAIT_X which we can't touch because it # CUSTOM_VENDOR_TRAIT_X which we can't touch because it
# was set externally on the provider. # was set externally on the provider.
# We also want to sync the COMPUTE_STATUS_DISABLED trait based # We also want to sync the COMPUTE_STATUS_DISABLED trait based
# on the related nova-compute service's disabled status. # on the related nova-compute service's disabled status.
traits = self._get_traits( traits = self._get_traits(
context, nodename, provider_tree=prov_tree) context, nodename, provider_tree=prov_tree)
prov_tree.update_traits(nodename, traits) prov_tree.update_traits(nodename, traits)
except NotImplementedError:
# TODO(mriedem): Remove the compatibility code in the U release.
LOG.warning('Compute driver "%s" does not implement the '
'"update_provider_tree" interface. Compatibility for '
'non-update_provider_tree interfaces will be removed '
'in a future release and result in an error to report '
'inventory for this compute service.',
CONF.compute_driver)
# update_provider_tree isn't implemented yet - try get_inventory
try:
inv_data = self.driver.get_inventory(nodename)
_normalize_inventory_from_cn_obj(inv_data, compute_node)
except NotImplementedError:
# Eventually all virt drivers will return an inventory dict in
# the format that the placement API expects and we'll be able
# to remove this code branch
inv_data = compute_utils.compute_node_to_inventory_dict(
compute_node)
prov_tree.update_inventory(nodename, inv_data)
self.provider_tree = prov_tree self.provider_tree = prov_tree

View File

@@ -1463,6 +1463,8 @@ def notify_about_instance_delete(notifier, context, instance,
phase=fields.NotificationPhase.END) phase=fields.NotificationPhase.END)
# TODO(mriedem): We should be able to remove this now that the ResourceTracker
# requires drivers to implement the update_provider_tree interface.
def compute_node_to_inventory_dict(compute_node): def compute_node_to_inventory_dict(compute_node):
"""Given a supplied `objects.ComputeNode` object, return a dict, keyed """Given a supplied `objects.ComputeNode` object, return a dict, keyed
by resource class, of various inventory information. by resource class, of various inventory information.

View File

@@ -449,7 +449,6 @@ def setup_rt(hostname, virt_resources=_VIRT_DRIVER_AVAIL_RESOURCES):
# Make sure we don't change any global fixtures during tests # Make sure we don't change any global fixtures during tests
virt_resources = copy.deepcopy(virt_resources) virt_resources = copy.deepcopy(virt_resources)
vd.get_available_resource.return_value = virt_resources vd.get_available_resource.return_value = virt_resources
vd.get_inventory.side_effect = NotImplementedError
def fake_upt(provider_tree, nodename, allocations=None): def fake_upt(provider_tree, nodename, allocations=None):
inventory = { inventory = {
@@ -1514,11 +1513,11 @@ class TestInitComputeNode(BaseTestCase):
class TestUpdateComputeNode(BaseTestCase): class TestUpdateComputeNode(BaseTestCase):
@mock.patch('nova.compute.resource_tracker.ResourceTracker.'
'_sync_compute_service_disabled_trait', new=mock.Mock())
@mock.patch('nova.objects.ComputeNode.save') @mock.patch('nova.objects.ComputeNode.save')
def test_existing_compute_node_updated_same_resources(self, save_mock): def test_existing_compute_node_updated_same_resources(self, save_mock):
self._setup_rt() self._setup_rt()
self.driver_mock.update_provider_tree.side_effect = NotImplementedError
# This is the same set of resources as the fixture, deliberately. We # This is the same set of resources as the fixture, deliberately. We
# are checking below to see that compute_node.save is not needlessly # are checking below to see that compute_node.save is not needlessly
@@ -1531,8 +1530,9 @@ class TestUpdateComputeNode(BaseTestCase):
self.rt._update(mock.sentinel.ctx, new_compute) self.rt._update(mock.sentinel.ctx, new_compute)
self.assertFalse(save_mock.called) self.assertFalse(save_mock.called)
# Even the compute node is not updated, get_inventory still got called. # Even the compute node is not updated, update_provider_tree
self.driver_mock.get_inventory.assert_called_once_with(_NODENAME) # still got called.
self.driver_mock.update_provider_tree.assert_called_once()
@mock.patch('nova.compute.resource_tracker.ResourceTracker.' @mock.patch('nova.compute.resource_tracker.ResourceTracker.'
'_sync_compute_service_disabled_trait', new=mock.Mock()) '_sync_compute_service_disabled_trait', new=mock.Mock())
@@ -1558,11 +1558,8 @@ class TestUpdateComputeNode(BaseTestCase):
@mock.patch('nova.compute.resource_tracker.ResourceTracker.' @mock.patch('nova.compute.resource_tracker.ResourceTracker.'
'_sync_compute_service_disabled_trait', new=mock.Mock()) '_sync_compute_service_disabled_trait', new=mock.Mock())
@mock.patch('nova.compute.resource_tracker.'
'_normalize_inventory_from_cn_obj')
@mock.patch('nova.objects.ComputeNode.save') @mock.patch('nova.objects.ComputeNode.save')
def test_existing_compute_node_updated_new_resources(self, save_mock, def test_existing_compute_node_updated_new_resources(self, save_mock):
norm_mock):
self._setup_rt() self._setup_rt()
orig_compute = _COMPUTE_NODE_FIXTURES[0].obj_clone() orig_compute = _COMPUTE_NODE_FIXTURES[0].obj_clone()
@@ -1580,42 +1577,6 @@ class TestUpdateComputeNode(BaseTestCase):
self.rt._update(mock.sentinel.ctx, new_compute) self.rt._update(mock.sentinel.ctx, new_compute)
save_mock.assert_called_once_with() save_mock.assert_called_once_with()
# The get_inventory() is not implemented, it shouldn't call
# _normalize_inventory_from_cn_obj
norm_mock.assert_not_called()
@mock.patch('nova.compute.resource_tracker.'
'_normalize_inventory_from_cn_obj')
@mock.patch('nova.objects.ComputeNode.save')
def test_existing_node_get_inventory_implemented(self, save_mock,
norm_mock):
"""The get_inventory() virt driver method is only implemented for some
virt drivers. This method returns inventory information for a
node/provider in a way that the placement API better understands.
"""
self._setup_rt()
self.driver_mock.update_provider_tree.side_effect = NotImplementedError
# Emulate a driver that has implemented the newish get_inventory() virt
# driver method
self.driver_mock.get_inventory.side_effect = [mock.sentinel.inv_data]
orig_compute = _COMPUTE_NODE_FIXTURES[0].obj_clone()
self.rt.compute_nodes[_NODENAME] = orig_compute
self.rt.old_resources[_NODENAME] = orig_compute
# Deliberately changing local_gb to trigger updating inventory
new_compute = orig_compute.obj_clone()
new_compute.local_gb = 210000
self.rt._update(mock.sentinel.ctx, new_compute)
save_mock.assert_called_once_with()
norm_mock.assert_called_once_with(mock.sentinel.inv_data, new_compute)
# Assert a warning was logged about using a virt driver that does not
# implement update_provider_tree.
self.assertIn('Compute driver "%s" does not implement the '
'"update_provider_tree" interface.' %
CONF.compute_driver, self.stdlog.logger.output)
@mock.patch('nova.compute.resource_tracker.ResourceTracker.' @mock.patch('nova.compute.resource_tracker.ResourceTracker.'
'_sync_compute_service_disabled_trait') '_sync_compute_service_disabled_trait')
@@ -1654,14 +1615,10 @@ class TestUpdateComputeNode(BaseTestCase):
@mock.patch('nova.objects.ComputeNode.save') @mock.patch('nova.objects.ComputeNode.save')
def test_existing_node_update_provider_tree_implemented( def test_existing_node_update_provider_tree_implemented(
self, save_mock, mock_sync_disabled): self, save_mock, mock_sync_disabled):
"""The update_provider_tree() virt driver method is only implemented """The update_provider_tree() virt driver method must be implemented
for some virt drivers. This method returns inventory, trait, and by all virt drivers. This method returns inventory, trait, and
aggregate information for resource providers in a tree associated with aggregate information for resource providers in a tree associated with
the compute node. If this method doesn't raise a NotImplementedError, the compute node.
it triggers _update() to try get_inventory() and then
compute_node_to_inventory_dict() to produce the inventory data with
which to call the update_from_provider_tree() method of the reporting
client instead.
""" """
fake_inv = { fake_inv = {
orc.VCPU: { orc.VCPU: {
@@ -1721,7 +1678,6 @@ class TestUpdateComputeNode(BaseTestCase):
ptree, new_compute.hypervisor_hostname) ptree, new_compute.hypervisor_hostname)
self.rt.reportclient.update_from_provider_tree.assert_called_once_with( self.rt.reportclient.update_from_provider_tree.assert_called_once_with(
mock.sentinel.ctx, ptree, allocations=None) mock.sentinel.ctx, ptree, allocations=None)
self.driver_mock.get_inventory.assert_not_called()
ptree.update_traits.assert_called_once_with( ptree.update_traits.assert_called_once_with(
new_compute.hypervisor_hostname, new_compute.hypervisor_hostname,
[] []

View File

@@ -972,9 +972,6 @@ class ComputeDriver(object):
node, as well as the inventory, aggregates, and traits associated with node, as well as the inventory, aggregates, and traits associated with
those resource providers. those resource providers.
This method supersedes get_inventory(): if this method is implemented,
get_inventory() is not used.
Implementors of this interface are expected to set ``allocation_ratio`` Implementors of this interface are expected to set ``allocation_ratio``
and ``reserved`` values for inventory records, which may be based on and ``reserved`` values for inventory records, which may be based on
configuration options, e.g. ``[DEFAULT]/cpu_allocation_ratio``, configuration options, e.g. ``[DEFAULT]/cpu_allocation_ratio``,
@@ -1048,12 +1045,6 @@ class ComputeDriver(object):
""" """
raise NotImplementedError() raise NotImplementedError()
def get_inventory(self, nodename):
"""Return a dict, keyed by resource class, of inventory information for
the supplied node.
"""
raise NotImplementedError()
def capabilities_as_traits(self): def capabilities_as_traits(self):
"""Returns this driver's capabilities dict where the keys are traits """Returns this driver's capabilities dict where the keys are traits

View File

@@ -167,8 +167,6 @@ class PowerVMDriver(driver.ComputeDriver):
this. this.
:return: Dictionary describing resources. :return: Dictionary describing resources.
""" """
# TODO(efried): Switch to get_inventory, per blueprint
# custom-resource-classes-pike
# Do this here so it refreshes each time this method is called. # Do this here so it refreshes each time this method is called.
self.host_wrapper = pvm_ms.System.get(self.adapter)[0] self.host_wrapper = pvm_ms.System.get(self.adapter)[0]
return self._get_available_resource() return self._get_available_resource()

View File

@@ -0,0 +1,8 @@
---
upgrade:
- |
Compatibility code for compute drivers that do not implement the
`update_provider_tree`__ interface has been removed. All compute drivers
must now implement ``update_provider_tree``.
__ https://docs.openstack.org/nova/latest/reference/update-provider-tree.html