openstack
/
nova
Code Issues Proposed changes

ProviderTree.get_provider_uuids: Top-down ordering 

It will become important in update_from_provider_tree to be able to walk
the providers in a ProviderTree in a sane and predictable order.
Otherwise, when flushing multiple adds/deletes, we will have no reliable
way to avoid creating orphans (which will fail).

Here we change ProviderTree.get_provider_uuids from returning a set() to
returning a list which is guaranteed to be in top-down order.  We do not
guarantee the order in which siblings appear, or where nephews appear
relative to their uncles; just that a child will always appear after its
parent (and, by extension, after all its ancestors).

Change-Id: I2fb691e019177c502ec651390faf3740a2d49045
blueprint: nested-resource-providers
This commit is contained in:
Eric Fried 2018-01-22 15:53:04 -06:00
parent e3de95e3b3
commit 8014449f2c
4 changed files with 44 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
nova/compute/provider_tree.py
View File

@ -86,10 +86,12 @@ class _Provider(object):
inventory, traits, aggregates)
def get_provider_uuids(self):
"""Returns a set of UUIDs of this provider and all its descendants."""
ret = set([self.uuid])
"""Returns a list, in top-down traversal order, of UUIDs of this
provider and all its descendants.
"""
ret = [self.uuid]
for child in self.children.values():
ret |= child.get_provider_uuids()
ret.extend(child.get_provider_uuids())
return ret
def find(self, search):
@ -227,7 +229,8 @@ class ProviderTree(object):
self.roots.append(p)
def get_provider_uuids(self, name_or_uuid=None):
"""Return a set of the UUIDs of all providers (in a subtree).
"""Return a list, in top-down traversable order, of the UUIDs of all
providers (in a subtree).
:param name_or_uuid: Provider name or UUID representing the root of a
subtree for which to return UUIDs. If not
@ -239,10 +242,10 @@ class ProviderTree(object):
return self._find_with_lock(name_or_uuid).get_provider_uuids()
# If no name_or_uuid, get UUIDs for all providers recursively.
ret = set()
ret = []
with self.lock:
for root in self.roots:
ret |= root.get_provider_uuids()
ret.extend(root.get_provider_uuids())
return ret
def populate_from_iterable(self, provider_dicts):
@ -276,7 +279,7 @@ class ProviderTree(object):
# NOTE(efried): Can't use get_provider_uuids directly because we're
# already under lock.
for root in self.roots:
all_parents |= root.get_provider_uuids()
all_parents |= set(root.get_provider_uuids())
missing_parents = set()
for pd in to_add_by_uuid.values():
parent_uuid = pd.get('parent_provider_uuid')

16
nova/tests/functional/api/openstack/placement/test_report_client.py
View File

