Skip to content

Commit

Permalink
Fix {min|max}_version in ironic Adapter setup
Browse files Browse the repository at this point in the history
Change If625411f40be0ba642baeb02950f568f43673655 introduced
nova.utils.get_ksa_adapter, which accepts min_version and max_version
kwargs to be passed through to the ksa Adapter constructor. These are
supposed to represent minimum and maximum *major* API versions,
min_version was erroneously set to *microversions* when setting up the
Adapter for ironicclient. This commit changes it to a major version.
(Microversion negotiation is done within ironicclient itself.)

Also, this bug went latent for several releases because a) it only seems
to be triggered when region_name is given in the conf; but also b)
ironicclient has code to discover a reasonable endpoint if passed None.
So this change also adds a warning log if we try and fail to discover
the endpoint via ksa.

Conflicts:
	nova/tests/unit/virt/ironic/test_client_wrapper.py
This was just because the old microversion was 1.38 instead of 1.46. The
patch still changes it to 1.0.

Change-Id: I34a3f8d4a496217eb01790e2d124111625bf5f85
Closes-Bug: #1825583
(cherry picked from commit 13278be)
(cherry picked from commit e6ca383)
  • Loading branch information
Eric Fried committed Apr 25, 2019
1 parent d8c5a2f commit 35bda4e
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 3 deletions.
4 changes: 2 additions & 2 deletions nova/tests/unit/virt/ironic/test_client_wrapper.py
Expand Up @@ -81,7 +81,7 @@ def test__get_client_session(self, mock_ir_cli, mock_session):
# nova.utils.get_ksa_adapter().get_endpoint()
self.get_ksa_adapter.assert_called_once_with(
'baremetal', ksa_auth=self.get_auth_plugin.return_value,
ksa_session='session', min_version=(1, 38),
ksa_session='session', min_version=(1, 0),
max_version=(1, ksa_disc.LATEST))
expected = {'session': 'session',
'max_retries': CONF.ironic.api_max_retries,
Expand All @@ -107,7 +107,7 @@ def test__get_client_session_service_not_found(self, mock_ir_cli,
# nova.utils.get_endpoint_data
self.get_ksa_adapter.assert_called_once_with(
'baremetal', ksa_auth=self.get_auth_plugin.return_value,
ksa_session='session', min_version=(1, 38),
ksa_session='session', min_version=(1, 0),
max_version=(1, ksa_disc.LATEST))
# When get_endpoint_data raises any ServiceNotFound, None is returned.
expected = {'session': 'session',
Expand Down
8 changes: 7 additions & 1 deletion nova/virt/ironic/client_wrapper.py
Expand Up @@ -117,14 +117,20 @@ def _get_client(self, retry_on_conflict=True):
ksa_adap = utils.get_ksa_adapter(
nova.conf.ironic.DEFAULT_SERVICE_TYPE,
ksa_auth=auth_plugin, ksa_session=sess,
min_version=IRONIC_API_VERSION,
min_version=(IRONIC_API_VERSION[0], 0),
max_version=(IRONIC_API_VERSION[0], ks_disc.LATEST))
ironic_url = ksa_adap.get_endpoint()
ironic_url_none_reason = 'returned None'
except exception.ServiceNotFound:
# NOTE(efried): No reason to believe service catalog lookup
# won't also fail in ironic client init, but this way will
# yield the expected exception/behavior.
ironic_url = None
ironic_url_none_reason = 'raised ServiceNotFound'

if ironic_url is None:
LOG.warning("Could not discover ironic_url via keystoneauth1: "
"Adapter.get_endpoint %s", ironic_url_none_reason)

try:
cli = ironic.client.get_client(IRONIC_API_VERSION[0],
Expand Down

0 comments on commit 35bda4e

Please sign in to comment.