From 3c1943f8d9974477d88ba94336d94db6c4346e01 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Wed, 1 Mar 2017 14:16:29 -0500 Subject: [PATCH] Check for 204 case in DynamicVendorData Before b9587587c2f3d6f871dec07562c16e1d82bc05e6 we were doing an explicit check on the response status codes, but after that change, we just rely on the bool override in requests.Reponse which is True for any repsonse code less than 400. In the 204 case we won't have response text, so we should handle that before trying to load the json repsonse text. Because TypeError and ValueError are handled it would just result in a warning if you got a 204, unless CONF.api.vendordata_dynamic_failure_fatal was True in which case it's a failure (as shown in the unit test update here). Change-Id: Iba95fd0112ae4510ef21fdb44fcb281b14567c07 Closes-Bug: #1669084 --- nova/api/metadata/vendordata_dynamic.py | 2 +- nova/tests/unit/test_identity.py | 6 +++++- nova/tests/unit/test_metadata.py | 8 ++++++-- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/nova/api/metadata/vendordata_dynamic.py b/nova/api/metadata/vendordata_dynamic.py index db481225badf..7044caab1095 100644 --- a/nova/api/metadata/vendordata_dynamic.py +++ b/nova/api/metadata/vendordata_dynamic.py @@ -96,7 +96,7 @@ class DynamicVendorData(vendordata.VendorDataDriver): res = self.session.request(url, 'POST', data=jsonutils.dumps(body), verify=verify, headers=headers, timeout=timeout) - if res: + if res and res.text: # TODO(mikal): Use the Cache-Control response header to do some # sensible form of caching here. return jsonutils.loads(res.text) diff --git a/nova/tests/unit/test_identity.py b/nova/tests/unit/test_identity.py index ad75d86049f3..e2c12832cc6b 100644 --- a/nova/tests/unit/test_identity.py +++ b/nova/tests/unit/test_identity.py @@ -39,7 +39,11 @@ class FakeResponse(object): def __nonzero__(self): # python 2 - return self.status_code == 200 + return self.status_code < 400 + + @property + def text(self): + return self.content class IdentityValidationTest(test.NoDBTestCase): diff --git a/nova/tests/unit/test_metadata.py b/nova/tests/unit/test_metadata.py index f3f9a7f00587..a43c03c27205 100644 --- a/nova/tests/unit/test_metadata.py +++ b/nova/tests/unit/test_metadata.py @@ -54,6 +54,7 @@ from nova import test from nova.tests.unit.api.openstack import fakes from nova.tests.unit import fake_block_device from nova.tests.unit import fake_network +from nova.tests.unit import test_identity from nova.tests import uuidsentinel as uuids from nova import utils from nova.virt import netutils @@ -845,9 +846,10 @@ class OpenStackMetadataTestCase(test.TestCase): def _test_vendordata2_response_inner(self, request_mock, response_code, include_rest_result=True): - request_mock.return_value.status_code = response_code + fake_response = test_identity.FakeResponse(response_code) if include_rest_result: - request_mock.return_value.text = '{"color": "blue"}' + fake_response.content = '{"color": "blue"}' + request_mock.return_value = fake_response with utils.tempdir() as tmpdir: jsonfile = os.path.join(tmpdir, 'test.json') @@ -905,6 +907,8 @@ class OpenStackMetadataTestCase(test.TestCase): @mock.patch.object(session.Session, 'request') def test_vendor_data_response_vendordata2_no_content(self, request_mock): + # Make it a failure if no content was returned and we don't handle it. + self.flags(vendordata_dynamic_failure_fatal=True, group='api') self._test_vendordata2_response_inner(request_mock, requests.codes.NO_CONTENT, include_rest_result=False)