Skip to content

[OSDOCS-11968]: Proxy docs for HCP#87863

Merged
lahinson merged 1 commit intoopenshift:mainfrom
lahinson:osdocs-11968-hcp-proxy
Mar 19, 2025
Merged

[OSDOCS-11968]: Proxy docs for HCP#87863
lahinson merged 1 commit intoopenshift:mainfrom
lahinson:osdocs-11968-hcp-proxy

Conversation

@lahinson
Copy link
Contributor

@lahinson lahinson commented Jan 30, 2025

Version(s): 4.18+

Issue: https://issues.redhat.com/browse/OSDOCS-11968

Link to docs preview: https://87863--ocpdocs-pr.netlify.app/openshift-enterprise/latest/hosted_control_planes/hcp-networking.html

QE review:

  • QE has approved this change.

Additional information: This is ready to merge, but will not be merged until the HCP docs are released on March. 20.

@lahinson lahinson added this to the Planned for 4.18 GA milestone Jan 30, 2025
@openshift-ci openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 30, 2025
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented Jan 30, 2025

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 2, 2025
@lahinson lahinson force-pushed the osdocs-11968-hcp-proxy branch from b7c2109 to 29e1d9f Compare February 4, 2025 20:49
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 4, 2025
@lahinson lahinson force-pushed the osdocs-11968-hcp-proxy branch 5 times, most recently from 687249f to 9399a1f Compare February 18, 2025 18:43
@lahinson lahinson force-pushed the osdocs-11968-hcp-proxy branch from 9399a1f to 3b24552 Compare February 24, 2025 18:07
Copy link

@csrwng csrwng left a comment

Choose a reason for hiding this comment

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

Just one comment, otherwise lgtm


[source,terminal]
---
{"ignition":{"config":{"merge":[{"httpHeaders":[{"name":"Authorization","value":"Bearer ..."},{"name":"TargetConfigVersionHash","value":"a4c1b0dd"}],"source":"https://ignition.controlplanehost.example.com/ignition","verification":{}}],"replace":{"verification":{}}},"proxy":{},"security":{"tls":{"certificateAuthorities":[{"source":"...","verification":{}}]}},"timeouts":{},"version":"3.2.0"},"passwd":{},"storage":{},"systemd":{}}
Copy link

@csrwng csrwng Feb 26, 2025

Choose a reason for hiding this comment

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

nit:
It would be good to include values for the proxy fields in the stub for this example:

{"ignition":{"config":{"merge":[{"httpHeaders":[{"name":"Authorization","value":"Bearer ..."},{"name":"TargetConfigVersionHash","value":"a4c1b0dd"}],"source":"https://ignition.controlplanehost.example.com/ignition","verification":{}}],"replace":{"verification":{}}},"proxy":{"httpProxy":"http://proxy.example.org:3128", "httpsProxy":"https://proxy.example.org:3129", "noProxy":"host.example.org"},"security":{"tls":{"certificateAuthorities":[{"source":"...","verification":{}}]}},"timeouts":{},"version":"3.2.0"},"passwd":{},"storage":{},"systemd":{}}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, @csrwng! I added the values to the proxy field.

@lahinson lahinson force-pushed the osdocs-11968-hcp-proxy branch from 3b24552 to 55e3720 Compare February 27, 2025 17:16
@jiezhao16
Copy link

lgtm

@lahinson lahinson added the peer-review-needed Signifies that the peer review team needs to review this PR label Feb 27, 2025
@xenolinux xenolinux added peer-review-in-progress Signifies that the peer review team is reviewing this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Feb 28, 2025
Copy link
Contributor

@xenolinux xenolinux left a comment

Choose a reason for hiding this comment

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

Overall looks good! 2 comments only.

LGTM


* The Ingress Operator needs access to validate external canary routes.

In a hosted cluster, traffic that originates from the Control Plane Operator, Ingress Operator, OAuth server, and OpenShift API server pods should be sent through the data plane to the configured proxy and then to its final destination.
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
In a hosted cluster, traffic that originates from the Control Plane Operator, Ingress Operator, OAuth server, and OpenShift API server pods should be sent through the data plane to the configured proxy and then to its final destination.
In a hosted cluster, traffic that originates from the Control Plane Operator, Ingress Operator, OAuth server, and OpenShift API server pods must be sent through the data plane to the configured proxy and then to its final destination.

Whenever possible, rewrite in a more direct way. Do not use should to refer to an action that must be performed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both good points. Will fix.

[id="hcp-proxy-ignition_{context}"]
= Compute nodes that need to access an ignition endpoint

When compute nodes need a proxy to access the ignition endpoint, the proxy must be configured in the user-data stub that is configured on the compute node when it is created. For cases where machines need a proxy to access the ignition URL, the proxy configuration is included in the stub.
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
When compute nodes need a proxy to access the ignition endpoint, the proxy must be configured in the user-data stub that is configured on the compute node when it is created. For cases where machines need a proxy to access the ignition URL, the proxy configuration is included in the stub.
When compute nodes need a proxy to access the ignition endpoint, you must configure the proxy in the user-data stub that is configured on the compute node when it is created. For cases where machines need a proxy to access the ignition URL, the proxy configuration is included in the stub.
  • Active vs passive voice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will fix.

@xenolinux xenolinux added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-in-progress Signifies that the peer review team is reviewing this PR labels Feb 28, 2025
@lahinson lahinson force-pushed the osdocs-11968-hcp-proxy branch from 55e3720 to 9330674 Compare March 3, 2025 17:43
@lahinson lahinson marked this pull request as draft March 3, 2025 18:51
@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 Mar 3, 2025
@lahinson lahinson marked this pull request as ready for review March 19, 2025 13:37
@lahinson lahinson merged commit 17bedd7 into openshift:main Mar 19, 2025
2 checks passed
@lahinson
Copy link
Contributor Author

/cherrypick enterprise-4.18

@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 Mar 19, 2025
@lahinson
Copy link
Contributor Author

/cherrypick enterprise-4.19

@openshift-cherrypick-robot

@lahinson: new pull request created: #90637

Details

In response to this:

/cherrypick enterprise-4.18

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-sigs/prow repository.

@openshift-cherrypick-robot

@lahinson: new pull request created: #90638

Details

In response to this:

/cherrypick enterprise-4.19

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-sigs/prow repository.

@openshift-ci
Copy link

openshift-ci bot commented Mar 19, 2025

@lahinson: all tests passed!

Full PR test history. Your PR dashboard.

Details

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-sigs/prow repository. I understand the commands that are listed here.

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

Labels

branch/enterprise-4.18 branch/enterprise-4.19 ok-to-merge peer-review-done Signifies that the peer review team has reviewed this PR size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants