-
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
(PUP-11593) Gentoo: Fix systemd service provider #8919
Conversation
Can one of the admins verify this patch? |
0562094
to
ee5692c
Compare
Hi @bastelfreak could you rebase on main and resolve conflicts? |
@joshcooper updated it |
@@ -27,6 +27,7 @@ | |||
defaultfor 'os.name' => :ubuntu | |||
notdefaultfor 'os.name' => :ubuntu, 'os.release.major' => ["10.04", "12.04", "14.04", "14.10"] # These are using upstart | |||
defaultfor 'os.name' => :cumuluslinux, 'os.release.major' => ["3", "4"] | |||
defaultfor 'os.name' => :gentoo |
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.
If you're on Gentoo and don't have systemd, will this provider be incorrectly chosen, because it has a higher specificity than the usual gentoo
provider?
confine 'os.name' => :gentoo |
Or does puppet exclude the systemd
provider and default to gentoo
because the systemctl
command isn't available and the confine resolves to 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.
the confine resolves to false and the provider is excluded.
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.
Sorry forgot to mention could you add a test like
puppet/spec/unit/provider/service/systemd_spec.rb
Lines 166 to 171 in 03b3df3
it "should be the default provider on LinuxMint#{ver}" do | |
allow(Facter).to receive(:value).with('os.family').and_return(:debian) | |
allow(Facter).to receive(:value).with('os.name').and_return(:LinuxMint) | |
allow(Facter).to receive(:value).with('os.release.major').and_return("#{ver}") | |
expect(provider_class).to be_default | |
end |
Also if you want this in 7.x, please submit a backport PR to that branch (since it relies on legacy facts)
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.
added a test, let me know it that's fine for you. After it's merged I will raise a PR for the 7.x branch.
Systemd is available on Gentoo since a long time (although Gentoo supports multiple init systems). This change marks the provider as default for Gentoo.
Systemd is available on Gentoo since a long time (although Gentoo supports multiple init systems). This change marks the provider as default for Gentoo.