Merge ~smoser/cloud-init:fix/1771382-ds-identify-ensure-sane-path into cloud-init:master

[Scott Moser]

ds-identify: ensure that we have certain tokens in PATH.

SuSE builds were not getting a PATH set in generator's environment.
This may seem like mis-configuration on the system, but caused ds-identify
to fail to find blkid (or any other program).

The change here just ensures that we get /sbin /usr/sbin /bin /usr/bin
into the PATH when main is run.

LP: #1771382

see commit message

[Robert Schweikert (rjschwei)]

Thanks

review: Approve

[Scott Moser (smoser)]

Just to write this down somewhere... the adding paths and searching (as compared to just appending PATH=$PATH:/usr/bin:....) doesnt cost a lot of time.

### Running ":" 100k times as benchmark
$ time sh /tmp/my.sh "/my/random:/path:/here" : $(seq 1 100000)
/my/random:/path:/here

real 0m0.162s
user 0m0.133s
sys 0m0.032s

### Running ensure_sane_path 100k times.
$ time sh /tmp/my.sh "/my/random:/path:/here" ensure_sane_path $(seq 1 100000)
/my/random:/path:/here:/sbin:/usr/sbin:/bin:/usr/bin

real 0m0.586s
user 0m0.543s
sys 0m0.045s

### Run with a larger input path.
$ time sh /tmp/my.sh "$PATH" ensure_sane_path $(seq 1 100000)
/home/smoser/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin:/snap/bin:/var/lib/snapd/snap/bin

real 0m0.635s
user 0m0.611s
sys 0m0.026s

So the cost of 100000 runs of ensure_sane_path is somewhere in the realm
of .5 seconds. That is .000005 for the one run we'll do.

very small.

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

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

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

This will work until some other setup doesn't put blkid in our "sane" path list. Previously our "sane" line was having a proper PATH defined.

review: Approve

[Scott Moser (smoser)]

An upstream commit landed for this bug.

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

diff --git a/tests/unittests/test_ds_identify.py b/tests/unittests/test_ds_identify.py
index 7f12be5..64d9f9f 100644
--- a/tests/unittests/test_ds_identify.py
+++ b/tests/unittests/test_ds_identify.py
@@ -175,7 +175,9 @@ class DsIdentifyBase(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_no_dmi', 'files'):
+        passthrough = ('mocks', 'func', 'args', 'policy_dmi',
+                       'policy_no_dmi', 'files')
+        for k in passthrough:
             if k in data:
                 xwargs[k] = data[k]
             if k in kwargs:
@@ -535,6 +537,25 @@ class TestDsIdentify(DsIdentifyBase):
         del mycfg['files'][ds_smartos.METADATA_SOCKFILE]
         self._check_via_dict(mycfg, rc=RC_NOT_FOUND, policy_dmi="disabled")
 
+    def test_path_env_gets_set_from_main(self):
+        """PATH environment should always have some tokens when main is run.
+
+        We explicitly call main as we want to ensure it updates PATH."""
+        cust = copy.deepcopy(VALID_CFG['NoCloud'])
+        rootd = self.tmp_dir()
+        mpp = 'main-printpath'
+        pre = "MYPATH="
+        cust['files'][mpp] = (
+            'PATH="/mycust/path"; main; r=$?; echo ' + pre + '$PATH; exit $r;')
+        ret = self._check_via_dict(
+            cust, RC_FOUND,
+            func=".", args=[os.path.join(rootd, mpp)], rootd=rootd)
+        line = [l for l in ret.stdout.splitlines() if l.startswith(pre)][0]
+        toks = line.replace(pre, "").split(":")
+        expected = ["/sbin", "/bin", "/usr/sbin", "/usr/bin", "/mycust/path"]
+        self.assertEqual(expected, [p for p in expected if p in toks],
+                         "path did not have expected tokens")
+
 
 class TestIsIBMProvisioning(DsIdentifyBase):
     """Test the is_ibm_provisioning method in ds-identify."""
diff --git a/tools/ds-identify b/tools/ds-identify
index dc7ac3a..ce0477a 100755
--- a/tools/ds-identify
+++ b/tools/ds-identify
@@ -187,6 +187,16 @@ block_dev_with_label() {
     return 0
 }
 
+ensure_sane_path() {
+    local t
+    for t in /sbin /usr/sbin /bin /usr/bin; do
+        case ":$PATH:" in
+            *:$t:*|*:$t/:*) continue;;
+        esac
+        PATH="${PATH:+${PATH}:}$t"
+    done
+}
+
 read_fs_info() {
     cached "${DI_BLKID_OUTPUT}" && return 0
     # do not rely on links in /dev/disk which might not be present yet.
@@ -1464,6 +1474,7 @@ _main() {
 
 main() {
     local ret=""
+    ensure_sane_path
     [ -d "$PATH_RUN_CI" ] || mkdir -p "$PATH_RUN_CI"
     if [ "${1:+$1}" != "--force" ] && [ -f "$PATH_RUN_CI_CFG" ] &&
         [ -f "$PATH_RUN_DI_RESULT" ]; then