Easier Linux FHS support #8636

ekohl · 2021-06-15T11:12:51Z

The goal of these patches is to make Linux FHS mode easier. Right now multiple distributions struggle and maintain similar patches.

The idea of this series is to introduce a Linux FHS run mode. The distributions can then choose to use this without having to patch too much. Perhaps install.rb can even ensure this runmode is selected.

Currently a draft since this requires some discussion. First of all, I'd like to get agreement on whether the general approach makes sense. If it doesn't and will never be accepted, then there's no point in trying to push for this.

puppetlabs-jenkins · 2021-06-15T11:12:53Z

Can one of the admins verify this patch?

ekohl · 2021-06-15T11:16:02Z

lib/puppet/provider/package/puppet_gem.rb

@@ -6,20 +6,7 @@

  confine :true => Facter.value(:aio_agent_version)


I debated this, but if we implement gem_cmd in the runmode then is there still a need to confine this to AIO?

I'd like to keep this confined to the AIO Ruby, otherwise it will serve the same purpose as the regular gem provider.

Having gem_cmd as part of the run mode is a good idea though, as it keeps things uncluttered in the provider.

My reasoning was that as a user I can then simply use puppet_gem to ensure a gem is installed in the Puppet environment. However, it can also lead to conflicts with the regular gem provider. So I can see the argument for either. We can also first clean up the platform independence and decide on it later while keeping the status quo.

ekohl · 2021-06-15T11:17:04Z

lib/puppet/util/run_mode.rb

@@ -14,7 +14,7 @@ def self.[](name)
        if Puppet::Util::Platform.windows?
          @run_modes[name] ||= WindowsRunMode.new(name)
        else
-          @run_modes[name] ||= UnixRunMode.new(name)
+          @run_modes[name] ||= LinuxFHSRunMode.new(name)


I wonder how we can signal this runmode. Can install.rb do something about this? I don't like the idea of it patching code but maybe it should?

I'm still thinking about this. Ideally we should maintain the same path structure as before when running puppet as a gem or in development mode with bundler.

Honestly patching might be a cleaner solution... it's a bit weird because paths are overridable in install.rb but they are still hardcoded here. We should find a way to keep them in sync.

I agree. Another option is to write a file (like an ini file or whatever) with the paths as install.rb defined them somewhere in the gem directory and load it it at runtime.

Due to the way run mode and settings are intertwined, we can't specify the run mode in puppet.conf. You could potentially load paths from a different ini file, unrelated to settings.... Or just patch.

Interesting, FreeBSD has its own paths (mostly the same but under /usr/local/). Patching this line when packaging on FreeBSD is indeed an option if providing a FreeBSDRunMode class is acceptable, and that would make my life easier 😉

I am wondering if __FILE__ could be used to guess what class to use. If it match /opt/puppetlabs/puppet/lib/.* we have an AIO Puppet, and otherwise some OS-detection just like windows do might help?

Due to the way run mode and settings are intertwined, we can't specify the run mode in puppet.conf. You could potentially load paths from a different ini file, unrelated to settings.... Or just patch.

That was the path I investigated: somehow do that in install.rb (hence the clean up PR you saw). I still don't quite know what it should be. It's almost like install.rb should write out the answers and load them.

Or what I also considered: specify a runmode to install.rb and load the answers from RunMode.

I welcome any suggestions what would be preferred.

Interesting, FreeBSD has its own paths (mostly the same but under /usr/local/). Patching this line when packaging on FreeBSD is indeed an option if providing a FreeBSDRunMode class is acceptable, and that would make my life easier 😉

I had hoped to use this to unify the various Linux distros, but if this makes it easier for BSDs then that's a nice bonus.

On the install.rb path I came up with #9490. By making it use RunMode we can then simplify patch the RunMode to get all the correct paths.

ekohl · 2021-06-15T11:18:30Z

lib/puppet/util/run_mode.rb

+        # TODO: not always
+        '/usr/lib64/pkgconfig'


Should it return nil instead since we don't really want to override it?

I think so, as the default pkgconfig path should be picked up automatically without the need for us to specify it.

ekohl · 2021-06-23T09:11:25Z

I noted that https://github.com/puppetlabs/puppet/blob/main/lib/puppet/defaults.rb also contains paths that are hardcoded. What's the reasoning for keeping that out of run mode?

GabrielNagy · 2021-06-23T10:10:03Z

I noted that https://github.com/puppetlabs/puppet/blob/main/lib/puppet/defaults.rb also contains paths that are hardcoded. What's the reasoning for keeping that out of run mode?

I can't think of a particular reason. For me it would make sense to have something like this:

diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index b2e3239590..a90a173e4c 100644
--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -54,7 +54,7 @@ module Puppet
         nil
       end
     else
-      '/opt/puppetlabs/puppet/vendor_modules'
+      Puppet.run_mode.vendor_module_dir
     end
   end
 
diff --git a/lib/puppet/util/run_mode.rb b/lib/puppet/util/run_mode.rb
index da1af047e2..78acbd0231 100644
--- a/lib/puppet/util/run_mode.rb
+++ b/lib/puppet/util/run_mode.rb
@@ -81,6 +81,10 @@ module Puppet
       def log_dir
         which_dir("/var/log/puppetlabs/puppet", "~/.puppetlabs/var/log")
       end
+
+      def vendor_module_dir
+        "/opt/puppetlabs/puppet/vendor_modules"
+      end
     end
 
     class WindowsRunMode < RunMode

Thoughts @joshcooper? IMO we should centralize hardcoded defaults in a single place if possible.

@ekohl can you open a PUP ticket to track this issue?

ekohl · 2021-06-30T13:12:02Z

@ekohl can you open a PUP ticket to track this issue?

I think so, but right now I'm swamped with other work.

anarcat

take this with a grain of salt as this is a little beyond my depth, but from debian's side of things, the paths look sane. LGTM.

ekohl · 2022-10-14T14:40:20Z

@ekohl can you open a PUP ticket to track this issue?

Better late than never? #8938 is the first bit of work that I think can be reviewed and (hopefully) merged.

The idea is that distro native packages can use this runmode without having to patch the code base. This is still not complete since there's no automatic detection of when to use this.

ekohl · 2024-06-24T23:14:23Z

Rebased now that the runmode changes have been merged. This is now still incomplete since there's no proper detection, but I at least added support for user environment variables that were previously TODOs.

ekohl commented Jun 15, 2021

View reviewed changes

ekohl force-pushed the fhs-paths branch from 5abbdfe to e486a1d Compare October 14, 2022 00:12

anarcat approved these changes Oct 14, 2022

View reviewed changes

ekohl mentioned this pull request Oct 14, 2022

(PUP-11655) Use run_mode for better platform independence #8938

Closed

joshcooper added the enhancement New feature or request label Nov 15, 2023

ekohl added 2 commits June 25, 2024 01:12

Introduce a Linux FHS run mode

75290b4

The idea is that distro native packages can use this runmode without having to patch the code base. This is still not complete since there's no automatic detection of when to use this.

[WIP] Use Linux FHS mode

19788ba

ekohl force-pushed the fhs-paths branch from e486a1d to 19788ba Compare June 24, 2024 23:12

ekohl mentioned this pull request Jul 24, 2024

Remove deprecated DESTDIR and unused things from install.rb #9425

Merged

ekohl mentioned this pull request Sep 27, 2024

Load install paths from RunMode #9490

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Easier Linux FHS support #8636

Easier Linux FHS support #8636

ekohl commented Jun 15, 2021

puppetlabs-jenkins commented Jun 15, 2021

ekohl Jun 15, 2021

GabrielNagy Jun 18, 2021

ekohl Jun 18, 2021

ekohl Jun 15, 2021

GabrielNagy Jun 18, 2021

ekohl Jun 18, 2021

joshcooper Aug 1, 2024

smortex Aug 1, 2024

smortex Aug 1, 2024

ekohl Aug 1, 2024

ekohl Sep 27, 2024

ekohl Jun 15, 2021

GabrielNagy Jun 18, 2021

ekohl commented Jun 23, 2021

GabrielNagy commented Jun 23, 2021

ekohl commented Jun 30, 2021

anarcat left a comment

ekohl commented Oct 14, 2022

ekohl commented Jun 24, 2024

		@@ -6,20 +6,7 @@

		confine :true => Facter.value(:aio_agent_version)

Easier Linux FHS support #8636

Are you sure you want to change the base?

Easier Linux FHS support #8636

Conversation

ekohl commented Jun 15, 2021

puppetlabs-jenkins commented Jun 15, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ekohl commented Jun 23, 2021

GabrielNagy commented Jun 23, 2021

ekohl commented Jun 30, 2021

anarcat left a comment

Choose a reason for hiding this comment

ekohl commented Oct 14, 2022

ekohl commented Jun 24, 2024