openstack/ironic
Code Issues Proposed changes

Receive and store agent version on heartbeat

Browse Source 
This patch enables receiving agent_version as part of heartbeat, and
stores this information on driver_internal_info. This is so that Ironic
can dynamically adjust which features and parameters it uses based on
which version of the agent is being used.

Change-Id: I400adba5d908b657751a83971811e8586f46c673
Partial-Bug: #1602265
This commit is contained in:
Sam Betts 2016-09-02 12:45:26 +01:00
parent 5f563d924c
commit b642f28be4
17 changed files with 232 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
api-ref/source/baremetal-api-v1-misc.inc
View File

@ -95,6 +95,9 @@ Normal response codes: 202
Error response codes: 400 404
.. versionadded:: 1.36 ``agent_version`` parameter for passing the version of
the Ironic Python Agent to Ironic during heartbeat
Request
-------
@ -102,3 +105,4 @@ Request
- node_ident: node_ident
- callback_url: callback_url
- agent_version: agent_version

7
api-ref/source/parameters.yaml
View File

@ -87,6 +87,13 @@ volume_target_id:
required: true
type: string
agent_version:
description: |
The version of the ironic-python-agent ramdisk, sent back to the Bare Metal
service and stored during provisioning.
in: query
required: true
type: string
callback_url:
description: |
The URL of an active ironic-python-agent ramdisk, sent back to the Bare

6
doc/source/contributor/webapi-version-history.rst
View File

@ -2,6 +2,12 @@
REST API Version History
========================
1.36 (Queens, 10.0.0)
---------------------
Added ``agent_version`` parameter to deploy heartbeat request for version
negotiation with Ironic Python Agent features.
1.35 (Queens, 9.2.0)
---------------------

19
ironic/api/controllers/v1/ramdisk.py
View File

@ -152,12 +152,17 @@ class HeartbeatController(rest.RestController):
"""Controller handling heartbeats from deploy ramdisk."""
@expose.expose(None, types.uuid_or_name, wtypes.text,
status_code=http_client.ACCEPTED)
def post(self, node_ident, callback_url):
wtypes.text, status_code=http_client.ACCEPTED)
def post(self, node_ident, callback_url, agent_version=None):
"""Process a heartbeat from the deploy ramdisk.
:param node_ident: the UUID or logical name of a node.
:param callback_url: the URL to reach back to the ramdisk.
:param agent_version: The version of the agent that is heartbeating.
``None`` indicates that the agent that is heartbeating is a version
before sending agent_version was introduced so agent v3.0.0 (the
last release before sending agent_version was introduced) will be
assumed.
:raises: NodeNotFound if node with provided UUID or name was not found.
:raises: InvalidUuidOrName if node_ident is not valid name or UUID.
:raises: NoValidHost if RPC topic for node could not be retrieved.
@ -167,6 +172,10 @@ class HeartbeatController(rest.RestController):
if not api_utils.allow_ramdisk_endpoints():
raise exception.NotFound()
if agent_version and not api_utils.allow_agent_version_in_heartbeat():
raise exception.InvalidParameterValue(
_('Field "agent_version" not recognised'))
cdict = pecan.request.context.to_policy_values()
policy.authorize('baremetal:node:ipa_heartbeat', cdict, cdict)
@ -178,6 +187,6 @@ class HeartbeatController(rest.RestController):
e.code = http_client.BAD_REQUEST
raise
pecan.request.rpcapi.heartbeat(pecan.request.context,
rpc_node.uuid, callback_url,
topic=topic)
pecan.request.rpcapi.heartbeat(
pecan.request.context, rpc_node.uuid, callback_url,
agent_version, topic=topic)

10
ironic/api/controllers/v1/utils.py
View File

@ -602,6 +602,16 @@ def allow_node_rebuild_with_configdrive():
versions.MINOR_35_REBUILD_CONFIG_DRIVE)
def allow_agent_version_in_heartbeat():
"""Check if agent version is allowed to be passed into heartbeat.
Version 1.36 of the API added the ability for agents to pass their version
information to Ironic on heartbeat.
"""
return (pecan.request.version.minor >=
versions.MINOR_36_AGENT_VERSION_HEARTBEAT)
def get_controller_reserved_names(cls):
"""Get reserved names for a given controller.

