commit b67ac0c4087834a3989995af42416b7969538e57 Author: Julia Kreger Date: Tue Aug 11 13:13:09 2020 -0700 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) diff --git a/ironic/drivers/modules/agent_client.py b/ironic/drivers/modules/agent_client.py index eba9e6d..59b395b 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 1bea741..f872992 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): @@ -490,6 +493,61 @@ class TestAgentClientAttempts(base.TestCase): 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 error = 'Connection Timeout' 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 0000000..babe89d --- /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.