-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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.
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
Update init.rb to support Nvidia's Cumulus Linux #9453
base: main
Are you sure you want to change the base?
Conversation
Cumulus Linux uses systemd, not system-v's init. This fixes errors like: Failed to call refresh: Could not find init script for 'telegraf'
Can one of the admins verify this patch? |
@dpkgbofh the test failures are unrelated to your PR. Could you rebase your PR on |
Hi Josh, |
Hi @dpkgbofh Could you rebase your PR onto
|
Hi Josh, that better? Please excuse my ignorance! Cheers, Jan |
@@ -21,7 +21,7 @@ def self.defpath | |||
end | |||
|
|||
# Debian and Ubuntu should use the Debian provider. | |||
confine :false => %w[Debian Ubuntu].include?(Puppet.runtime[:facter].value('os.name')) | |||
confine :false => %w[Debian Ubuntu Cumulus].include?(Puppet.runtime[:facter].value('os.name')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already set systemd
as the default provider in
defaultfor 'os.name' => :cumuluslinux, 'os.release.major' => %w[3 4] |
init
? Is the os.name
incorrect in the systemd provider?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what is what facter returns on Cumulus:
root@edd-sw30:~# facter -p os.name
Cumulus
root@edd-sw30:~# facter -p os.release.major
12
root@edd-sw30:~#
The exclusion solution was suggested somewhere on serverfault and is working for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The exclusion solution was suggested somewhere on serverfault
The pattern we've been following is to specify systemd
as the default for a given platform and then exclude older versions in situations where the platform uses init
or something else. For example, we do this for debian
puppet/lib/puppet/provider/service/systemd.rb
Lines 27 to 28 in dc0e3ba
defaultfor 'os.name' => :debian | |
notdefaultfor 'os.name' => :debian, 'os.release.major' => %w[5 6 7] # These are using the "debian" method |
This way it's future compatible (so long as systemd
remains the default) and in theory it helps when documenting which providers are enabled by default on each platform. If we just exclude Cumulus from init
, it's not clear what provider is "next" precedence-wise.
I think the root issue is this line is expecting the os.name
fact to return cumuluslinux
(case-insensitively)
defaultfor 'os.name' => :cumuluslinux, 'os.release.major' => %w[3 4] |
Maybe it did that in previous versions of Cumulus? Or maybe facter 4 is reporting the os.name
fact differently on Cumulus? What is the output of facter os
? Could you try installing puppet-agent 6 on Cumulus and see what facter os
reports?
Related commits
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
root@edd-sw30:~# puppet --version
7.23.0
root@edd-sw30:~# facter os
{
architecture => "amd64",
distro => {
codename => "cumulus",
description => "Cumulus Linux",
id => "Cumulus-linux",
release => {
full => "12.5",
major => "12",
minor => "5"
}
},
family => "Debian",
hardware => "x86_64",
name => "Cumulus",
release => {
full => "12.5",
major => "12",
minor => "5"
},
selinux => {
enabled => false
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, the root cause is because facter 3 used to report os.name => "CumulusLinux"
So systemd
would be selected as the default due to
defaultfor 'os.name' => :cumuluslinux, 'os.release.major' => %w[3 4] |
However, facter 4 reports os.name => "Cumulus"
, hence this issue. To ensure compatibility with facter 3 and 4, could you add the following to the systemd
provider:
defaultfor 'os.name' => :cumulus, 'os.release.major' => %w[5]
I'm assuming cumulus 3 and 4 are EOL?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Josh,
agreed!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dpkgbofh I unresolved this based on my comment:
To ensure compatibility with facter 3 and 4, could you add the following to the systemd provider
defaultfor 'os.name' => :cumulus, 'os.release.major' => %w[5]
At which point, I don't think the change to the init
provider is needed?
Cumulus Linux uses systemd, not system-v's init.
This fixes errors like:
Failed to call refresh: Could not find init script for 'telegraf'