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 30d9cb47e62b62d570e1792515e16abf1ac3cd56)

3 files changed, 94 insertions, 3 deletions

diff --git a/ironic/drivers/modules/agent_client.py b/ironic/drivers/modules/agent_client.py
index 090007d8f..4cb349509 100644
--- a/ironic/drivers/modules/agent_client.py
+++ b/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)
 
diff --git a/ironic/tests/unit/drivers/modules/test_agent_client.py b/ironic/tests/unit/drivers/modules/test_agent_client.py
index 1f8cc1f6a..cb794c94c 100644
--- a/ironic/tests/unit/drivers/modules/test_agent_client.py
+++ b/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):
@@ -606,6 +616,61 @@ class TestAgentClientAttempts(base.TestCase):
             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
         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 000000000..babe89da5
--- /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.