pkg/asset/manifests/proxy: Create new proxy manifest by patrickdillon · Pull Request #1866 · openshift/installer

patrickdillon · 2019-06-18T20:44:30Z

Add proxy manifest to create config files and Proxy CR when users have a proxy for their cluster.

This PR uses a similar template to other manifests. There is some special logic for handling the NO_PROXY values. First, it generates values that need to be added to the NO_PROXY. Then there is a check to make sure if the user added those values, we will only include it once. I'm not sure if this check is necessary. (There is not currently a check to make sure the user's values are unique, but we could handle that in validation.)

JIRA: CORS-1074

patrickdillon · 2019-06-18T21:02:17Z

/cc @abhinavdahiya

abhinavdahiya · 2019-06-18T21:13:56Z

The asset currently is not being pushed to the cluster.. this assets needs to be wired like other config.openshift.io objects like

installer/pkg/asset/manifests/operators.go

Line 124 in a99bd59

m.FileList = append(m.FileList, infra.Files()...)

secondly another commit that updates the assets dep graph https://github.com/openshift/installer/blob/master/docs/design/assetgeneration.md#dependency-graph will be nice.

Update vendor dir to pull in the ProxyStatus which was added to proxies.config.openshift.io.

patrickdillon · 2019-06-20T14:25:13Z

secondly another commit that updates the assets dep graph https://github.com/openshift/installer/blob/master/docs/design/assetgeneration.md#dependency-graph will be nice.

I have updated the graph. manifests.Proxy shows up but I don't see an installConfig.Proxy. Should installConfig.Proxy be in the graph as well? My branch is up-to-date.

patrickdillon · 2019-06-20T15:03:49Z

[padillon@localhost installer]$ cat assets/manifests/cluster-proxy-01-config.yaml 
apiVersion: config.openshift.io/v1
kind: Proxy
metadata:
  creationTimestamp: null
  name: cluster
spec:
  httpProxy: http://padillon:plaintextpassword@example.com
  httpsProxy: https://padillon:plaintextpassword@example.com
  noProxy: cluster.com
status:
  httpProxy: http://padillon:plaintextpassword@example.com
  httpsProxy: https://padillon:plaintextpassword@example.com
  noProxy: 10.128.0.0/14,127.0.0.1,172.30.0.0/16,cluster.com,etcd-0.padillon-dr.devcluster.openshift.com,etcd-1.padillon-dr.devcluster.openshift.com,etcd-2.padillon-dr.devcluster.openshift.com,https://api-int.padillon-dr.devcluster.openshift.com:6443,https://api.padillon-dr.devcluster.openshift.com:6443,localhost

EDIT: added sorting to status.noProxy values

abhinavdahiya · 2019-06-20T16:43:28Z

[padillon@localhost installer]$ cat assets/manifests/cluster-proxy-01-config.yaml 
apiVersion: config.openshift.io/v1
kind: Proxy
metadata:
  creationTimestamp: null
  name: cluster
spec:
  httpProxy: http://padillon:plaintextpassword@example.com
  httpsProxy: https://padillon:plaintextpassword@example.com
  noProxy: cluster.com
status:
  httpProxy: http://padillon:plaintextpassword@example.com
  httpsProxy: https://padillon:plaintextpassword@example.com
  noProxy: 10.128.0.0/14,127.0.0.1,172.30.0.0/16,cluster.com,etcd-0.padillon-dr.devcluster.openshift.com,etcd-1.padillon-dr.devcluster.openshift.com,etcd-2.padillon-dr.devcluster.openshift.com,https://api-int.padillon-dr.devcluster.openshift.com:6443,https://api.padillon-dr.devcluster.openshift.com:6443,localhost

EDIT: added sorting to status.noProxy values

@patrickdillon
few things pop-out:
https://api-int.padillon-dr.devcluster.openshift.com:6443 https://api.padillon-dr.devcluster.openshift.com:6443 are URI, when I think these should have been just the dns names right?

Update the design doc asset graph to include the addition of the proxy manifest.

patrickdillon · 2019-06-21T02:48:33Z

https://api-int.padillon-dr.devcluster.openshift.com:6443 https://api.padillon-dr.devcluster.openshift.com:6443 are URI, when I think these should have been just the dns names right?

I removed the https protocol so it now reads:

noProxy: 10.128.0.0/14,127.0.0.1,172.30.0.0/16,api-int.padillon-dr.devcluster.openshift.com:6443,api.padillon-dr.devcluster.openshift.com:6443,cluster.com,etcd-0.padillon-dr.devcluster.openshift.com,etcd-1.padillon-dr.devcluster.openshift.com,etcd-2.padillon-dr.devcluster.openshift.com,localhost

I should probably remove the :6443 port postfix too, right?

Also did you see my question about installConfig.Proxy in the graph: #1866 (comment)?

abhinavdahiya · 2019-06-21T04:58:58Z

secondly another commit that updates the assets dep graph https://github.com/openshift/installer/blob/master/docs/design/assetgeneration.md#dependency-graph will be nice.

I have updated the graph. manifests.Proxy shows up but I don't see an installConfig.Proxy. Should installConfig.Proxy be in the graph as well? My branch is up-to-date.

