Merge ~chad.smith/cloud-init:bug/ec2-tests-unmocked-metadata into cloud-init:master

[Chad Smith]

ec2 tests: Stop leaking calls through unmocked metadata addresses

DataSourceEc2 behavior changed to first check a minimum acceptable
metadata version uri http://169.154.169.254/<min_version>/instance-id,
retrying on 404, until the metadata service is available. After the
metadata service is up, the datasource inspects preferred
extended_metadata_versions for availability. Unit tests only mocked the
preferred extended_metadata_version so all Ec2 tests were retrying
attempts against
http://169.254.169.254/meta-data/<min-version>/instance-id adding a lot of
time cost to the unit test runs.

This branch uses httpretty to properly mock the following:
  - 404s from metadata on undesired extended_metadata_version test routes
    - https://169.254.169.254/meta-data/2016-09-02/instance-id
  - full metadata dictionary represented on min_metadata_version
    - https://169.254.169.254/meta-data/2016-09-02/*

The branch also tightens httpretty to raise a MockError for any URL
which isn't mocked via httpretty.HTTPretty.allow_net_connect=False.

LP: #1714117

ec2 tests: Stop leaking calls through unmocked metadata addresses

DataSourceEc2 behavior changed to first check a minimum acceptable
metadata version uri http://169.154.169.254/<min_version>/instance-id,
retrying on 404, until the metadata service is available. After the
metadata service is up, the datasource inspects preferred
extended_metadata_versions for availability. Unit tests only mocked the
preferred extended_metadata_version so all Ec2 tests were retrying
attempts against
http://169.254.169.254/meta-data/<min-version>/instance-id adding a lot of
time cost to the unit test runs.

This branch uses httpretty to properly mock the following:
  - 404s from metadata on undesired extended_metadata_version test routes
    - https://169.254.169.254/meta-data/2016-09-02/instance-id
  - full metadata dictionary represented on min_metadata_version
    - https://169.254.169.254/meta-data/2016-09-02/*

The branch also tightens httpretty to raise a MockError for any URL
which isn't mocked via httpretty.HTTPretty.allow_net_connect=False.

LP: #1714117

[Joshua Powers (powersj)]

Ran on the test system and was back to under 20 seconds for each environment. Add link to bug (LP: 1714117), fix style tests, and this is good to go!

tests/unittests/test_datasource/test_ec2.py:6:1: F401 're' imported but unused
tests/unittests/test_datasource/test_ec2.py:203:24: E114 indentation is not a multiple of four (comment)
tests/unittests/test_datasource/test_ec2.py:207:24: E114 indentation is not a multiple of four (comment)
tests/unittests/test_datasource/test_ec2.py:208:24: E111 indentation is not a multiple of four

review: Approve

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

FAILED: Continuous integration, rev:d817633406d69224ae24924244e10a039a84bdbb
https://jenkins.ubuntu.com/server/job/cloud-init-ci/243/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

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

review: Needs Fixing (continuous-integration)

53e97d2... by Chad Smith on 2017-08-31

flakes

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

PASSED: Continuous integration, rev:53e97d2da7fd9f080d6774bfa7394e139a457c0d
https://jenkins.ubuntu.com/server/job/cloud-init-ci/244/
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/244/rebuild

review: Approve (continuous-integration)

diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py
index e1ce644..b7a84e2 100644
--- a/tests/unittests/test_datasource/test_ec2.py
+++ b/tests/unittests/test_datasource/test_ec2.py
@@ -116,6 +116,9 @@ def register_mock_metaserver(base_url, data):
     In the index, references to lists or dictionaries have a trailing /.
     """
     def register_helper(register, base_url, body):
+        if not isinstance(base_url, str):
+            register(base_url, body)
+            return
         base_url = base_url.rstrip("/")
         if isinstance(body, str):
             register(base_url, body)
@@ -138,7 +141,7 @@ def register_mock_metaserver(base_url, data):
             register(base_url, '\n'.join(vals) + '\n')
             register(base_url + '/', '\n'.join(vals) + '\n')
         elif body is None:
-            register(base_url, 'not found', status_code=404)
+            register(base_url, 'not found', status=404)
 
     def myreg(*argc, **kwargs):
         # print("register_url(%s, %s)" % (argc, kwargs))
@@ -161,38 +164,47 @@ class TestEc2(test_helpers.HttprettyTestCase):
         self.datasource = ec2.DataSourceEc2
         self.metadata_addr = self.datasource.metadata_urls[0]
 
-    @property
-    def metadata_url(self):
-        return '/'.join([
-            self.metadata_addr,
-            self.datasource.min_metadata_version, 'meta-data', ''])
-
-    @property
-    def userdata_url(self):
-        return '/'.join([
-            self.metadata_addr,
-            self.datasource.min_metadata_version, 'user-data'])
+    def data_url(self, version):
+        """Return a metadata url based on the version provided."""
+        return '/'.join([self.metadata_addr, version, 'meta-data', ''])
 
     def _patch_add_cleanup(self, mpath, *args, **kwargs):
         p = mock.patch(mpath, *args, **kwargs)
         p.start()
         self.addCleanup(p.stop)
 
-    def _setup_ds(self, sys_cfg, platform_data, md, ud=None):
+    def _setup_ds(self, sys_cfg, platform_data, md, md_version=None):
+        self.uris = []
         distro = {}
         paths = helpers.Paths({})
         if sys_cfg is None:
             sys_cfg = {}
         ds = self.datasource(sys_cfg=sys_cfg, distro=distro, paths=paths)
+        if not md_version:
+            md_version = ds.min_metadata_version
         if platform_data is not None:
             self._patch_add_cleanup(
                 "cloudinit.sources.DataSourceEc2._collect_platform_data",
                 return_value=platform_data)
 
         if md:
-            register_mock_metaserver(self.metadata_url, md)
-            register_mock_metaserver(self.userdata_url, ud)
-
+            httpretty.HTTPretty.allow_net_connect = False
+            all_versions = (
+                [ds.min_metadata_version] + ds.extended_metadata_versions)
+            for version in all_versions:
+                metadata_url = self.data_url(version)
+                if version == md_version:
+                    # Register all metadata for desired version
+                    register_mock_metaserver(metadata_url, md)
+                else:
+                    instance_id_url = metadata_url + 'instance-id'
+                    if version == ds.min_metadata_version:
+                        # Add min_metadata_version service availability check
+                        register_mock_metaserver(
+                            instance_id_url, DEFAULT_METADATA['instance-id'])
+                    else:
+                        # Register 404s for all unrequested extended versions
+                        register_mock_metaserver(instance_id_url, None)
         return ds
 
     @httpretty.activate
@@ -297,6 +309,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',
@@ -307,6 +320,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()