From 75865619cc98dc9d8fe161fc8fcbaf05bfcd066b Mon Sep 17 00:00:00 2001 From: Michael Still Date: Fri, 29 Jul 2016 02:50:18 +1000 Subject: [PATCH] Add more vd2 unit tests Add more unit tests for vendordata2, as requested on the intial review. While doing this I realized that a HTTP status of "NO CONTENT" is valid, but will result in nothing being added to the config drive. We handle that case by just having an empty section. Additionally, I've decided that thrown exceptions for REST service requests shouldn't bubble up like they did initially, as that would stop instance boot. Instead, log them and then add an empty section to the config drive as well. Change-Id: If82312d9ca22a87929b947bcf7fed33a108cc720 Blueprint: vendordata-reboot --- nova/api/metadata/vendordata_dynamic.py | 18 +++-- nova/tests/unit/test_metadata.py | 93 +++++++++++++++++++++++-- 2 files changed, 97 insertions(+), 14 deletions(-) diff --git a/nova/api/metadata/vendordata_dynamic.py b/nova/api/metadata/vendordata_dynamic.py index 2736936770..9d13826e11 100644 --- a/nova/api/metadata/vendordata_dynamic.py +++ b/nova/api/metadata/vendordata_dynamic.py @@ -19,7 +19,6 @@ import requests from oslo_log import log as logging from oslo_serialization import jsonutils -from oslo_utils import excutils from nova.api.metadata import vendordata import nova.conf @@ -77,8 +76,7 @@ class DynamicVendorData(vendordata.VendorDataDriver): timeout=timeout) if res.status_code in (requests.codes.OK, requests.codes.CREATED, - requests.codes.ACCEPTED, - requests.codes.NO_CONTENT): + requests.codes.ACCEPTED): # TODO(mikal): Use the Cache-Control response header to do some # sensible form of caching here. return jsonutils.loads(res.text) @@ -87,13 +85,13 @@ class DynamicVendorData(vendordata.VendorDataDriver): except (TypeError, ValueError, requests.exceptions.RequestException, requests.exceptions.SSLError) as e: - with excutils.save_and_reraise_exception(): - LOG.warning(_LW('Error from dynamic vendordata service ' - '%(service_name)s at %(url)s: %(error)s'), - {'service_name': service_name, - 'url': url, - 'error': e}, - instance=self.instance) + LOG.warning(_LW('Error from dynamic vendordata service ' + '%(service_name)s at %(url)s: %(error)s'), + {'service_name': service_name, + 'url': url, + 'error': e}, + instance=self.instance) + return {} def get(self): j = {} diff --git a/nova/tests/unit/test_metadata.py b/nova/tests/unit/test_metadata.py index 8070e4f55e..1b60edde1f 100644 --- a/nova/tests/unit/test_metadata.py +++ b/nova/tests/unit/test_metadata.py @@ -39,6 +39,7 @@ from nova.api.metadata import base from nova.api.metadata import handler from nova.api.metadata import password from nova.api.metadata import vendordata +from nova.api.metadata import vendordata_dynamic from nova import block_device from nova.compute import flavors from nova.conductor import api as conductor_api @@ -834,9 +835,9 @@ class OpenStackMetadataTestCase(test.TestCase): for k, v in mydata.items(): self.assertEqual(vd[k], v) - @mock.patch.object(requests, 'request') - def test_vendor_data_response_vendordata2(self, request_mock): - request_mock.return_value.status_code = requests.codes.OK + def _test_vendordata2_response_inner(self, request_mock, response_code, + include_rest_result=True): + request_mock.return_value.status_code = response_code request_mock.return_value.text = '{"color": "blue"}' with utils.tempdir() as tmpdir: @@ -871,7 +872,91 @@ class OpenStackMetadataTestCase(test.TestCase): vd = jsonutils.loads(mdinst.lookup(vdpath)) self.assertEqual('10.0.0.1', vd['static'].get('ldap')) self.assertEqual('10.0.0.2', vd['static'].get('ad')) - self.assertEqual('blue', vd['web'].get('color')) + + if include_rest_result: + self.assertEqual('blue', vd['web'].get('color')) + else: + self.assertEqual({}, vd['web']) + + @mock.patch.object(requests, 'request') + def test_vendor_data_response_vendordata2_ok(self, request_mock): + self._test_vendordata2_response_inner(request_mock, + requests.codes.OK) + + @mock.patch.object(requests, 'request') + def test_vendor_data_response_vendordata2_created(self, request_mock): + self._test_vendordata2_response_inner(request_mock, + requests.codes.CREATED) + + @mock.patch.object(requests, 'request') + def test_vendor_data_response_vendordata2_accepted(self, request_mock): + self._test_vendordata2_response_inner(request_mock, + requests.codes.ACCEPTED) + + @mock.patch.object(requests, 'request') + def test_vendor_data_response_vendordata2_no_content(self, request_mock): + self._test_vendordata2_response_inner(request_mock, + requests.codes.NO_CONTENT, + include_rest_result=False) + + def _test_vendordata2_response_inner_exceptional( + self, request_mock, log_mock, exc): + request_mock.side_effect = exc('Ta da!') + + with utils.tempdir() as tmpdir: + jsonfile = os.path.join(tmpdir, 'test.json') + with open(jsonfile, 'w') as f: + f.write(jsonutils.dumps({'ldap': '10.0.0.1', + 'ad': '10.0.0.2'})) + + self.flags(vendordata_providers=['StaticJSON', 'DynamicJSON'], + vendordata_jsonfile_path=jsonfile, + vendordata_dynamic_targets=[ + 'web@http://fake.com/foobar'] + ) + + inst = self.instance.obj_clone() + mdinst = fake_InstanceMetadata(self, inst) + + # verify the new format as well + vdpath = "/openstack/2016-10-06/vendor_data2.json" + vd = jsonutils.loads(mdinst.lookup(vdpath)) + self.assertEqual('10.0.0.1', vd['static'].get('ldap')) + self.assertEqual('10.0.0.2', vd['static'].get('ad')) + + # and exception should result in nothing being added, but no error + self.assertEqual({}, vd['web']) + self.assertTrue(log_mock.called) + + @mock.patch.object(vendordata_dynamic.LOG, 'warning') + @mock.patch.object(requests, 'request') + def test_vendor_data_response_vendordata2_type_error(self, request_mock, + log_mock): + self._test_vendordata2_response_inner_exceptional( + request_mock, log_mock, TypeError) + + @mock.patch.object(vendordata_dynamic.LOG, 'warning') + @mock.patch.object(requests, 'request') + def test_vendor_data_response_vendordata2_value_error(self, request_mock, + log_mock): + self._test_vendordata2_response_inner_exceptional( + request_mock, log_mock, ValueError) + + @mock.patch.object(vendordata_dynamic.LOG, 'warning') + @mock.patch.object(requests, 'request') + def test_vendor_data_response_vendordata2_request_error(self, + request_mock, + log_mock): + self._test_vendordata2_response_inner_exceptional( + request_mock, log_mock, requests.exceptions.RequestException) + + @mock.patch.object(vendordata_dynamic.LOG, 'warning') + @mock.patch.object(requests, 'request') + def test_vendor_data_response_vendordata2_ssl_error(self, + request_mock, + log_mock): + self._test_vendordata2_response_inner_exceptional( + request_mock, log_mock, requests.exceptions.SSLError) def test_network_data_presence(self): inst = self.instance.obj_clone()