Merge ~smoser/cloud-init:fix/1766401-ibmcloud-upgrade-xenial into cloud-init:master

[Scott Moser]

IBMCloud: Disable config-drive and nocloud only if IBMCloud is enabled.

Ubuntu images on IBMCloud for 16.04 have some seed data in
/var/lib/cloud/data/seed/nocloud-net. In order to have systems with
IBMCloud enabled, we modified ds-identify detection to skip that seed
if the system was on IBMCloud. That change did not consider the
fact that IBMCloud might not be in the datasource list.

There was similar logic in the ConfigDrive datasource in ds-identify
and the datasource itself.

Config drive is now updated to only check and avoid IBMCloud if IBMCloud
is enabled. The check in ds-identify for nocloud was dropped. If a
user provides a nocloud seed on IBMCloud, then that can be used.

This means that systems running Xenial will continue to get their
old datasources.

LP: #1766401

see commit message

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

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

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

review: Needs Fixing (continuous-integration)

[Scott Moser (smoser)]

PPA at https://launchpad.net/~smoser/+archive/ubuntu/lp1766401 has a build with a proposed fix.

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

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

review: Approve (continuous-integration)

[Scott Moser (smoser)]

I've tested upgrading the from xenial image to the build in the ppa.
It seems to have gone correctly for each of the instances I launched.
Here are the results:
The paste is a result of running 'collect-info' from
  https://gist.github.com/smoser/94195d335b5af32efe1be056e413e130/

Basically I used our 'launch-softlayer' script to launch
via 'template' and 'oscode' and with and without user-data for each.
So 4 total instances.

Then collect info from each, upgrade, reboot, collect-info:

 $ wget https://gist.githubusercontent.com/smoser/94195d335b5af32efe1be056e413e130/raw/collect-info -O collect-info
 $ chmod 755 collect-info
 $ echo $(hostname) before $(./collect-info before $(hostname) | pastebinit)
 $ mv /var/log/cloud-init* .
 $ apt-add-repository -y ppa:smoser/lp1766401 && apt-get update -q
 $ apt-get install cloud-init
 $ reboot
 $ echo $(hostname) after $(./collect-info after $(hostname) | pastebinit)

xt-oscode-with before http://paste.ubuntu.com/p/gW9gR5WQnR/
xt-oscode-with after http://paste.ubuntu.com/p/Z7pM4bGjPy/
xt-oscode-without before http://paste.ubuntu.com/p/JJK3mm6gZ2/
xt-oscode-without after http://paste.ubuntu.com/p/RrSNSJnNbg/
xt-template-with before http://paste.ubuntu.com/p/2kFZ9wb768/
xt-template-with after http://paste.ubuntu.com/p/QtyYy3MBKm/
xt-template-without before http://paste.ubuntu.com/p/ZJZS8CkTYm/
xt-template-without after http://paste.ubuntu.com/p/GdPr4YT7jV/

[Chad Smith (chad.smith)]

Tested as well on systems broken by the previous SRU'd cloud-init. A 'cloud-init clean --reboot' properly detects the ConfigDrive datasource which presents the proper network configuration.
+1

Validated both clean and dirty reboot scenarios with user-data to confirm proper behavior.

review: Approve

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

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

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

review: Needs Fixing (continuous-integration)

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

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

review: Approve (continuous-integration)

[Chad Smith (chad.smith)]

review: Approve

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

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

review: Approve (continuous-integration)

[Chad Smith (chad.smith)]

An upstream commit landed for this bug.

To view that commit see the following URL:
https://git.launchpad.net/cloud-init/commit/?id=11172924

diff --git a/cloudinit/sources/DataSourceConfigDrive.py b/cloudinit/sources/DataSourceConfigDrive.py
index c7b5fe5..121cf21 100644
--- a/cloudinit/sources/DataSourceConfigDrive.py
+++ b/cloudinit/sources/DataSourceConfigDrive.py
@@ -69,7 +69,8 @@ class DataSourceConfigDrive(openstack.SourceMixin, sources.DataSource):
                 util.logexc(LOG, "Failed reading config drive from %s", sdir)
 
         if not found:
-            for dev in find_candidate_devs():
+            dslist = self.sys_cfg.get('datasource_list')
+            for dev in find_candidate_devs(dslist=dslist):
                 try:
                     # Set mtype if freebsd and turn off sync
                     if dev.startswith("/dev/cd"):