The dependency graph is better assets... Not parts of assets. So I think it's expected that manifest.proxy which is an asset has dependency on entire install config asset.

abhinavdahiya · 2019-06-21T05:00:35Z

I should probably remove the :6443 port postfix too, right?

Yes.

I think you can net.ParseURL and then use Host field to add to NoProxy list...

Create a proxy manifest asset for proxies.config.openshift.io. The Proxy spec is based on values read from the install config. This commit introduces logic to create the NoProxy status value by combining the user-provided values from the install config with default values for the cluster, such as the internal API server. These values are combined into a comma-separated string of unique values. JIRA: CORS-1074 & CORS-1075

patrickdillon · 2019-06-21T14:09:39Z

noProxy: 10.128.0.0/14,127.0.0.1,172.30.0.0/16,api-int.padillon-dr.devcluster.openshift.com,api.padillon-dr.devcluster.openshift.com,cluster.com,etcd-0.padillon-dr.devcluster.openshift.com,etcd-1.padillon-dr.devcluster.openshift.com,etcd-2.padillon-dr.devcluster.openshift.com,localhost

openshift-ci-robot · 2019-06-21T15:36:14Z

@patrickdillon: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/prow/e2e-openstack	`ddb589f`	link	`/test e2e-openstack`
ci/prow/e2e-aws-scaleup-rhel7	`ddb589f`	link	`/test e2e-aws-scaleup-rhel7`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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

abhinavdahiya · 2019-06-21T15:57:12Z

/lgtm

openshift-ci-robot · 2019-06-21T15:57:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, patrickdillon

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [abhinavdahiya]

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

openshift-ci-robot requested review from mtnbikenc and vrutkovs June 18, 2019 20:44

openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 18, 2019

openshift-ci-robot requested a review from abhinavdahiya June 18, 2019 21:02

abhinavdahiya reviewed Jun 18, 2019

View reviewed changes

Comment thread pkg/asset/manifests/proxy.go Outdated

abhinavdahiya reviewed Jun 18, 2019

View reviewed changes

Comment thread pkg/asset/manifests/proxy.go Outdated

abhinavdahiya reviewed Jun 18, 2019

View reviewed changes

Comment thread pkg/asset/manifests/proxy.go Outdated

abhinavdahiya reviewed Jun 18, 2019

View reviewed changes

Comment thread pkg/asset/manifests/proxy.go Outdated

abhinavdahiya reviewed Jun 18, 2019

View reviewed changes

Comment thread pkg/asset/manifests/proxy.go Outdated

abhinavdahiya reviewed Jun 18, 2019

View reviewed changes

Comment thread pkg/asset/manifests/proxy.go Outdated

abhinavdahiya reviewed Jun 18, 2019

View reviewed changes

Comment thread pkg/asset/manifests/proxy.go Outdated

abhinavdahiya reviewed Jun 18, 2019

View reviewed changes

Comment thread Gopkg.lock Outdated

abhinavdahiya mentioned this pull request Jun 19, 2019

Add proxy configuration to bootstrap node #1832

Merged

Bump vendor dependencies for openshift/api/configv1/proxy.

e1c200c

Update vendor dir to pull in the ProxyStatus which was added to proxies.config.openshift.io.

patrickdillon force-pushed the proxy-manifest branch from ddcb42a to 285272f Compare June 20, 2019 14:24

openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 20, 2019

patrickdillon force-pushed the proxy-manifest branch from 285272f to ca992ab Compare June 20, 2019 15:11

abhinavdahiya reviewed Jun 20, 2019

View reviewed changes

Comment thread pkg/asset/manifests/proxy.go Outdated

Update asset graph for Proxy manifest.

8e73b7e

Update the design doc asset graph to include the addition of the proxy manifest.

patrickdillon force-pushed the proxy-manifest branch from ca992ab to 598008d Compare June 21, 2019 02:44

abhinavdahiya reviewed Jun 21, 2019

View reviewed changes

Comment thread pkg/asset/manifests/proxy.go Outdated

patrickdillon force-pushed the proxy-manifest branch from 598008d to ddb589f Compare June 21, 2019 14:07

openshift-ci-robot assigned abhinavdahiya Jun 21, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 21, 2019

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 21, 2019

openshift-merge-robot merged commit 0502d44 into openshift:master Jun 21, 2019

patrickdillon deleted the proxy-manifest branch October 24, 2019 18:21

Conversation

patrickdillon commented Jun 18, 2019

Uh oh!

patrickdillon commented Jun 18, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

abhinavdahiya commented Jun 18, 2019

Uh oh!

patrickdillon commented Jun 20, 2019

Uh oh!

patrickdillon commented Jun 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

abhinavdahiya commented Jun 20, 2019

Uh oh!

Uh oh!

patrickdillon commented Jun 21, 2019

Uh oh!

abhinavdahiya commented Jun 21, 2019

Uh oh!

abhinavdahiya commented Jun 21, 2019

Uh oh!

Uh oh!

patrickdillon commented Jun 21, 2019

Uh oh!

openshift-ci-robot commented Jun 21, 2019

Uh oh!

abhinavdahiya commented Jun 21, 2019

Uh oh!

openshift-ci-robot commented Jun 21, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

patrickdillon commented Jun 20, 2019 •

edited

Loading