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.

Change-Id: Ibd7e17fb00747485c8072289fff9b28d4da17c39
Story: 2008002
Task: 40649
(cherry picked from commit 30d9cb47e6)

ironic/drivers/modules/agent_client.py

@@ -207,15 +207,35 @@ 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,
                        'code': response.status_code})
             raise exception.AgentAPIError(node=node.uuid,
                                           status=response.status_code,
-                                          error=result.get('faultstring'))
+                                          error=faultstring)
 
         self._raise_if_typeerror(result, node, method)
 

ironic/tests/unit/drivers/modules/test_agent_client.py

@@ -43,7 +43,8 @@ class MockResponse(object):
 
 
 class MockCommandStatus(MockResponse):
-    def __init__(self, status, name='fake', error=None):
+    def __init__(self, status, name='fake', error=None,
+                 status_code=http_client.OK):
         super().__init__({
             'commands': [
                 {'command_name': name,
@@ -54,6 +55,12 @@ class MockCommandStatus(MockResponse):
         })
 
 
+class MockFault(MockResponse):
+    def __init__(self, faultstring, status_code=http_client.BAD_REQUEST):
+        super().__init__({'faultstring': faultstring},
+                         status_code=status_code)
+
+
 class MockNode(object):
     def __init__(self):
         self.uuid = 'uuid'
@@ -73,6 +80,9 @@ class MockNode(object):
             'driver_info': self.driver_info,
         }
 
+    def save(self):
+        pass
+
 
 class TestAgentClient(base.TestCase):
     def setUp(self):
@@ -605,6 +615,61 @@ class TestAgentClientAttempts(base.TestCase):
             timeout=60,
             verify=True)
 
+    @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 = '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 = [
+            MockFault(error),
+            MockResponse(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,
+            verify=True)
+        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 = '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 = [
+            MockFault(error)
+        ]
+
+        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,
+            verify=True)
+        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

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.