From b67ac0c4087834a3989995af42416b7969538e57 Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Tue, 11 Aug 2020 13:13:09 -0700 Subject: [PATCH] Handle an older agent with agent_token Well, as the author of the agent token work, I could have sworn we covered this case, but it seems not and agent commands could fail. What was occuring, when the token is optional, we were not backing down on the agent client and failing even though we could detect the failure and handle it accordingly. Note: This change had to be slightly re-worked and thus was not a perfectly clean backport due to changes made earlier in the victoria development cycle in I62e9086441fa2b164aee42f7489d12aed4076f49. As a result, the helper class for faults is not present in this change set and the messages returned from the tests had to be passed slightly differently using the original FakeResponse class. Change-Id: Ibd7e17fb00747485c8072289fff9b28d4da17c39 Story: 2008002 Task: 40649 (cherry picked from commit 30d9cb47e62b62d570e1792515e16abf1ac3cd56) (cherry picked from commit 980bb3f9465746e34b846f1e337ccceaccbf1bc7) --- ironic/drivers/modules/agent_client.py | 22 ++++++- .../unit/drivers/modules/test_agent_client.py | 58 +++++++++++++++++++ ...-older-agent-command-5930124fd03bb327.yaml | 6 ++ 3 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/handle-older-agent-command-5930124fd03bb327.yaml diff --git a/ironic/drivers/modules/agent_client.py b/ironic/drivers/modules/agent_client.py index eba9e6de83..59b395bc4a 100644 --- a/ironic/drivers/modules/agent_client.py +++ b/ironic/drivers/modules/agent_client.py @@ -140,8 +140,28 @@ class AgentClient(object): 'res': result.get('command_result'), 'error': error, 'code': response.status_code}) - if response.status_code >= http_client.BAD_REQUEST: + faultstring = result.get('faultstring') + if 'agent_token' in faultstring and agent_token: + # NOTE(TheJulia) We have an agent that is out of date. + # which means I guess grenade updates the agent image + # for upgrades... :( + if not CONF.require_agent_token: + LOG.warning('Agent command %(method)s for node %(node)s ' + 'failed. Expected 2xx HTTP status code, got ' + '%(code)d. Error suggests an older ramdisk ' + 'which does not support ``agent_token``. ' + 'Removing the token for the next retry.', + {'method': method, 'node': node.uuid, + 'code': response.status_code}) + i_info = node.driver_internal_info + i_info.pop('agent_secret_token') + node.driver_internal_info = i_info + node.save() + msg = ('Node {} does not appear to support ' + 'agent_token and it is not required. Next retry ' + 'will be without the token.').format(node.uuid) + raise exception.AgentConnectionFailed(reason=msg) LOG.error('Agent command %(method)s for node %(node)s failed. ' 'Expected 2xx HTTP status code, got %(code)d.', {'method': method, 'node': node.uuid, diff --git a/ironic/tests/unit/drivers/modules/test_agent_client.py b/ironic/tests/unit/drivers/modules/test_agent_client.py index 1bea741880..f872992618 100644 --- a/ironic/tests/unit/drivers/modules/test_agent_client.py +++ b/ironic/tests/unit/drivers/modules/test_agent_client.py @@ -55,6 +55,9 @@ class MockNode(object): 'instance_info': self.instance_info } + def save(self): + pass + class TestAgentClient(base.TestCase): def setUp(self): @@ -489,6 +492,61 @@ class TestAgentClientAttempts(base.TestCase): params={'wait': 'false'}, timeout=60) + @mock.patch.object(retrying.time, 'sleep', autospec=True) + def test__command_succeed_after_agent_token(self, mock_sleep): + self.config(require_agent_token=False) + mock_sleep.return_value = None + error = {'faultstring': 'Unknown Argument: "agent_token"'} + response_data = {'status': 'ok'} + method = 'standby.run_image' + image_info = {'image_id': 'test_image'} + params = {'image_info': image_info} + i_info = self.node.driver_internal_info + i_info['agent_secret_token'] = 'meowmeowmeow' + self.client.session.post.side_effect = [ + MockResponse(json.dumps(error), + status_code=http_client.BAD_REQUEST), + MockResponse(json.dumps(response_data)), + ] + + response = self.client._command(self.node, method, params) + self.assertEqual(2, self.client.session.post.call_count) + self.assertEqual(response, response_data) + self.client.session.post.assert_called_with( + self.client._get_command_url(self.node), + data=self.client._get_command_body(method, params), + params={'wait': 'false'}, + timeout=60) + self.assertNotIn('agent_secret_token', self.node.driver_internal_info) + + @mock.patch.object(retrying.time, 'sleep', autospec=True) + def test__command_fail_agent_token_required(self, mock_sleep): + self.config(require_agent_token=True) + mock_sleep.return_value = None + error = {'faultstring': 'Unknown Argument: "agent_token"'} + method = 'standby.run_image' + image_info = {'image_id': 'test_image'} + params = {'image_info': image_info} + i_info = self.node.driver_internal_info + i_info['agent_secret_token'] = 'meowmeowmeow' + self.client.session.post.side_effect = [ + MockResponse(json.dumps(error), + status_code=http_client.BAD_REQUEST), + ] + + self.assertRaises(exception.AgentAPIError, + self.client._command, + self.node, method, params) + self.assertEqual(1, self.client.session.post.call_count) + self.client.session.post.assert_called_with( + self.client._get_command_url(self.node), + data=self.client._get_command_body(method, params), + params={'wait': 'false', 'agent_token': 'meowmeowmeow'}, + timeout=60) + self.assertEqual( + 'meowmeowmeow', + self.node.driver_internal_info.get('agent_secret_token')) + @mock.patch.object(retrying.time, 'sleep', autospec=True) def test__command_succeed_after_one_timeout(self, mock_sleep): mock_sleep.return_value = None diff --git a/releasenotes/notes/handle-older-agent-command-5930124fd03bb327.yaml b/releasenotes/notes/handle-older-agent-command-5930124fd03bb327.yaml new file mode 100644 index 0000000000..babe89da54 --- /dev/null +++ b/releasenotes/notes/handle-older-agent-command-5930124fd03bb327.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixes an issue with agent token handling where the agent has not been + upgraded resulting in an AgentAPIError, when the token is not required. + The conductor now retries without sending an agent token.