4
ironic/api/controllers/v1/versions.py
View File

@ -72,6 +72,7 @@ BASE_VERSION = 1
# v1.33: Add node storage interface
# v1.34: Add physical network field to port.
# v1.35: Add ability to provide configdrive when rebuilding node.
# v1.36: Add Ironic Python Agent version support.
MINOR_0_JUNO = 0
MINOR_1_INITIAL_VERSION = 1
@ -109,6 +110,7 @@ MINOR_32_VOLUME = 32
MINOR_33_STORAGE_INTERFACE = 33
MINOR_34_PORT_PHYSICAL_NETWORK = 34
MINOR_35_REBUILD_CONFIG_DRIVE = 35
MINOR_36_AGENT_VERSION_HEARTBEAT = 36
# When adding another version, update:
# - MINOR_MAX_VERSION
@ -116,7 +118,7 @@ MINOR_35_REBUILD_CONFIG_DRIVE = 35
# explanation of what changed in the new version
# - common/release_mappings.py, RELEASE_MAPPING['master']['api']
MINOR_MAX_VERSION = MINOR_35_REBUILD_CONFIG_DRIVE
MINOR_MAX_VERSION = MINOR_36_AGENT_VERSION_HEARTBEAT
# String representations of the minor and maximum versions
_MIN_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_1_INITIAL_VERSION)

4
ironic/common/release_mappings.py
View File

