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

Proposed by Scott Moser
Status: Merged
Approved by: Scott Moser
Approved revision: 67270f244f3a96f9328aa983cd957fa55b3f1240
Merge reported by: Scott Moser
Merged at revision: b4ae0e1fb8a48a83ea325cf032eb1acb196ee31c
Proposed branch: ~smoser/cloud-init:fix/1771382-ds-identify-ensure-sane-path
Merge into: cloud-init:master
Diff against target: 70 lines (+33/-1)
2 files modified
tests/unittests/ (+22/-1)
tools/ds-identify (+11/-0)
Reviewer Review Type Date Requested Status
Ryan Harper Approve
Server Team CI bot continuous-integration Approve
Robert Schweikert (community) Approve
Review via email:

Commit message

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

Description of the change

see commit message

To post a comment you must log in.
Revision history for this message
Robert Schweikert (rjschwei) wrote :


review: Approve
Revision history for this message
Scott Moser (smoser) wrote :

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/random:/path:/here" : $(seq 1 100000)

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

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

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

### Run with a larger input path.
$ time sh /tmp/ "$PATH" ensure_sane_path $(seq 1 100000)

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.

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:67270f244f3a96f9328aa983cd957fa55b3f1240
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:

review: Approve (continuous-integration)
Revision history for this message
Ryan Harper (raharper) wrote :

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
Revision history for this message
Scott Moser (smoser) wrote :

An upstream commit landed for this bug.

To view that commit see the following URL:

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/tests/unittests/ b/tests/unittests/
2index 7f12be5..64d9f9f 100644
3--- a/tests/unittests/
4+++ b/tests/unittests/
5@@ -175,7 +175,9 @@ class DsIdentifyBase(CiTestCase):
6 def _call_via_dict(self, data, rootd=None, **kwargs):
7 # return output of with a dict input like VALID_CFG[item]
8 xwargs = {'rootd': rootd}
9- for k in ('mocks', 'args', 'policy_dmi', 'policy_no_dmi', 'files'):
10+ passthrough = ('mocks', 'func', 'args', 'policy_dmi',
11+ 'policy_no_dmi', 'files')
12+ for k in passthrough:
13 if k in data:
14 xwargs[k] = data[k]
15 if k in kwargs:
16@@ -535,6 +537,25 @@ class TestDsIdentify(DsIdentifyBase):
17 del mycfg['files'][ds_smartos.METADATA_SOCKFILE]
18 self._check_via_dict(mycfg, rc=RC_NOT_FOUND, policy_dmi="disabled")
20+ def test_path_env_gets_set_from_main(self):
21+ """PATH environment should always have some tokens when main is run.
23+ We explicitly call main as we want to ensure it updates PATH."""
24+ cust = copy.deepcopy(VALID_CFG['NoCloud'])
25+ rootd = self.tmp_dir()
26+ mpp = 'main-printpath'
27+ pre = "MYPATH="
28+ cust['files'][mpp] = (
29+ 'PATH="/mycust/path"; main; r=$?; echo ' + pre + '$PATH; exit $r;')
30+ ret = self._check_via_dict(
31+ cust, RC_FOUND,
32+ func=".", args=[os.path.join(rootd, mpp)], rootd=rootd)
33+ line = [l for l in ret.stdout.splitlines() if l.startswith(pre)][0]
34+ toks = line.replace(pre, "").split(":")
35+ expected = ["/sbin", "/bin", "/usr/sbin", "/usr/bin", "/mycust/path"]
36+ self.assertEqual(expected, [p for p in expected if p in toks],
37+ "path did not have expected tokens")
40 class TestIsIBMProvisioning(DsIdentifyBase):
41 """Test the is_ibm_provisioning method in ds-identify."""
42diff --git a/tools/ds-identify b/tools/ds-identify
43index dc7ac3a..ce0477a 100755
44--- a/tools/ds-identify
45+++ b/tools/ds-identify
46@@ -187,6 +187,16 @@ block_dev_with_label() {
47 return 0
48 }
50+ensure_sane_path() {
51+ local t
52+ for t in /sbin /usr/sbin /bin /usr/bin; do
53+ case ":$PATH:" in
54+ *:$t:*|*:$t/:*) continue;;
55+ esac
56+ PATH="${PATH:+${PATH}:}$t"
57+ done
60 read_fs_info() {
61 cached "${DI_BLKID_OUTPUT}" && return 0
62 # do not rely on links in /dev/disk which might not be present yet.
63@@ -1464,6 +1474,7 @@ _main() {
65 main() {
66 local ret=""
67+ ensure_sane_path
68 [ -d "$PATH_RUN_CI" ] || mkdir -p "$PATH_RUN_CI"
69 if [ "${1:+$1}" != "--force" ] && [ -f "$PATH_RUN_CI_CFG" ] &&
70 [ -f "$PATH_RUN_DI_RESULT" ]; then


People subscribed via source and target branches