Skip to content

Commit 77e7df2

Browse files
priteaupriteau
authored
Merge pull request #85 from stackhpc/upstream/2023.1-2024-10-21
Synchronise 2023.1 with upstream
2 parents 07bb6fa + 0869bf1 commit 77e7df2

File tree

6 files changed

+135
-51
lines changed

6 files changed

+135
-51
lines changed

‎ironic_python_agent/hardware.py‎

Lines changed: 40 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2958,46 +2958,65 @@ def _collect_udev(io_dict):
29582958
io_dict[f'udev/{kname}'] = buf
29592959

29602960

2961-
def _compare_extensions(ext1, ext2):
2962-
mgr1 = ext1.obj
2963-
mgr2 = ext2.obj
2964-
return mgr2.evaluate_hardware_support() - mgr1.evaluate_hardware_support()
2961+
def _compare_managers(hwm1, hwm2):
2962+
return hwm2['support'] - hwm1['support']
2963+
2964+
2965+
def _get_extensions():
2966+
return stevedore.ExtensionManager(
2967+
namespace='ironic_python_agent.hardware_managers',
2968+
invoke_on_load=True
2969+
)
29652970

29662971

29672972
def get_managers():
29682973
"""Get a list of hardware managers in priority order.
29692974
2970-
Use stevedore to find all eligible hardware managers, sort them based on
2971-
self-reported (via evaluate_hardware_support()) priorities, and return them
2972-
in a list. The resulting list is cached in _global_managers.
2975+
This exists as a backwards compatibility shim, returning a simple list
2976+
of managers where expected. New usages should use get_managers_detail.
29732977
29742978
:returns: Priority-sorted list of hardware managers
29752979
:raises HardwareManagerNotFound: if no valid hardware managers found
29762980
"""
2981+
return [hwm['manager'] for hwm in get_managers_detail()]
2982+
2983+
2984+
def get_managers_detail():
2985+
"""Get detailed information about hardware managers
2986+
2987+
Use stevedore to find all eligible hardware managers, sort them based on
2988+
self-reported (via evaluate_hardware_support()) priorities, and return a
2989+
dict containing the manager object, it's class name, and hardware support
2990+
value. The resulting list is cached in _global_managers.
2991+
2992+
:returns: list of dictionaries representing hardware managers and metadata
2993+
:raises HardwareManagerNotFound: if no valid hardware managers found
2994+
"""
29772995
global _global_managers
29782996

29792997
if not _global_managers:
2980-
extension_manager = stevedore.ExtensionManager(
2981-
namespace='ironic_python_agent.hardware_managers',
2982-
invoke_on_load=True)
2983-
2984-
# There will always be at least one extension available (the
2985-
# GenericHardwareManager).
2986-
extensions = sorted(extension_manager,
2987-
key=functools.cmp_to_key(_compare_extensions))
29882998

29892999
preferred_managers = []
29903000

2991-
for extension in extensions:
2992-
if extension.obj.evaluate_hardware_support() > 0:
2993-
preferred_managers.append(extension.obj)
3001+
for extension in _get_extensions():
3002+
hwm = extension.obj
3003+
hardware_support = hwm.evaluate_hardware_support()
3004+
if hardware_support > 0:
3005+
preferred_managers.append({
3006+
'name': hwm.__class__.__name__,
3007+
'manager': hwm,
3008+
'support': hardware_support
3009+
})
29943010
LOG.info('Hardware manager found: %s',
29953011
extension.entry_point_target)
29963012

29973013
if not preferred_managers:
29983014
raise errors.HardwareManagerNotFound
29993015

3000-
_global_managers = preferred_managers
3016+
hwms = sorted(preferred_managers,
3017+
key=functools.cmp_to_key(_compare_managers))
3018+
3019+
_global_managers = hwms
30013020

30023021
return _global_managers
30033022

