openstack/ironic-python-agent
Code Issues Proposed changes

Make inspection URL optional if the collectors are provided

Browse Source 
With the new in-band inspection, we can derive the callback URL from
the Ironic URL, there is no need to duplicate it. This change uses
the presence of collectors as a sign to run inspection.

The previous approach of setting an inspection URL, with or without
explicitly setting collectors, still works for compatibility with
ironic-inspector.

Change-Id: Ie4279ee6d2995c9686f1dcdef1d6e5dc1dd20871
This commit is contained in:
Dmitry Tantsur 2023-12-19 19:30:16 +01:00
parent 0d4ae976c2
commit 6cd36a750f
No known key found for this signature in database
GPG Key ID: 315B2AF9FD216C60
6 changed files with 122 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
doc/source/admin/how_it_works.rst
View File

@ -49,6 +49,18 @@ for example::
Make sure your DHCP environment is set to boot IPA by default.
If you use the new built-in :ironic-doc:`Ironic in-band inspection
<admin/inspection/index.html>`, it is enough to only set a list of collectors
(see `inspection data`_), for example::
ipa-inspection-collectors=default,logs
Then the correct callback URL will be determined from the Ironic URL in
``ipa-api-url``.
Instance agent
~~~~~~~~~~~~~~
For the cases where the infrastructure operator and cloud user are the same,
an additional tool exists that can be installed alongside the agent inside
a running instance. This is the ``ironic-collect-introspection-data``

2
ironic_python_agent/agent.py
View File

@ -526,7 +526,7 @@ class IronicPythonAgent(base.ExecuteCommandMixin):
# interfaces to perform those actions over.
self._wait_for_interface()
if cfg.CONF.inspection_callback_url:
if self.api_urls or cfg.CONF.inspection_callback_url:
try:
# Attempt inspection. This may fail, and previously
# an error would be logged.

15
ironic_python_agent/config.py
View File

