1127521 - runner.cfg shows a diff on every puppet run

Status: RESOLVED FIXED

(Infrastructure & Operations :: RelOps: Puppet, task)

Description

[Dustin J. Mitchell [:dustin] (he/him)]

Notice: /Stage[main]/Runner/File[/opt/runner/runner.cfg]/content:
--- /opt/runner/runner.cfg      2015-01-29 13:08:23.682897750 -0800
+++ /tmp/puppet-file20150129-3059-18r88yj-0     2015-01-29 13:18:18.780904432 -0800
@@ -14,7 +14,7 @@
 GIT_SHARE_BASE_DIR=/builds/git-shared
 INFLUXDB_URL=*mumble*
 INFLUXDB_NAME=runner_tasks
-PATH=/usr/bin:/sbin:/bin:/usr/sbin:/usr/local/bin
+PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/bin/X11:/usr/local/bin
 
 [hg]
 tools_path = /tools/checkouts/build-tools

Notice: /Stage[main]/Runner/File[/opt/runner/runner.cfg]/content: content changed '{md5}bd3ef4ae116dd70cf0572bfedf6264cb' to '{md5}889e4e35d56defd097ee299ab778686c'

Comment 1

[Morgan Phillips [:mrrrgn]]

Attachment: puppet-issues.dump

So, what's happened is that we set the path based on whatever the user has at startup. It's trying bake export PATH=$PATH:/usr/local/bin the runner config.

The issue we're seeing, is that apparently the path isn't consistent across all of our hosts across reboots. I'm still thinking about how to approach this. It wasn't my doing, so I'm not yet sure what changing this could break. Also, I need to wrap my head around whether or not this is a bad thing to do.

At the very least, if we decide to keep this, it needs comments in the manifest. This was rather confusing to debug! >.<

Comment 2

[Morgan Phillips [:mrrrgn]]

Ahh, this was added in ea14f564a94e

changeset:   2680:ea14f564a94e
parent:      2678:e200fa718ce8
user:        [e-mail]
date:        Wed Jan 07 09:02:29 2015 -0500
summary:     Bug 1094293 - Add initial Yosemite(10.10) slaves to releng CI r=dustin

Comment 3

[Morgan Phillips [:mrrrgn]]

Attachment: puppet.comment.diff

At the very least, this comment is necessary IMO.

Comment 4

[Dustin J. Mitchell [:dustin] (he/him)]

Yikes!  I think the path should either be appended at runtime (so runner gets a patch to expand `$PATH` when it reads its config) or set statically per platform.  Keying it to the environment where puppet was run is just going to confuse things, as nobody really gives consideration to setting PATH properly before running puppet.

Comment 5

[Morgan Phillips [:mrrrgn]]

Attachment: puppet.hardcode.path.diff

Comment 6

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8557273 [details] [diff] [review]
puppet.hardcode.path.diff

Review of attachment 8557273 [details] [diff] [review]:
-----------------------------------------------------------------

sorry, I forgot to click "Publish"