@@ -211,7 +212,7 @@ def write_injected_files(files):
                 util.logexc(LOG, "Failed writing file: %s", filename)
 
 
-def find_candidate_devs(probe_optical=True):
+def find_candidate_devs(probe_optical=True, dslist=None):
     """Return a list of devices that may contain the config drive.
 
     The returned list is sorted by search order where the first item has
@@ -227,6 +228,9 @@ def find_candidate_devs(probe_optical=True):
         * either vfat or iso9660 formated
         * labeled with 'config-2' or 'CONFIG-2'
     """
+    if dslist is None:
+        dslist = []
+
     # query optical drive to get it in blkid cache for 2.6 kernels
     if probe_optical:
         for device in OPTICAL_DEVICES:
@@ -257,7 +261,8 @@ def find_candidate_devs(probe_optical=True):
     devices = [d for d in candidates
                if d in by_label or not util.is_partition(d)]
 
-    if devices:
+    LOG.debug("devices=%s dslist=%s", devices, dslist)
+    if devices and "IBMCloud" in dslist:
         # IBMCloud uses config-2 label, but limited to a single UUID.
         ibm_platform, ibm_path = get_ibm_platform()
         if ibm_path in devices:
diff --git a/tests/unittests/test_ds_identify.py b/tests/unittests/test_ds_identify.py
index ad7fe41..4d8a436 100644
--- a/tests/unittests/test_ds_identify.py
+++ b/tests/unittests/test_ds_identify.py
@@ -184,17 +184,18 @@ class DsIdentifyBase(CiTestCase):
             data, RC_FOUND, dslist=[data.get('ds'), DS_NONE])
 
     def _check_via_dict(self, data, rc, dslist=None, **kwargs):
-        found_rc, out, err, cfg, files = self._call_via_dict(data, **kwargs)
+        ret = self._call_via_dict(data, **kwargs)
         good = False
         try:
-            self.assertEqual(rc, found_rc)
+            self.assertEqual(rc, ret.rc)
             if dslist is not None:
-                self.assertEqual(dslist, cfg['datasource_list'])
+                self.assertEqual(dslist, ret.cfg['datasource_list'])
             good = True
         finally:
             if not good:
-                _print_run_output(rc, out, err, cfg, files)
-        return rc, out, err, cfg, files
+                _print_run_output(ret.rc, ret.stdout, ret.stderr, ret.cfg,
+                                  ret.files)
+        return ret
 
 
 class TestDsIdentify(DsIdentifyBase):
@@ -245,13 +246,40 @@ class TestDsIdentify(DsIdentifyBase):
     def test_config_drive(self):
         """ConfigDrive datasource has a disk with LABEL=config-2."""
         self._test_ds_found('ConfigDrive')
-        return
 
     def test_config_drive_upper(self):
         """ConfigDrive datasource has a disk with LABEL=CONFIG-2."""
         self._test_ds_found('ConfigDriveUpper')
         return
 
