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

OSDOCS-2466: Created configuration instructions for cluster-wide proxy. #39830

Merged
merged 1 commit into from
Jan 31, 2022
Merged

OSDOCS-2466: Created configuration instructions for cluster-wide proxy. #39830

merged 1 commit into from
Jan 31, 2022

Conversation

EricPonvelle
Copy link
Contributor

@EricPonvelle EricPonvelle commented Dec 13, 2021 •
edited

@EricPonvelle EricPonvelle added this to the Next Release milestone Dec 13, 2021
@openshift-ci openshift-ci bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Dec 13, 2021
@netlify
Copy link

netlify bot commented Dec 13, 2021 •
edited

✔️ Deploy Preview for osdocs ready!

🔨 Explore the source changes: d7decb3

🔍 Inspect the deploy log: https://app.netlify.com/sites/osdocs/deploys/61f815304feae00007858a78

😎 Browse the preview: https://deploy-preview-39830--osdocs.netlify.app/openshift-rosa/latest/networking/configuring-cluster-wide-proxy

@openshift-ci openshift-ci bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 14, 2021
yingzhanredhat
yingzhanredhat reviewed Dec 15, 2021
View reviewed changes
modules/osd-cluster-wide-proxy.adoc Outdated
<other arguments here> \
--additional-trust-bundle-file $CA_BUNDLE_FILE \
--http-proxy $HTTP_PROXY \
--https-proxy $HTTPS_PROXY
Copy link

Choose a reason for hiding this comment

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

This card has not been implemented yet. It isn't ok to expose this function.

Copy link
Contributor

Choose a reason for hiding this comment

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

@yingzhanredhat it is the expectation that when this feature and documentation goes live, both ocm and rosa CLI will support it. We have confirmation that ocm is expected to match the arguments of rosa so I think it is safe to leave in as part of this PR.

mrbarge
mrbarge reviewed Dec 15, 2021
View reviewed changes
modules/osd-cluster-wide-proxy.adoc Outdated Show resolved Hide resolved
modules/osd-cluster-wide-proxy.adoc Outdated Show resolved Hide resolved
modules/osd-cluster-wide-proxy.adoc Outdated Show resolved Hide resolved
modules/osd-cluster-wide-proxy.adoc Outdated Show resolved Hide resolved
modules/osd-cluster-wide-proxy.adoc Outdated Show resolved Hide resolved
@EricPonvelle EricPonvelle force-pushed the OSDOCS-2466_ClusterwideProxy branch 2 times, most recently from 7e6a0b3 to 94ddb9c Compare December 15, 2021 18:52
@openshift-ci openshift-ci bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 15, 2021
mrbarge
mrbarge reviewed Dec 16, 2021
View reviewed changes
modules/osd-cluster-wide-proxy.adoc Outdated Show resolved Hide resolved
modules/osd-cluster-wide-proxy.adoc Outdated Show resolved Hide resolved
yingzhanredhat
yingzhanredhat reviewed Dec 16, 2021
View reviewed changes
modules/osd-cluster-wide-proxy.adoc Outdated
<other arguments here> \
--additional-trust-bundle-file <path-to-CA-bundle-file> \
--http-proxy http://<username>:<pswd>@<ip>:<port> \
--https-proxy https://<username>:<pswd>@<ip>:<port>
Copy link

Choose a reason for hiding this comment

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

--https-proxy field can also use **http**://<username>:<pswd>@<ip>:<port>
Do we need to explain?

Copy link

Choose a reason for hiding this comment

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

I think <other arguments here> is simple.User will confuse about how to configure

Copy link
Contributor

@mrbarge mrbarge Dec 16, 2021 •
edited

Choose a reason for hiding this comment

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

Agree on the https-proxy point. Not sure what is the best way to represent that.
Maybe

Suggested change
--https-proxy https://<username>:<pswd>@<ip>:<port>
--https-proxy http(s)://<username>:<pswd>@<ip>:<port>

