Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Easier Linux FHS support #8636

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Easier Linux FHS support #8636

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
75290b4
Introduce a Linux FHS run mode
ekohl Jun 15, 2021
19788ba
[WIP] Use Linux FHS mode
ekohl Jun 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 43 additions & 1 deletion lib/puppet/util/run_mode.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 😉

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

end
end

Expand Down Expand Up @@ -105,6 +105,48 @@ def vendor_module_dir
end
end

class LinuxFHSRunMode < RunMode
def conf_dir
which_dir("/etc/puppet", File.join(ENV.fetch("XDG_CONFIG_HOME", "~/.config"), "puppet"))
end

def code_dir
File.join(conf_dir, 'code')
end

def var_dir
which_dir("/var/lib/puppet", File.join(ENV.fetch("XDG_DATA_HOME", "~/.local/share"), "puppet"))
end

def public_dir
which_dir("/var/cache/puppet/public", File.join(ENV.fetch("XDG_CACHE_HOME", "~/.cache"), "puppet", "public"))
end

def run_dir
which_dir("/run/puppet", File.join(ENV.fetch("XDG_RUNTIME_DIR") { "/run/user/#{Etc.getpwuid.uid}" }, "puppet"))
end

def log_dir
which_dir("/var/log/puppet", File.join(ENV.fetch("XDG_STATE_HOME", "~/.local/state"), "puppet", "logs"))
end

def pkg_config_path
# automatically picked up
end

def gem_cmd
'/usr/bin/gem'
end

def common_module_dir
'/usr/share/puppet/modules'
end

def vendor_module_dir
'/usr/share/puppet/vendor_modules'
end
end

class WindowsRunMode < RunMode
def conf_dir
which_dir(File.join(windows_common_base("puppet/etc")), "~/.puppetlabs/etc/puppet")
Expand Down