+    def test_config_drive_seed(self):
+        """Config Drive seed directory."""
+        self._test_ds_found('ConfigDrive-seed')
+
+    def test_config_drive_interacts_with_ibmcloud_config_disk(self):
+        """Verify ConfigDrive interaction with IBMCloud.
+
+        If ConfigDrive is enabled and not IBMCloud, then ConfigDrive
+        should claim the ibmcloud 'config-2' disk.
+        If IBMCloud is enabled, then ConfigDrive should skip."""
+        data = copy.deepcopy(VALID_CFG['IBMCloud-config-2'])
+        files = data.get('files', {})
+        if not files:
+            data['files'] = files
+        cfgpath = 'etc/cloud/cloud.cfg.d/99_networklayer_common.cfg'
+
+        # with list including IBMCloud, config drive should be not found.
+        files[cfgpath] = 'datasource_list: [ ConfigDrive, IBMCloud ]\n'
+        ret = self._check_via_dict(data, shell_true)
+        self.assertEqual(
+            ret.cfg.get('datasource_list'), ['IBMCloud', 'None'])
+
+        # But if IBMCloud is not enabled, config drive should claim this.
+        files[cfgpath] = 'datasource_list: [ ConfigDrive, NoCloud ]\n'
+        ret = self._check_via_dict(data, shell_true)
+        self.assertEqual(
+            ret.cfg.get('datasource_list'), ['ConfigDrive', 'None'])
+
     def test_ibmcloud_template_userdata_in_provisioning(self):
         """Template provisioned with user-data during provisioning stage.
 
@@ -307,6 +335,37 @@ class TestDsIdentify(DsIdentifyBase):
         self._check_via_dict(
             data, rc=RC_FOUND, dslist=['ConfigDrive', DS_NONE])
 
+    def test_ibmcloud_with_nocloud_seed(self):
+        """NoCloud seed should be preferred over IBMCloud.
+
+        A nocloud seed should be preferred over IBMCloud even if enabled.
+        Ubuntu 16.04 images have <vlc>/seed/nocloud-net. LP: #1766401."""
+        data = copy.deepcopy(VALID_CFG['IBMCloud-config-2'])
+        files = data.get('files', {})
+        if not files:
+            data['files'] = files
+        files.update(VALID_CFG['NoCloud-seed']['files'])
+        ret = self._check_via_dict(data, shell_true)
+        self.assertEqual(
+            ['NoCloud', 'IBMCloud', 'None'],
+            ret.cfg.get('datasource_list'))
+
+    def test_ibmcloud_with_configdrive_seed(self):
+        """ConfigDrive seed should be preferred over IBMCloud.
+
+        A ConfigDrive seed should be preferred over IBMCloud even if enabled.
+        Ubuntu 16.04 images have a fstab entry that mounts the
+        METADATA disk into <vlc>/seed/config_drive. LP: ##1766401."""
+        data = copy.deepcopy(VALID_CFG['IBMCloud-config-2'])
+        files = data.get('files', {})
+        if not files:
+            data['files'] = files
+        files.update(VALID_CFG['ConfigDrive-seed']['files'])
+        ret = self._check_via_dict(data, shell_true)
+        self.assertEqual(
+            ['ConfigDrive', 'IBMCloud', 'None'],
+            ret.cfg.get('datasource_list'))
+
     def test_policy_disabled(self):
         """A Builtin policy of 'disabled' should return not found.
 
@@ -684,6 +743,12 @@ VALID_CFG = {
              },
         ],
     },
+    'ConfigDrive-seed': {
+        'ds': 'ConfigDrive',
+        'files': {
+            os.path.join(P_SEED_DIR, 'config_drive', 'openstack',
+                         'latest', 'meta_data.json'): 'md\n'},
+    },
     'Hetzner': {
         'ds': 'Hetzner',
         'files': {P_SYS_VENDOR: 'Hetzner\n'},
diff --git a/tools/ds-identify b/tools/ds-identify
index 7fff5d1..9f0d96f 100755
--- a/tools/ds-identify
+++ b/tools/ds-identify
@@ -601,7 +601,6 @@ dscheck_NoCloud() {
         *\ ds=nocloud*) return ${DS_FOUND};;
     esac
 
-    is_ibm_cloud && return ${DS_NOT_FOUND}
     for d in nocloud nocloud-net; do
         check_seed_dir "$d" meta-data user-data && return ${DS_FOUND}
         check_writable_seed_dir "$d" meta-data user-data && return ${DS_FOUND}
@@ -612,11 +611,12 @@ dscheck_NoCloud() {
     return ${DS_NOT_FOUND}
 }
 
+is_ds_enabled() {
+    local name="$1" pad=" ${DI_DSLIST} "
+    [ "${pad#* $name }" != "${pad}" ]
+}
+
 check_configdrive_v2() {
-    is_ibm_cloud && return ${DS_NOT_FOUND}
-    if has_fs_with_label CONFIG-2 config-2; then
-        return ${DS_FOUND}
-    fi
     # look in /config-drive <vlc>/seed/config_drive for a directory
     # openstack/YYYY-MM-DD format with a file meta_data.json
     local d=""
@@ -631,6 +631,13 @@ check_configdrive_v2() {
         debug 1 "config drive seeded directory had only 'latest'"
         return ${DS_FOUND}
     fi
+
+    is_ds_enabled "IBMCloud"
+    debug 1 "is_ds_enabled returned $?: $DI_DSLIST"
+    is_ds_enabled "IBMCloud" && is_ibm_cloud && return ${DS_NOT_FOUND}
+    if has_fs_with_label CONFIG-2 config-2; then
+        return ${DS_FOUND}
+    fi
     return ${DS_NOT_FOUND}
 }