Skip to content

Conversation

@aneta-petrova
Copy link
Member

What changes are you introducing?

Merging all the existing IPv4 and IPv6 sections with installation requirements into one module.

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

This is part of preparing the installation prerequisites for the new installation guide in #4087

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

This is based on #3545 which didn't get merged then but now came in handy.

Contributor 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.16/Katello 4.18 (Satellite 6.18)
  • Foreman 3.15/Katello 4.17
  • Foreman 3.14/Katello 4.16 (Satellite 6.17; orcharhino 7.4)
  • Foreman 3.13/Katello 4.15 (EL9 only)
  • Foreman 3.12/Katello 4.14 (Satellite 6.16; orcharhino 7.2 on EL9 only; orcharhino 7.3)
  • Foreman 3.11/Katello 4.13 (orcharhino 6.11 on EL8 only; orcharhino 7.0 on EL8+EL9; orcharhino 7.1 with Leapp)
  • Foreman 3.10/Katello 4.12
  • Foreman 3.9/Katello 4.11 (Satellite 6.15; orcharhino 6.8/6.9/6.10)
  • We do not accept PRs for Foreman older than 3.9.

Review checklists

Tech review (performed by an Engineer who did not author the PR; can be skipped if tech review is unnecessary):

  • The PR documents a recommended, user-friendly path.
  • The PR removes steps that have been made unnecessary or obsolete.
  • Any steps introduced or updated in the PR have been tested to confirm that they lead to the documented end result.

Style review (by a Technical Writer who did not author the PR):

  • The PR conforms with the team's style guidelines.
  • The PR introduces documentation that describes a user story rather than a product feature.

@github-actions github-actions bot added Needs tech review Requires a review from the technical perspective Needs style review Requires a review from docs style/grammar perspective Needs testing Requires functional testing labels Sep 4, 2025
@github-actions
Copy link

github-actions bot commented Sep 4, 2025

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I know it's a draft, but adding some notes for related work.

Comment on lines 27 to 29
* {Project} does not support configuring an HTTP proxy using a direct IPv6 address.
Instead, configure the HTTP proxy with a FQDN that resolves to the IPv6 address.
Using an IPv6 address as the HTTP proxy URL causes it to fail.
Copy link
Member

Choose a reason for hiding this comment

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

Katello/katello#11432 removes 1 bug in this area. I just don't know for sure if there aren't more lurking. I'd like to discuss this in the team to see if we can drop this text.

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'll keep this in for now, then, and please let me know if you find out it's safe to remove.

Copy link
Member

Choose a reason for hiding this comment

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

If we can verify this works and resolve the bug then we can drop it for Foreman 3.16 and newer.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @ianballou, I see you approved the linked Katello PR. Do you know if it's safe to remove this bullet point?

@aneta-petrova aneta-petrova removed Needs tech review Requires a review from the technical perspective Needs style review Requires a review from docs style/grammar perspective Needs testing Requires functional testing labels Sep 4, 2025
@aneta-petrova aneta-petrova force-pushed the review-ipv6-and-ipv4-requirements branch 2 times, most recently from 951e8fc to 693c13e Compare September 4, 2025 13:13
@aneta-petrova aneta-petrova force-pushed the review-ipv6-and-ipv4-requirements branch from 693c13e to 2eae196 Compare September 4, 2025 13:15
@aneta-petrova aneta-petrova marked this pull request as ready for review September 4, 2025 13:32
@aneta-petrova aneta-petrova added Needs tech review Requires a review from the technical perspective Needs style review Requires a review from docs style/grammar perspective labels Sep 4, 2025
@aneta-petrova
Copy link
Member Author

Hi @ekohl and/or @evgeni, can one of you please re-review and, if possible, ack?

Hi dear writers, can someone please do a style review?

@aneta-petrova aneta-petrova added tech review done No issues from the technical perspective and removed Needs tech review Requires a review from the technical perspective labels Sep 8, 2025
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.

style-wise LGTM

@aneta-petrova aneta-petrova 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 Sep 9, 2025
@aneta-petrova aneta-petrova merged commit d7107ad into theforeman:master Sep 9, 2025
10 checks passed
@aneta-petrova aneta-petrova deleted the review-ipv6-and-ipv4-requirements branch September 9, 2025 09:47
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I had a last look at it, but I see by now you merged it.


The following requirements apply to installations in an IPv4 network:

* Ensure an IPv6 loopback is configured on the base system.
Copy link
Member

Choose a reason for hiding this comment

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

Style nit: I don't think you need ensure a requirement

Suggested change
* Ensure an IPv6 loopback is configured on the base system.
* An IPv6 loopback configured on the base system.

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'll style-nit you back: I'd prefer to keep this a full sentence (verb and all) to make the action item in the requirement clear. An option could be An IPv6 loopback must be configured on the base system. but that would introduce passive voice, which is frowned upon.

Comment on lines +13 to +14
Do not disable it.
* Do not disable IPv6 in kernel by adding the `ipv6.disable=1` kernel parameter.
Copy link
Member

Choose a reason for hiding this comment

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

Not a blocker, but technically the requirement we have is IPv6 loopback. An implication of ipv6.disable=1 is that it disables this IPv6 loopback. You could argue it shouldn't be listed as a separate requirement but maybe more as an example for the "do not disable" line.

Would something like this work?

Suggested change
Do not disable it.
* Do not disable IPv6 in kernel by adding the `ipv6.disable=1` kernel parameter.
Do not disable it.
Use of the `ipv6.disable=1` kernel parameter or the `net.ipv6.conf.lo.disable_ipv6 = 1` sysctl option will break.

I tried to avoid the duplicate "do not" on 2 lines just below each other.

Copy link
Member Author

Choose a reason for hiding this comment

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

Comment on lines +27 to +28
* {Project} does not support configuring an HTTP proxy using a direct IPv6 address.
Instead, configure the HTTP proxy with a FQDN that resolves to the IPv6 address.
Copy link
Member

Choose a reason for hiding this comment

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

I'm pretty sure this was a Katello-only limitation.

Suggested change
* {Project} does not support configuring an HTTP proxy using a direct IPv6 address.
Instead, configure the HTTP proxy with a FQDN that resolves to the IPv6 address.
ifdef::katello,orcharhino,satellite[]
* {Project} does not support configuring an HTTP proxy using a direct IPv6 address.
Instead, configure the HTTP proxy with a FQDN that resolves to the IPv6 address.
endif::[]

Copy link
Member Author

Choose a reason for hiding this comment

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

aneta-petrova added a commit that referenced this pull request Sep 9, 2025
* Merge all IPv4/IPv6 requirements into one module
* Exclude a prereq on content from foreman-el/deb

---------

Co-authored-by: Maximilian Kolb <[email protected]>
(cherry picked from commit d7107ad)
@aneta-petrova
Copy link
Member Author

Merged to "master" and cherry-picked:

c22b1b2..8a63c8a 3.16 -> 3.16

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.

4 participants