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

Fix image builder #3544

Merged
merged 3 commits into from
Feb 5, 2025
Merged

Conversation

maximiliankolb
Copy link
Contributor

What changes are you introducing?

hide procedures for upstream that are only relevent for RHEL7/6 + some minor cleanup

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.)

please review commit by commit; I suggest to squash the commits before merging unless someone comments otherwise.

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
  • 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 2, 2025

The PR preview for 58c5923 is available at theforeman-foreman-documentation-preview-pr-3544.surge.sh

The following output files are affected by this PR:

show diff

show diff as HTML

@@ -65,8 +65,3 @@ This creates a new `rhel6-cloud.qcow2` file in the location where you enter the
====
You must manually resize the partitions of instances based on the image in accordance with the disk space in the flavor that is applied to the instance.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I consider this out of scope unless someone provides a suggestion.

Copy link
Contributor

@Lennonka Lennonka left a comment

Choose a reason for hiding this comment

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

Thank you for removing the duplicate next steps!

Generally looks good to me. My comment isn't blocking your PR.

@@ -10,8 +10,10 @@ include::modules/proc_registering-a-host.adoc[leveloffset=+1]

include::modules/proc_installing-and-configuring-puppet-agent-manually.adoc[leveloffset=+1]

ifdef::satellite[]
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure. Aren't they still relevant for upstream?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically yes, upstream users could still use RHEL 6 (???) or RHEL 7/Oracle Linux 7 with paid support. Though I'd argue that it's OK to drop this by now.

line 13 was basically a judgement call by me. I am happy to adjust this to "if not orcharhino".

@maximiliankolb maximiliankolb added style review done No issues from docs style/grammar perspective Needs tech review Requires a review from the technical perspective labels Jan 17, 2025
@maximiliankolb
Copy link
Contributor Author

PR was agreed upon in chat.

@maximiliankolb maximiliankolb merged commit fefd696 into theforeman:master Feb 5, 2025
8 of 9 checks passed
@maximiliankolb maximiliankolb deleted the fix_image_builder branch February 5, 2025 11:13
maximiliankolb added a commit that referenced this pull request Feb 5, 2025
* Show custom EL7+EL6 images for Satellite only
* Use dashes in favor of underscores in file names
* Remove duplicate next steps

(cherry picked from commit fefd696)
maximiliankolb added a commit that referenced this pull request Feb 5, 2025
* Show custom EL7+EL6 images for Satellite only
* Use dashes in favor of underscores in file names
* Remove duplicate next steps

(cherry picked from commit fefd696)
@maximiliankolb
Copy link
Contributor Author

Merged to "master" and cherry-picked:
1775fa9..d86ce0f 3.13 -> 3.13
dea218b..6a507dd 3.12 -> 3.12

@maximiliankolb maximiliankolb removed the Needs tech review Requires a review from the technical perspective label Feb 5, 2025
@maximiliankolb maximiliankolb mentioned this pull request Feb 5, 2025
9 tasks
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants