Fail deploy if agent returns >= 400

Currently the status code returned by IPA's POST /v1/commands API is
completely ignored by Ironic. This causes deploys to get stuck, as
future heartbeats will see that the command has not yet been accepted
and will continue to wait.

If IPA returns an error code to Ironic's agent client, we should bubble
that exception up, causing a deploy failure higher in the chain.

This adds an AgentAPIError exception class, and raises it when the agent
returns a 400 status code or higher when issuing a command.

Co-Authored-By: Jay Faulkner <jay@jvf.cc>

Related-bug: 1542506
Change-Id: I07fb8115d254e877d8781207eaec203e3fdf8ad6

ironic/common/exception.py

@@ -753,3 +753,8 @@ class PortgroupPhysnetInconsistent(IronicException):
 class VifInvalidForAttach(Conflict):
     _msg_fmt = _("Unable to attach VIF %(vif)s to node %(node)s. Reason: "
                  "%(reason)s")
+
+
+class AgentAPIError(IronicException):
+    _msg_fmt = _('Agent API for node %(node)s returned status %(status)s with '
+                 'error %(error)s')

ironic/drivers/modules/agent_client.py

@@ -16,6 +16,7 @@ from ironic_lib import metrics_utils
 from oslo_log import log
 from oslo_serialization import jsonutils
 import requests
+from six.moves import http_client
 
 from ironic.common import exception
 from ironic.common.i18n import _
@@ -90,6 +91,16 @@ class AgentClient(object):
                    'res': result.get('command_result'),
                    'error': result.get('command_error'),
                    'code': response.status_code})
+
+        if response.status_code >= http_client.BAD_REQUEST:
+            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'))
+
         return result
 
     @METRICS.timer('AgentClient.get_commands_status')

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

@@ -25,11 +25,10 @@ from ironic.tests import base
 
 
 class MockResponse(object):
-    status_code = http_client.OK
-
-    def __init__(self, text):
+    def __init__(self, text, status_code=http_client.OK):
         assert isinstance(text, six.string_types)
         self.text = text
+        self.status_code = status_code
 
     def json(self):
         return json.loads(self.text)
@@ -133,6 +132,25 @@ class TestAgentClient(base.TestCase):
                          {'method': method, 'node': self.node.uuid,
                           'error': error}, str(e))
 
+    def test__command_error_code(self):
+        response_text = '{"faultstring": "you dun goofd"}'
+        self.client.session.post.return_value = MockResponse(
+            response_text, status_code=http_client.BAD_REQUEST)
+        method = 'standby.run_image'
+        image_info = {'image_id': 'test_image'}
+        params = {'image_info': image_info}
+
+        url = self.client._get_command_url(self.node)
+        body = self.client._get_command_body(method, params)
+
+        self.assertRaises(exception.AgentAPIError,
+                          self.client._command,
+                          self.node, method, params)
+        self.client.session.post.assert_called_once_with(
+            url,
+            data=body,
+            params={'wait': 'false'})
+
     def test_get_commands_status(self):
         with mock.patch.object(self.client.session, 'get',
                                autospec=True) as mock_get:

releasenotes/notes/add-agent-api-error-77ec6c272390c488.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - Fixes propagation of HTTP errors from **ironic-python-agent** commands. Now
+    an operation is aborted on receiving HTTP error status from the ramdisk.