choria-plugins / puppet-agent

Choria Plugin capable of managing the Puppet Agent
https://choria.io
Apache License 2.0
1 stars 8 forks source link

Rely on Puppet 7 built-in windows utils #52

Closed smortex closed 3 years ago

smortex commented 3 years ago

Prior to Puppet 7, the win32-process and win32-servic gems where bundled in the Puppet AIO package for windows, but where removed as Puppet gained support for these features in it's core:

Update the code to use the new API when available, and fall-back to the previous code.

This unbreaks the module on windows with Puppet 7.


Initial issue before full re-write These gem where previously installed with Puppet, but where removed from Puppet 7 bundle: https://github.com/puppetlabs/puppet/commit/321f36fac7856fdd8af82f80d236fb2603a825ea https://github.com/puppetlabs/puppet/commit/50a908694916e812e53b5ab5b0a5302ed9ba9811 Add them as dependencies to the agent to unbreak it on windows. Fixes #51 Things I am not sure about: * which version to stick to. I chose to keep the version previously used in Puppet but it might be better to update them? * this will install the dependencies on all OSes, but it is only useful on windows. Maybe it's worth adding something to avoid installing unneeded dependencies?
ripienaar commented 3 years ago

So in Puppet are people supposed to install these by hand or did they move to some other solution that these are not needed anymore?

smortex commented 3 years ago

It seems it's not needed anymore in core Puppet, and that they implemented the missing bits in Puppet directly:

While walking to $WORK, I was thinking about adding a level in the agents Hiera hierarchy like os/%{facts.os.family}/puppet_aio/%{facts.mcollective.puppet_major_version}.yaml which would allow to ship a data/os/windows/puppet_aio/7.yaml that list these dependencies only when needed… I does not look straightforward though.

Maybe using the Puppet 7 API when available and falling back to the older code makes more sense? What do you think?

ripienaar commented 3 years ago

Yeah, it looks reasonably easy to call their code instead of the gem when on P7, bit annoying but I reckon we can adjust this so these dependencies are just not needed anymore in time which would be great.

So we keep it for P6 and call their code in P7, bit complex, if you'd like me to do the work I am happy to, of course you can also give it a go?

smortex commented 3 years ago

It took me some time but it seems to be working as expected :tada:

smortex commented 3 years ago

I though that it would be possible to use the same code (provided by Puppet) for all platforms, but it seems that Puppet wrappers always wait for a spawned process to terminate so I stuck to the minimal change that fix the issue and allows an easy removal when this can be done.