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

Remove upstream EL7 client repository #3596

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

ekohl
Copy link
Member

@ekohl ekohl commented Jan 20, 2025

What changes are you introducing?

Foreman 3.14 has dropped the EL7 client repository and this documents that change.

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

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

I have not completely reviewed all the content bits to see if they're still correct.

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.13/Katello 4.15 (EL9 only)
  • Foreman 3.12/Katello 4.14 (Satellite 6.16)
  • Foreman 3.11/Katello 4.13 (orcharhino 6.11 on EL8 only; orcharhino 7.0 on EL8+EL9)
  • 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)
  • We do not accept PRs for Foreman older than 3.7.

Copy link

github-actions bot commented Jan 20, 2025

The PR preview for 200d9e6 could not be generated

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.

one minor suggestion that does not block this PR. Style-wise LGTM; tech ACK: matches https://yum.theforeman.org/client/

  • Client for EL7 for Foreman 3.13 ✔️ as expected
  • Client for EL7 for Foreman nightly: ❌ as expected

@@ -157,11 +157,7 @@
:RepoRHEL8ServerSatelliteUtils: satellite-utils-{RepoSatelliteVersion}-for-rhel-8-x86_64-rpms

// RHEL 7 Satellite repos
:RepoRHEL7ServerSatelliteMaintenanceProjectVersion: rhel-7-server-satellite-maintenance-{RepoSatelliteVersion}-rpms
:RepoRHEL7ServerSatelliteServerProjectVersion: rhel-7-server-satellite-{RepoSatelliteVersion}-rpms
:RepoRHEL7ServerSatelliteServerProjectVersionPrevious: rhel-7-server-satellite-{ProjectVersionPrevious}-rpms
:RepoRHEL7ServerSatelliteToolsProjectVersion: rhel-7-server-satellite-client-6-rpms
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this attributes is only used once as value for another attribute. Out of scope but could be simplified:

$ rg -i RepoRHEL7ServerSatelliteToolsProjectVersion
guides/common/attributes-satellite.adoc
163::RepoRHEL7ServerSatelliteToolsProjectVersion: rhel-7-server-satellite-client-6-rpms
169::project-client-RHEL7-url: {RepoRHEL7ServerSatelliteToolsProjectVersion}

Copy link
Member Author

Choose a reason for hiding this comment

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

I debated that, but I find it worrying we don't have project-client-RHEL9-url either so I decided to leave it out of scope. There's also a chance we'll just drop RHEL 7 in Satellite, but I don't know what the plan for that is right now.

@maximiliankolb maximiliankolb added tech review done No issues from the technical perspective style review done No issues from docs style/grammar perspective labels Jan 20, 2025
@@ -67,7 +67,7 @@ To enable, enter the following command:
----
$ hammer activation-key content-override \
--name "_My_Activation_Key_" \
--content-label {project-client-RHEL7-url} \
--content-label {project-client-name} \
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't look right in builds:

  • Satellite:
    image
  • Katello:
    image

I think we should stick with an -url attribute.

Copy link
Member Author

Choose a reason for hiding this comment

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

@maximiliankolb in 3ec2864 you decided to add this line, but I don't think it ever was correct. At least in upstream I don't see any documentation that ever creates the client repository. Any suggestion on what to do here?

Should we introduce project-client-label?

Copy link
Contributor

Choose a reason for hiding this comment

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

To be fair, it already was there before me PR:

$ git checkout 3ec2864a7f1071257d7e8a3b07c2ad912fb825b6^
$ rg content-label
guides/doc-Content_Management_Guide/topics/Managing_Activation_Keys.adoc
161:--content-label {project-client-RHEL7-url} \
236:--content-label _content_label_ \

I just retested the commands from this procedure:

$ hammer activation-key create --name "maximilian-1" --unlimited-hosts --lifecycle-environment "development" --content-view "CCV AlmaLinux 9" --organization-id 1
$ hammer activation-key list --organization-id 1 | grep maximilian
$ hammer activation-key info --id 46 --organization-id 1
$ hammer activation-key product-content --content-access-mode-all true --name "maximilian-1" --organization-id 1
$ hammer activation-key content-override --name "maximilian-1" --content-label "MYORG_test_product_yum_repo_yum" --value 1 --organization-id 1

I did not spot any error. -> I think we only have to fix the attribute.

Copy link
Member Author

Choose a reason for hiding this comment

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

To be fair, it already was there before me PR:

I didn't trace the full history and only did a simple git blame.


RHEL 7 is out of maintenance since June 2024 and at the same time CentOS Linux 7 went end of life.
With Foreman 3.14 the client repository is no longer built for EL 7.
This primairly affects Katello and OpenSCAP users.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This primairly affects Katello and OpenSCAP users.
This primarily affects Katello and OpenSCAP users.

=== EL 7 client repositories dropped

RHEL 7 is out of maintenance since June 2024 and at the same time CentOS Linux 7 went end of life.
With Foreman 3.14 the client repository is no longer built for EL 7.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
With Foreman 3.14 the client repository is no longer built for EL 7.
With Foreman 3.14, the client repository is no longer built for EL 7.

@pr-processor pr-processor bot added the Waiting on contributor Requires an action from the author label Jan 20, 2025
@@ -28,6 +28,7 @@ For more information, see {ManagingHostsDocURL}Registering_Hosts_by_Using_the_Bo
<2> Include the `--force` option to register the client that has been previously registered to a standalone {SmartProxy}.
<3> Include the `--puppet-ca-port 8141` option if you use Puppet.

ifdef::satellite[]
Copy link
Member Author

Choose a reason for hiding this comment

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

Note to self: I found out this whole procedure is only included in Satellite and Orcharhino so this probably isn't needed at all.

@ekohl
Copy link
Member Author

ekohl commented Jan 22, 2025

I've opened #3606 to add the release note. With conferences coming up I don't think I'll have time to figure out the content situation. If anyone does, feel free to take over. Otherwise please keep the feedback coming on where we should take this.

ekohl added 6 commits January 22, 2025 14:43
Since 51f5a90 and
9169132 these have been unused.
This was introduced in 2e9d3be but
never used.

Fixes: 2e9d3be ("Further modularization of upgrading guide (theforeman#2599)")
This used part of the client tooling repository, but it should really
use the attribute to prevent them going out of sync whenever it changes.
This allows using it in every context where a name is required.
The option says --content-label so a name should be used.
Foreman 3.14 drops EL7 builds for the client repositories and since the
packages are gone, we can no longer mention it.
@pr-processor pr-processor bot added Needs re-review and removed Waiting on contributor Requires an action from the author Needs re-review labels Jan 22, 2025
@ekohl ekohl marked this pull request as draft January 22, 2025 15:07
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants