(#163) Add AIO binary to the PATH

[smortex]

This match the Bolt behavior of using Puppet's version of Ruby to run tasks, and helps with tasks which rely on Puppet components such as facter.

Fixes #163

[bastelfreak]

Is it safe to add the path or should we add a check if it actually exists? could. this be a backwards incompatible change (do we now get executables in a differen version that might break things)?

[smortex]

Is it safe to add the path or should we add a check if it actually exists? could. this be a backwards incompatible change (do we now get executables in a differen version that might break things)?

That makes sense!

[ripienaar]

It probably is backward compatible, but since it appears this is what Bolt does its probably more in the realm of a bug fix than anything else?

If people want specific ruby in their tasks they should #!/specific/ruby else the right thing is the ruby that runs mco and puppet?

[smortex]

It probably is backward compatible, but since it appears this is what Bolt does its probably more in the realm of a bug fix than anything else?

If people want specific ruby in their tasks they should #!/specific/ruby else the right thing is the ruby that runs mco and puppet?

That looks 100% reasonable to me

[smortex]

Hum, I am wrong with Bolt: it always use the PATH from the user: on a bare metal host with not yet puppet installed and a sane environment setup, nothing is working as expected with Bolt. Since in our base puppet profile, we manage sudo configuration and ensure it has:

Defaults secure_path="/opt/puppetlabs/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"

Then all bolt invocations succeed because Bolt rely on sudo to change user.

So prepending the AIO path might be okayish, but might not be the best solution as far as choria is concerned. I now vaguely recall the idea of adding some configuration directive to setup custom environment variables for the spawned processes.

[ripienaar]

Haha this is about where we got to last time round this also. No good option.

Maybe we pretend it by default and provide a way to override path during task runs in config.

but for sure it appears there won’t be a good answer.

[smortex]

Well, bolt command run has:

Remote environment options
        --env-var ENVIRONMENT_VARIABLES
                                     Environment variables to set on the target.

But this can not be set (yet?) in a configuration file. Also, it's not available when running tasks or plans :upside_down_face:

[ripienaar]

wtf :)

well, I have no oppinions (other than burn it all to the ground) so happy to go with your recommendation for whats best option here.

[smortex]

If you ignore PATH, I see reasons to set custom environment variables when running random commands with bolt command run, but not when running tasks or plans so as far as Bolt is concerned, this is probably okay (just add parameters to tasks and plans if needed).

PATH is a different story: running a task in different environments has different outcomes (e.g. a task might not find a command or use a different version of an interpreter).

So setting random environment variables seems not a good idea to me.

[neomilium]

IMHO:

• only PATH env variable need attention, custom variables do not make sense right here ;
• AIO path should be prepend to PATH as it done in this PR to use AIO binaries by default ;
• an new config may be added to fully overwrite PATH env var.

This way, main use cases are working out of the box and corner-cases can be solved by config entry.

[ripienaar]

OK, thanks for the input here, merged this!