Regarding the <other arguments here>, I also agree that it is simplistic, but I am not sure what is the best way to represent that other arguments will be required when creating a cluster (and we can't be able to predict what each customer's individual argument set may look like). Maybe we just point them at the install docs? (ie https://docs.openshift.com/rosa/rosa_getting_started/rosa-installing-rosa.html)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mrbarge @yingzhanredhat Fixed the address with the http(s) change, but for the link, would this be the OSD version? https://docs.openshift.com/dedicated/osd_cluster_create/creating-your-cluster.html

I can add a note in the considerations section with a link to the respective documentation.

Copy link

Choose a reason for hiding this comment

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

If only link to the creating cluster url, it is still confused. I think we should point that user need prepare a CCS cluster with VPC which the proxy can be accessible.

@mrbarge
Copy link
Contributor

mrbarge commented Dec 16, 2021

@EricPonvelle This is looking good, thank you! I added a couple more comments but am otherwise happy.

@EricPonvelle EricPonvelle force-pushed the OSDOCS-2466_ClusterwideProxy branch 2 times, most recently from c926b7e to 6baf3ae Compare December 17, 2021 21:28
@EricPonvelle EricPonvelle force-pushed the OSDOCS-2466_ClusterwideProxy branch 2 times, most recently from 8ea8c3a to 240586a Compare January 5, 2022 21:26
@EricPonvelle
Copy link
Contributor Author

@mrbarge Check out the updates to the ROSA documentation. I conditioned the CCS item to only display on the OSD side. Is this correct?

@mrbarge
Copy link
Contributor

mrbarge commented Jan 6, 2022

Thanks @EricPonvelle ! Yes that looks good to me.

@yingzhanredhat
Copy link

@EricPonvelle I will verify the command when the it is ready.And everything else is fine
@mrbarge Another question:when the doc is exposed,is it supported to create a cluster with proxy on GCP?If not,is it necessary to indicate that GCP is not supported at present?

@mrbarge
Copy link
Contributor

mrbarge commented Jan 10, 2022

@yingzhanredhat Re GCP that's a good point, I will sync with you separately on that topic outside of this PR concerning the state of GCP support.

@mrbarge
Copy link
Contributor

mrbarge commented Jan 10, 2022

(With regards to GCP support) I think for now it's fine to leave it as-is, just in case we're able to go live with GCP support included. Even if someone tries to run the command for a GCP cluster and we don't have GCP support yet, they will receive the message "cluster_wide_proxy is only supported for AWS clusters" so it will be self-explanatory what the problem is.

bergerhoffer
bergerhoffer reviewed Jan 13, 2022
View reviewed changes
_topic_maps/_topic_map_osd.yml Outdated Show resolved Hide resolved
_topic_maps/_topic_map_rosa.yml Outdated Show resolved Hide resolved
networking/proxies/images Outdated Show resolved Hide resolved
networking/proxies/configuring-cluster-wide-proxy.adoc Outdated Show resolved Hide resolved
networking/proxies/configuring-cluster-wide-proxy.adoc Outdated Show resolved Hide resolved
modules/osd-cluster-wide-proxy.adoc Outdated Show resolved Hide resolved
modules/rosa-cluster-wide-proxy.adoc Outdated Show resolved Hide resolved
modules/osd-cluster-wide-proxy.adoc Outdated Show resolved Hide resolved
modules/osd-cluster-wide-proxy.adoc Outdated Show resolved Hide resolved
modules/osd-cluster-wide-proxy.adoc Outdated Show resolved Hide resolved
@openshift-ci openshift-ci bot removed the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 13, 2022
@openshift-ci openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 13, 2022
@EricPonvelle EricPonvelle force-pushed the OSDOCS-2466_ClusterwideProxy branch 2 times, most recently from 0635d7e to 1001255 Compare January 14, 2022 20:58
@nalhadef
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 17, 2022
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 31, 2022
@maxwelldb maxwelldb self-requested a review January 31, 2022 17:02
@maxwelldb maxwelldb added the peer-review-done Signifies that the peer review team has reviewed this PR label Jan 31, 2022
@maxwelldb
Copy link
Contributor

/lgtm

@EricPonvelle EricPonvelle merged commit 6b1d6d8 into openshift:main Jan 31, 2022
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 31, 2022
@EricPonvelle
Copy link
Contributor Author

/cherry-pick enterprise-4.9

maxwelldb
maxwelldb reviewed Jan 31, 2022
View reviewed changes
Copy link
Contributor

@maxwelldb maxwelldb left a comment

Choose a reason for hiding this comment

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

Reviewed for OSD output. After leveloffset fix, seems copacetic to me.

@EricPonvelle
Copy link
Contributor Author

/cherry-pick enterprise-4.10

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Jan 31, 2022
Merged
@openshift-cherrypick-robot
Copy link

@EricPonvelle: new pull request created: #41219

In response to this:

/cherry-pick enterprise-4.9

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-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Jan 31, 2022
Merged
@openshift-cherrypick-robot
Copy link

@EricPonvelle: new pull request created: #41220

In response to this:

/cherry-pick enterprise-4.10

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.

@EricPonvelle EricPonvelle deleted the OSDOCS-2466_ClusterwideProxy branch January 31, 2022 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrbarge mrbarge mrbarge left review comments

@bergerhoffer bergerhoffer bergerhoffer left review comments

@maxwelldb maxwelldb maxwelldb left review comments

@yingzhanredhat yingzhanredhat yingzhanredhat left review comments

Assignees

@maxwelldb maxwelldb

@nalhadef nalhadef

Labels
branch/enterprise-4.9 branch/enterprise-4.10 lgtm Indicates that a PR is ready to be merged. 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
Milestone
Next Release
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@EricPonvelle @mrbarge @yingzhanredhat @nalhadef @maxwelldb @openshift-cherrypick-robot @bergerhoffer