Only pull associated *sharing* providers
It was discussed and decided [1] that we only want to be pulling down,
caching, and passing to update_provider_tree providers associated via
aggregate with the compute node's provider tree if they are sharing
providers. Otherwise we'll get e.g. all the *other* compute nodes which
are also associated with a sharing provider.
[1] https://review.openstack.org/#/c/540111/4/specs/rocky/approved/update-provider-tree.rst@48
Closes-Bug: #1750084
(cherry picked from commit d2152f3094)
Conflicts:
nova/scheduler/client/report.py
nova/tests/functional/api/openstack/placement/test_report_client.py
nova/tests/unit/scheduler/client/test_report.py
NOTE(efried): Conflicts due to changes in this series:
https://review.openstack.org/#/q/project:openstack/nova+topic:bug/1734625+branch:master
Change-Id: Iab366da7623e5e31b8416e89fee7d418f7bf9b30
This commit is contained in:
Eric Fried
parent
e708799c04
commit
f0a722d1ef
|
|
@ -19,6 +19,7 @@ import re
|
|||
import time
|
||||
|
||||
from keystoneauth1 import exceptions as ks_exc
|
||||
import os_traits
|
||||
from oslo_log import log as logging
|
||||
from oslo_middleware import request_id
|
||||
from oslo_utils import versionutils
|
||||
|
|
@ -458,9 +459,10 @@ class SchedulerReportClient(object):
|
|||
raise exception.ResourceProviderRetrievalFailed(uuid=uuid)
|
||||
|
||||
@safe_connect
|
||||
def _get_providers_in_aggregates(self, agg_uuids):
|
||||
def _get_sharing_providers(self, agg_uuids):
|
||||
"""Queries the placement API for a list of the resource providers
|
||||
associated with any of the specified aggregates.
|
||||
associated with any of the specified aggregates and possessing the
|
||||
MISC_SHARES_VIA_AGGREGATE trait.
|
||||
|
||||
:param agg_uuids: Iterable of string UUIDs of aggregates to filter on.
|
||||
:return: A list of dicts of resource provider information, which may be
|
||||
|
|
@ -471,10 +473,16 @@ class SchedulerReportClient(object):
|
|||
return []
|
||||
|
||||
qpval = ','.join(agg_uuids)
|
||||
# TODO(efried): Need a ?having_traits=[...] on this API!
|
||||
resp = self.get("/resource_providers?member_of=in:" + qpval,
|
||||
version='1.3')
|
||||
if resp.status_code == 200:
|
||||
return resp.json()['resource_providers']
|
||||
rps = []
|
||||
for rp in resp.json()['resource_providers']:
|
||||
traits = self._get_provider_traits(rp['uuid'])
|
||||
if os_traits.MISC_SHARES_VIA_AGGREGATE in traits:
|
||||
rps.append(rp)
|
||||
return rps
|
||||
|
||||
# Some unexpected error
|
||||
placement_req_id = get_placement_request_id(resp)
|
||||
|
|
@ -769,7 +777,7 @@ class SchedulerReportClient(object):
|
|||
|
||||
if refresh_sharing:
|
||||
# Refresh providers associated by aggregate
|
||||
for rp in self._get_providers_in_aggregates(aggs):
|
||||
for rp in self._get_sharing_providers(aggs):
|
||||
if not self._provider_tree.exists(rp['uuid']):
|
||||
# NOTE(efried): Right now sharing providers are always
|
||||
# treated as roots. This is deliberate. From the
|
||||
|
|
|
|||
|
|
@ -128,7 +128,7 @@ class SchedulerReportClientTests(test.TestCase):
|
|||
# We should also have empty sets of aggregate and trait
|
||||
# associations
|
||||
self.assertEqual(
|
||||
[], self.client._get_providers_in_aggregates([uuids.agg]))
|
||||
[], self.client._get_sharing_providers([uuids.agg]))
|
||||
self.assertFalse(
|
||||
self.client._provider_tree.have_aggregates_changed(
|
||||
self.compute_uuid, []))
|
||||
|
|
|
|||
|
|
@ -1193,11 +1193,11 @@ class TestProviderOperations(SchedulerReportClientTestCase):
|
|||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_get_provider_traits')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_get_providers_in_aggregates')
|
||||
'_get_sharing_providers')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_get_providers_in_tree')
|
||||
def test_ensure_resource_provider_exists_in_cache(self, get_rpt_mock,
|
||||
get_pia_mock, get_trait_mock, get_agg_mock, create_rp_mock):
|
||||
get_shr_mock, get_trait_mock, get_agg_mock, create_rp_mock):
|
||||
# Override the client object's cache to contain a resource provider
|
||||
# object for the compute host and check that
|
||||
# _ensure_resource_provider() doesn't call _get_resource_provider() or
|
||||
|
|
@ -1219,7 +1219,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
|
|||
set(),
|
||||
set(['CUSTOM_BRONZE'])
|
||||
]
|
||||
get_pia_mock.return_value = [
|
||||
get_shr_mock.return_value = [
|
||||
{
|
||||
'uuid': uuids.shr1,
|
||||
'name': 'sharing1',
|
||||
|
|
@ -1232,7 +1232,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
|
|||
},
|
||||
]
|
||||
self.client._ensure_resource_provider(self.context, cn.uuid)
|
||||
get_pia_mock.assert_called_once_with(set([uuids.agg1, uuids.agg2]))
|
||||
get_shr_mock.assert_called_once_with(set([uuids.agg1, uuids.agg2]))
|
||||
self.assertTrue(self.client._provider_tree.exists(uuids.shr1))
|
||||
self.assertTrue(self.client._provider_tree.exists(uuids.shr2))
|
||||
# _get_provider_aggregates and _traits were called thrice: one for the
|
||||
|
|
@ -1270,11 +1270,11 @@ class TestProviderOperations(SchedulerReportClientTestCase):
|
|||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_get_provider_traits')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_get_providers_in_aggregates')
|
||||
'_get_sharing_providers')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_get_providers_in_tree')
|
||||
def test_ensure_resource_provider_get(self, get_rpt_mock, get_pia_mock,
|
||||
get_trait_mock, get_agg_mock, create_rp_mock):
|
||||
def test_ensure_resource_provider_get(self, get_rpt_mock, get_shr_mock,
|
||||
get_trait_mock, get_agg_mock, create_rp_mock):
|
||||
# No resource provider exists in the client's cache, so validate that
|
||||
# if we get the resource provider from the placement API that we don't
|
||||
# try to create the resource provider.
|
||||
|
|
@ -1286,7 +1286,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
|
|||
|
||||
get_agg_mock.return_value = set([uuids.agg1])
|
||||
get_trait_mock.return_value = set(['CUSTOM_GOLD'])
|
||||
get_pia_mock.return_value = []
|
||||
get_shr_mock.return_value = []
|
||||
|
||||
self.client._ensure_resource_provider(self.context, uuids.compute_node)
|
||||
|
||||
|
|
@ -1306,7 +1306,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
|
|||
self.assertFalse(
|
||||
self.client._provider_tree.has_traits(uuids.compute_node,
|
||||
['CUSTOM_SILVER']))
|
||||
get_pia_mock.assert_called_once_with(set([uuids.agg1]))
|
||||
get_shr_mock.assert_called_once_with(set([uuids.agg1]))
|
||||
self.assertTrue(self.client._provider_tree.exists(uuids.compute_node))
|
||||
self.assertFalse(create_rp_mock.called)
|
||||
|
||||
|
|
@ -1673,8 +1673,10 @@ class TestProviderOperations(SchedulerReportClientTestCase):
|
|||
self.assertEqual(uuids.request_id,
|
||||
logging_mock.call_args[0][1]['placement_req_id'])
|
||||
|
||||
def test_get_providers_in_aggregates(self):
|
||||
# Ensure _get_providers_in_aggregates() returns a list of resource
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_get_provider_traits')
|
||||
def test_get_sharing_providers(self, mock_get_traits):
|
||||
# Ensure _get_sharing_providers() returns a list of resource
|
||||
# provider dicts if it finds resource provider records from the
|
||||
# placement API
|
||||
resp_mock = mock.Mock(status_code=200)
|
||||
|
|
@ -1699,22 +1701,26 @@ class TestProviderOperations(SchedulerReportClientTestCase):
|
|||
resp_mock.json.return_value = {'resource_providers': rpjson}
|
||||
self.ks_adap_mock.get.return_value = resp_mock
|
||||
|
||||
result = self.client._get_providers_in_aggregates([uuids.agg1,
|
||||
uuids.agg2])
|
||||
mock_get_traits.side_effect = [
|
||||
set(['MISC_SHARES_VIA_AGGREGATE', 'CUSTOM_FOO']),
|
||||
set(['CUSTOM_BAR']),
|
||||
]
|
||||
result = self.client._get_sharing_providers([uuids.agg1, uuids.agg2])
|
||||
|
||||
expected_url = ('/resource_providers?member_of=in:' +
|
||||
','.join((uuids.agg1, uuids.agg2)))
|
||||
self.ks_adap_mock.get.assert_called_once_with(
|
||||
expected_url, raise_exc=False, microversion='1.3')
|
||||
self.assertEqual(rpjson, result)
|
||||
self.assertEqual(rpjson[:1], result)
|
||||
|
||||
def test_get_providers_in_aggregates_emptylist(self):
|
||||
self.assertEqual([], self.client._get_providers_in_aggregates([]))
|
||||
def test_get_sharing_providers_emptylist(self):
|
||||
self.assertEqual(
|
||||
[], self.client._get_sharing_providers([]))
|
||||
self.ks_adap_mock.get.assert_not_called()
|
||||
|
||||
@mock.patch.object(report.LOG, 'error')
|
||||
def test_get_providers_in_aggregates_error(self, logging_mock):
|
||||
# Ensure _get_providers_in_aggregates() logs an error and raises if the
|
||||
def test_get_sharing_providers_error(self, logging_mock):
|
||||
# Ensure _get_sharing_providers() logs an error and raises if the
|
||||
# placement API call doesn't respond 200
|
||||
resp_mock = mock.Mock(status_code=503)
|
||||
self.ks_adap_mock.get.return_value = resp_mock
|
||||
|
|
@ -1723,7 +1729,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
|
|||
|
||||
uuid = uuids.agg
|
||||
self.assertRaises(exception.ResourceProviderRetrievalFailed,
|
||||
self.client._get_providers_in_aggregates, [uuid])
|
||||
self.client._get_sharing_providers, [uuid])
|
||||
|
||||
expected_url = '/resource_providers?member_of=in:' + uuid
|
||||
self.ks_adap_mock.get.assert_called_once_with(
|
||||
|
|
@ -2224,8 +2230,8 @@ class TestAssociations(SchedulerReportClientTestCase):
|
|||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_get_provider_traits')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_get_providers_in_aggregates')
|
||||
def test_refresh_associations_no_last(self, mock_pia_get, mock_trait_get,
|
||||
'_get_sharing_providers')
|
||||
def test_refresh_associations_no_last(self, mock_shr_get, mock_trait_get,
|
||||
mock_agg_get):
|
||||
"""Test that associations are refreshed when stale."""
|
||||
uuid = uuids.compute_node
|
||||
|
|
@ -2236,7 +2242,7 @@ class TestAssociations(SchedulerReportClientTestCase):
|
|||
self.client._refresh_associations(uuid)
|
||||
mock_agg_get.assert_called_once_with(uuid)
|
||||
mock_trait_get.assert_called_once_with(uuid)
|
||||
mock_pia_get.assert_called_once_with(mock_agg_get.return_value)
|
||||
mock_shr_get.assert_called_once_with(mock_agg_get.return_value)
|
||||
self.assertIn(uuid, self.client.association_refresh_time)
|
||||
self.assertTrue(
|
||||
self.client._provider_tree.in_aggregates(uuid, [uuids.agg1]))
|
||||
|
|
@ -2252,8 +2258,8 @@ class TestAssociations(SchedulerReportClientTestCase):
|
|||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_get_provider_traits')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_get_providers_in_aggregates')
|
||||
def test_refresh_associations_no_refresh_sharing(self, mock_pia_get,
|
||||
'_get_sharing_providers')
|
||||
def test_refresh_associations_no_refresh_sharing(self, mock_shr_get,
|
||||
mock_trait_get,
|
||||
mock_agg_get):
|
||||
"""Test refresh_sharing=False."""
|
||||
|
|
@ -2265,7 +2271,7 @@ class TestAssociations(SchedulerReportClientTestCase):
|
|||
self.client._refresh_associations(uuid, refresh_sharing=False)
|
||||
mock_agg_get.assert_called_once_with(uuid)
|
||||
mock_trait_get.assert_called_once_with(uuid)
|
||||
mock_pia_get.assert_not_called()
|
||||
mock_shr_get.assert_not_called()
|
||||
self.assertIn(uuid, self.client.association_refresh_time)
|
||||
self.assertTrue(
|
||||
self.client._provider_tree.in_aggregates(uuid, [uuids.agg1]))
|
||||
|
|
@ -2281,10 +2287,10 @@ class TestAssociations(SchedulerReportClientTestCase):
|
|||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_get_provider_traits')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_get_providers_in_aggregates')
|
||||
'_get_sharing_providers')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_associations_stale')
|
||||
def test_refresh_associations_not_stale(self, mock_stale, mock_pia_get,
|
||||
def test_refresh_associations_not_stale(self, mock_stale, mock_shr_get,
|
||||
mock_trait_get, mock_agg_get):
|
||||
"""Test that refresh associations is not called when the map is
|
||||
not stale.
|
||||
|
|
@ -2294,7 +2300,7 @@ class TestAssociations(SchedulerReportClientTestCase):
|
|||
self.client._refresh_associations(uuid)
|
||||
mock_agg_get.assert_not_called()
|
||||
mock_trait_get.assert_not_called()
|
||||
mock_pia_get.assert_not_called()
|
||||
mock_shr_get.assert_not_called()
|
||||
self.assertFalse(self.client.association_refresh_time)
|
||||
|
||||
@mock.patch.object(report.LOG, 'debug')
|
||||
|
|
@ -2303,8 +2309,8 @@ class TestAssociations(SchedulerReportClientTestCase):
|
|||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_get_provider_traits')
|
||||
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
|
||||
'_get_providers_in_aggregates')
|
||||
def test_refresh_associations_time(self, mock_pia_get, mock_trait_get,
|
||||
'_get_sharing_providers')
|
||||
def test_refresh_associations_time(self, mock_shr_get, mock_trait_get,
|
||||
mock_agg_get, log_mock):
|
||||
"""Test that refresh associations is called when the map is stale."""
|
||||
uuid = uuids.compute_node
|
||||
|
|
@ -2312,14 +2318,14 @@ class TestAssociations(SchedulerReportClientTestCase):
|
|||
self.client._provider_tree.new_root('compute', uuid, 1)
|
||||
mock_agg_get.return_value = set([])
|
||||
mock_trait_get.return_value = set([])
|
||||
mock_pia_get.return_value = []
|
||||
mock_shr_get.return_value = []
|
||||
|
||||
# Called a first time because association_refresh_time is empty.
|
||||
now = time.time()
|
||||
self.client._refresh_associations(uuid)
|
||||
mock_agg_get.assert_called_once_with(uuid)
|
||||
mock_trait_get.assert_called_once_with(uuid)
|
||||
mock_pia_get.assert_called_once_with(set())
|
||||
mock_shr_get.assert_called_once_with(set())
|
||||
log_mock.assert_has_calls([
|
||||
mock.call('Refreshing aggregate associations for resource '
|
||||
'provider %s, aggregates: %s', uuid, 'None'),
|
||||
|
|
@ -2331,7 +2337,7 @@ class TestAssociations(SchedulerReportClientTestCase):
|
|||
# Clear call count.
|
||||
mock_agg_get.reset_mock()
|
||||
mock_trait_get.reset_mock()
|
||||
mock_pia_get.reset_mock()
|
||||
mock_shr_get.reset_mock()
|
||||
|
||||
with mock.patch('time.time') as mock_future:
|
||||
# Not called a second time because not enough time has passed.
|
||||
|
|
@ -2339,14 +2345,14 @@ class TestAssociations(SchedulerReportClientTestCase):
|
|||
self.client._refresh_associations(uuid)
|
||||
mock_agg_get.assert_not_called()
|
||||
mock_trait_get.assert_not_called()
|
||||
mock_pia_get.assert_not_called()
|
||||
mock_shr_get.assert_not_called()
|
||||
|
||||
# Called because time has passed.
|
||||
mock_future.return_value = now + report.ASSOCIATION_REFRESH + 1
|
||||
self.client._refresh_associations(uuid)
|
||||
mock_agg_get.assert_called_once_with(uuid)
|
||||
mock_trait_get.assert_called_once_with(uuid)
|
||||
mock_pia_get.assert_called_once_with(set())
|
||||
mock_shr_get.assert_called_once_with(set())
|
||||
|
||||
|
||||
class TestComputeNodeToInventoryDict(test.NoDBTestCase):
|
||||
|
|
|
|||
Loading…
Reference in New Issue