openstack/nova
Code Issues Proposed changes

pci: create PCI tracker in RT._init_compute_node

Browse Source 
The end goal is to have the ResourceTracker's constructor be responsible
for creating the ComputeNode object/DB record and do all setup of
various resource tracking, including PCI device handling, querying the
aggregate and associated resource pool inventory information and
communication with the virt driver to gather host resources.

Before we can do that, however, we need to consolidate the creation of
various tracking entities into the ResourceTracker._init_compute_node()
method. This patch moves the creation of the
nova.pci.manager.PciDevTracker into the _init_compute_node() method and
cleans up a number of unit tests that were manually setting the
ResourceTracker.compute_node attribute to a fixture value (and in so
doing improperly circumventing the initialization process by relying on
the ResourceTracker.update_available_resource() to populate resource
information in that ComputeNode object.

Change-Id: I39a5436ee86e2d9b90f8f3e185353a04b277d434
Related-bug: #1357453
Related-bug: #1357491
This commit is contained in:
Jay Pipes
2016-05-11 09:13:11 -04:00
parent 421c52d9d3
commit d7cc32b514
3 changed files with 128 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files

34
nova/compute/resource_tracker.py
View File

@@ -366,6 +366,7 @@ class ResourceTracker(object):
# to initialize
if self.compute_node:
self._copy_resources(resources)
self._setup_pci_tracker(context, resources)
return
# now try to get the compute node record from the
@@ -373,6 +374,7 @@ class ResourceTracker(object):
self.compute_node = self._get_compute_node(context)
if self.compute_node:
self._copy_resources(resources)
self._setup_pci_tracker(context, resources)
return
# there was no local copy and none in the database
@@ -386,6 +388,20 @@ class ResourceTracker(object):
'%(host)s:%(node)s'),
{'host': self.host, 'node': self.nodename})
self._setup_pci_tracker(context, resources)
def _setup_pci_tracker(self, context, resources):
if not self.pci_tracker:
n_id = self.compute_node.id if self.compute_node else None
self.pci_tracker = pci_manager.PciDevTracker(context, node_id=n_id)
if 'pci_passthrough_devices' in resources:
dev_json = resources.pop('pci_passthrough_devices')
self.pci_tracker.update_devices_from_hypervisor_resources(
dev_json)
dev_pools_obj = self.pci_tracker.stats.to_device_pools_obj()
self.compute_node.pci_device_pools = dev_pools_obj
def _copy_resources(self, resources):
"""Copy resource values to initialise compute_node and related
data structures.
@@ -487,15 +503,6 @@ class ResourceTracker(object):
if self.disabled:
return
if 'pci_passthrough_devices' in resources:
# TODO(jaypipes): Move this into _init_compute_node()
if not self.pci_tracker:
n_id = self.compute_node.id if self.compute_node else None
self.pci_tracker = pci_manager.PciDevTracker(context,
node_id=n_id)
dev_json = resources.pop('pci_passthrough_devices')
self.pci_tracker.update_devices_from_hypervisor_resources(dev_json)
# Grab all instances assigned to this node:
instances = objects.InstanceList.get_by_host_and_node(
context, self.host, self.nodename,
@@ -522,12 +529,9 @@ class ResourceTracker(object):
# this periodic task, and also because the resource tracker is not
# notified when instances are deleted, we need remove all usages
# from deleted instances.
if self.pci_tracker:
self.pci_tracker.clean_usage(instances, migrations, orphans)
dev_pools_obj = self.pci_tracker.stats.to_device_pools_obj()
self.compute_node.pci_device_pools = dev_pools_obj
else:
self.compute_node.pci_device_pools = objects.PciDevicePoolList()
self.pci_tracker.clean_usage(instances, migrations, orphans)
dev_pools_obj = self.pci_tracker.stats.to_device_pools_obj()
self.compute_node.pci_device_pools = dev_pools_obj
self._report_final_resource_view()

11
nova/tests/unit/compute/test_resource_tracker.py
View File

@@ -284,16 +284,6 @@ class BaseTestCase(test.TestCase):
compute.update(values)
return compute
def _create_compute_node_obj(self, context):
# Use the db representation of a compute node returned
# by _create_compute_node() to create an equivalent compute
# node object.
compute = self._create_compute_node()
compute_obj = objects.ComputeNode()
compute_obj = objects.ComputeNode._from_db_object(
context, compute_obj, compute)
return compute_obj
def _create_service(self, host="fakehost", compute=None):
if compute:
compute = [compute]
@@ -448,7 +438,6 @@ class BaseTestCase(test.TestCase):
driver = self._driver()
tracker = resource_tracker.ResourceTracker(host, driver, node)
tracker.compute_node = self._create_compute_node_obj(self.context)
return tracker

136
nova/tests/unit/compute/test_tracker.py
View File

@@ -433,11 +433,16 @@ class TestUpdateAvailableResources(BaseTestCase):
self.rt.update_available_resource(mock.sentinel.ctx)
return update_mock
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance',
return_value=objects.InstancePCIRequests(requests=[]))
@mock.patch('nova.objects.PciDeviceList.get_by_compute_node',
return_value=objects.PciDeviceList())
@mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename')
@mock.patch('nova.objects.MigrationList.get_in_progress_by_host_and_node')
@mock.patch('nova.objects.InstanceList.get_by_host_and_node')
def test_no_instances_no_migrations_no_reserved(self, get_mock, migr_mock,
get_cn_mock):
get_cn_mock, pci_mock,
instance_pci_mock):
self.flags(reserved_host_disk_mb=0,
reserved_host_memory_mb=0)
self._setup_rt()
@@ -491,11 +496,16 @@ class TestUpdateAvailableResources(BaseTestCase):
self.assertTrue(obj_base.obj_equal_prims(expected_resources,
self.rt.compute_node))
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance',
return_value=objects.InstancePCIRequests(requests=[]))
@mock.patch('nova.objects.PciDeviceList.get_by_compute_node',
return_value=objects.PciDeviceList())
@mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename')
@mock.patch('nova.objects.MigrationList.get_in_progress_by_host_and_node')
@mock.patch('nova.objects.InstanceList.get_by_host_and_node')
def test_no_instances_no_migrations_reserved_disk_and_ram(
self, get_mock, migr_mock, get_cn_mock):
self, get_mock, migr_mock, get_cn_mock, pci_mock,
instance_pci_mock):
self.flags(reserved_host_disk_mb=1024,
reserved_host_memory_mb=512)
self._setup_rt()
@@ -537,11 +547,16 @@ class TestUpdateAvailableResources(BaseTestCase):
self.assertTrue(obj_base.obj_equal_prims(expected_resources,
self.rt.compute_node))
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance',
return_value=objects.InstancePCIRequests(requests=[]))
@mock.patch('nova.objects.PciDeviceList.get_by_compute_node',
return_value=objects.PciDeviceList())
@mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename')
@mock.patch('nova.objects.MigrationList.get_in_progress_by_host_and_node')
@mock.patch('nova.objects.InstanceList.get_by_host_and_node')
def test_some_instances_no_migrations(self, get_mock, migr_mock,
get_cn_mock):
get_cn_mock, pci_mock,
instance_pci_mock):
self.flags(reserved_host_disk_mb=0,
reserved_host_memory_mb=0)
@@ -590,11 +605,16 @@ class TestUpdateAvailableResources(BaseTestCase):
self.assertTrue(obj_base.obj_equal_prims(expected_resources,
self.rt.compute_node))
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance',
return_value=objects.InstancePCIRequests(requests=[]))
@mock.patch('nova.objects.PciDeviceList.get_by_compute_node',
return_value=objects.PciDeviceList())
@mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename')
@mock.patch('nova.objects.MigrationList.get_in_progress_by_host_and_node')
@mock.patch('nova.objects.InstanceList.get_by_host_and_node')
def test_orphaned_instances_no_migrations(self, get_mock, migr_mock,
get_cn_mock):
get_cn_mock, pci_mock,
instance_pci_mock):
self.flags(reserved_host_disk_mb=0,
reserved_host_memory_mb=0)
@@ -663,12 +683,17 @@ class TestUpdateAvailableResources(BaseTestCase):
self.assertTrue(obj_base.obj_equal_prims(expected_resources,
self.rt.compute_node))
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance',
return_value=objects.InstancePCIRequests(requests=[]))
@mock.patch('nova.objects.PciDeviceList.get_by_compute_node',
return_value=objects.PciDeviceList())
@mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename')
@mock.patch('nova.objects.MigrationList.get_in_progress_by_host_and_node')
@mock.patch('nova.objects.Instance.get_by_uuid')
@mock.patch('nova.objects.InstanceList.get_by_host_and_node')
def test_no_instances_source_migration(self, get_mock, get_inst_mock,
migr_mock, get_cn_mock):
migr_mock, get_cn_mock, pci_mock,
instance_pci_mock):
# We test the behavior of update_available_resource() when
# there is an active migration that involves this compute node
# as the source host not the destination host, and the resource
@@ -734,12 +759,17 @@ class TestUpdateAvailableResources(BaseTestCase):
self.assertTrue(obj_base.obj_equal_prims(expected_resources,
self.rt.compute_node))
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance',
return_value=objects.InstancePCIRequests(requests=[]))
@mock.patch('nova.objects.PciDeviceList.get_by_compute_node',
return_value=objects.PciDeviceList())
@mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename')
@mock.patch('nova.objects.MigrationList.get_in_progress_by_host_and_node')
@mock.patch('nova.objects.Instance.get_by_uuid')
@mock.patch('nova.objects.InstanceList.get_by_host_and_node')
def test_no_instances_dest_migration(self, get_mock, get_inst_mock,
migr_mock, get_cn_mock):
migr_mock, get_cn_mock, pci_mock,
instance_pci_mock):
# We test the behavior of update_available_resource() when
# there is an active migration that involves this compute node
# as the destination host not the source host, and the resource
@@ -802,12 +832,17 @@ class TestUpdateAvailableResources(BaseTestCase):
self.assertTrue(obj_base.obj_equal_prims(expected_resources,
self.rt.compute_node))
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance',
return_value=objects.InstancePCIRequests(requests=[]))
@mock.patch('nova.objects.PciDeviceList.get_by_compute_node',
return_value=objects.PciDeviceList())
@mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename')
@mock.patch('nova.objects.MigrationList.get_in_progress_by_host_and_node')
@mock.patch('nova.objects.Instance.get_by_uuid')
@mock.patch('nova.objects.InstanceList.get_by_host_and_node')
def test_no_instances_dest_evacuation(self, get_mock, get_inst_mock,
migr_mock, get_cn_mock):
migr_mock, get_cn_mock, pci_mock,
instance_pci_mock):
# We test the behavior of update_available_resource() when
# there is an active evacuation that involves this compute node
# as the destination host not the source host, and the resource
@@ -867,6 +902,10 @@ class TestUpdateAvailableResources(BaseTestCase):
self.assertTrue(obj_base.obj_equal_prims(expected_resources,
self.rt.compute_node))
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance',
return_value=objects.InstancePCIRequests(requests=[]))
@mock.patch('nova.objects.PciDeviceList.get_by_compute_node',
return_value=objects.PciDeviceList())
@mock.patch('nova.objects.MigrationContext.get_by_instance_uuid',
return_value=None)
@mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename')
@@ -876,7 +915,9 @@ class TestUpdateAvailableResources(BaseTestCase):
def test_some_instances_source_and_dest_migration(self, get_mock,
get_inst_mock, migr_mock,
get_cn_mock,
get_mig_ctxt_mock):
get_mig_ctxt_mock,
pci_mock,
instance_pci_mock):
# We test the behavior of update_available_resource() when
# there is an active migration that involves this compute node
# as the destination host AND the source host, and the resource
@@ -946,11 +987,13 @@ class TestUpdateAvailableResources(BaseTestCase):
class TestInitComputeNode(BaseTestCase):
@mock.patch('nova.objects.PciDeviceList.get_by_compute_node',
return_value=objects.PciDeviceList())
@mock.patch('nova.objects.ComputeNode.create')
@mock.patch('nova.objects.Service.get_by_compute_host')
@mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename')
def test_no_op_init_compute_node(self, get_mock, service_mock,
create_mock):
create_mock, pci_mock):
self._setup_rt()
resources = copy.deepcopy(_VIRT_DRIVER_AVAIL_RESOURCES)
@@ -962,11 +1005,15 @@ class TestInitComputeNode(BaseTestCase):
self.assertFalse(service_mock.called)
self.assertFalse(get_mock.called)
self.assertFalse(create_mock.called)
self.assertTrue(pci_mock.called)
self.assertFalse(self.rt.disabled)
@mock.patch('nova.objects.PciDeviceList.get_by_compute_node',
return_value=objects.PciDeviceList())
@mock.patch('nova.objects.ComputeNode.create')
@mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename')
def test_compute_node_loaded(self, get_mock, create_mock):
def test_compute_node_loaded(self, get_mock, create_mock,
pci_mock):
self._setup_rt()
def fake_get_node(_ctx, host, node):
@@ -983,9 +1030,12 @@ class TestInitComputeNode(BaseTestCase):
self.assertFalse(create_mock.called)
self.assertFalse(self.rt.disabled)
@mock.patch('nova.objects.PciDeviceList.get_by_compute_node',
return_value=objects.PciDeviceList(objects=[]))
@mock.patch('nova.objects.ComputeNode.create')
@mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename')
def test_compute_node_created_on_empty(self, get_mock, create_mock):
def test_compute_node_created_on_empty(self, get_mock, create_mock,
pci_tracker_mock):
self._setup_rt()
get_mock.side_effect = exc.NotFound
@@ -1017,6 +1067,7 @@ class TestInitComputeNode(BaseTestCase):
# The expected compute represents the initial values used
# when creating a compute node.
expected_compute = objects.ComputeNode(
id=42,
host_ip=resources['host_ip'],
vcpus=resources['vcpus'],
memory_mb=resources['memory_mb'],
@@ -1036,6 +1087,7 @@ class TestInitComputeNode(BaseTestCase):
cpu_allocation_ratio=cpu_alloc_ratio,
disk_allocation_ratio=disk_alloc_ratio,
stats={},
pci_device_pools=objects.PciDevicePoolList(objects=[])
)
# Forcing the flags to the values we know
@@ -1043,6 +1095,13 @@ class TestInitComputeNode(BaseTestCase):
self.rt.cpu_allocation_ratio = cpu_alloc_ratio
self.rt.disk_allocation_ratio = disk_alloc_ratio
def set_cn_id():
# The PCI tracker needs the compute node's ID when starting up, so
# make sure that we set the ID value so we don't get a Cannot load
# 'id' in base class error
self.rt.compute_node.id = 42 # Has to be a number, not a mock
create_mock.side_effect = set_cn_id
self.rt._init_compute_node(mock.sentinel.ctx, resources)
self.assertFalse(self.rt.disabled)
@@ -1051,6 +1110,8 @@ class TestInitComputeNode(BaseTestCase):
create_mock.assert_called_once_with()
self.assertTrue(obj_base.obj_equal_prims(expected_compute,
self.rt.compute_node))
pci_tracker_mock.assert_called_once_with(mock.sentinel.ctx,
42)
def test_copy_resources_adds_allocation_ratios(self):
self.flags(cpu_allocation_ratio=4.0, ram_allocation_ratio=3.0,
@@ -1058,8 +1119,10 @@ class TestInitComputeNode(BaseTestCase):
self._setup_rt()
resources = copy.deepcopy(_VIRT_DRIVER_AVAIL_RESOURCES)
compute_node = copy.deepcopy(_COMPUTE_NODE_FIXTURES[0])
self.rt.compute_node = compute_node
# TODO(jaypipes): Remove this manual setting of the RT.compute_node
# attribute once the compute node object is always created in the
# RT's constructor.
self.rt.compute_node = copy.deepcopy(_COMPUTE_NODE_FIXTURES[0])
self.rt._copy_resources(resources)
self.assertEqual(4.0, self.rt.compute_node.cpu_allocation_ratio)
@@ -1516,7 +1579,6 @@ class TestMoveClaim(BaseTestCase):
super(TestMoveClaim, self).setUp()
self._setup_rt()
self.rt.compute_node = copy.deepcopy(_COMPUTE_NODE_FIXTURES[0])
self.instance = _INSTANCE_FIXTURES[0].obj_clone()
self.flavor = _INSTANCE_TYPE_OBJ_FIXTURES[1]
@@ -1527,15 +1589,20 @@ class TestMoveClaim(BaseTestCase):
self.elevated = mock.MagicMock()
self.ctx.elevated.return_value = self.elevated
# Initialise extensible resource trackers
self.flags(reserved_host_disk_mb=0, reserved_host_memory_mb=0)
with test.nested(
mock.patch('nova.objects.InstanceList.get_by_host_and_node'),
mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename',
return_value=copy.deepcopy(_COMPUTE_NODE_FIXTURES[0])),
mock.patch('nova.objects.InstancePCIRequests.get_by_instance',
return_value=objects.InstancePCIRequests(requests=[])),
mock.patch('nova.objects.PciDeviceList.get_by_compute_node',
return_value=objects.PciDeviceList()),
mock.patch('nova.objects.InstanceList.get_by_host_and_node',
return_value=objects.InstanceList()),
mock.patch('nova.objects.MigrationList.'
'get_in_progress_by_host_and_node')
) as (inst_list_mock, migr_mock):
inst_list_mock.return_value = objects.InstanceList(objects=[])
migr_mock.return_value = objects.MigrationList(objects=[])
'get_in_progress_by_host_and_node',
return_value=objects.MigrationList())
) as (cn_mock, inst_pci_mock, pci_dev_mock, inst_list_mock, migr_mock):
self.rt.update_available_resource(self.ctx)
def register_mocks(self, pci_mock, inst_list_mock, inst_by_uuid,
@@ -1592,11 +1659,14 @@ class TestMoveClaim(BaseTestCase):
expected = copy.deepcopy(self.rt.compute_node)
self.adjust_expected(expected, self.flavor)
create_mig_mock = mock.patch.object(self.rt, '_create_migration')
mig_ctxt_mock = mock.patch('nova.objects.MigrationContext',
return_value=mig_context_obj)
with create_mig_mock as migr_mock, mig_ctxt_mock as ctxt_mock:
migr_mock.return_value = _MIGRATION_FIXTURES['source-only']
with test.nested(
mock.patch('nova.objects.InstancePCIRequests.get_by_instance',
return_value=objects.InstancePCIRequests(requests=[])),
mock.patch.object(self.rt, '_create_migration',
return_value=_MIGRATION_FIXTURES['source-only']),
mock.patch('nova.objects.MigrationContext',
return_value=mig_context_obj)
) as (int_pci_mock, migr_mock, ctxt_mock):
claim = self.rt.resize_claim(
self.ctx, self.instance, self.flavor, None)
self.assertEqual(1, ctxt_mock.call_count)
@@ -1710,9 +1780,21 @@ class TestMoveClaim(BaseTestCase):
# update_available_resource to initialise extensible resource trackers
src_rt = self.rt
(dst_rt, _, _) = setup_rt("other-host", "other-node")
dst_rt.compute_node = copy.deepcopy(_COMPUTE_NODE_FIXTURES[0])
inst_list_mock.return_value = objects.InstanceList(objects=[])
dst_rt.update_available_resource(self.ctx)
with test.nested(
mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename',
return_value=copy.deepcopy(_COMPUTE_NODE_FIXTURES[0])),
mock.patch('nova.objects.InstancePCIRequests.get_by_instance',
return_value=objects.InstancePCIRequests(requests=[])),
mock.patch('nova.objects.PciDeviceList.get_by_compute_node',
return_value=objects.PciDeviceList()),
mock.patch('nova.objects.InstanceList.get_by_host_and_node',
return_value=objects.InstanceList()),
mock.patch('nova.objects.MigrationList.'
'get_in_progress_by_host_and_node',
return_value=objects.MigrationList())
) as (cn_mock, inst_pci_mock, pci_dev_mock, inst_list_mock, migr_mock):
dst_rt.update_available_resource(self.ctx)
# Register the instance with dst_rt
expected = copy.deepcopy(dst_rt.compute_node)