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

HOSTEDCP-1184: Document IPv6/IPv4/DualStack deployments for Hypershift in Baremetal #3008

Merged
merged 1 commit into from Oct 5, 2023

Conversation

jparrill
Copy link
Contributor

Which issue(s) this PR fixes:
Fixes #HOSTEDCP-1184

Checklist

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

@openshift-ci-robot
Copy link

openshift-ci-robot commented Sep 13, 2023

@jparrill: This pull request references HOSTEDCP-1184 which is a valid jira issue.

In response to this:

Which issue(s) this PR fixes:
Fixes #HOSTEDCP-1184

Checklist

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Sep 13, 2023
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 13, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 13, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@netlify
Copy link

netlify bot commented Sep 13, 2023

Deploy Preview for hypershift-docs ready!

Name Link
🔨 Latest commit 9f2a5b5
🔍 Latest deploy log https://app.netlify.com/sites/hypershift-docs/deploys/651a9ad4f74249000804727b
😎 Deploy Preview https://deploy-preview-3008--hypershift-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@openshift-ci openshift-ci bot added area/documentation Indicates the PR includes changes for documentation and removed do-not-merge/needs-area labels Sep 13, 2023
Copy link
Member

@bryan-cox bryan-cox left a comment

Choose a reason for hiding this comment

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

I reviewed the first 10 files. I'll try to review the rest later.

docs/content/how-to/mce/Dual/dns.md Outdated Show resolved Hide resolved
docs/content/how-to/mce/Dual/dns.md Outdated Show resolved Hide resolved
docs/content/how-to/mce/Dual/dns.md Outdated Show resolved Hide resolved
docs/content/how-to/mce/Dual/hostedcluster/worker-nodes.md Outdated Show resolved Hide resolved
docs/content/how-to/mce/Dual/hostedcluster/worker-nodes.md Outdated Show resolved Hide resolved
docs/content/how-to/mce/Dual/hypervisor/index.md Outdated Show resolved Hide resolved
@jparrill jparrill force-pushed the doc/HOSTEDCP-1184 branch 2 times, most recently from f9de6e1 to 54d6b67 Compare September 19, 2023 10:43
.gitignore Show resolved Hide resolved
docs/content/labs/Dual/dns.md Outdated Show resolved Hide resolved
docs/content/labs/IPv4/mgmt-cluster/index.md Outdated Show resolved Hide resolved
docs/content/labs/Dual/dns.md Outdated Show resolved Hide resolved
docs/content/labs/Dual/dns.md Outdated Show resolved Hide resolved
Copy link
Member

@bryan-cox bryan-cox left a comment

Choose a reason for hiding this comment

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

Made it 36 files in this time. It seems like quite a few of the /Dual/ markdown files are duplicated in the /IPv4/ section as well. Could they just reference the same markdown file if the content is the same?

docs/content/labs/Dual/hostedcluster/baremetalhost.md Outdated Show resolved Hide resolved
docs/content/labs/Dual/hostedcluster/baremetalhost.md Outdated Show resolved Hide resolved
docs/content/labs/Dual/hostedcluster/hostedcluster.md Outdated Show resolved Hide resolved
docs/content/labs/Dual/hostedcluster/infraenv.md Outdated Show resolved Hide resolved
docs/content/labs/Dual/hostedcluster/nodepool.md Outdated Show resolved Hide resolved
docs/content/labs/IPv4/hostedcluster/baremetalhost.md Outdated Show resolved Hide resolved
docs/content/labs/IPv4/hostedcluster/hostedcluster.md Outdated Show resolved Hide resolved
docs/content/labs/IPv4/hostedcluster/index.md Outdated Show resolved Hide resolved
@jparrill
Copy link
Contributor Author

Made it 36 files in this time. It seems like quite a few of the /Dual/ markdown files are duplicated in the /IPv4/ section as well. Could they just reference the same markdown file if the content is the same?

Thanks for the review!, it's a huge PR, sorry about that.

Yes, when the file contains the same, I've placed the files in common/ folder but if it contains some differences, I maintain a copy of each one in their own folder.

jparrill added a commit to jparrill/hypershift-disconnected that referenced this pull request Sep 19, 2023
Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
@jparrill jparrill marked this pull request as ready for review September 20, 2023 07:17
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 20, 2023
@jparrill jparrill force-pushed the doc/HOSTEDCP-1184 branch 2 times, most recently from ed4b823 to 7ea6d39 Compare September 20, 2023 07:28
jparrill added a commit to jparrill/hypershift-disconnected that referenced this pull request Sep 20, 2023
Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
jparrill added a commit to jparrill/hypershift-disconnected that referenced this pull request Sep 20, 2023
Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
@jparrill jparrill force-pushed the doc/HOSTEDCP-1184 branch 3 times, most recently from 29931b9 to 77037a2 Compare September 20, 2023 15:42
@jparrill
Copy link
Contributor Author

