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

[3006.x] Make rpms more universal by replacing systemd_* scriptlets with RHEL7/8 styled forward-compatible definitions #66030

Merged
merged 5 commits into from
Feb 14, 2024

Conversation

ScriptAutomate
Copy link
Contributor

@ScriptAutomate ScriptAutomate commented Feb 8, 2024

What does this PR do?

Uses systemd_* scriptlet definitions from RHEL 7/8 that are forward-compatible with RHEL 9, allowing for the RPMs to be built on newer systems while the built artifacts are still compatible with RHEL 7/8/9.

What issues does this PR fix or reference?

Fixes: #65987

Previous Behavior

systemd_* scriptlet definitions were not compatible with RHEL 7/8 systems (including Photon OS 3, Photon OS 4, AlmaLinux 8, etc.).

New Behavior

systemd_* scriptlet definitions were used from RHEL 7/8 build macros, allowing for more universal compatibility on RHEL 7/8/9 systems, regardless of OS that package is built on. Definitions have been tested on RHEL 9 systemd and work as expected.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

Yes

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

@ScriptAutomate ScriptAutomate requested a review from a team as a code owner February 8, 2024 16:54
@ScriptAutomate ScriptAutomate requested review from twangboy and removed request for a team February 8, 2024 16:54
@salt-project-bot-prod-environment salt-project-bot-prod-environment bot changed the title Make rpms more universal by replacing systemd_* scriptlets with RHEL7/8 styled forward-compatible definitions [3006.x] Make rpms more universal by replacing systemd_* scriptlets with RHEL7/8 styled forward-compatible definitions Feb 8, 2024
@ScriptAutomate ScriptAutomate marked this pull request as draft February 8, 2024 17:04
@ScriptAutomate ScriptAutomate self-assigned this Feb 8, 2024
@ScriptAutomate
Copy link
Contributor Author

ScriptAutomate commented Feb 8, 2024

Requires a test review and updates to:

The upgrades and downgrades need to:

  • Prior to downgrade/upgrade: Grab pid of currently running service(s)
  • After downgrade/upgrade: Grab pid of currently running service(s)
  • Verify that the pids differ, which would show that the services have restarted

Though, technically, if you downgrade to one of the older versions (v3006.6 and earlier), this test will fail because the RPMs used for older versions haven't been updated yet. So, this may be a case of ensuring it is setup for upgrade/downgrade service validation where the version is >3006.6 ?

tests/pytests/pkg/integration/test_systemd_config.py might be a good reference point for how to retrieve the information via systemd, since that is what is being managed here.

s0undt3ch
s0undt3ch previously approved these changes Feb 13, 2024
Copy link
Collaborator

@s0undt3ch s0undt3ch left a comment

Choose a reason for hiding this comment

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

Don't know enough to validate the spec file changes.
@dwoz or @dmurphy18 ?

@s0undt3ch s0undt3ch requested review from dwoz and dmurphy18 February 13, 2024 21:37
twangboy
twangboy previously approved these changes Feb 14, 2024
Copy link
Contributor

@dmurphy18 dmurphy18 left a comment

Choose a reason for hiding this comment

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

need to check comments for post

pkg/rpm/salt.spec Outdated Show resolved Hide resolved
pkg/rpm/salt.spec Outdated Show resolved Hide resolved
pkg/rpm/salt.spec Outdated Show resolved Hide resolved
pkg/rpm/salt.spec Outdated Show resolved Hide resolved
@twangboy twangboy dismissed stale reviews from s0undt3ch and themself via b69b814 February 14, 2024 18:16
ScriptAutomate and others added 5 commits February 14, 2024 11:24
Looked up the actual code for the macros being replaced
Made sure we're doing the proper if statements
Cleaned up the tests a little
@twangboy twangboy force-pushed the make-rpms-more-universal branch from a538e7e to fbfba41 Compare February 14, 2024 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test:pkg Run the package tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants