Merge ~smoser/cloud-init:bug/1683038-ec2-no-warn-on-explicit into cloud-init:master

[Scott Moser]

Ec2: do not warn user of strict if Ec2 is explicitly set.

If the system is configured with datasource_list of either just
'Ec2' or 'Ec2' and 'None', then consider this as if they had set
strict_id = true. A change is made to the signature of
'warn_if_necessary' to make unit testing easier.

Additionally, fix a bug in ds-identify. If the user configured:
  datasource_list: ["Ec2", "None"]
then ds-identify would write
  datasource_list: ["Ec2", "None", "None"]
which would break the logic to avoid warning.

LP: #1683038

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

PASSED: Continuous integration, rev:11c0967458cbe80e36f64ee53c47d0a448a3e137
https://jenkins.ubuntu.com/server/job/cloud-init-ci/351/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/351
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/351
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/351
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/351

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

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

Generally looks fine, one question w.r.t confirming explict_dslist check

[Scott Moser (smoser)]

On Mon, 22 May 2017, Ryan Harper wrote:

> Generally looks fine, one question w.r.t confirming explict_dslist check
>
> Diff comments:
>
> > diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py
> > index 2f9c7ed..818a639 100644
> > --- a/cloudinit/sources/DataSourceEc2.py
> > +++ b/cloudinit/sources/DataSourceEc2.py
> > @@ -266,6 +267,12 @@ def parse_strict_mode(cfgval):
> > return mode, sleep
> >
> >
> > +def _is_explicit_dslist(dslist):
> > + if 'Ec2' not in dslist:
> > + return False
> > + return (len(dslist) == 1 or (len(dslist) == 2 and 'None' in dslist))
>
> if we know dslist is len == 2, then We could ensure it looks like:
> ['Ec2', None]
>
> Right? That's exactly what we're looking for? Verus this [None, 'Ec2']
> which would pass this test but not be what we're looking for IIUC

Yeah, I avoided checking for 'Ec2' because if this datasoruce is running,
then Ec2 is in the list. But I think it makes more sense to be explicit
Will cahnge as you sigguested.
  if dslist == ['Ec2', 'None'] or dslist == ['Ec2']

sm

[Scott Moser (smoser)]

some responses in line. i'll fix some things too. thanks for review.

[Scott Moser (smoser)]

I've pushed some new commits, please see and review.
thanks.

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

PASSED: Continuous integration, rev:d0e1e415ef282ff28e8a142a0ff8389dbbb83567
https://jenkins.ubuntu.com/server/job/cloud-init-ci/383/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/383
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/383
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/383
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-s390x/383
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/383

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

review: Approve (continuous-integration)

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

PASSED: Continuous integration, rev:194e15a6785f695ed5d228fdf6601837df3fb781
https://jenkins.ubuntu.com/server/job/cloud-init-ci/384/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/384
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/384
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/384
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-s390x/384
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/384

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

review: Approve (continuous-integration)

[Chad Smith (chad.smith)]

review: Approve

[Chad Smith (chad.smith)]

pre-approved on assumption of a new unit test for behavior, if the following branch[1] lands before this one you could create a CiTestCase setting with_logs = True and self.assertIn(" not warning", self.logs.getvalue())

References:
[1] https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/324450

[Scott Moser (smoser)]

I split the ds-identify portion out into
 https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/324557
that way i can get it in now, and fix the ec2 warn portion itself with a nice test.

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

PASSED: Continuous integration, rev:4556902992eed670704cee83637e9870b6b946a2
https://jenkins.ubuntu.com/server/job/cloud-init-ci/393/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/393
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/393
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/393
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-s390x/393
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/393

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

review: Approve (continuous-integration)

e8fcbfd... by Scott Moser on 2017-05-30

Add a docstring for _collect_platform_data.

c4d0d40... by Scott Moser on 2017-05-30

initial test_ec2.py

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