/test e2e-kubevirt-aws-ovn

@jparrill
Copy link
Contributor Author

/retest

@@ -91,6 +91,117 @@ nav:
- how-to/powervs/create-cluster-powervs.md
- how-to/powervs/create-infra-separately.md
- how-to/powervs/prerequisites-and-env-guide.md
- 'Self-Managed Laboratories':
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for this effort, this is looking great! Can we have this section to be just another category of "how to"? along with AWS and other sections we have there.
Then a subcategory of this would be also "architecture", so all the self managed lab info is self contained there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem putting a new section in "how-to-guides" is, once you dropdown on this last section (it's very down bellow) the browser makes you go up because in fact it's a new page, so it's uncomfortable to keep going down every time you get into a new window. This is one of the reasons why I separated this section.

Regarding architecture section, I think it's better to keep it in Reference because it's not related with any Laboratory itself but the organically way to create the environment, it's like reference architectures.

Copy link
Member

@bryan-cox bryan-cox left a comment

Choose a reason for hiding this comment

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

lgtm overall

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 28, 2023
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Sep 28, 2023
…t in Baremetal

Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>

!!! note

This step is mandatory for both Disconnected and Connected environments. Additionally, it holds significance for both Virtualized and Bare Metal environments. The key distinction lies in the location where the resources will be configured. In a non-virtualized environment, a more robust solution like Bind is recommended instead of a lightweight dnsmasq.
Copy link

Choose a reason for hiding this comment

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

@jparrill If this step is mandatory for both connected and disconnected environments, does it override the info that we already have published for configuring DNS in the official docs?
You can see the source for the official docs related to DNS here: https://github.com/stolostron/rhacm-docs/blob/2.9_stage/clusters/hosted_control_planes/hosted_bare_metal_dns.adoc and https://github.com/stolostron/rhacm-docs/blob/2.9_stage/clusters/hosted_control_planes/hosted_bare_metal_dns.adoc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I don't think so (actually it's mostly the same), the only thing I would add is the same but in IPv6 and Dual stack (I made up the IP addresses).

which in IPv6 would be:

api.example.krnl.es.    IN A 2620:52:0:1306::5
api.example.krnl.es.    IN A 2620:52:0:1306::6
api.example.krnl.es.    IN A 2620:52:0:1306::7
api-int.example.krnl.es.    IN A 2620:52:0:1306::5
api-int.example.krnl.es.    IN A 2620:52:0:1306::6
api-int.example.krnl.es.    IN A 2620:52:0:1306::7
`*`.apps.example.krnl.es. IN A 2620:52:0:1306::10

And for Dual stack, they should resolve in both stacks depending how do you execute the dig command dig A google.com (For IPv4) or dig AAAA google.com (For IPv6)

Copy link

Choose a reason for hiding this comment

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

@jparrill When I move this content to the official docs, do you think it would be best if I mentioned the already existing docs about DNS as a prereq to this? I'm trying to figure out what the best way is to present this info keeping in mind that we have other docs about DNS as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reuse existing documentation will be always better (IMHO), only makes sense to create new doc if the case differs from the current one.


Please ensure you modify the appropriate fields to align with your laboratory environment.

!!! warning
Copy link

Choose a reason for hiding this comment

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

@jparrill Does this warning apply only to the upstream docs, or should I include it in the official docs, too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, at least until the first Z version of 4.14 get released.

@enxebre
Copy link
Member

enxebre commented Oct 5, 2023

/approve

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 5, 2023
@openshift-ci-robot
Copy link

openshift-ci-robot commented Oct 5, 2023

@jparrill: This pull request references HOSTEDCP-1184 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.15.0" version, but no target version was set.

In response to this:

Which issue(s) this PR fixes:
Fixes #HOSTEDCP-1184

Checklist

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Copy link
Member

@bryan-cox bryan-cox left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 5, 2023
@openshift-ci-robot
Copy link

openshift-ci-robot commented Oct 5, 2023

@jparrill: This pull request references HOSTEDCP-1184 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.15.0" version, but no target version was set.

In response to this:

Which issue(s) this PR fixes:
Fixes #HOSTEDCP-1184

Checklist

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 5, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bryan-cox, enxebre, jparrill

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jparrill
Copy link
Contributor Author

jparrill commented Oct 5, 2023

/retest-required

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 04c64ea and 2 for PR HEAD 9f2a5b5 in total

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 5, 2023

@jparrill: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@openshift-ci openshift-ci bot merged commit f7e15f8 into openshift:main Oct 5, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/documentation Indicates the PR includes changes for documentation jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants