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.

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

Merged
merged 1 commit into from
Nov 21, 2023

Conversation

bastelfreak
Copy link
Contributor

@bastelfreak bastelfreak commented Jun 20, 2022

Systemd is available on Gentoo since a long time (although Gentoo supports multiple init systems). This change marks the provider as default for Gentoo.

@bastelfreak bastelfreak requested a review from a team as a code owner June 20, 2022 10:53
@bastelfreak bastelfreak requested a review from a team June 20, 2022 10:53
@puppetlabs-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

@bastelfreak bastelfreak force-pushed the gentoo branch 3 times, most recently from 0562094 to ee5692c Compare June 20, 2022 11:08
@bastelfreak bastelfreak changed the title Gentoo: Fix systemd service provider (PUP-11593) Gentoo: Fix systemd service provider Jul 18, 2022
@bastelfreak bastelfreak reopened this Sep 18, 2022
@CLAassistant
Copy link

CLAassistant commented Apr 19, 2023

CLA assistant check
All committers have signed the CLA.

@joshcooper
Copy link
Contributor

Hi @bastelfreak could you rebase on main and resolve conflicts?

@bastelfreak
Copy link
Contributor Author

@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
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor

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

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)

Copy link
Contributor Author

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.
@joshcooper joshcooper added the bug Something isn't working label Nov 15, 2023
@joshcooper joshcooper merged commit 79853b8 into puppetlabs:main Nov 21, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants