Merge ~smoser/cloud-init:bug/1715128-ec2-used-on-openstack into cloud-init:master

[Scott Moser]

Ec2: only attempt to operate at local mode on known platforms.

This change makes the DataSourceEc2Local do nothing unless it is on
actual AWS platform. The motivation is twofold:

a.) It is generally safer to only make this function available to Ec2
clones that explicitly identify themselves to the guest. (It also
gives them a reason to supply identification code to cloud-init.)

b.) On non-intel OpenStack platforms ds-identify would enable both the Ec2
and OpenStack sources. That is because there is not good data (such as
dmi) to positively identify the platform. Previously that would be fine
as OpenStack would run first and be successful. The change to add Ec2Local
meant that an Ec2 now runs first.

The best case for 'b' would be a slow down as attempts at the Ec2 metadata
service time out. The discovered case was worse.

Additionally we add a simple check for datatype of 'network' in the
metadata before attempting to read it.

LP: #1715128

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

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

review: Approve (continuous-integration)

[Scott Moser (smoser)]

This will return ec2 to being searched after openstack.
but there are still a couple things i have to think about the stack trace a bit.

a0ef705... by Scott Moser on 2017-09-07

Ec2: return None if in network_config there is no network config.

also here, change the signature of convert_ec2_metadata_network_config
to be reasonable, since it only ever referenced the 'network' entry
in metadata.

still may raise exception and stack trace on invalid data.

2eea4ec... by Scott Moser on 2017-09-07

update tests for function signature change.

c65d1e0... by Scott Moser on 2017-09-07

network_md is mandatory

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

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

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

review: Needs Fixing (continuous-integration)

5e2ce57... by Scott Moser on 2017-09-07

flake8

[Chad Smith (chad.smith)]

+1 with a unit test here http://paste.ubuntu.com/25485368/

review: Approve

d99b689... by Scott Moser on 2017-09-07

add chad's test.

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

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

review: Approve (continuous-integration)

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

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

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

review: Needs Fixing

[Ryan Harper (raharper)]

I'm OK with the current approach of blowing up; but I'd like to see a follow up on validating that the config after conversion has a change of doing something useful.

review: Approve

[Scott Moser (smoser)]

ok. given the approve aboev i'm going to pull this

 http://paste.ubuntu.com/25485967/

that patch there has a bit more strict checking, but falling back rathe rthan raising a value error means "hiding" this error.

so i'm leaving it as it is right now.

diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py
index 07c12bb..41367a8 100644
--- a/cloudinit/sources/DataSourceEc2.py
+++ b/cloudinit/sources/DataSourceEc2.py
@@ -27,6 +27,8 @@ SKIP_METADATA_URL_CODES = frozenset([uhelp.NOT_FOUND])
 STRICT_ID_PATH = ("datasource", "Ec2", "strict_id")
 STRICT_ID_DEFAULT = "warn"
 
+_unset = "_unset"
+
 
 class Platforms(object):
     ALIYUN = "AliYun"
@@ -57,7 +59,7 @@ class DataSourceEc2(sources.DataSource):
 
     _cloud_platform = None
 
-    _network_config = None  # Used for caching calculated network config v1
+    _network_config = _unset  # Used for caching calculated network config v1
 
     # Whether we want to get network configuration from the metadata service.
     get_network_metadata = False
@@ -284,10 +286,24 @@ class DataSourceEc2(sources.DataSource):
     @property
     def network_config(self):
         """Return a network config dict for rendering ENI or netplan files."""
-        if self._network_config is None:
-            if self.metadata is not None:
-                self._network_config = convert_ec2_metadata_network_config(
-                    self.metadata)
+        if self._network_config != _unset:
+            return self._network_config
+
+        if self.metadata is None:
+            # this would happen if get_data hadn't been called. leave as _unset
+            LOG.warning(
+                "Unexpected call to network_config when metadata is None.")
+            return None
+
+        result = None
+        net_md = self.metadata.get('network')
+        if isinstance(net_md, dict):
+            result = convert_ec2_metadata_network_config(net_md)
+        else:
+            LOG.warning("unexpected metadata 'network' key not valid: %s",
+                        net_md)
+        self._network_config = result
+
         return self._network_config
 
     def _crawl_metadata(self):
@@ -321,6 +337,14 @@ class DataSourceEc2Local(DataSourceEc2):
     """
     get_network_metadata = True  # Get metadata network config if present
 
+    def get_data(self):
+        supported_platforms = (Platforms.AWS,)
+        if self.cloud_platform not in supported_platforms:
+            LOG.debug("Local Ec2 mode only supported on %s, not %s",
+                      supported_platforms, self.cloud_platform)
+            return False
+        return super(DataSourceEc2Local, self).get_data()
+
 
 def read_strict_mode(cfgval, default):
     try:
@@ -434,10 +458,13 @@ def _collect_platform_data():
     return data
 
 
-def convert_ec2_metadata_network_config(metadata=None, macs_to_nics=None):
+def convert_ec2_metadata_network_config(network_md, macs_to_nics=None):
     """Convert ec2 metadata to network config version 1 data dict.
 
-    @param: metadata: Dictionary of metadata crawled from EC2 metadata url.
+    @param: network_md: 'network' portion of EC2 metadata.
+       generally formed as {"interfaces": {"macs": {}} where
+       'macs' is a dictionary with mac address as key and contents like:
+       {"device-number": "0", "interface-id": "...", "local-ipv4s": ...}
     @param: macs_to_name: Optional dict mac addresses and the nic name. If
        not provided, get_interfaces_by_mac is called to get it from the OS.
 
@@ -446,7 +473,7 @@ def convert_ec2_metadata_network_config(metadata=None, macs_to_nics=None):
     netcfg = {'version': 1, 'config': []}
     if not macs_to_nics:
         macs_to_nics = net.get_interfaces_by_mac()
-    macs_metadata = metadata['network']['interfaces']['macs']
+    macs_metadata = network_md['interfaces']['macs']
     for mac, nic_name in macs_to_nics.items():
         nic_metadata = macs_metadata.get(mac)
         if not nic_metadata:
diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py
index 9fb9048..a7301db 100644
--- a/tests/unittests/test_datasource/test_ec2.py
+++ b/tests/unittests/test_datasource/test_ec2.py
@@ -279,6 +279,27 @@ class TestEc2(test_helpers.HttprettyTestCase):
         ret = ds.get_data()
         self.assertTrue(ret)
 
+    def test_ec2_local_returns_false_on_non_aws(self):
+        """DataSourceEc2Local returns False when platform is not AWS."""
+        self.datasource = ec2.DataSourceEc2Local
+        ds = self._setup_ds(
+            platform_data=self.valid_platform_data,
+            sys_cfg={'datasource': {'Ec2': {'strict_id': False}}},
+            md=DEFAULT_METADATA)
+        platform_attrs = [
+            attr for attr in ec2.Platforms.__dict__.keys()
+            if not attr.startswith('__')]
+        for attr_name in platform_attrs:
+            platform_name = getattr(ec2.Platforms, attr_name)
+            if platform_name != 'AWS':
+                ds._cloud_platform = platform_name
+                ret = ds.get_data()
+                self.assertFalse(ret)
+                message = (
+                    "Local Ec2 mode only supported on ('AWS',),"
+                    ' not {0}'.format(platform_name))
+                self.assertIn(message, self.logs.getvalue())
+
     @httpretty.activate
     @mock.patch('cloudinit.sources.DataSourceEc2.util.is_FreeBSD')
     def test_ec2_local_returns_false_on_bsd(self, m_is_freebsd):
@@ -336,8 +357,8 @@ class TestConvertEc2MetadataNetworkConfig(test_helpers.CiTestCase):
         super(TestConvertEc2MetadataNetworkConfig, self).setUp()
         self.mac1 = '06:17:04:d7:26:09'
         self.network_metadata = {
-            'network': {'interfaces': {'macs': {
-                self.mac1: {'public-ipv4s': '172.31.2.16'}}}}}
+            'interfaces': {'macs': {
+                self.mac1: {'public-ipv4s': '172.31.2.16'}}}}
 
     def test_convert_ec2_metadata_network_config_skips_absent_macs(self):
         """Any mac absent from metadata is skipped by network config."""
@@ -357,7 +378,7 @@ class TestConvertEc2MetadataNetworkConfig(test_helpers.CiTestCase):
         macs_to_nics = {self.mac1: 'eth9'}
         network_metadata_ipv6 = copy.deepcopy(self.network_metadata)
         nic1_metadata = (
-            network_metadata_ipv6['network']['interfaces']['macs'][self.mac1])
+            network_metadata_ipv6['interfaces']['macs'][self.mac1])
         nic1_metadata['ipv6s'] = '2620:0:1009:fd00:e442:c88d:c04d:dc85/64'
         nic1_metadata.pop('public-ipv4s')
         expected = {'version': 1, 'config': [
@@ -373,7 +394,7 @@ class TestConvertEc2MetadataNetworkConfig(test_helpers.CiTestCase):
         macs_to_nics = {self.mac1: 'eth9'}
         network_metadata_both = copy.deepcopy(self.network_metadata)
         nic1_metadata = (
-            network_metadata_both['network']['interfaces']['macs'][self.mac1])
+            network_metadata_both['interfaces']['macs'][self.mac1])
         nic1_metadata['ipv6s'] = '2620:0:1009:fd00:e442:c88d:c04d:dc85/64'
         expected = {'version': 1, 'config': [
             {'mac_address': self.mac1, 'type': 'physical',