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

Turn chronyd module into a short prereq (3.11) #3413

Merged
merged 1 commit into from
Nov 7, 2024

Conversation

asteflova
Copy link
Member

@asteflova asteflova commented Nov 1, 2024

What changes are you introducing?

I'd like to drop the chronyd module on branches 3.11 and lower too.

Why are you introducing these changes? (Explanation, links to references, issues, etc.)

Originally, I made this change only on 3.12 and above and thought it would be enough: #3345 Then I came across a Jira ticket that pointed out the issue on earlier versions, which made me think that it's probably worth it to apply the fix everywhere after all.

Anything else to add? (Considerations, potential downsides, alternative solutions you have explored, etc.)

Checklists

  • I am okay with my commits getting squashed when you merge this PR.
  • I am familiar with the contributing guidelines.

Please cherry-pick my commits into:

  • Foreman 3.12/Katello 4.14 (Satellite 6.16)
  • Foreman 3.11/Katello 4.13
  • Foreman 3.10/Katello 4.12
  • Foreman 3.9/Katello 4.11 (Satellite 6.15; orcharhino 6.8/6.9/6.10)
  • Foreman 3.8/Katello 4.10
  • Foreman 3.7/Katello 4.9 (Satellite 6.14)
  • Foreman 3.6/Katello 4.8
  • Foreman 3.5/Katello 4.7 (Satellite 6.13; orcharhino 6.6/6.7)
  • We do not accept PRs for Foreman older than 3.5.

The prerequisite avoids mentioning any specific timekeeping protocol (previously: NTP) or service (previously: chronyd) because these are not the only options.

(cherry picked from commit f21fc89)
Copy link
Contributor

@maximiliankolb maximiliankolb left a comment

Choose a reason for hiding this comment

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

Diff LGTM and URLs work. Please check guides/common/modules/proc_configuring-a-host-for-registration.adoc and guides/common/modules/proc_adding-an-amazon-ec2-connection-to-server.adoc for xrefs.

@pr-processor pr-processor bot added Waiting on contributor Requires an action from the author and removed Not yet reviewed labels Nov 4, 2024
@asteflova asteflova added the Needs style review Requires a review from docs style/grammar perspective label Nov 4, 2024
@asteflova
Copy link
Member Author

Diff LGTM and URLs work. Please check guides/common/modules/proc_configuring-a-host-for-registration.adoc and guides/common/modules/proc_adding-an-amazon-ec2-connection-to-server.adoc for xrefs.

That's a good catch. This PR is only for branches 3.10 and lower. The links need to be removed in all branches. Is it okay if we do it in a separate PR? #3419

@maximiliankolb
Copy link
Contributor

@asteflova Yes, works for me!

@asteflova asteflova added style review done No issues from docs style/grammar perspective and removed Needs style review Requires a review from docs style/grammar perspective labels Nov 4, 2024
@asteflova
Copy link
Member Author

It looks like this PR is only valid for 3.11 and 3.10, due to the link to RHEL 9 docs. I updated the cherry-pick list in the description and will file a new PR (without the RHEL 9 link) for the remaining branches.

@asteflova asteflova changed the title Turn chronyd module into a short prereq Turn chronyd module into a short prereq (3.11) Nov 4, 2024
Copy link
Contributor

@maximiliankolb maximiliankolb left a comment

Choose a reason for hiding this comment

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

diff LGTM with the follow-up PR #3419

@asteflova
Copy link
Member Author

This is just a cherry pick of changes that don't depend on any specific version > tech review done.

@asteflova asteflova added the tech review done No issues from the technical perspective label Nov 7, 2024
@asteflova asteflova merged commit 34e4099 into theforeman:3.11 Nov 7, 2024
7 of 8 checks passed
@asteflova asteflova deleted the drop_chronyd_3-11 branch November 7, 2024 07:53
asteflova added a commit that referenced this pull request Nov 7, 2024
The prerequisite avoids mentioning any specific timekeeping protocol (previously: NTP) or service (previously: chronyd) because these are not the only options.

(cherry picked from commit f21fc89)
(cherry picked from commit 34e4099)
@asteflova
Copy link
Member Author

Merged to 3.11 and cherry-picked:

ef35981..9fb73fa 3.10 -> 3.10 (conflict, resolved)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
style review done No issues from docs style/grammar perspective tech review done No issues from the technical perspective Waiting on contributor Requires an action from the author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants