(#64) Allow service to be configured for non-Windows.

[treydock]

Check the service state rather than PID for non-Windows This allows for the continued checking of 'puppet' service but also allows checking other service names like 'puppet-run.service' used by The Foreman module for Puppet. This change is backwards incompatible due to the change in configuration key

Fixes #64

[treydock]

Marked as draft while I test internally.

[ripienaar]

We support only AIO Puppet and its settings. If foreman invents their own things, that's their business, I cant chase every crazy idea :)

[treydock]

This is just allowing some other mechanism to check Puppet's service. If someone's Puppet module, whether that be Foreman or some other group, turns off puppet.service and uses something different, it would never be possible to check that with this code. There is already a configuration option for the Windows service and supporting Puppet Enterprise, this just expands on that so it's no longer specific to configuring Windows.

[treydock]

I could make this backwards compatible if that's a concern. Then there'd be a new setting and this just gives some parity between Windows and non-Windows in terms of how services are checked.

[ghoneycutt]

We are not asking that we substantially change the support platforms, just that this option is configurable not just for Windows but for any platform. Specifically we are running puppet in a cron like manner using a systemd timer which has a different service name.

Would this be acceptable if the change was backward compatible instead of removing the windows_service config option?

[ripienaar]

At the very least it would need to be backward compatible. But we have a very strong stance to only support AIO behaviours with only very slight exceptions where community is specifically maintaining those - such as FreeBSD which Puppet does not support.

In the dark days we had this wild west where everyone had modules of their own and installed stuff absolutely everywhere and calling things whatever they wanted and it was unsupportable.

Once AIO came about these modules were written because suddenly the situation became supportable as everyone suddenly was the same on all setups. AIO is the very reason I was able to even ship this, it would have to be a very strong argument to make me suddenly go back to days where everyone does whatever they like and module authors have to carry that complexity.

This particular agent pre-dates AIO so it might have some knobs to facilitate the old pre-AIO days, but that's not a reason to add more. Choria.io Puppet modules is a distribution of Choria for AIO.

The right fix here is for foreman to make its modules compatible or configurable.

[treydock]

Even before AIO was a thing, Puppet still shipped with a service that ran the Ruby daemon and I remember the bad-ol-days when that daemon would eat up CPU unnecessarily and I vaguely recall Puppetlab's solution via tickets was to run via cron and not a service. I think that's one reason the community has adopted to frequently run Puppet agent via scheduled tasks such as cron or systemd timers and not rely on a Ruby daemon.

I've made this PR backwards compatible. Rather than look at the PID, ie only support daemon, the daemon's service state will be checked using Puppet code which will work for AIO daemon as well as systemd timers. This also brings some parity with how Linux and Windows check for Puppet daemon. They will now both look at the service state, just using different Puppet code.

[treydock]

Verified:

One host has Puppet disabled and the other runs Puppet via systemd timer that was configured using the agent config to set service: puppet-run.timer.

$ mco puppet status -F networking.hostname='/OMIT1|OMIT2/'

 * [ ============================================================> ] 2 / 2

      OMIT1: Currently stopped; last completed run 1 minutes 13 seconds ago
   OMIT2: Currently idling; last completed run 45 seconds ago

Summary of Applying:

   false = 2

Summary of Daemon Running:

   running = 1
   stopped = 1

Summary of Enabled:

   enabled = 2

Summary of Idling:

    true = 1
   false = 1

Summary of Status:

    idling = 1
   stopped = 1


Finished processing 2 / 2 hosts in 1099.18 ms

Ran Puppet via the systemd service, shows running while the one stopped still shows stopped:

$ mco puppet status -F networking.hostname='/OMIT1|OMIT2/'

 * [ ============================================================> ] 2 / 2

      OMIT1: Currently stopped; last completed run 1 minutes 59 seconds ago
   OMIT2: Currently applying a catalog; last completed run 1 minutes 31 seconds ago

Summary of Applying:

    true = 1
   false = 1

Summary of Daemon Running:

   running = 1
   stopped = 1

Summary of Enabled:

   enabled = 2

Summary of Idling:

   false = 2

Summary of Status:

   applying a catalog = 1
              stopped = 1


Finished processing 2 / 2 hosts in 1068.45 ms

[ripienaar]

So this is reporting timer based one as idling? When it’s not in the process tree it’s just waiting for next timer to fire?

If that’s true then that’s just wrong. Idling specifically means a daemon running but doing nothing.

[treydock]

A systemd timer reports the same status in systemd as a service, so if you do systemctl is-active puppet.service you'd get running and if you do systemctl is-active puppet-run.timer you'd also get running. A systemd timer is a daemon that periodically executes a service unit. So rather than puppet.service sleeping and periodically doing things, there is a timer that is sleeping and periodically doing things. So if you did like systemctl status puppet-run.timer you'd see the same active status like any other daemon as well as the journald logs about when that timer is doing things.

This PR can also be useful to treat Linux service check just like Windows service check, so irregardless of how we'll be using it, I feel like this PR is still useful as it seems like a way to bring some parity with how different OSes are checked and also doing it in a way that requires less custom code, ie using Puppet's service provider rather than custom PID checks.

[ripienaar]

Cron is also a daemon that periodically execute a program.

I understand how it works, rest assured, we dont need to explain what a timer does.

Idle means PUPPET is idling, this is not puppet idling. It's not making some service name configurable, its entirely re-defining what the idle state means, I am not happy with that.

[treydock]

Then what if we instead did

service: puppet-run.service

In that case we've now made use of this new configuration option and it will show stopped and not idle when Puppet's service isn't running cause all we've done is configured the service we actually use and not the service name AIO ships with.

This change isn't entirely about just supporting a cron/timer but making the service name configurable and checking a service via Puppet's service provider rather than PID. This isn't changing what idling means, cause the puppet.service will still show the same states with this change. This just makes the name configurable and how we consume this new feature is not being imposed on anyone using this change, so we aren't changing how idle is defined except with our specific site configuration which is just consuming an improvement in this plugin.

[treydock]

I see this as how Puppet is configured. Puppet knows how to check service status, but you can also configure it in such a way to do different things with checking status. That doesn't redefine how Puppet defines "stopped" or "running", simply allows sites with specific needs to adapt the feature to do extra things. Right now this plugin simply doesn't use "@puppet_service" for Linux, and only using it for Windows. This change makes it so both Windows and non-Windows both consume the configured service name and check the service state. How we use such a feature doesn't change what this feature does.

[ripienaar]

You say its not changing the meaning of idle - well then when the agent reports "idle" with your proposed change does it mean the puppet process is still running just not applying a manifest. Thats what idle means.

I am not adding support for non AIO setups.

/usr/lib/systemd/system/puppet.service
/usr/lib/systemd/system/pxp-agent.service

That's what we support.