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-11433) Use systemd by default in CentOS/RHEL #8862

Closed
wants to merge 1 commit into from

Conversation

kajinamit
Copy link
Contributor

Since CentOS7/RHEL7, systemd is used to manage services. This change
changes the default provider in CentOS/RHEL from the old redhat
provider based on init, to the systemd provider, so that we no longer
need to update the provider code when adding support for a new version
of CentOS/RHEL.

The old redhat provider is still used for CentOS/RHEL<7 which are using
init to manage services.

@kajinamit kajinamit requested a review from a team January 24, 2022 12:35
@kajinamit kajinamit requested a review from a team as a code owner January 24, 2022 12:35
@kajinamit kajinamit requested a review from a team January 24, 2022 12:35
@puppetlabs-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

Since CentOS7/RHEL7, systemd is used to manage services. This change
changes the default provider in CentOS/RHEL from the old redhat
provider based on init, to the systemd provider, so that we no longer
need to update the provider code when adding support for a new version
of CentOS/RHEL.

The old redhat provider is still used for CentOS/RHEL<7 which are using
init to manage services.
@@ -8,7 +8,8 @@

commands :chkconfig => "/sbin/chkconfig", :service => "/sbin/service"

defaultfor :osfamily => :redhat
defaultfor :osfamily => :redhat, :operatingsystemmajrelease => (4..6).to_a
defaultfor :operatingsystem => :amazon, :operatingsystemmajrelease => ["2017"]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this will break some versions of Amazon Linux, as the operatingsystemmajrelease is not always 2017 (at least in some of the versions we test against):

{
        "operatingsystem": "Amazon",
        "operatingsystemmajrelease": "2",
        "operatingsystemrelease": "2",
        "os": {
          "architecture": "x86_64",
          "distro": {
            "codename": "2017.12",
            "description": "Amazon Linux release 2 (2017.12) LTS Release Candidate",
            "id": "Amazon",
            "release": {
              "full": "2",
              "major": "2"
            }
          },
          "family": "RedHat",
          "hardware": "x86_64",
          "name": "Amazon",
          "release": {
            "full": "2",
            "major": "2"
          },
          "selinux": {
            "enabled": false
          }
        },
        "osfamily": "RedHat",
}

Also are there other platforms that report osfamily as RedHat but have a different operatingmajrelease version scheme?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I don't think this will work. For example, on Rocky the osfamily is RedHat but the operatingsystem is Rocky so this provider would no longer be the default for that platform.

Copy link
Contributor

Choose a reason for hiding this comment

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

@kajinamit did you have a chance to see my comment above? Is there another way to do this without breaking defaultfor behavior for existing platforms?

Also we recently merged changes for AL 2023 and it looks like there are conflicts that need resolving.

@CLAassistant
Copy link

CLAassistant commented Apr 19, 2023

CLA assistant check
All committers have signed the CLA.

@joshcooper joshcooper added the bug Something isn't working label Nov 15, 2023
@mhashizume
Copy link
Contributor

Since this PR is stale, closing this in favor of #9207

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