Skip to content

Conversation

@bergerhoffer
Copy link
Contributor

@bergerhoffer bergerhoffer commented Sep 13, 2019

@bergerhoffer bergerhoffer added this to the Future Release milestone Sep 13, 2019
@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 13, 2019
@openshift-docs-preview-bot

The preview will be available shortly at:

@bergerhoffer
Copy link
Contributor Author

Thanks for the feedback @danehans! I've adjusted the field descriptions if you can take another look. Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

I see @danehans asking for this restriction above, but I'm curious about why. My understanding is that you can use a HTTPS URI for httpProxy (e.g. to avoid leaking your proxy-connection creds), although you might bump into containers/image#699 if the proxy doesn't support any of the encryption options currently compiled into containers/image (and that would go for httpsProxy too).

Copy link

@gpei gpei Sep 26, 2019

Choose a reason for hiding this comment

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

I once had a try with using https URI for httpProxy, network-operator will complain about it.

Copy link
Member

Choose a reason for hiding this comment

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

What error message did you see?

Copy link

Choose a reason for hiding this comment

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

It's a quite long time ago, so I just spin up a new cluster with such setting.
In network-operator pod log, I could see such message, also a clear prompt.

2019/09/27 02:42:21 Updated ClusterOperator with conditions:
- lastTransitionTime: "2019-09-27T02:42:21Z"
  message: The configuration is invalid for proxy 'cluster' (httpProxy requires a
    'http' URI scheme). Use 'oc edit proxy.config.openshift.io cluster' to fix.
  reason: InvalidProxyConfig

Copy link
Member

Choose a reason for hiding this comment

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

Followed up here, where that check landed.

Copy link
Member

Choose a reason for hiding this comment

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

You also only need this (for proxy connections) if you're using an https proxy URI. No proxy cert to include if all your proxy connections are http.

@bergerhoffer bergerhoffer added peer-review-needed Signifies that the peer review team needs to review this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Sep 24, 2019
@bergerhoffer bergerhoffer force-pushed the OSDOCS-640 branch 2 times, most recently from 32053b6 to 289a1ac Compare September 24, 2019 17:41
@openshift-ci-robot openshift-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. 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 Sep 24, 2019
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 24, 2019
@bergerhoffer
Copy link
Contributor Author

Preview bot isn't working, so posted a build to the file share site. The "Configuring the cluster-wide proxy during installation" file has been included in all appropriate provider/install types. Links to each install section are below:

* These two sections restructured all content under the "Creating the installation files for AWS" heading (broke up a single procedure into two procedures and inserted configuring proxy procedure in between). Needs to be reviewed carefully.

@bergerhoffer bergerhoffer changed the title [WIP] OSDOCS-640: Adding docs for configuring proxy during installation OSDOCS-640: Adding docs for configuring proxy during installation Sep 24, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 24, 2019
@bergerhoffer bergerhoffer added the peer-review-needed Signifies that the peer review team needs to review this PR label Sep 24, 2019
Copy link
Contributor

Choose a reason for hiding this comment

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

If restricted network is not going to be supported in 4.2, you should consider removing this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kalexand-rh can confirm for me, but the docs for these restricted network are in for 4.2, so they should be supported. Also, this sentence will only appear in the restricted docs, so if they don't go out for some reason, then this won't appear.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit confused by this paragraph. First it talks about removing manifests, then it states to generate the manifests.

Copy link
Member

Choose a reason for hiding this comment

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

First it talks about removing manifests, then it states to generate the manifests.

You: "Installer, make your usual manifests." Installer: "Here you go" You: "Forget about these. The rest look good. Carry on."

However we want to explain that workflow works for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was pulled from the original module that I broke up, but I agree that it is confusing.

@kalexand-rh - I don't have the background to know why the steps are the way that they are. Do you have any reword suggestions that would make this clearer, or know who might be good to ask on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry I missed your comment @wking before I added mine - thanks for the explanation!

@kalexand-rh let's look at this together tomorrow.

Copy link
Contributor

Choose a reason for hiding this comment

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

@wking correct me if I'm wrong, but this section should be removed due to https://bugzilla.redhat.com/show_bug.cgi?id=1755073.

Copy link
Member

Choose a reason for hiding this comment

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

I think so, but prob wait until openshift/installer#2402 lands or is otherwise sorted.

@danehans
Copy link
Contributor

@bergerhoffer this is coming along nicely. I provided a few comments. This PR covers proxy at install time. Will a separate PR address configuring Proxy for day 2?

@danehans
Copy link
Contributor

@bergerhoffer UPI-based proxy installs require a newer ami from 4.1. Are the AMI references being updated as part of the general 4.2 docs? cc: @wking @ewolinetz

@wking
Copy link
Member

wking commented Sep 25, 2019

Are the AMI references being updated...

If we haven't yet, 4.2 should definitely bump to https://github.com/openshift/installer/blob/release-4.2/data/data/rhcos.json

@bergerhoffer
Copy link
Contributor Author

@bergerhoffer
Copy link
Contributor Author

@danehans I've made a few updates. I'm going to go ahead and merge this PR because we have a round of reviews over the next few days that I'd like this to be included for. But we can continue to refine these docs as necessary in a separate PR.

When you ask about day 2 configuration of the proxy, are you talking about enabling the proxy on an existing cluster, which was added by PR #16562 (and can be viewed here). Or are there some other configuration docs that we're missing?

spec:
httpProxy: http://<username>:<pswd>@<ip>:<port> <1>
httpsProxy: https://<username>:<pswd>@<ip>:<port> <2>
httpsProxy: http://<username>:<pswd>@<ip>:<port> <2>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@danehans Please verify that the changes to this file are appropriate in response to your comments on the install docs that use these similar examples and descriptions. It can be previewed here.

@bergerhoffer bergerhoffer merged commit 9abb118 into openshift:master Sep 25, 2019
@bergerhoffer
Copy link
Contributor Author

/cherrypick enterprise-4.2

@openshift-cherrypick-robot

@bergerhoffer: new pull request created: #16871

Details

In response to this:

/cherrypick enterprise-4.2

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.

@vikram-redhat
Copy link
Contributor


include::modules/installation-aws-config-yaml.adoc[leveloffset=+2]

include::modules/installation-configure-proxy.adoc[leveloffset=+2]
Copy link

Choose a reason for hiding this comment

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

I have a doubt whether we should add it here. As I know, only UPI way support proxy setting until IPI supports using a preconfigured VPC. This section is for IPI-on-AWS.

Copy link
Member

Choose a reason for hiding this comment

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

You can use a proxy on AWS IPI, the proxy just has to live outside the VPC and still be reachable by the cluster. This is how CI works now. Not as nice as what's in flight with openshift/release#4719, but still something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gpei Are you suggesting that this should not be included here for AWS IPI (and Azure and GCP)? I had confirmed with @katherinedube the appropriate supported list here: https://jira.coreos.com/browse/OSDOCS-640?focusedCommentId=130922&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-130922

Choose a reason for hiding this comment

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

As Trevor mentioned, until we support deploying to a pre-existing VPC (planned for the next release), the proxy must live outside the VPC and be reachable by the nodes. While I agree it's not ideal, I'm not sure we should be saying it doesn't work unless it really doesn't work.

Copy link

Choose a reason for hiding this comment

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

yeah, I understand that we can use proxy for IPI install. But QE was told UPI must be used for proxy testing in 4.2, so all proxy related testing were scheduled with UPI installs in QE's 4.2 test matrix.

Copy link
Contributor

Choose a reason for hiding this comment

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

I traced it back to a discussion Daneyon had with Xiaoli to remove the testing of ipi installs (at the time that it might have been the belief), but after ipi was tested, it wasn't the case.

@katherinedube IPI was being used for testing as a stop-gap until UPI CI was complete. It was identified early on by @derekwaynecarr that UPI is required due to the lack of existing VPC support for IPI. As I mention in my above comment, IPI can technically work, but I don't see the requirements for IPI proxy meeting real world use cases.

Copy link

@katherinedube katherinedube Sep 27, 2019

Choose a reason for hiding this comment

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

@danehans Just to be clear, even if we're going to say upi support is required for proxy this should only effect public cloud provider ipi deployments. Meaning I don't know this is really a generic ipi problem across all of our ipi providers like OSP. As it is today in the text matrix, it's all ipi providers - so nothing is being tested with OSP (not sure this should have been excluded.) Otherwise, why couldn't a public cloud provider use proxy with ipi? Could you provider some additional details as to why it wouldn't be practical? Thanks!

Choose a reason for hiding this comment

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

I totally agree with @danehans 's statement.
IPI's prerequisite is having internet connectivity. That means, cluster itself already have internet connection, why still need proxy? Even proxy is enabled, how to validate all components and operators are really getting outside via proxy but not via its own internet connectivity?
In QE's testing, we dropped internet connectivity, and enable proxy, we found the following bugs:
Bug 1753467 - [proxy] no proxy is set for kube-controller-manager

That indicates if we do not drop internet connectivity for proxy testing, our test result is NOT persuadable, or else, we would miss the above bugs. But in the whole IPI install process, we have no way to drop internet connectivity.

Copy link
Member

Choose a reason for hiding this comment

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

In QE's testing, we dropped internet connectivity, and enable proxy, we found...

We definitely want to support proxy on blackholed UPI; I don't think anyone opposes that. Once we have bring-your-own-VPC, we all agree we want to support proxy there too. And testing in those environments makes me reasonably confident that at least OpenShift components will respect a voluntary proxy. The only contention seems to be whether we want to support proxy for installer-provided-everything. And the pushback seems to be mostly around "who wants this?". But I don't see anything about that case that would be a support burden, so I'd rather not add confusion by walling it off as an unsupported niche. Maybe customers want a proxy for MitM snooping on our egress (and they are also convinced from our blackhole support that we are playing nice). Or maybe they want a proxy for some other reason. But I much prefer the optics of "you may not care about this on IPI" to "we don't support this (suggesting it does not work reliably) on IPI". From a QE angle, we don't test all possible config permutations on all platforms. I'm fine leaving IPI+proxy covered just in CI, and relying on QE in the blackholed cases to turn up bugs like they're already doing.

Copy link

@jianlinliu jianlinliu Sep 29, 2019

Choose a reason for hiding this comment

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

From a QE angle, we don't test all possible config permutations on all platforms.

Yeah, agree. QE did not run proxy testing in a blackholed IPI (due to in 4.2 IPI, we have no way to create a blackholed env), but only proxy testing in a blackholed UPI.

I like "we don't support this (suggesting it does not work reliably) on IPI".


include::modules/installation-azure-config-yaml.adoc[leveloffset=+2]

include::modules/installation-configure-proxy.adoc[leveloffset=+2]
Copy link

Choose a reason for hiding this comment

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

Same here, we only have IPI-on-Azure in 4.2

include::modules/installation-gcp-config-yaml.adoc[leveloffset=+2]

include::modules/installation-configure-proxy.adoc[leveloffset=+2]

Copy link

Choose a reason for hiding this comment

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

Same here, this section is for IPI-on-GCP

Copy link

@gpei gpei Sep 27, 2019

Choose a reason for hiding this comment

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

And for proxy support on GCP, we still have a testblocker bug https://bugzilla.redhat.com/show_bug.cgi?id=1753930, which was targeted to 4.3. We might need to add it to known issue in the release note.

Copy link
Member

Choose a reason for hiding this comment

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

openshift/installer#2405 is in the merge queue, which would unblock a 4.2 backport. But yeah, restricting proxy GCP to UPI in the meantime would be a good, conservative choice.

Copy link
Member

Choose a reason for hiding this comment

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

The 4.3/master changes all landed. Hopefully we'll have the 4.2 backports soon.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link

Choose a reason for hiding this comment

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

https://bugzilla.redhat.com/show_bug.cgi?id=1753930 was already verified with 4.3 build.
But if the 4.2 backports were not merged into 4.2.0, customer still have the problem when 4.2.0 is shipped.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI - with PR #17080 we're limiting support to UPI installs only, so support is only for UPI for GCP now.

@danehans Were the PRs for https://bugzilla.redhat.com/show_bug.cgi?id=1753930 backported to 4.2, or is this still a problem in 4.2 for GCP UPI?

If not, you mentioned there was a workaround of adding 'metadata.google.internal.' to noProxy - let me know if this is something that we need to document or not.

Copy link

Choose a reason for hiding this comment

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

@bergerhoffer The cloned bug for 4.2 https://bugzilla.redhat.com/show_bug.cgi?id=1759245 has been verified now, it's not an issue for 4.2.0 GCP UPI anymore, thanks!

Copy link

@gpei gpei Sep 26, 2019

Choose a reason for hiding this comment

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

I once had a try with using https URI for httpProxy, network-operator will complain about it.

@gpei gpei mentioned this pull request Oct 10, 2019
wking added a commit to wking/openshift-docs that referenced this pull request Oct 29, 2019
Drop the wording from aa39bf3 (OSDOCS-640: Adding docs for
configuring proxy during installation, 2019-09-12, openshift#16635), because
the network operator explicitly supports both the 'http' and 'https'
schemes since openshift/cluster-network-operator@42dbcf8955 (Refactors
PR to focus on http/https/no proxy reconciliation, 2019-08-06,
openshift/cluster-network-operator#245), which landed before
release-4.2 split off from the network operator's master.
@therevoman
Copy link

My client is using a transparent proxy.. no url to put into http(s)Proxy.. however any outbound traffic needs a CA in the chain... additionalTrustedCA only sort of works for installation and post-install tasks.

@bergerhoffer
Copy link
Contributor Author

@danehans Can you address @therevoman's situation above?

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

Labels

branch/enterprise-4.2 peer-review-done Signifies that the peer review team has reviewed this PR size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.