@ -114,8 +114,7 @@ class SchedulerReportClientTests(test.TestCase):
# _ensure_resource_provider)
ptree = self.client.get_provider_tree_and_ensure_root(
self.compute_uuid)
self.assertEqual(set([self.compute_uuid]),
ptree.get_provider_uuids())
self.assertEqual([self.compute_uuid], ptree.get_provider_uuids())
# Now let's update status for our compute node.
self.client.update_compute_node(self.context, self.compute_node)
@ -149,8 +148,7 @@ class SchedulerReportClientTests(test.TestCase):
# Providers and inventory show up nicely in the provider tree
ptree = self.client.get_provider_tree_and_ensure_root(
self.compute_uuid)
self.assertEqual(set([self.compute_uuid]),
ptree.get_provider_uuids())
self.assertEqual([self.compute_uuid], ptree.get_provider_uuids())
self.assertTrue(ptree.has_inventory(self.compute_uuid))
# Update allocations with our instance
@ -197,8 +195,7 @@ class SchedulerReportClientTests(test.TestCase):
ptree = self.client.get_provider_tree_and_ensure_root(
self.compute_uuid)
# The compute node is still there
self.assertEqual(set([self.compute_uuid]),
ptree.get_provider_uuids())
self.assertEqual([self.compute_uuid], ptree.get_provider_uuids())
# But the inventory is gone
self.assertFalse(ptree.has_inventory(self.compute_uuid))
@ -353,10 +350,11 @@ class SchedulerReportClientTests(test.TestCase):
self.assertEqual(set([self.compute_uuid, uuids.ss1, uuids.ss2,
uuids.pf1, uuids.pf2, uuids.sip, uuids.ss3,
uuids.sbw]),
prov_tree.get_provider_uuids())
set(prov_tree.get_provider_uuids()))
# Narrow the field to just our compute subtree.
self.assertEqual(set([self.compute_uuid, uuids.pf1, uuids.pf2]),
prov_tree.get_provider_uuids(self.compute_uuid))
self.assertEqual(
set([self.compute_uuid, uuids.pf1, uuids.pf2]),
set(prov_tree.get_provider_uuids(self.compute_uuid)))
# Validate traits for a couple of providers
self.assertFalse(prov_tree.have_traits_changed(

35
nova/tests/unit/compute/test_provider_tree.py
View File

@ -51,9 +51,10 @@ class TestProviderTree(test.NoDBTestCase):
self.assertFalse(pt.exists(uuids.non_existing_rp))
self.assertFalse(pt.exists('noexist'))
self.assertEqual(set([cn1.uuid]),
self.assertEqual([cn1.uuid],
pt.get_provider_uuids(name_or_uuid=cn1.uuid))
self.assertEqual(set([cn1.uuid, cn2.uuid]), pt.get_provider_uuids())
self.assertEqual(set([cn1.uuid, cn2.uuid]),
set(pt.get_provider_uuids()))
numa_cell0_uuid = pt.new_child('numa_cell0', cn1.uuid)
numa_cell1_uuid = pt.new_child('numa_cell1', cn1.hypervisor_hostname)
@ -73,11 +74,11 @@ class TestProviderTree(test.NoDBTestCase):
# Now we've got a 3-level tree under cn1 - check provider UUIDs again
self.assertEqual(
set([cn1.uuid, numa_cell0_uuid, pf1_cell0_uuid, numa_cell1_uuid]),
pt.get_provider_uuids(name_or_uuid=cn1.uuid))
set(pt.get_provider_uuids(name_or_uuid=cn1.uuid)))
self.assertEqual(
set([cn1.uuid, cn2.uuid, numa_cell0_uuid, pf1_cell0_uuid,
numa_cell1_uuid]),
pt.get_provider_uuids())
set(pt.get_provider_uuids()))
self.assertRaises(
ValueError,
@ -140,7 +141,7 @@ class TestProviderTree(test.NoDBTestCase):
pt = provider_tree.ProviderTree()
# Empty list is a no-op
pt.populate_from_iterable([])
self.assertEqual(set(), pt.get_provider_uuids())
self.assertEqual([], pt.get_provider_uuids())
def test_populate_from_iterable_error_orphan_cycle(self):
pt = provider_tree.ProviderTree()
@ -230,7 +231,7 @@ class TestProviderTree(test.NoDBTestCase):
def validate_root(expected_uuids):
# Make sure we have all and only the expected providers
self.assertEqual(expected_uuids, pt.get_provider_uuids())
self.assertEqual(expected_uuids, set(pt.get_provider_uuids()))
# Now make sure they're in the right hierarchy. Cheat: get the
# actual _Provider to make it easier to walk the tree (ProviderData
# doesn't include children).
@ -313,6 +314,19 @@ class TestProviderTree(test.NoDBTestCase):
pt.populate_from_iterable([])
validate_root(expected_uuids)
# Since we have a complex tree, test the ordering of get_provider_uuids
# We can't predict the order of siblings, or where nephews will appear
# relative to their uncles, but we can guarantee that any given child
# always comes after its parent (and by extension, its ancestors too).
puuids = pt.get_provider_uuids()
for desc in (uuids.child1, uuids.child2):
self.assertTrue(puuids.index(desc) > puuids.index(uuids.root))
for desc in (uuids.grandchild1_1, uuids.grandchild1_2):
self.assertTrue(puuids.index(desc) > puuids.index(uuids.child1))
for desc in (uuids.ggc1_2_1, uuids.ggc1_2_2, uuids.ggc1_2_3):
self.assertTrue(
puuids.index(desc) > puuids.index(uuids.grandchild1_2))
def test_populate_from_iterable_with_root_update(self):
# Ensure we can update hierarchies, including adding children, in a
# tree that's already populated. This tests the case where a given
@ -333,7 +347,7 @@ class TestProviderTree(test.NoDBTestCase):
},
]
pt.populate_from_iterable(plist)
expected_uuids = set([uuids.root])
expected_uuids = [uuids.root]
self.assertEqual(expected_uuids, pt.get_provider_uuids())
# Let's add a child updating the name and generation for the root.
@ -353,7 +367,7 @@ class TestProviderTree(test.NoDBTestCase):
},
]
pt.populate_from_iterable(plist)
expected_uuids = set([uuids.root, uuids.child1])
expected_uuids = [uuids.root, uuids.child1]
self.assertEqual(expected_uuids, pt.get_provider_uuids())
def test_populate_from_iterable_disown_grandchild(self):
@ -384,12 +398,11 @@ class TestProviderTree(test.NoDBTestCase):
},
]
pt.populate_from_iterable(plist)
self.assertEqual(set([uuids.root, uuids.child, uuids.grandchild]),
self.assertEqual([uuids.root, uuids.child, uuids.grandchild],
pt.get_provider_uuids())
self.assertTrue(pt.exists(uuids.grandchild))
pt.populate_from_iterable([child])
self.assertEqual(set([uuids.root, uuids.child]),
pt.get_provider_uuids())
self.assertEqual([uuids.root, uuids.child], pt.get_provider_uuids())
self.assertFalse(pt.exists(uuids.grandchild))
def test_has_inventory_changed_no_existing_rp(self):

6
nova/tests/unit/scheduler/client/test_report.py
View File

@ -1421,7 +1421,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
# At this point we should get all the providers.
self.assertEqual(
set([uuids.root, uuids.child1, uuids.child2, uuids.grandchild]),
self.client._provider_tree.get_provider_uuids())
set(self.client._provider_tree.get_provider_uuids()))
@mock.patch('nova.compute.provider_tree.ProviderTree.exists')
@mock.patch('nova.compute.provider_tree.ProviderTree.get_provider_uuids')
@ -1461,7 +1461,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
[mock.call(uuid, generation=42, force=True)
for uuid in tree_uuids])
self.assertEqual(tree_uuids,
self.client._provider_tree.get_provider_uuids())
set(self.client._provider_tree.get_provider_uuids()))
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
'_get_providers_in_tree')
@ -1481,7 +1481,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
mock_create.assert_called_once_with(uuids.root, uuids.root,
parent_provider_uuid=None)
mock_refresh.assert_not_called()
self.assertEqual(set([uuids.cn]),
self.assertEqual([uuids.cn],
self.client._provider_tree.get_provider_uuids())
def test_get_allocation_candidates(self):