Merge ~larsks/cloud-init:bug/ec2-tests into cloud-init:master

[Lars Kellogg-Stedman]

test_ec2: metadata tests were mocking wrong urls

The ec2 metadata tests were only mocking one version of the metadata
api, but requests were made against both. This fixes _setup_ds to
register mock data at both versions of the API, and adds a fallback
handler to raise an http error on requests to un-mocked requests.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:1884378859ccd5638dc2e52d1082b89e892c7ad6
https://jenkins.ubuntu.com/server/job/cloud-init-ci/214/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/214/rebuild

review: Approve (continuous-integration)

[Scott Moser (smoser)]

Lars,
As this is right now, it is definitely 'needs_fixing'.
 - you are not registering / using bad_uri_handler at all.
 - as a result your commit message doesn't match the code changes.

I recall from talking to you that this is because 'tox -e xenial' (xenial/ubuntu 16.04 having an older version of httppretty). The memory is fuzzy in my head though.

I do think we want to register a 'not found' response / exception, and make sure nothing ever gets out, as our unit tests shouldn't hit external resources.

If we simply can't achieve that with 16.04 versions of packages, then i'd be ok for these tests to be skipIf in that scenario.

review: Needs Fixing

[Chad Smith (chad.smith)]

Lars, thank you for this. I replaced this content with a slightly different approach which only mocks the instance-id of the extended_metadata_versions as that's the only url that is checked for 404s when determining which metadata service version to use.

I'm marking as Rejected only because a slightly different take for bad_url_handling and unit test setup to make a minimally mocked set of metadata endpoints.

For your and my reference in future, it looks like the following allows httpretty to raise MockErrors for unmocked url attempts from unit tests:

httpretty.HTTPretty.allow_net_connect=False.

[Chad Smith (chad.smith)]

https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/330040 pulls in your content

[Lars Kellogg-Stedman (larsks)]

I do appreciate the chance to fix my own merge requests rather than having them replaced, but it's too late for that. In any case I'm glad it's working. You'll note that the final version of my code simply wasn't using the bad_uri_handler anymore, because there was no way to support that under Xenial.

review: Disapprove

[Lars Kellogg-Stedman (larsks)]

Actually, I didn't mean to reject this merge request. I'm not familiar enough with launchpad.

review: Approve

Unmerged commits

1884378... by Lars Kellogg-Stedman on 2017-08-25

test_ec2: metadata tests were mocking wrong urls

The ec2 metadata tests were only mocking one version of the metadata
api, but requests were made against both. This fixes _setup_ds to
register mock data at both versions of the API.

29025c3... by Lars Kellogg-Stedman on 2016-12-08

url_helper: fail gracefully if oauthlib is not available

We are unable to ship python-oauthlib in RHEL. This commit allows
imports of url_helper to succeed even when oauthlib is unavailable
and OauthUrlHelper.oauth_headers to raise a NotImplementedException
when called.

diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py
index 33d0261..7c9ac70 100644
--- a/tests/unittests/test_datasource/test_ec2.py
+++ b/tests/unittests/test_datasource/test_ec2.py
@@ -167,17 +167,36 @@ class TestEc2(test_helpers.HttprettyTestCase):
             self.datasource.min_metadata_version, 'meta-data', ''])
 
     @property
+    def metadata_urls(self):
+        return ['/'.join([self.metadata_addr, version, 'meta-data'])
+                for version in
+                [self.datasource.min_metadata_version] +
+                self.datasource.extended_metadata_versions]
+
+    @property
     def userdata_url(self):
         return '/'.join([
             self.metadata_addr,
             self.datasource.min_metadata_version, 'user-data'])
 
+    @property
+    def userdata_urls(self):
+        return ['/'.join([self.metadata_addr, version, 'user-data'])
+                for version in
+                [self.datasource.min_metadata_version] +
+                self.datasource.extended_metadata_versions]
+
     def _patch_add_cleanup(self, mpath, *args, **kwargs):
         p = mock.patch(mpath, *args, **kwargs)
         p.start()
         self.addCleanup(p.stop)
 
+    def bad_uri_handler(self, req, uri, headers):
+        print('Request for invalid URL:', uri)
+        return (999, headers, 'Invalid request for %s' % uri)
+
     def _setup_ds(self, sys_cfg, platform_data, md, ud=None):
+        self.uris = []
         distro = {}
         paths = helpers.Paths({})
         if sys_cfg is None:
@@ -189,8 +208,10 @@ class TestEc2(test_helpers.HttprettyTestCase):
                 return_value=platform_data)
 
         if md:
-            register_mock_metaserver(self.metadata_url, md)
-            register_mock_metaserver(self.userdata_url, ud)
+            for url in self.metadata_urls:
+                register_mock_metaserver(url, md)
+            for url in self.userdata_urls:
+                register_mock_metaserver(url, ud)
 
         return ds
 
@@ -268,6 +289,7 @@ class TestEc2(test_helpers.HttprettyTestCase):
         Then the metadata services is crawled for more network config info.
         When the platform data is valid, return True.
         """
+
         m_is_bsd.return_value = False
         m_dhcp.return_value = [{
             'interface': 'eth9', 'fixed-address': '192.168.2.9',
@@ -278,6 +300,7 @@ class TestEc2(test_helpers.HttprettyTestCase):
             platform_data=self.valid_platform_data,
             sys_cfg={'datasource': {'Ec2': {'strict_id': False}}},
             md=DEFAULT_METADATA)
+
         ret = ds.get_data()
         self.assertTrue(ret)
         m_dhcp.assert_called_once_with()