From 003e27fb96238551a3d13fa09fb969cfb3eff7ed Mon Sep 17 00:00:00 2001
From: Jim Bach <jabach@blizzard.com>
Date: Mon, 18 Jun 2018 11:23:13 -0700
Subject: [PATCH] Added error handling for discoveryurl

This adds an immediate failure response if the etcd discovery service returns
a bad status code. Before Magnum would continue to run and fail to configure
but with vague information of its failure. This would cause Magnum to generally
wait until the entire timeout before failing.

Change-Id: Iebd51e5dc8a3e3c285cb0c2af35c19f6f37ed0a7
Task: 22193
Story: 2002584
---
 magnum/drivers/heat/template_def.py                       | 6 +++++-
 .../unit/conductor/handlers/test_k8s_cluster_conductor.py | 8 ++++++--
 magnum/tests/unit/drivers/test_template_definition.py     | 4 ++++
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/magnum/drivers/heat/template_def.py b/magnum/drivers/heat/template_def.py
index 0849c325b9..d69bed6e15 100755
--- a/magnum/drivers/heat/template_def.py
+++ b/magnum/drivers/heat/template_def.py
@@ -313,7 +313,11 @@ class BaseTemplateDefinition(TemplateDefinition):
                 CONF.cluster.etcd_discovery_service_endpoint_format %
                 {'size': cluster.master_count})
             try:
-                discovery_url = requests.get(discovery_endpoint).text
+                discovery_request = requests.get(discovery_endpoint)
+                if discovery_request.status_code != requests.codes.ok:
+                    raise exception.GetDiscoveryUrlFailed(
+                        discovery_endpoint=discovery_endpoint)
+                discovery_url = discovery_request.text
             except req_exceptions.RequestException as err:
                 LOG.error(six.text_type(err))
                 raise exception.GetDiscoveryUrlFailed(
diff --git a/magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py b/magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py
index 01c0240f4e..a793c0907e 100644
--- a/magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py
+++ b/magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py
@@ -161,6 +161,7 @@ class TestClusterConductorWithK8s(base.TestCase):
                               '"1","modifiedIndex":10,"createdIndex":10}}')
         mock_resp = mock.MagicMock()
         mock_resp.text = expected_result
+        mock_resp.status_code = 200
         mock_get.return_value = mock_resp
         cluster = objects.Cluster(self.context, **self.cluster_dict)
         mock_driver.return_value = k8s_dr.Driver()
@@ -502,6 +503,7 @@ class TestClusterConductorWithK8s(base.TestCase):
                               '"1","modifiedIndex":10,"createdIndex":10}}')
         mock_resp = mock.MagicMock()
         mock_resp.text = expected_result
+        mock_resp.status_code = 200
         mock_get.return_value = mock_resp
         cluster = objects.Cluster(self.context, **self.cluster_dict)
         mock_driver.return_value = k8s_coreos_dr.Driver()
@@ -580,7 +582,8 @@ class TestClusterConductorWithK8s(base.TestCase):
             reqget):
         self.cluster_template_dict['cluster_distro'] = 'coreos'
         self.cluster_dict['discovery_url'] = None
-        mock_req = mock.MagicMock(text='http://tokentest/h1/h2/h3')
+        mock_req = mock.MagicMock(text='http://tokentest/h1/h2/h3',
+                                  status_code=200)
         reqget.return_value = mock_req
         cluster_template = objects.ClusterTemplate(
             self.context, **self.cluster_template_dict)
@@ -763,7 +766,8 @@ class TestClusterConductorWithK8s(base.TestCase):
         CONF.set_override('etcd_discovery_service_endpoint_format',
                           'http://etcd/test?size=%(size)d',
                           group='cluster')
-        mock_req = mock.MagicMock(text='https://address/token')
+        mock_req = mock.MagicMock(text='https://address/token',
+                                  status_code=200)
         reqget.return_value = mock_req
 
         (template_path,
diff --git a/magnum/tests/unit/drivers/test_template_definition.py b/magnum/tests/unit/drivers/test_template_definition.py
index 27a75ca66d..de6ace81ec 100644
--- a/magnum/tests/unit/drivers/test_template_definition.py
+++ b/magnum/tests/unit/drivers/test_template_definition.py
@@ -555,6 +555,7 @@ class AtomicK8sTemplateDefinitionTestCase(BaseTemplateDefinitionTestCase):
         expected_discovery_url = 'http://etcd/token'
         mock_resp = mock.MagicMock()
         mock_resp.text = expected_discovery_url
+        mock_resp.status_code = 200
         mock_get.return_value = mock_resp
         mock_cluster = mock.MagicMock()
         mock_cluster.master_count = 10
@@ -592,6 +593,7 @@ class AtomicK8sTemplateDefinitionTestCase(BaseTemplateDefinitionTestCase):
     def test_k8s_get_discovery_url_not_found(self, mock_get):
         mock_resp = mock.MagicMock()
         mock_resp.text = ''
+        mock_resp.status_code = 200
         mock_get.return_value = mock_resp
 
         fake_cluster = mock.MagicMock()
@@ -964,6 +966,7 @@ class AtomicSwarmTemplateDefinitionTestCase(base.TestCase):
         expected_discovery_url = 'http://etcd/token'
         mock_resp = mock.MagicMock()
         mock_resp.text = expected_discovery_url
+        mock_resp.status_code = 200
         mock_get.return_value = mock_resp
         mock_cluster = mock.MagicMock()
         mock_cluster.discovery_url = None
@@ -979,6 +982,7 @@ class AtomicSwarmTemplateDefinitionTestCase(base.TestCase):
     def test_swarm_get_discovery_url_not_found(self, mock_get):
         mock_resp = mock.MagicMock()
         mock_resp.text = ''
+        mock_resp.status_code = 200
         mock_get.return_value = mock_resp
 
         fake_cluster = mock.MagicMock()