From 22ea3fb3b72b614bb1a052a04b5cf5a5359b5b40 Mon Sep 17 00:00:00 2001
From: Jim Rollenhagen <jim@jimrollenhagen.com>
Date: Mon, 18 Jan 2016 14:24:29 -0800
Subject: [PATCH] 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                    |  5 ++++
 ironic/drivers/modules/agent_client.py        | 11 +++++++++
 .../unit/drivers/modules/test_agent_client.py | 24 ++++++++++++++++---
 .../add-agent-api-error-77ec6c272390c488.yaml |  4 ++++
 4 files changed, 41 insertions(+), 3 deletions(-)
 create mode 100644 releasenotes/notes/add-agent-api-error-77ec6c272390c488.yaml

diff --git a/ironic/common/exception.py b/ironic/common/exception.py
index f619b5b12a..05e8533f7a 100644
--- a/ironic/common/exception.py
+++ b/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')
diff --git a/ironic/drivers/modules/agent_client.py b/ironic/drivers/modules/agent_client.py
index e908bd646c..fb45948c62 100644
--- a/ironic/drivers/modules/agent_client.py
+++ b/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')
diff --git a/ironic/tests/unit/drivers/modules/test_agent_client.py b/ironic/tests/unit/drivers/modules/test_agent_client.py
index 5f81decaa0..3c8ca31fdd 100644
--- a/ironic/tests/unit/drivers/modules/test_agent_client.py
+++ b/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:
diff --git a/releasenotes/notes/add-agent-api-error-77ec6c272390c488.yaml b/releasenotes/notes/add-agent-api-error-77ec6c272390c488.yaml
new file mode 100644
index 0000000000..c28c89bf98
--- /dev/null
+++ b/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.