cloud-init-dev team mailing list archive

[Scott Moser <smoser@xxxxxxxxxx>]

Scott Moser has proposed merging ~smoser/cloud-init:bug/1715128-ec2-used-on-openstack into cloud-init:master.

Commit message:
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 two fold:

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.

LP: #1715128

Requested reviews:
  cloud-init commiters (cloud-init-dev)
Related bugs:
  Bug #1715128 in cloud-init (Ubuntu): "Crashes in convert_ec2_metadata_network_config on ScalingStack bos01 (ppc64el)"
  https://bugs.launchpad.net/ubuntu/+source/cloud-init/+bug/1715128
  Bug #1715241 in cloud-init: "ds-identify openstack returns not found on non-intel"
  https://bugs.launchpad.net/cloud-init/+bug/1715241

For more details, see:
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/330361
-- 
Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:bug/1715128-ec2-used-on-openstack into cloud-init:master.

diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py
index 07c12bb..dfab55c 100644
--- a/cloudinit/sources/DataSourceEc2.py
+++ b/cloudinit/sources/DataSourceEc2.py
@@ -321,6 +321,14 @@ class DataSourceEc2Local(DataSourceEc2):
     """
     get_network_metadata = True  # Get metadata network config if present
 
+    def get_data(self):
+        supported = (Platforms.AWS,)
+        if self.cloud_platform not in supported:
+            LOG.debug("Local Ec2 mode only supported on %s, not %s",
+                      supported, self.cloud_platform)
+            return False
+        return super(DataSourceEc2Local, self).get_data()
+
 
 def read_strict_mode(cfgval, default):
     try:
diff --git a/tests/unittests/test_ds_identify.py b/tests/unittests/test_ds_identify.py
index 1a81a89..c389029 100644
--- a/tests/unittests/test_ds_identify.py
+++ b/tests/unittests/test_ds_identify.py
@@ -11,6 +11,10 @@ from cloudinit.tests.helpers import (
 
 UNAME_MYSYS = ("Linux bart 4.4.0-62-generic #83-Ubuntu "
                "SMP Wed Jan 18 14:10:15 UTC 2017 x86_64 GNU/Linux")
+UNAME_PPC64EL = ("Linux diamond 4.4.0-83-generic #106-Ubuntu SMP "
+                 "Mon Jun 26 17:53:54 UTC 2017 "
+                 "ppc64le ppc64le ppc64le GNU/Linux")
+
 BLKID_EFI_ROOT = """
 DEVNAME=/dev/sda1
 UUID=8B36-5390