FAILED: Continuous integration, rev:0c5f1e428c05f9eacb19e88fb9473ba8bf13992d
https://jenkins.ubuntu.com/server/job/cloud-init-ci/424/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/424/console
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/424/console
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/424/console
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-s390x/424/console
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/424/console

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

review: Needs Fixing (continuous-integration)

092ea40... by Scott Moser on 2017-05-31

Add test for warn_if_necessary.

Change here also changes function signature of warn_if_necessary
to allow for more direct testing without using 'activate.'

[Scott Moser (smoser)]

I have to think some more on the 'FIXME' bits here.
But i'd like feedback on teh added unit tests.

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

FAILED: Continuous integration, rev:4999cd8ef760973e37a92b2755b7454849fbcb00
https://jenkins.ubuntu.com/server/job/cloud-init-ci/429/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/429/console
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/429/console
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/429/console
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-s390x/429/console
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/429/console

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

review: Needs Fixing (continuous-integration)

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

PASSED: Continuous integration, rev:092ea4027f241e6272588ae622b1ebd2692dde08
https://jenkins.ubuntu.com/server/job/cloud-init-ci/430/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/430
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/430
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/430
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-s390x/430
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/430

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

review: Approve (continuous-integration)

a247285... by Scott Moser on 2017-05-31

add warning on unexpected situation strict_id=true and platform unknown.

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

PASSED: Continuous integration, rev:a2472853a66847a90b2e6892e3a22733518dbc8c
https://jenkins.ubuntu.com/server/job/cloud-init-ci/431/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/431
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/431
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/431
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-s390x/431
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/431

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

review: Approve (continuous-integration)

[Scott Moser (smoser)]

I think that the signature change is fine.
the other fixme I think is best fixed by just having ds-identify write an entry in the config that says that it wrote a datasource_list, and how it did that.

Ie, it could declare that it essentially copied the datasource list like this:
   di_datasource_list_method: copy

or something to that affect. Then cloud-init could read that. A 'copy' there would be different than 'search'. The copy would show no warning, but the search would.

I'm not sure stil..
We *should* grab the other parts of this merge proposal and get them into main.

[Scott Moser (smoser)]

I grabbed the initial test portions of this merge proposal and put them into another
at https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/327534
note, those do not have the tests for WarnIfNecessary

Unmerged commits

a247285... by Scott Moser on 2017-05-31

add warning on unexpected situation strict_id=true and platform unknown.

092ea40... by Scott Moser on 2017-05-31

Add test for warn_if_necessary.

Change here also changes function signature of warn_if_necessary
to allow for more direct testing without using 'activate.'

c4d0d40... by Scott Moser on 2017-05-30

initial test_ec2.py

e8fcbfd... by Scott Moser on 2017-05-30

Add a docstring for _collect_platform_data.

e358e2f... by Scott Moser on 2017-05-18

Ec2: do not warn user of strict if Ec2 is explicitly set.

If the system is configured with datasource_list of either just
'Ec2' or 'Ec2' and 'None', then consider this as if they had set
strict_id = true.

LP: #1683038

diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py
index 9e2fdc0..d0a2f8e 100644
--- a/cloudinit/sources/DataSourceEc2.py
+++ b/cloudinit/sources/DataSourceEc2.py
@@ -69,7 +69,8 @@ class DataSourceEc2(sources.DataSource):
         LOG.debug("strict_mode: %s, cloud_platform=%s",
                   strict_mode, self.cloud_platform)
         if strict_mode == "true" and self.cloud_platform == Platforms.UNKNOWN:
-            return False
+            if not _is_explicit_dslist(self.sys_cfg.get('datasource_list')):
+                return False
         elif self.cloud_platform == Platforms.NO_EC2_METADATA:
             return False
 
@@ -229,10 +230,15 @@ class DataSourceEc2(sources.DataSource):
     def activate(self, cfg, is_new_instance):
         if not is_new_instance:
             return
-        if self.cloud_platform == Platforms.UNKNOWN:
-            warn_if_necessary(
-                util.get_cfg_by_path(cfg, STRICT_ID_PATH, STRICT_ID_DEFAULT),
-                cfg)
+        # FIXME: Think about this function signature change.
+        # This change allows 'warn_if_necessary' unit test to do more
+        # without testing 'activate'.  I need to test though the upgrade
+        # adn reboot path where an old obj.pkl file has 'activate' called
+        # on it.  I think it is fine... as the new code will have a
+        # cloud_platform on it already.
+        warn_if_necessary(
+            util.get_cfg_by_path(cfg, STRICT_ID_PATH, STRICT_ID_DEFAULT),
+            cfg, platform=self.cloud_platform)
 
 
 def read_strict_mode(cfgval, default):
@@ -273,7 +279,17 @@ def parse_strict_mode(cfgval):
     return mode, sleep
 
 
-def warn_if_necessary(cfgval, cfg):
+def _is_explicit_dslist(dslist):
+    """Is dslist explicitly configured to search Ec2 datasource only?"""
+    if not dslist:
+        return False
+    return dslist in (['Ec2'], ['Ec2', 'None'])
+
+
+def warn_if_necessary(cfgval, cfg, platform):
+    if platform != Platforms.UNKNOWN:
+        return
+
     try:
         mode, sleep = parse_strict_mode(cfgval)
     except ValueError as e:
@@ -283,6 +299,18 @@ def warn_if_necessary(cfgval, cfg):
     if mode == "false":
         return
 
+    dslist = cfg.get('datasource_list')
+    if _is_explicit_dslist(cfg.get('datasource_list')):
+        LOG.debug("mode=%s but explicit dslist (%s), not warning.",
+                  mode, dslist)
+        return
+
+    if mode == "true":
+        # Unknown platform and strict is true. get_data should have
+        # returned false.
+        LOG.warning("Unexpected error.  strict_mode=%s platform=%s (%s)",
+                    mode, platform, cfgval)
+
     warnings.show_warning('non_ec2_md', cfg, mode=True, sleep=sleep)
 
 
@@ -316,10 +344,16 @@ def identify_platform():
 
 
 def _collect_platform_data():
-    # returns a dictionary with all lower case values:
-    #   uuid: system-uuid from dmi or /sys/hypervisor
-    #   uuid_source: 'hypervisor' (/sys/hypervisor/uuid) or 'dmi'
-    #   serial: dmi 'system-serial-number' (/sys/.../product_serial)
+    """Returns a dictionary of platform info from dmi or /sys/hypervisor.
+
+    Keys in the dictionary are as follows:
+       uuid: system-uuid from dmi or /sys/hypervisor
+       uuid_source: 'hypervisor' (/sys/hypervisor/uuid) or 'dmi'
+       serial: dmi 'system-serial-number' (/sys/.../product_serial)
+
+    On Ec2 instances experimentation is that product_serial is upper case,
+    and product_uuid is lower case.  This returns lower case values for both.
+    """
     data = {}
     try:
         uuid = util.load_file("/sys/hypervisor/uuid").strip()
diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py
new file mode 100644
index 0000000..443eda9
--- /dev/null
+++ b/tests/unittests/test_datasource/test_ec2.py
@@ -0,0 +1,274 @@
+# This file is part of cloud-init. See LICENSE file for license information.
+
+import httpretty
+import mock
+
+from .. import helpers as test_helpers
+from cloudinit import helpers
+from cloudinit.sources import DataSourceEc2 as ec2
+
+
+# collected from api version 2009-04-04/ with
+# python3 -c 'import json
+# from cloudinit.ec2_utils import get_instance_metadata as gm
+# print(json.dumps(gm("2009-04-04"), indent=1, sort_keys=True))'
+DEFAULT_METADATA = {
+    "ami-id": "ami-80861296",
+    "ami-launch-index": "0",
+    "ami-manifest-path": "(unknown)",
+    "block-device-mapping": {"ami": "/dev/sda1", "root": "/dev/sda1"},
+    "hostname": "ip-10-0-0-149",
+    "instance-action": "none",
+    "instance-id": "i-0052913950685138c",
+    "instance-type": "t2.micro",
+    "local-hostname": "ip-10-0-0-149",
+    "local-ipv4": "10.0.0.149",
+    "placement": {"availability-zone": "us-east-1b"},
+    "profile": "default-hvm",
+    "public-hostname": "",
+    "public-ipv4": "107.23.188.247",
+    "public-keys": {"brickies": ["ssh-rsa AAAAB3Nz....w== brickies"]},
+    "reservation-id": "r-00a2c173fb5782a08",
+    "security-groups": "wide-open"
+}
+
+
+def register_ssh_keys(rfunc, base_url, keys_data):
+    """handle ssh key inconsistencies.
+
+    public-keys in the ec2 metadata is inconsistently formated compared
+    to other entries.
+    Given keys_data of {name1: pubkey1, name2: pubkey2}
+
+    This registers the following urls:
+       base_url                 0={name1}\n1={name2} # (for each name)
+       base_url/                0={name1}\n1={name2} # (for each name)
+       base_url/0               openssh-key
+       base_url/0/              openssh-key
+       base_url/0/openssh-key   {pubkey1}
+       base_url/0/openssh-key/  {pubkey1}
+       ...
+    """
+
+    base_url = base_url.rstrip("/")
+    odd_index = '\n'.join(
+        ["{0}={1}".format(n, name)
+         for n, name in enumerate(sorted(keys_data))])
+
+    rfunc(base_url, odd_index)
+    rfunc(base_url + "/", odd_index)
+
+    for n, name in enumerate(sorted(keys_data)):
+        val = keys_data[name]
+        if isinstance(val, list):
+            val = '\n'.join(val)
+        burl = base_url + "/%s" % n
+        rfunc(burl, "openssh-key")
+        rfunc(burl + "/", "openssh-key")
+        rfunc(burl + "/%s/openssh-key" % name, val)
+        rfunc(burl + "/%s/openssh-key/" % name, val)
+
+
+def register_mock_metaserver(base_url, data):
+    """Register with httpretty a ec2 metadata like service serving 'data'.
+
+    If given a dictionary, it will populate urls under base_url for
+    that dictionary.  For example, input of
+       {"instance-id": "i-abc", "mac": "00:16:3e:00:00:00"}
+    populates
+       base_url  with 'instance-id\nmac'
+       base_url/ with 'instance-id\nmac'
+       base_url/instance-id with i-abc
+       base_url/mac with 00:16:3e:00:00:00
+    """
+    def register_helper(register, base_url, body):
+        if isinstance(body, str):
+            register(base_url, body)
+        elif isinstance(body, list):
+            register(base_url.rstrip('/'), '\n'.join(body) + '\n')
+            register(base_url.rstrip('/') + '/', '\n'.join(body) + '\n')
+        elif isinstance(body, dict):
+            vals = []
+            for k, v in body.items():
+                if k == 'public-keys':
+                    register_ssh_keys(
+                        register, base_url.rstrip('/') + '/public-keys/', v)
+                    continue
+                if isinstance(v, (str, list)):
+                    suffix = k.rstrip('/')
+                else:
+                    suffix = k.rstrip('/') + '/'
+                vals.append(suffix)
+                url = base_url.rstrip('/') + '/' + suffix
+                register_helper(register, url, v)
+            register(base_url.rstrip('/'), '\n'.join(vals) + '\n')
+            register(base_url.rstrip('/') + '/', '\n'.join(vals) + '\n')
+        elif body is None:
+            register(base_url.rstrip('/'), 'not found', status_code=404)
+
+    def myreg(*argc, **kwargs):
+        # print("register_url(%s, %s)" % (argc, kwargs))
+        return httpretty.register_uri(httpretty.GET, *argc, **kwargs)
+
+    register_helper(myreg, base_url, data)
+
+
+class TestEc2(test_helpers.HttprettyTestCase):
+    valid_platform_data = {
+        'uuid': 'ec212f79-87d1-2f1d-588f-d86dc0fd5412',
+        'uuid_source': 'dmi',
+        'serial': 'ec212f79-87d1-2f1d-588f-d86dc0fd5412',
+    }
+
+    def setUp(self):
+        super(TestEc2, self).setUp()
+        self.metadata_addr = ec2.DataSourceEc2.metadata_urls[0]
+        self.api_ver = '2009-04-04'
+
+    @property
+    def metadata_url(self):
+        return '/'.join([self.metadata_addr, self.api_ver, 'meta-data', ''])
+
+    @property
+    def userdata_url(self):
+        return '/'.join([self.metadata_addr, self.api_ver, 'user-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=None, platform_data=None, md=None, ud=None):
+        distro = {}
+        paths = helpers.Paths({})
+        if sys_cfg is None:
+            sys_cfg = {}
+        ds = ec2.DataSourceEc2(sys_cfg=sys_cfg, distro=distro, paths=paths)
+        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)
+
+        return ds
+
+    @httpretty.activate
+    def test_valid_platform_with_strict_true(self):
+        """Valid platform data should return true with strict_id true."""
+        ds = self._setup_ds(
+            platform_data=self.valid_platform_data,
+            sys_cfg={'datasource': {'Ec2': {'strict_id': True}}},
+            md=DEFAULT_METADATA)
+        ret = ds.get_data()
+        self.assertEqual(True, ret)
+
+    @httpretty.activate
+    def test_valid_platform_with_strict_false(self):
+        """Valid platform data should return true with strict_id false."""
+        ds = self._setup_ds(
+            platform_data=self.valid_platform_data,
+            sys_cfg={'datasource': {'Ec2': {'strict_id': False}}},
+            md=DEFAULT_METADATA)
+        ret = ds.get_data()
+        self.assertEqual(True, ret)
+
+    @httpretty.activate
+    def test_unknown_platform_with_strict_true(self):
+        """Unknown platform data with strict_id true should return False."""
+        uuid = 'ab439480-72bf-11d3-91fc-b8aded755F9a'
+        ds = self._setup_ds(
+            platform_data={'uuid': uuid, 'uuid_source': 'dmi', 'serial': ''},
+            sys_cfg={'datasource': {'Ec2': {'strict_id': True}}},
+            md=DEFAULT_METADATA)
+        ret = ds.get_data()
+        self.assertEqual(False, ret)
+
+    @httpretty.activate
+    def test_unknown_platform_with_strict_false(self):
+        """Unknown platform data with strict_id false should return True."""
+        uuid = 'ab439480-72bf-11d3-91fc-b8aded755F9a'
+        ds = self._setup_ds(
+            platform_data={'uuid': uuid, 'uuid_source': 'dmi', 'serial': ''},
+            sys_cfg={'datasource': {'Ec2': {'strict_id': False}}},
+            md=DEFAULT_METADATA)
+        ret = ds.get_data()
+        self.assertEqual(True, ret)
+
+
+class TestWarnIfNecessary(test_helpers.CiTestCase):
+    """Test warn_if_necessary behaves as expected.
+
+    warn_if_necessary is called with the value of the 'strict_id' setting.
+    It should call show_warning if a warning should be shown."""
+
+    with_logs = True
+
+    @mock.patch("cloudinit.sources.DataSourceEc2.warnings.show_warning")
+    def test_no_warnings_if_platform_identified(self, m_show_warning):
+        """If platform is something known, then no warninings expected."""
+        ec2.warn_if_necessary("warn", cfg={}, platform=ec2.Platforms.AWS)
+        ec2.warn_if_necessary("warn", cfg={}, platform=ec2.Platforms.BRIGHTBOX)
+        ec2.warn_if_necessary("warn", cfg={}, platform=ec2.Platforms.SEEDED)
+        ec2.warn_if_necessary("warn", cfg={}, platform=ec2.Platforms.ALIYUN)
+        self.assertEqual(0, m_show_warning.call_count)
+
+    @mock.patch("cloudinit.sources.DataSourceEc2.warnings.show_warning")
+    def test_warn_if_platform_unknown_and_strict_true(self, m_show_warning):
+        """If platform is unknown and strict is warn, warnings expected."""
+        ec2.warn_if_necessary("warn", cfg={}, platform=ec2.Platforms.UNKNOWN)
+        self.assertEqual(1, m_show_warning.call_count)
+
+    @mock.patch("cloudinit.sources.DataSourceEc2.warnings.show_warning")
+    def test_no_warn_if_platform_unknown_and_not_strict(self, m_show_warning):
+        """If platform is unknown but strict is false, no warnings."""
+        ec2.warn_if_necessary("false", cfg={}, platform=ec2.Platforms.UNKNOWN)
+        self.assertEqual(0, m_show_warning.call_count)
+
+    @mock.patch("cloudinit.sources.DataSourceEc2.warnings.show_warning")
+    def test_no_warn_if_unknown_strict_true_explicit_ds(self, m_show_warning):
+        """No warnings on strict_id=warn, platform=unknown, ec2 only."""
+        ec2.warn_if_necessary("false", cfg={'datasource_list': ['Ec2']},
+                              platform=ec2.Platforms.UNKNOWN)
+        self.assertEqual(0, m_show_warning.call_count)
+
+    @mock.patch("cloudinit.sources.DataSourceEc2.warnings.show_warning")
+    def test_no_warn_if_unknown_strict_true_exp_and_none(self, m_show_warning):
+        """No warnings on strict_id=warn, platform=unknown, Ec2, None.
+
+        FIXME: If ds-identify writes datasource_list due to
+               strict_id = warn and DS_MAYBE, will this code path
+               be taken?  That *should* get a warning.
+
+        If datasource_list contains ['Ec2', 'None'], then no warning
+        should be seen as this is explicit configuration of Ec2."""
+        ec2.warn_if_necessary(
+            "warn", cfg={'datasource_list': ['Ec2', 'None']},
+            platform=ec2.Platforms.UNKNOWN)
+        self.assertEqual(0, m_show_warning.call_count)
+
+    @mock.patch("cloudinit.sources.DataSourceEc2.warnings.show_warning")
+    def test_warn_if_unknown_strict_true_noexp_and_none(self, m_show_warning):
+        """Warn if strict_id=warn, platform=unknown, but not explicit list.
+
+        This is not an 'explicit' configuration because it has extra
+        entry in its datasource_list.
+        """
+        ec2.warn_if_necessary(
+            "warn", cfg={'datasource_list': ['OpenStack', 'Ec2', 'None']},
+            platform=ec2.Platforms.UNKNOWN)
+        self.assertNotIn('Unexpected error', self.logs.getvalue())
+        self.assertEqual(1, m_show_warning.call_count)
+
+    @mock.patch("cloudinit.sources.DataSourceEc2.warnings.show_warning")
+    def test_additional_warn_on_strict_and_unknown(self, m_show_warning):
+        """If strict_id is true and platform is unknown, unexpected error."""
+        ec2.warn_if_necessary(
+            "true", cfg={}, platform=ec2.Platforms.UNKNOWN)
+        self.assertIn('Unexpected error', self.logs.getvalue())
+        self.assertEqual(1, m_show_warning.call_count)
+
+
+# vi: ts=4 expandtab