puppetlabs / puppet

Server automation framework and application
https://puppet.com/open-source/#osp
Apache License 2.0
7.46k stars 2.19k forks source link

Load install paths from RunMode #9490

Open ekohl opened 1 month ago

ekohl commented 1 month ago

This avoids duplicating the defaults that already exist within the run mode. It also opens up the path to more run modes where the paths could be different (https://github.com/puppetlabs/puppet/pull/8636).

To do so, it first cleans things up by removing an unused import. Then it starts using Puppet::Util::Platform to remove the use of facter. This is ok, because Puppet::Util::RunMode relies on Puppet::Util::Platform. After that, prerequisite checking isn't really needed anymore so it's dropped. It wasn't really maintained anyway.

Then comes the first big refactor: setting the options before loading the parser. This has the benefit that it shows the actual defaults in the help text and removes all selection later in the code. The last patch loads the values from RunMode where possible.

There are further clean ups possible (and I have them locally), but this patch is big enough now.

I also took a stab at completely rewriting everything to a Rake task, but that's a lot more complex for now. Another option I had a look at was (ab)using extensions to install the man pages and additional files, but that was even more complex. So for now I opted to at least work within the existing script.

puppetlabs-jenkins commented 1 month ago

Can one of the admins verify this patch?

puppetlabs-jenkins commented 1 month ago

Can one of the admins verify this patch?

ekohl commented 1 month ago

@joshcooper I'd appreciate your thoughts on the direction of this. I'd prefer to avoid loading application code (which can depend on other parts, like I already needed to hack the root feature check) but I'm not sure I see another way.