@@ -23,6 +27,8 @@ TYPE=ext4
 PARTUUID=30c65c77-e07d-4039-b2fb-88b1fb5fa1fc
 """
 
+POLICY_FOUND_ONLY = "search,found=all,maybe=none,notfound=disabled"
+POLICY_FOUND_OR_MAYBE = "search,found=all,maybe=all,notfound=disabled"
 DI_DEFAULT_POLICY = "search,found=all,maybe=all,notfound=enabled"
 DI_DEFAULT_POLICY_NO_DMI = "search,found=all,maybe=all,notfound=disabled"
 
@@ -48,6 +54,7 @@ P_SEED_DIR = "var/lib/cloud/seed"
 P_DSID_CFG = "etc/cloud/ds-identify.cfg"
 
 MOCK_VIRT_IS_KVM = {'name': 'detect_virt', 'RET': 'kvm', 'ret': 0}
+MOCK_UNAME_IS_PPC64 = {'name': 'uname', 'out': UNAME_PPC64EL, 'ret': 0}
 
 
 class TestDsIdentify(CiTestCase):
@@ -55,7 +62,7 @@ class TestDsIdentify(CiTestCase):
 
     def call(self, rootd=None, mocks=None, args=None, files=None,
              policy_dmi=DI_DEFAULT_POLICY,
-             policy_nodmi=DI_DEFAULT_POLICY_NO_DMI):
+             policy_no_dmi=DI_DEFAULT_POLICY_NO_DMI):
         if args is None:
             args = []
         if mocks is None:
@@ -81,7 +88,7 @@ class TestDsIdentify(CiTestCase):
             "PATH_ROOT='%s'" % rootd,
             ". " + self.dsid_path,
             'DI_DEFAULT_POLICY="%s"' % policy_dmi,
-            'DI_DEFAULT_POLICY_NO_DMI="%s"' % policy_nodmi,
+            'DI_DEFAULT_POLICY_NO_DMI="%s"' % policy_no_dmi,
             ""
         ]
 
@@ -137,7 +144,7 @@ class TestDsIdentify(CiTestCase):
     def _call_via_dict(self, data, rootd=None, **kwargs):
         # return output of self.call with a dict input like VALID_CFG[item]
         xwargs = {'rootd': rootd}
-        for k in ('mocks', 'args', 'policy_dmi', 'policy_nodmi', 'files'):
+        for k in ('mocks', 'args', 'policy_dmi', 'policy_no_dmi', 'files'):
             if k in data:
                 xwargs[k] = data[k]
             if k in kwargs:
@@ -261,6 +268,29 @@ class TestDsIdentify(CiTestCase):
         self._check_via_dict(mydata, rc=RC_FOUND, dslist=['AliYun', DS_NONE],
                              policy_dmi=policy)
 
+    def test_default_openstack_intel_is_found(self):
+        """On Intel, openstack must be identified."""
+        self._test_ds_found('OpenStack')
+
+    def test_openstack_on_non_intel_is_maybe(self):
+        """On non-Intel, openstack without dmi info is maybe.
+
+        nova does not identify itself on platforms other than intel.
+           https://bugs.launchpad.net/cloud-init/+bugs?field.tag=dsid-nova""";
+
+        data = VALID_CFG['OpenStack'].copy()
+        del data['files'][P_PRODUCT_NAME]
+        data.update({'policy_dmi': POLICY_FOUND_OR_MAYBE,
+                     'policy_no_dmi': POLICY_FOUND_OR_MAYBE})
+
+        # this should show not found as default uname in tests is intel.
+        # and intel openstack requires positive identification.
+        self._check_via_dict(data, RC_NOT_FOUND, dslist=None)
+
+        # updating the uname to ppc64 though should get a maybe.
+        data.update({'mocks': [MOCK_VIRT_IS_KVM, MOCK_UNAME_IS_PPC64]})
+        self._check_via_dict(data, RC_FOUND, dslist=['OpenStack', 'None'])
+
 
 def blkid_out(disks=None):
     """Convert a list of disk dictionaries into blkid content."""
@@ -341,6 +371,13 @@ VALID_CFG = {
         'files': {P_PRODUCT_SERIAL: 'GoogleCloud-8f2e88f\n'},
         'mocks': [MOCK_VIRT_IS_KVM],
     },
+    'OpenStack': {
+        'ds': 'OpenStack',
+        'files': {P_PRODUCT_NAME: 'OpenStack Nova\n'},
+        'mocks': [MOCK_VIRT_IS_KVM],
+        'policy_dmi': POLICY_FOUND_ONLY,
+        'policy_no_dmi': POLICY_FOUND_ONLY,
+    },
     'ConfigDrive': {
         'ds': 'ConfigDrive',
         'mocks': [
diff --git a/tools/ds-identify b/tools/ds-identify
index 33bd299..ee5e05a 100755
--- a/tools/ds-identify
+++ b/tools/ds-identify
@@ -833,6 +833,12 @@ dscheck_OpenStack() {
         return ${DS_FOUND}
     fi
 
+    # LP: #1715241 : arch other than intel are not identified properly.
+    case "$DI_UNAME_MACHINE" in
+        i?86|x86_64) :;;
+        *) return ${DS_MAYBE};;
+    esac
+
     return ${DS_NOT_FOUND}
 }