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.

Sign up for GitHub

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

Jump to bottom

Update IPv4/IPv6 installation prerequisites #3545

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Update IPv4/IPv6 installation prerequisites #3545

wants to merge 4 commits into from

Conversation

aneta-petrova
Copy link
Member

@aneta-petrova aneta-petrova commented Jan 3, 2025 •
edited
Loading

What changes are you introducing?

  • Making the existing IPv6 installation section a lot shorter (by dropping a bit of fluff)
  • Excluding the information about dual-stack installations not being supported from non-Satellite builds (based on https://community.theforeman.org/t/ip6-dual-stack-support/31486)
    * Expanding the existing IPv6 section with information on IPv4 installations

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

This started with https://issues.redhat.com/browse/SAT-29764 which requests adding the information about IPv4 and it seemed like a good opportunity to implement a few other changes that had been discussed at various times.

EDIT: The IPv4 changes now have their own PR #3553

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

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 3, 2025 •
edited
Loading

The PR preview for 5e04042 is available at theforeman-foreman-documentation-preview-pr-3545.surge.sh

The following output files are affected by this PR:

show diff

show diff as HTML

@aneta-petrova aneta-petrova force-pushed the SAT-29764_ipv6_loopback branch from 9d74bc6 to 93b2342 Compare January 3, 2025 12:04
@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 Jan 3, 2025
@aneta-petrova
Copy link
Member Author

Hi @lhellebr, I'm trying to address the issue with disabling IPv6 in kernel that you discovered (https://issues.redhat.com/browse/SAT-29764). Do you think that cb69625 would work?

@lhellebr
Copy link

lhellebr commented Jan 6, 2025

I've added one comment, otherwise looks good

ekohl
ekohl requested changes Jan 6, 2025
View reviewed changes
guides/common/modules/con_ipv4-and-ipv6-requirements.adoc Outdated Show resolved Hide resolved
guides/common/modules/con_ipv4-and-ipv6-requirements.adoc Outdated

* You must deploy an external DHCP IPv6 server as a separate, unmanaged service to bootstrap clients into GRUB2.
GRUB2 configures IPv6 networking by using DHCPv6 or by assigning a static IPv6 address.
This is required because the DHCP server in {RHEL} (ISC DHCP) does not provide an integration API for managing IPv6 records, therefore the {SmartProxy} DHCP plugin that provides DHCP management is limited to IPv4 subnets.
Copy link
Member

Choose a reason for hiding this comment

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

Preempting customer questions: Infoblox does have an API to manage DHCPv6. Can I use that? The answer is no, so perhaps just reduce it to say we currently can't manage DHCPv6?

Copy link
Member Author

Choose a reason for hiding this comment

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

No change, because #3541 removes this line.

guides/common/modules/con_ipv4-and-ipv6-requirements.adoc Outdated Show resolved Hide resolved
guides/common/modules/con_ipv4-and-ipv6-requirements.adoc Outdated Show resolved Hide resolved
guides/common/modules/con_ipv4-and-ipv6-requirements.adoc Outdated Show resolved Hide resolved
guides/common/modules/con_ipv4-and-ipv6-requirements.adoc Show resolved Hide resolved
@pr-processor pr-processor bot added the Waiting on contributor Requires an action from the author label Jan 6, 2025
@Lennonka
Copy link
Contributor

Lennonka commented Jan 6, 2025

@asteflova @ekohl Actually, I have an update in progress in #3541. Perhaps we could incorporate it in this PR?

@aneta-petrova
Copy link
Member Author

@asteflova @ekohl Actually, I have an update in progress in #3541. Perhaps we could incorporate it in this PR?

Nice! I just reviewed and acked #3541 @Lennonka Once it gets merged, I'll pull the changes in here.

@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 6, 2025
@aneta-petrova aneta-petrova mentioned this pull request Jan 8, 2025
Merged
9 tasks
@aneta-petrova
Copy link
Member Author

Part of the feedback resulted in changes that can't be cherry-picked to all versions, which is why I raised a separate PR to address the IPv4 requirements #3553.

@aneta-petrova aneta-petrova force-pushed the SAT-29764_ipv6_loopback branch from e0c6274 to 81c3947 Compare January 10, 2025 10:39
@aneta-petrova
Copy link
Member Author

Rebased to include changes from #3541 and #3553.

@Lennonka and @ekohl, can you please re-review?

Lennonka
Lennonka reviewed Jan 13, 2025
View reviewed changes
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.

LGTM style wise

guides/common/modules/con_ipv4-and-ipv6-requirements.adoc
Comment on lines +40 to +41
Although {Project} provisioning templates include IPv6 support for PXE and HTTP (iPXE) provisioning, the only tested and certified provisioning workflow is the UEFI HTTP Boot provisioning.
This limitation only relates to users who plan to use {Project} to provision hosts.
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
Although {Project} provisioning templates include IPv6 support for PXE and HTTP (iPXE) provisioning, the only tested and certified provisioning workflow is the UEFI HTTP Boot provisioning.
This limitation only relates to users who plan to use {Project} to provision hosts.

I think that these lines no longer apply because of #3532 (TFTP ~ PXE). I think we can just remove them.

@ShimShtein Can you please confirm? Or is there another limitation?

Copy link
Member

Choose a reason for hiding this comment

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

I can confirm that I am working on enabling PXE provisioning in IPv6 networking.

As far as limitations go - I don't see anything yet

maximiliankolb
maximiliankolb reviewed Jan 14, 2025
View reviewed changes
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.

some mostly optional comments. I prefer to always say "HTTP proxy" to clearly differentiate between Smart Proxies and HTTP proxies.

guides/common/modules/con_ipv4-and-ipv6-requirements.adoc Outdated Show resolved Hide resolved
guides/common/modules/con_ipv4-and-ipv6-requirements.adoc Outdated Show resolved Hide resolved
guides/common/modules/con_ipv4-and-ipv6-requirements.adoc Outdated Show resolved Hide resolved
guides/common/modules/con_ipv4-and-ipv6-requirements.adoc Outdated Show resolved Hide resolved
guides/common/modules/con_ipv4-and-ipv6-requirements.adoc Outdated Show resolved Hide resolved
@maximiliankolb maximiliankolb mentioned this pull request Jan 14, 2025
Merged
9 tasks
@aneta-petrova aneta-petrova force-pushed the SAT-29764_ipv6_loopback branch from 7d0bcc8 to 5e04042 Compare January 14, 2025 12:18
@aneta-petrova
Copy link
Member Author

I implemented feedback from @maximiliankolb (thank you!) > style review done.

Tech review is still needed so I'll wait for @ekohl and/or @ShimShtein to review as requested above.

@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 Jan 14, 2025
maximiliankolb
maximiliankolb approved these changes Jan 14, 2025
View reviewed changes
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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Lennonka Lennonka Lennonka left review comments

@ShimShtein ShimShtein ShimShtein left review comments

@maximiliankolb maximiliankolb maximiliankolb approved these changes

@ekohl ekohl Awaiting requested review from ekohl

Assignees
No one assigned
Labels
Needs tech review Requires a review from the technical perspective style review done No issues from docs style/grammar perspective
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@aneta-petrova @lhellebr @Lennonka @ekohl @ShimShtein @maximiliankolb