@ -23,7 +23,7 @@ CONF = cfg.CONF
APARAMS = utils.get_agent_params()
INSPECTION_DEFAULT_COLLECTOR = 'default,logs'
INSPECTION_DEFAULT_COLLECTORS = 'default,logs'
INSPECTION_DEFAULT_DHCP_WAIT_TIMEOUT = 60
cli_opts = [
@ -162,16 +162,17 @@ cli_opts = [
'A special value "mdns" can be specified to fetch the '
'URL using multicast DNS service discovery. '
'Can be supplied as "ipa-inspection-callback-url" '
'kernel parameter.'),
'kernel parameter. If the URL is not provided but '
'inspection_collectors is not empty, the URL is detected '
'from api_url.'),
cfg.StrOpt('inspection_collectors',
default=APARAMS.get('ipa-inspection-collectors',
INSPECTION_DEFAULT_COLLECTOR),
default=APARAMS.get('ipa-inspection-collectors'),
help='Comma-separated list of plugins providing additional '
'hardware data for inspection, empty value gives '
'a minimum required set of plugins. '
'hardware data for inspection. The default are %s. '
'Can be supplied as "ipa-inspection-collectors" '
'kernel parameter.'),
'kernel parameter.'
% INSPECTION_DEFAULT_COLLECTORS),
cfg.IntOpt('inspection_dhcp_wait_timeout',
min=0,

18
ironic_python_agent/inspector.py
View File

@ -52,8 +52,9 @@ def extension_manager(names):
def _get_collector_names():
return [x.strip() for x in CONF.inspection_collectors.split(',')
if x.strip()]
collectors = (CONF.inspection_collectors
or config.INSPECTION_DEFAULT_COLLECTORS)
return [x.strip() for x in collectors.split(',') if x.strip()]
def inspect():
@ -66,7 +67,8 @@ def inspect():
was not found in inspector cache. None is also returned if
inspector support is not enabled.
"""
if not CONF.inspection_callback_url:
if (not CONF.inspection_callback_url
and not (CONF.inspection_collectors and CONF.api_url)):
LOG.info('Inspection is disabled, skipping')
return
@ -121,6 +123,14 @@ _RETRY_WAIT_MAX = 30
_RETRY_ATTEMPTS = 5
def _get_urls():
urls = CONF.inspection_callback_url or CONF.api_url
urls = list(filter(None, urls.split(',')))
if not CONF.inspection_callback_url:
urls = [url.rstrip('/') + "/v1/continue_inspection" for url in urls]
return urls
def call_inspector(data, failures):
"""Post data to inspector."""
data['error'] = failures.get_error()
@ -139,7 +149,7 @@ def call_inspector(data, failures):
if CONF.global_request_id:
headers["X-OpenStack-Request-ID"] = CONF.global_request_id
urls = list(filter(None, CONF.inspection_callback_url.split(',')))
urls = _get_urls()
@tenacity.retry(
retry=tenacity.retry_if_exception_type(

76
ironic_python_agent/tests/unit/test_inspector.py
View File

@ -49,7 +49,7 @@ class AcceptingFailure(mock.Mock):
class TestMisc(base.IronicAgentTest):
def test_default_collector_loadable(self):
defaults = config.INSPECTION_DEFAULT_COLLECTOR.split(',')
defaults = config.INSPECTION_DEFAULT_COLLECTORS.split(',')
# default should go first
self.assertEqual('default', defaults[0])
# logs much go last
@ -71,7 +71,6 @@ class TestInspect(base.IronicAgentTest):
def setUp(self):
super(TestInspect, self).setUp()
CONF.set_override('inspection_callback_url', 'http://foo/bar')
CONF.set_override('inspection_collectors', '')
self.mock_collect = AcceptingFailure()
self.mock_ext = mock.Mock(spec=['plugin', 'name'],
plugin=self.mock_collect)
@ -85,6 +84,35 @@ class TestInspect(base.IronicAgentTest):
self.mock_collect.assert_called_with_failure()
mock_call.assert_called_with_failure()
self.assertEqual('uuid1', result)
mock_ext_mgr.assert_called_once_with(
inspector._COLLECTOR_NS, ['default', 'logs'],
name_order=True, on_missing_entrypoints_callback=mock.ANY)
def test_ok_with_ironic_url(self, mock_ext_mgr, mock_call):
CONF.set_override('api_url', 'http://url')
CONF.set_override('inspection_callback_url', '')
CONF.set_override('inspection_collectors', 'default')
mock_ext_mgr.return_value = [self.mock_ext]
mock_call.return_value = {'uuid': 'uuid1'}
result = inspector.inspect()
self.mock_collect.assert_called_with_failure()
mock_call.assert_called_with_failure()
self.assertEqual('uuid1', result)
mock_ext_mgr.assert_called_once_with(
inspector._COLLECTOR_NS, ['default'],
name_order=True, on_missing_entrypoints_callback=mock.ANY)
def test_disabled(self, mock_ext_mgr, mock_call):
CONF.set_override('inspection_callback_url', '')
mock_ext_mgr.return_value = [self.mock_ext]
result = inspector.inspect()
self.mock_collect.assert_not_called()
mock_call.assert_not_called()
self.assertIsNone(result)
@mock.patch('ironic_lib.mdns.get_endpoint', autospec=True)
def test_mdns(self, mock_mdns, mock_ext_mgr, mock_call):
@ -169,6 +197,25 @@ class TestCallInspector(base.IronicAgentTest):
timeout=30)
self.assertEqual(mock_post.return_value.json.return_value, res)
def test_use_api_url(self, mock_post):
CONF.set_override('inspection_callback_url', '')
CONF.set_override('api_url', 'http://url1/,http://url2/baremetal')
failures = utils.AccumulatedFailures()
data = collections.OrderedDict(data=42)
mock_post.return_value.status_code = 200
res = inspector.call_inspector(data, failures)
mock_post.assert_called_once_with(
'http://url1/v1/continue_inspection',
data='{"data": 42, "error": null}',
cert=None, verify=True,
headers={'Content-Type': 'application/json',
'Accept': 'application/json'},
timeout=30)
self.assertEqual(mock_post.return_value.json.return_value, res)
def test_send_failure(self, mock_post):
failures = mock.Mock(spec=utils.AccumulatedFailures)
failures.get_error.return_value = "boom"
@ -231,6 +278,31 @@ class TestCallInspector(base.IronicAgentTest):
data='{"data": 42, "error": null}', timeout=30),
])
def test_use_several_api_urls(self, mock_post):
CONF.set_override('inspection_callback_url', '')
CONF.set_override('api_url', 'http://url1/,http://url2/baremetal')
good_resp = mock.Mock(status_code=200)
mock_post.side_effect = [
requests.exceptions.ConnectionError, good_resp
]
failures = utils.AccumulatedFailures()
data = collections.OrderedDict(data=42)
mock_post.return_value.status_code = 200
res = inspector.call_inspector(data, failures)
mock_post.assert_has_calls([
mock.call('http://url1/v1/continue_inspection',
cert=None, verify=True, headers=mock.ANY,
data='{"data": 42, "error": null}', timeout=30),
mock.call('http://url2/baremetal/v1/continue_inspection',
cert=None, verify=True, headers=mock.ANY,
data='{"data": 42, "error": null}', timeout=30),
])
self.assertEqual(good_resp.json.return_value, res)
@mock.patch.object(inspector, '_RETRY_WAIT', 0.01)
@mock.patch.object(inspector, '_RETRY_WAIT_MAX', 1)
@mock.patch.object(inspector, '_RETRY_ATTEMPTS', 3)

13
releasenotes/notes/inspect-url-15547d48432cd2b5.yaml Normal file
View File

@ -0,0 +1,13 @@
---
features:
- |
When the new Ironic built-in inspection is used,
``ipa-inspection-callback-url`` can now be automatically derived from
``ipa-api-url``. In this case, inspection will be enabled if the
``ipa-inspection-collectors`` option is set.
upgrade:
- |
If you currently set the ``ipa-inspection-collectors`` option without
setting ``ipa-inspection-callback-url``, it will now cause inspection
to run. Update your boot configuration to only supply the collectors
when inspection is desired.