Flapping $::path forces to persist a new catalog into PuppetDB

[nbarrientos]

Hi,

When running Puppet interactively the value of $::path might differ from background runs as the former is taken from the login shell. This generates a catalog diff that perhaps will not apply any change on the target system but it will force a catalog replacement on PuppetDB which is kind of unnecessary IMHO.

I, [2018-02-27T09:38:52.953787 #3195]  INFO -- : Catalogs compiled for foo.example.com
I, [2018-02-27T09:38:54.314022 #3195]  INFO -- : Diffs computed for foo.example.com
  Exec[systemctl-daemon-reload] =>
   parameters =>
     path =>
      - /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin
      + /usr/sue/sbin:/usr/sue/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/opt/puppetlabs/bin:/root/bin:/sbin

https://github.com/camptocamp/puppet-systemd/blob/20a465b0d8751bc08913b556d0a5b7fdac139271/manifests/systemctl/daemon_reload.pp#L8

[ekohl]

Would you suggest using an explicit path? That makes sense to me.

[nbarrientos]

Our local patch is:

diff --git a/code/manifests/systemctl/daemon_reload.pp b/code/manifests/systemctl/daemon_reload.pp
index f42efdc..1d2d05c 100644
--- a/code/manifests/systemctl/daemon_reload.pp
+++ b/code/manifests/systemctl/daemon_reload.pp
@@ -3,8 +3,7 @@
 # @api public
 class systemd::systemctl::daemon_reload {
   exec { 'systemctl-daemon-reload':
-    command     => 'systemctl daemon-reload',
+    command     => '/usr/bin/systemctl daemon-reload',
     refreshonly => true,
-    path        => $::path,
   }
 }

It of course ignores other platforms where the path could be different :)

[ekohl]

I'm pretty sure that current Gentoo stable is /bin/systemctl but they're going to switch and it's not in metadata.json. You could also use path => ['/bin', '/usr/bin'] and that'd work on all platforms I think.

[bastelfreak]

@nbarrientos which puppet version do you use? Can you show our puppet.conf? I under stand the issue, but I'm not able to reproduce it with mit different puppet 5.x systems.

[nbarrientos]

Hi,

Thanks for replying :)

To "trigger" the issue the value of $PATH on the interactive shell that invokes Puppet interactively has to be different from the value that the Puppet daemon has.

# puppet --version
4.9.4
# cat /proc/$(pgrep puppet)/environ
LANG=en_US.UTF-8PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin
# facter --version && facter path
3.6.2 (commit c352bf42c7cbce7349fc70753beee4535340537c)
/usr/sue/sbin:/usr/sue/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/opt/puppetlabs/bin:/root/bin

In our case it's different because we do this:

# cat /etc/profile.d/puppet-agent.sh 
# Add /opt/puppetlabs/bin to the path for sh compatible users

if ! echo $PATH | grep -q /opt/puppetlabs/bin ; then
  export PATH=$PATH:/opt/puppetlabs/bin
fi

But I'd say that's totally normal to have a different path for login shells ;)

Here it is our puppet.conf, although I believe there's nothing relevant to this issue in it :)

# cat /etc/puppetlabs/puppet/puppet.conf 
# Installed by cernpuppet
[main]
ssldir                    = /var/lib/puppet/ssl
localcacert               = /etc/pki/tls/certs/foo-bundle.pem
[agent]
server                    = master.example.com
ca_server                 = ca.example.com
masterport                = 8144
ca_port                   = 8140
report                    = true
environment               = qa
splay                     = true
splaylimit                = 900
runinterval               = 1300
listen                    = false
certificate_revocation    = false
postrun_command           = /usr/bin/run-parts /etc/puppetlabs/puppet/post-scripts.d
usecacheonfailure         = false

Thanks!

[raphink]

Getting back to this. Using the $::path fact in exec resources is a standard practice, I'm not sure I want to "fix" this in the systemd module.

If this is problematic to you, you could override the fact path with a file in /etc/puppetlabs/facter/facts.d or in a wrapper when you launch Puppet, using the FACTER_path environment variable.

[traylenator]

Getting back to this. Using the $::path fact in exec resources is a standard practice,

Is it? It is camptocamp modules :-) Not sure I've seen it else where.

[raphink]

Maybe I'm too used to our own standard? 😕