@@ -3191,20 +3210,14 @@ def deduplicate_steps(candidate_steps):
31913210
all managers, key=manager, value=list of steps
31923211
:returns: A deduplicated dictionary of {hardware_manager: [steps]}
31933212
"""
3194-
support = dispatch_to_all_managers(
3195-
'evaluate_hardware_support')
3213+
support = {hwm['name']: hwm['support']
3214+
for hwm in get_managers_detail()}
31963215

31973216
steps = collections.defaultdict(list)
31983217
deduped_steps = collections.defaultdict(list)
31993218

32003219
for manager, manager_steps in candidate_steps.items():
32013220
# We cannot deduplicate steps with unknown hardware support
3202-
if manager not in support:
3203-
LOG.warning('Unknown hardware support for %(manager)s, '
3204-
'dropping steps: %(steps)s',
3205-
{'manager': manager, 'steps': manager_steps})
3206-
continue
3207-
32083221
for step in manager_steps:
32093222
# build a new dict of steps that's easier to filter
32103223
step['hwm'] = {'name': manager,

‎ironic_python_agent/tests/unit/base.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ def setUp(self):
6565

6666
ext_base._EXT_MANAGER = None
6767
hardware._CACHED_HW_INFO = None
68+
hardware._global_managers = None
6869

6970
def _set_config(self):
7071
self.cfg_fixture = self.useFixture(config_fixture.Config(CONF))

‎ironic_python_agent/tests/unit/extensions/test_clean.py‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,14 @@ def setUp(self):
3434
}
3535
self.version = {'generic': '1', 'specific': '1'}
3636

37+
@mock.patch('ironic_python_agent.hardware.get_managers_detail',
38+
autospec=True)
3739
@mock.patch('ironic_python_agent.hardware.get_current_versions',
3840
autospec=True)
3941
@mock.patch('ironic_python_agent.hardware.dispatch_to_all_managers',
4042
autospec=True)
4143
def test_get_clean_steps(self, mock_dispatch, mock_version,
42-
mock_cache_node):
44+
mock_managers, mock_cache_node):
4345
mock_version.return_value = self.version
4446

4547
manager_steps = {
@@ -118,13 +120,17 @@ def test_get_clean_steps(self, mock_dispatch, mock_version,
118120

119121
}
120122

121-
hardware_support = {
122-
'SpecificHardwareManager': 3,
123-
'FirmwareHardwareManager': 4,
124-
'DiskHardwareManager': 4
125-
}
123+
# NOTE(JayF): The real dict also has manager: hwm-object
124+
# but we don't use it in the code under test
125+
hwms = [
126+
{'name': 'SpecificHardwareManager', 'support': 3},
127+
{'name': 'FirmwareHardwareManager', 'support': 4},
128+
{'name': 'DiskHardwareManager', 'support': 4},
129+
]
130+
131+
mock_dispatch.side_effect = [manager_steps]
132+
mock_managers.return_value = hwms
126133

127-
mock_dispatch.side_effect = [manager_steps, hardware_support]
128134
expected_return = {
129135
'hardware_manager_version': self.version,
130136
'clean_steps': expected_steps

‎ironic_python_agent/tests/unit/extensions/test_deploy.py‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,14 @@ def setUp(self):
3232
}
3333
self.version = {'generic': '1', 'specific': '1'}
3434

35+
@mock.patch('ironic_python_agent.hardware.get_managers_detail',
36+
autospec=True)
3537
@mock.patch('ironic_python_agent.hardware.get_current_versions',
3638
autospec=True)
3739
@mock.patch('ironic_python_agent.hardware.dispatch_to_all_managers',
3840
autospec=True)
3941
def test_get_deploy_steps(self, mock_dispatch, mock_version,
40-
mock_cache_node):
42+
mock_managers, mock_cache_node):
4143
mock_version.return_value = self.version
4244

4345
manager_steps = {
@@ -116,13 +118,17 @@ def test_get_deploy_steps(self, mock_dispatch, mock_version,
116118

117119
}
118120

119-
hardware_support = {
120-
'SpecificHardwareManager': 3,
121-
'FirmwareHardwareManager': 4,
122-
'DiskHardwareManager': 4
123-
}
121+
# NOTE(JayF): The real dict also has manager: hwm-object
122+
# but we don't use it in the code under test
123+
hwms = [
124+
{'name': 'SpecificHardwareManager', 'support': 3},
125+
{'name': 'FirmwareHardwareManager', 'support': 4},
126+
{'name': 'DiskHardwareManager', 'support': 4},
127+
]
128+
129+
mock_dispatch.side_effect = [manager_steps]
130+
mock_managers.return_value = hwms
124131

125-
mock_dispatch.side_effect = [manager_steps, hardware_support]
126132
expected_return = {
127133
'hardware_manager_version': self.version,
128134
'deploy_steps': expected_steps

‎ironic_python_agent/tests/unit/test_hardware.py‎

Lines changed: 61 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -81,12 +81,22 @@
8181
]
8282

8383

84-
class FakeHardwareManager(hardware.GenericHardwareManager):
85-
def __init__(self, hardware_support):
86-
self._hardware_support = hardware_support
87-
84+
class FakeHardwareManager(hardware.HardwareManager):
8885
def evaluate_hardware_support(self):
89-
return self._hardware_support
86+
return self.support
87+
88+
89+
def _create_mock_hwm(name, support):
90+
def set_support(self, x):
91+
self.support = x
92+
93+
# note(JayF): This code creates a subclass of FakeHardwareManager with
94+
# a unique name. Since we actually use the class name in IPA
95+
# code as an identifier, we need to have a new class for each
96+
# mock.
97+
hwm = type(name, (FakeHardwareManager,), {'_set_support': set_support})()
98+
hwm._set_support(support)
99+
return hwm
90100

91101

92102
class TestHardwareManagerLoading(base.IronicAgentTest):
@@ -104,17 +114,58 @@ def setUp(self):
104114
fake_ep.attrs = ['fake attrs']
105115
ext1 = extension.Extension(
106116
'fake_generic0', fake_ep, None,
107-
FakeHardwareManager(hardware.HardwareSupport.GENERIC))
117+
_create_mock_hwm("fake_generic0",
118+
hardware.HardwareSupport.GENERIC))
108119
ext2 = extension.Extension(
109120
'fake_mainline0', fake_ep, None,
110-
FakeHardwareManager(hardware.HardwareSupport.MAINLINE))
121+
_create_mock_hwm("fake_mainline0",
122+
hardware.HardwareSupport.MAINLINE))
111123
ext3 = extension.Extension(
112-
'fake_generic1', fake_ep, None,
113-
FakeHardwareManager(hardware.HardwareSupport.GENERIC))
114-
self.correct_hw_manager = ext2.obj
124+
'fake_serviceprovider0', fake_ep, None,
125+
_create_mock_hwm("fake_serviceprovider0",
126+
hardware.HardwareSupport.SERVICE_PROVIDER))
127+
# Note(JayF): Ensure these are added in an order other than priority
128+
# order or else you may invalidate the entire test :)
115129
self.fake_ext_mgr = extension.ExtensionManager.make_test_instance([
116130
ext1, ext2, ext3
117131
])
132+
self.expected_detail_response = [
133+
{'name': 'fake_serviceprovider0',
134+
'support': hardware.HardwareSupport.SERVICE_PROVIDER,
135+
'manager': ext3.obj},
136+
{'name': 'fake_mainline0',
137+
'support': hardware.HardwareSupport.MAINLINE,
138+
'manager': ext2.obj},
139+
{'name': 'fake_generic0',
140+
'support': hardware.HardwareSupport.GENERIC,
141+
'manager': ext1.obj},
142+
]
143+
self.expected_get_managers_response = [ext3.obj, ext2.obj, ext1.obj]
144+
145+
@mock.patch.object(hardware, '_get_extensions', autospec=True)
146+
def test_get_managers(self, mock_extensions):
147+
"""Test to ensure get_managers sorts and returns a list of HWMs.
148+
149+
The most meaningful part of this test is ensuring HWMs are in priority
150+
order, with the highest hardware support value coming earlier in the
151+
list of classes.
152+
"""
153+
mock_extensions.return_value = self.fake_ext_mgr
154+
expected_names = [x.__class__.__name__
155+
for x in self.expected_get_managers_response]
156+
actual_names = [x.__class__.__name__
157+
for x in hardware.get_managers()]
158+
self.assertEqual(actual_names, expected_names)
159+
160+
@mock.patch.object(hardware, '_get_extensions', autospec=True)
161+
def test_get_managers_detail(self, mock_extensions):
162+
"""ensure get_manager_details returns a list of HWMs + metadata
163+
164+
These also need to be sorted in priority order
165+
"""
166+
mock_extensions.return_value = self.fake_ext_mgr
167+
self.assertEqual(hardware.get_managers_detail(),
168+
self.expected_detail_response)
118169

119170

120171
@mock.patch.object(hardware, '_udev_settle', lambda *_: None)

‎releasenotes/notes/only-run-evaluate-hardware-support-once-9ec1ae327b4e03f2.yaml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
fixes:
3+
- |
4+
Fixes bug 2066308, an issue where Ironic Python Agent would call
5+
evaluate_hardware_support multiple times on hardware manager plugins.
6+
Scanning for hardware and disks is time consuming, and caused timeouts
7+
on badly-performing nodes.

0 commit comments

Comments
 (0)