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

cluster up: add proxy support #12483

Merged
merged 1 commit into from
Jan 19, 2017
Merged

cluster up: add proxy support #12483

merged 1 commit into from
Jan 19, 2017

Conversation

csrwng
Copy link
Contributor

@csrwng csrwng commented Jan 13, 2017 •
edited
Loading

Adds arguments to cluster up to set http, https proxy and no_proxy

Fixes #11323

@csrwng
Copy link
Contributor Author

csrwng commented Jan 13, 2017

@bparees fyi

bparees
bparees reviewed Jan 13, 2017
View reviewed changes
pkg/bootstrap/docker/up.go
@@ -149,6 +149,9 @@ func NewCmdUp(name, fullName string, f *osclientcmd.Factory, out, errout io.Writ
cmd.Flags().StringArrayVarP(&config.Environment, "env", "e", config.Environment, "Specify a key-value pair for an environment variable to set on OpenShift container")
cmd.Flags().BoolVar(&config.ShouldInstallMetrics, "metrics", false, "If true, install metrics (experimental)")
cmd.Flags().BoolVar(&config.ShouldInstallLogging, "logging", false, "If true, install logging (experimental)")
cmd.Flags().StringVar(&config.HTTPProxy, "http-proxy", "", "HTTP proxy to use for master and builds")
cmd.Flags().StringVar(&config.HTTPProxy, "https-proxy", "", "HTTPS proxy to use for master and builds")
cmd.Flags().StringArrayVar(&config.NoProxy, "no-proxy", config.NoProxy, "List of hosts or subnets for which a proxy should not be used")
Copy link
Contributor

Choose a reason for hiding this comment

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

is it a list, or do you pass the arg multiple times, one per value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can pass it either a comma-separated list or specify multiple times, one per value

Copy link
Contributor

Choose a reason for hiding this comment

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

k

bparees
bparees reviewed Jan 13, 2017
View reviewed changes
docs/man/man1/oc-cluster-up.1
\fB\-\-persistent\-volumes\-dir\fP="/var/lib/origin/openshift.local.pv"
Directory on host for OpenShift persistent volumes

.PP
Copy link
Contributor

Choose a reason for hiding this comment

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

??

bparees
bparees reviewed Jan 13, 2017
View reviewed changes
docs/man/man1/openshift-cli-cluster-up.1
\fB\-\-persistent\-volumes\-dir\fP="/var/lib/origin/openshift.local.pv"
Directory on host for OpenShift persistent volumes

.PP
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like you need to rerun docs gen?

@bparees bparees self-assigned this Jan 13, 2017
@bparees
Copy link
Contributor

bparees commented Jan 16, 2017

@csrwng other than the question about the generated docs, lgtm

@bparees
Copy link
Contributor

bparees commented Jan 16, 2017

@csrwng will need this backported into 3.4 once it merges.

@csrwng csrwng mentioned this pull request Jan 17, 2017
Closed
@csrwng csrwng force-pushed the clusterup_proxy branch 2 times, most recently from 0e3dcb9 to c0135d5 Compare January 17, 2017 16:37
@csrwng
Copy link
Contributor Author

csrwng commented Jan 17, 2017 •
edited
Loading

Now using a fixed IP for the docker registry (172.30.1.1). This should make it easy to configure NO_PROXY for Docker. (It won't take a CIDR).

Items remaining:

  • Populate a default NO_PROXY value based on current host and registry CIDR
  • Check that the Docker proxy settings are reasonable on startup (and warn otherwise).

@csrwng csrwng force-pushed the clusterup_proxy branch 4 times, most recently from f5af096 to d37afc4 Compare January 17, 2017 21:51
@csrwng csrwng changed the title [WIP] cluster up: add proxy support cluster up: add proxy support Jan 17, 2017
@csrwng
Copy link
Contributor Author

csrwng commented Jan 17, 2017

@bparees should be ready for another review

@bparees
Copy link
Contributor

bparees commented Jan 17, 2017

lgtm

@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 18, 2017
@csrwng
Copy link
Contributor Author

csrwng commented Jan 18, 2017

Tested on windows, mac and linux
Note on 'Docker for Windows': setting the proxy server for Docker causes every container created to inherit the proxy settings, which means that for any local services to be accessible from other containers, the NO_PROXY list configured on the Docker daemon needs to include their name and the .cluster.local prefix.

@csrwng
Copy link
Contributor Author

csrwng commented Jan 18, 2017

[merge]

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to a837c86

@openshift-bot
Copy link
Contributor

[Test]ing while waiting on the merge queue

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to a837c86

@openshift-bot openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 18, 2017
@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/12985/) (Base Commit: 68d4d0a)

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 19, 2017 •
edited
Loading

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/13010/) (Base Commit: 0d637fe) (Image: devenv-rhel7_5717)

@openshift-bot openshift-bot merged commit e4b43ee into openshift:master Jan 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bparees bparees bparees left review comments

Assignees

@bparees bparees

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@csrwng @bparees @openshift-bot