@ -108,8 +108,8 @@ RELEASE_MAPPING = {
}
},
'master': {
'api': '1.35',
'rpc': '1.41',
'api': '1.36',
'rpc': '1.42',
'objects': {
'Node': ['1.21'],
'Conductor': ['1.2'],

41
ironic/conductor/manager.py
View File

@ -82,6 +82,11 @@ METRICS = metrics_utils.get_metrics_logger(__name__)
SYNC_EXCLUDED_STATES = (states.DEPLOYWAIT, states.CLEANWAIT, states.ENROLL)
# NOTE(sambetts) This list is used to keep track of deprecation warnings that
# have already been issued for deploy drivers that do not accept the
# agent_version parameter and need updating.
_SEEN_AGENT_VERSION_DEPRECATIONS = []
class ConductorManager(base_manager.BaseConductorManager):
"""Ironic Conductor manager main class."""
@ -89,7 +94,7 @@ class ConductorManager(base_manager.BaseConductorManager):
# NOTE(rloo): This must be in sync with rpcapi.ConductorAPI's.
# NOTE(pas-ha): This also must be in sync with
# ironic.common.release_mappings.RELEASE_MAPPING['master']
RPC_API_VERSION = '1.41'
RPC_API_VERSION = '1.42'
target = messaging.Target(version=RPC_API_VERSION)
@ -2549,23 +2554,51 @@ class ConductorManager(base_manager.BaseConductorManager):
@METRICS.timer('ConductorManager.heartbeat')
@messaging.expected_exceptions(exception.NoFreeConductorWorker)
def heartbeat(self, context, node_id, callback_url):
def heartbeat(self, context, node_id, callback_url, agent_version=None):
"""Process a heartbeat from the ramdisk.
:param context: request context.
:param node_id: node id or uuid.
:param agent_version: The version of the agent that is heartbeating. If
not provided it either indicates that the agent that is
heartbeating is a version before sending agent_version was
introduced or that we're in the middle of a rolling upgrade and the
RPC version is pinned so the API isn't passing us the
agent_version, in these cases assume agent v3.0.0 (the last release
before sending agent_version was introduced).
:param callback_url: URL to reach back to the ramdisk.
:raises: NoFreeConductorWorker if there are no conductors to process
this heartbeat request.
"""
LOG.debug('RPC heartbeat called for node %s', node_id)
if agent_version is None:
agent_version = '3.0.0'
def heartbeat_with_deprecation(task, callback_url, agent_version):
global _SEEN_AGENT_VERSION_DEPRECATIONS
# FIXME(sambetts) Remove this try/except statement in Rocky making
# taking the agent_version in the deploy driver heartbeat function
# mandatory.
try:
task.driver.deploy.heartbeat(task, callback_url, agent_version)
except TypeError:
deploy_driver_name = task.driver.deploy.__class__.__name__
if deploy_driver_name not in _SEEN_AGENT_VERSION_DEPRECATIONS:
LOG.warning('Deploy driver %s does not support '
'agent_version as part of the heartbeat '
'request, this will be required from Rocky '
'onward.', deploy_driver_name)
_SEEN_AGENT_VERSION_DEPRECATIONS.append(deploy_driver_name)
task.driver.deploy.heartbeat(task, callback_url)
# NOTE(dtantsur): we acquire a shared lock to begin with, drivers are
# free to promote it to an exclusive one.
with task_manager.acquire(context, node_id, shared=True,
purpose='heartbeat') as task:
task.spawn_after(self._spawn_worker, task.driver.deploy.heartbeat,
task, callback_url)
task.spawn_after(
self._spawn_worker, heartbeat_with_deprecation,
task, callback_url, agent_version)
@METRICS.timer('ConductorManager.vif_list')
@messaging.expected_exceptions(exception.NetworkError,

16
ironic/conductor/rpcapi.py
View File

@ -90,13 +90,14 @@ class ConductorAPI(object):
| 1.39 - Added timeout optional parameter to change_node_power_state
| 1.40 - Added inject_nmi
| 1.41 - Added create_port
| 1.42 - Added optional agent_version to heartbeat
"""
# NOTE(rloo): This must be in sync with manager.ConductorManager's.
# NOTE(pas-ha): This also must be in sync with
# ironic.common.release_mappings.RELEASE_MAPPING['master']
RPC_API_VERSION = '1.41'
RPC_API_VERSION = '1.42'
def __init__(self, topic=None):
super(ConductorAPI, self).__init__()
@ -744,17 +745,24 @@ class ConductorAPI(object):
return cctxt.call(context, 'do_node_clean',
node_id=node_id, clean_steps=clean_steps)
def heartbeat(self, context, node_id, callback_url, topic=None):
def heartbeat(self, context, node_id, callback_url, agent_version,
topic=None):
"""Process a node heartbeat.
:param context: request context.
:param node_id: node ID or UUID.
:param callback_url: URL to reach back to the ramdisk.
:param topic: RPC topic. Defaults to self.topic.
:param agent_version: the version of the agent that is heartbeating
"""
cctxt = self.client.prepare(topic=topic or self.topic, version='1.34')
new_kws = {}
version = '1.34'
if self.client.can_send_version('1.42'):
version = '1.42'
new_kws['agent_version'] = agent_version
cctxt = self.client.prepare(topic=topic or self.topic, version=version)
return cctxt.call(context, 'heartbeat', node_id=node_id,
callback_url=callback_url)
callback_url=callback_url, **new_kws)
def object_class_action_versions(self, context, objname, objmethod,
object_versions, args, kwargs):

3
ironic/drivers/base.py
View File

@ -402,11 +402,12 @@ class DeployInterface(BaseInterface):
"""
pass
def heartbeat(self, task, callback_url):
def heartbeat(self, task, callback_url, agent_version):
"""Record a heartbeat for the node.
:param task: a TaskManager instance containing the node to act on.
:param callback_url: a URL to use to call to the ramdisk.
:param agent_version: The version of the agent that is heartbeating
:return: None
"""
LOG.warning('Got heartbeat message from node %(node)s, but '

4
ironic/drivers/modules/agent_base_vendor.py
View File

@ -272,11 +272,12 @@ class HeartbeatMixin(object):
return (states.DEPLOYWAIT, states.CLEANWAIT)
@METRICS.timer('HeartbeatMixin.heartbeat')
def heartbeat(self, task, callback_url):
def heartbeat(self, task, callback_url, agent_version):
"""Process a heartbeat.
:param task: task to work with.
:param callback_url: agent HTTP API URL.
:param agent_version: The version of the agent that is heartbeating
"""
# NOTE(pas-ha) immediately skip the rest if nothing to do
if task.node.provision_state not in self.heartbeat_allowed_states:
@ -293,6 +294,7 @@ class HeartbeatMixin(object):
driver_internal_info = node.driver_internal_info
driver_internal_info['agent_url'] = callback_url
driver_internal_info['agent_version'] = agent_version
node.driver_internal_info = driver_internal_info
node.save()

27
ironic/tests/unit/api/controllers/v1/test_ramdisk.py
View File

@ -185,5 +185,30 @@ class TestHeartbeat(test_api_base.BaseApiTest):
self.assertEqual(http_client.ACCEPTED, response.status_int)
self.assertEqual(b'', response.body)
mock_heartbeat.assert_called_once_with(mock.ANY, mock.ANY,
node.uuid, 'url',
node.uuid, 'url', None,
topic='test-topic')
@mock.patch.object(rpcapi.ConductorAPI, 'heartbeat', autospec=True)
def test_ok_agent_version(self, mock_heartbeat):
node = obj_utils.create_test_node(self.context)
response = self.post_json(
'/heartbeat/%s' % node.uuid,
{'callback_url': 'url',
'agent_version': '1.4.1'},
headers={api_base.Version.string: str(api_v1.max_version())})
self.assertEqual(http_client.ACCEPTED, response.status_int)
self.assertEqual(b'', response.body)
mock_heartbeat.assert_called_once_with(mock.ANY, mock.ANY,
node.uuid, 'url', '1.4.1',
topic='test-topic')
@mock.patch.object(rpcapi.ConductorAPI, 'heartbeat', autospec=True)
def test_old_API_agent_version_error(self, mock_heartbeat):
node = obj_utils.create_test_node(self.context)
response = self.post_json(
'/heartbeat/%s' % node.uuid,
{'callback_url': 'url',
'agent_version': '1.4.1'},
headers={api_base.Version.string: '1.35'},
expect_errors=True)
self.assertEqual(http_client.BAD_REQUEST, response.status_int)

84
ironic/tests/unit/conductor/test_manager.py
View File

@ -6159,8 +6159,9 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
self.assertEqual(states.NOSTATE, node.target_provision_state)
self.assertIsNone(node.last_error)
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat')
@mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker')
def test_heartbeat(self, mock_spawn):
def test_heartbeat(self, mock_spawn, mock_heartbeat):
"""Test heartbeating."""
node = obj_utils.create_test_node(
self.context, driver='fake',
@ -6168,9 +6169,86 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
target_provision_state=states.ACTIVE)
self._start_service()
mock_spawn.reset_mock()
def fake_spawn(func, *args, **kwargs):
func(*args, **kwargs)
return mock.MagicMock()
mock_spawn.side_effect = fake_spawn
self.service.heartbeat(self.context, node.uuid, 'http://callback')
mock_spawn.assert_called_with(self.driver.deploy.heartbeat,
mock.ANY, 'http://callback')
mock_heartbeat.assert_called_with(mock.ANY, 'http://callback', '3.0.0')
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat')
@mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker')
def test_heartbeat_agent_version(self, mock_spawn, mock_heartbeat):
"""Test heartbeating."""
node = obj_utils.create_test_node(
self.context, driver='fake',
provision_state=states.DEPLOYING,
target_provision_state=states.ACTIVE)
self._start_service()
mock_spawn.reset_mock()
def fake_spawn(func, *args, **kwargs):
func(*args, **kwargs)
return mock.MagicMock()
mock_spawn.side_effect = fake_spawn
self.service.heartbeat(
self.context, node.uuid, 'http://callback', '1.4.1')
mock_heartbeat.assert_called_with(mock.ANY, 'http://callback', '1.4.1')
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat')
@mock.patch.object(manager, 'LOG', autospec=True)
@mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker')
def test_heartbeat_agent_version_deprecated(self, mock_spawn, log_mock,
mock_heartbeat):
"""Test heartbeating."""
node = obj_utils.create_test_node(
self.context, driver='fake',
provision_state=states.DEPLOYING,
target_provision_state=states.ACTIVE)
self._start_service()
mock_spawn.reset_mock()
def fake_spawn(func, *args, **kwargs):
func(*args, **kwargs)
return mock.MagicMock()
mock_spawn.side_effect = fake_spawn
mock_heartbeat.side_effect = [TypeError("Too many parameters"),
None, TypeError("Too many parameters"),
None]
# NOTE(sambetts) Test to make sure deploy driver that doesn't support
# version yet falls back to old behviour and logs a warning.
self.service.heartbeat(
self.context, node.uuid, 'http://callback', '1.4.1')
calls = [
mock.call(mock.ANY, 'http://callback', '1.4.1'),
mock.call(mock.ANY, 'http://callback')
]
mock_heartbeat.assert_has_calls(calls)
self.assertTrue(log_mock.warning.called)
# NOTE(sambetts) Test to make sure that the deprecation warning isn't
# thrown again.
log_mock.reset_mock()
mock_heartbeat.reset_mock()
self.service.heartbeat(
self.context, node.uuid, 'http://callback', '1.4.1')
calls = [
mock.call(mock.ANY, 'http://callback', '1.4.1'),
mock.call(mock.ANY, 'http://callback')
]
mock_heartbeat.assert_has_calls(calls)
self.assertFalse(log_mock.warning.called)
@mgr_utils.mock_record_keepalive

3
ironic/tests/unit/conductor/test_rpcapi.py
View File

@ -443,7 +443,8 @@ class RPCAPITestCase(db_base.DbTestCase):
'call',
node_id='fake-node',
callback_url='http://ramdisk.url:port',
version='1.34')
agent_version=None,
version='1.42')
def test_destroy_volume_connector(self):
fake_volume_connector = db_utils.get_test_volume_connector()

23
ironic/tests/unit/drivers/modules/test_agent_base_vendor.py
View File

@ -81,11 +81,14 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest):
agent_url = 'url-%s' % state
with task_manager.acquire(self.context, self.node.uuid,
shared=True) as task:
self.deploy.heartbeat(task, agent_url)
self.deploy.heartbeat(task, agent_url, '3.2.0')
self.assertFalse(task.shared)
self.assertEqual(
agent_url,
task.node.driver_internal_info['agent_url'])
self.assertEqual(
'3.2.0',
task.node.driver_internal_info['agent_version'])
self.assertEqual(0, ncrc_mock.call_count)
self.assertEqual(0, rti_mock.call_count)
self.assertEqual(0, cd_mock.call_count)
@ -105,7 +108,7 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest):
with task_manager.acquire(self.context, self.node.uuid,
shared=True) as task:
self.node.provision_state = state
self.deploy.heartbeat(task, 'url')
self.deploy.heartbeat(task, 'url', '1.0.0')
self.assertTrue(task.shared)
self.assertEqual(0, ncrc_mock.call_count)
self.assertEqual(0, rti_mock.call_count)
@ -125,7 +128,7 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest):
self.context, self.node['uuid'], shared=False) as task:
task.node.provision_state = states.DEPLOYWAIT
task.node.target_provision_state = states.ACTIVE
self.deploy.heartbeat(task, 'http://127.0.0.1:8080')
self.deploy.heartbeat(task, 'http://127.0.0.1:8080', '1.0.0')
failed_mock.assert_called_once_with(
task, mock.ANY, collect_logs=True)
log_mock.assert_called_once_with(
@ -155,7 +158,7 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest):
task.node.provision_state = states.DEPLOYWAIT
task.node.target_provision_state = states.ACTIVE
done_mock.side_effect = driver_failure
self.deploy.heartbeat(task, 'http://127.0.0.1:8080')
self.deploy.heartbeat(task, 'http://127.0.0.1:8080', '1.0.0')
# task.node.provision_state being set to DEPLOYFAIL
# within the driver_failue, hearbeat should not call
# deploy_utils.set_failed_state anymore
@ -178,7 +181,7 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest):
self.node.save()
with task_manager.acquire(
self.context, self.node.uuid, shared=False) as task:
self.deploy.heartbeat(task, 'http://127.0.0.1:8080')
self.deploy.heartbeat(task, 'http://127.0.0.1:8080', '1.0.0')
mock_touch.assert_called_once_with(mock.ANY)
mock_refresh.assert_called_once_with(mock.ANY, task)
@ -206,7 +209,7 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest):
failed_mock.side_effect = Exception()
with task_manager.acquire(
self.context, self.node.uuid, shared=False) as task:
self.deploy.heartbeat(task, 'http://127.0.0.1:8080')
self.deploy.heartbeat(task, 'http://127.0.0.1:8080', '1.0.0')
mock_touch.assert_called_once_with(mock.ANY)
mock_handler.assert_called_once_with(task, mock.ANY)
@ -234,7 +237,7 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest):
self.node.save()
with task_manager.acquire(
self.context, self.node.uuid, shared=False) as task:
self.deploy.heartbeat(task, 'http://127.0.0.1:8080')
self.deploy.heartbeat(task, 'http://127.0.0.1:8080', '1.0.0')
mock_touch.assert_called_once_with(mock.ANY)
mock_continue.assert_called_once_with(mock.ANY, task)
@ -257,7 +260,7 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest):
self.node.save()
with task_manager.acquire(
self.context, self.node.uuid, shared=False) as task:
self.deploy.heartbeat(task, 'http://127.0.0.1:8080')
self.deploy.heartbeat(task, 'http://127.0.0.1:8080', '1.0.0')
mock_continue.assert_called_once_with(mock.ANY, task)
mock_handler.assert_called_once_with(task, mock.ANY)
@ -274,9 +277,11 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest):
self.node.save()
with task_manager.acquire(
self.context, self.node.uuid, shared=False) as task:
self.deploy.heartbeat(task, 'http://127.0.0.1:8080')
self.deploy.heartbeat(task, 'http://127.0.0.1:8080', '3.2.0')
self.assertEqual('http://127.0.0.1:8080',
task.node.driver_internal_info['agent_url'])
self.assertEqual('3.2.0',
task.node.driver_internal_info['agent_version'])
mock_touch.assert_called_once_with(mock.ANY)

2
ironic/tests/unit/drivers/test_base.py
View File

@ -406,7 +406,7 @@ class TestDeployInterface(base.TestCase):
deploy = fake.FakeDeploy()
deploy.heartbeat(mock.Mock(node=mock.Mock(uuid='uuid',
driver='driver')),
'url')
'url', '3.2.0')
self.assertTrue(mock_log.called)

8
releasenotes/notes/heartbeat_agent_version-70f4e64b19b51d87.yaml Normal file
View File

@ -0,0 +1,8 @@
---
other:
- The agent heartbeat API (POST /v1/heartbeat/<node>) can now receive a new
``agent_version`` parameter. If received this will be stored in the node's
driver_internal_info['agent_version'] field. This information will be used
by the Bare Metal service to gracefully degrade support for agent features
that are requested by the Bare Metal service, ensuring that we don't
request a feature that an older ramdisk doesn't support.