configure Docker daemon to use proxy #322

bart0sh · 2019-02-20T12:39:07Z

This is a second fix for the proxy issues (see #306 for details)

Docker fails to pull images if running behind the proxy.
It causes weave-net pods to stuck in ImagePullBackOff state.

Configured proxy as explained in docker documentation:
https://docs.docker.com/config/daemon/systemd/#http-proxy
This should solve the issue.

Fixes: #306

Docker fails to pull images if running behind the proxy. It causes weave-net pods to stuck in ImagePullBackOff state. Configured proxy as explained in docker documentation: https://docs.docker.com/config/daemon/systemd/#http-proxy This should solve the issue. Fixes: #306 Signed-off-by: Ed Bartosh <eduard.bartosh@intel.com>

bart0sh · 2019-02-20T12:48:49Z

/cc @BenTheElder

bart0sh · 2019-02-20T12:59:40Z

/test pull-kind-conformance-parallel

bart0sh · 2019-02-20T13:32:07Z

@BenTheElder looks like failing test is not related to my changes. It's rather related to kubeadm changes. What should I do with this?

neolit123

thanks @bart0sh
mostly SGTM

neolit123 · 2019-02-20T13:42:37Z

pkg/cluster/internal/create/createcontext.go

@@ -146,6 +146,12 @@ func (cc *Context) ProvisionNodes() (nodeList map[string]*nodes.Node, err error)
 			return nodeList, err
 		}

+		cc.Status.Start(fmt.Sprintf("[%s] Configuring proxy 🐋", configNode.Name))


had to view this RAW and search the U+... in google as none of my Windows fonts support this glyph.
i think we need a different icon for the proxy and not a "whale".

also my suggestion to move these to constants was not accepted. :grumble:

It worked just fine fine for me in Mac Terminal. As for another icon, so far the code configures proxy for Docker, so I decided to use "whale" glyph.

Can you suggest where can I see a list of supported glyphs?

i guess the whale also makes sense. i'm going to leave this topic to @BenTheElder

This is fine for now and on par with the rest. I'll probably revisit this UX late next week. Output needs cleanup.

I'm not sure if we need a status for this step given how quick it should be, but like I said we need to look at cleaning this up anyhow.

Aside: what windows are you using Lubomir? Windows should have emoji fonts and even has some custom nonstandard emoji.

neolit123 · 2019-02-20T13:45:09Z

pkg/cluster/nodes/node.go

+
+// WriteFile writes temporary file on the host
+// and copies it to the node
+func (n *Node) WriteFile(dest, content string) error {


didn't we have a method for this already?

no as far as I can see. I'm planning to use this to refactor a couple of places in a separate PR.

neolit123 · 2019-02-20T13:46:14Z

/priority important-longterm
/kind feature

@BenTheElder looks like failing test is not related to my changes. It's rather related to kubeadm changes. What should I do with this?

yes, nothing for now.

bart0sh · 2019-02-20T20:06:20Z

/test pull-kind-conformance-parallel

BenTheElder · 2019-02-22T08:18:42Z

/assign
coming back to this in the morning, I expected to come back after the kubaadm fix upstream, but today was mostly tackling prow.k8s.io problems... 😞

BenTheElder

/lgtm
/approve
Thank you!

BenTheElder · 2019-02-23T02:18:57Z

pkg/cluster/nodes/node.go

+	}
+
+	// copy the temp file from the host to the node
+	if err := n.CopyTo(path, dest); err != nil {


I have a neat trick to avoid this tempfile but we can do it in a follow up:
node.Command("cp", "/dev/stdin", dest).SetInput(strings.NewReader(contents)).Run()
Will probably refactor to do this in other places to deal with docker installed via snap not having access to $TMPDIR.

looks nice :)

BenTheElder · 2019-02-23T02:20:54Z

pkg/cluster/internal/create/createcontext.go

@@ -146,6 +146,12 @@ func (cc *Context) ProvisionNodes() (nodeList map[string]*nodes.Node, err error)
 			return nodeList, err
 		}

+		cc.Status.Start(fmt.Sprintf("[%s] Configuring proxy 🐋", configNode.Name))


I'm not sure if we need a status for this step given how quick it should be, but like I said we need to look at cleaning this up anyhow.

Aside: what windows are you using Lubomir? Windows should have emoji fonts and even has some custom nonstandard emoji.

k8s-ci-robot · 2019-02-23T02:21:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bart0sh, BenTheElder

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [BenTheElder]

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

BenTheElder · 2019-02-23T02:21:44Z

pkg/cluster/internal/create/createcontext.go

+		cc.Status.Start(fmt.Sprintf("[%s] Configuring proxy 🐋", configNode.Name))
+		if err := node.SetProxy(); err != nil {
+			// TODO: logging here
+			return nodeList, errors.Wrapf(err, "failed to set proxy for %s", configNode.Name)


Also slightly iffy on this being a creation failure rather than a warning, but we can revisit this.

* Add iam permissions * Add iam permissions * Add iam permissions * Fix Files checks * Fix eks-custom-links fix * Fix eks-custom-links fix * Fix eks-custom-links fix * Fix eks-custom-links fix * Fix eks-custom-links fix * Fix eks-custom-links fix * Fix [Docs Checks] Empty lines * Fix [Docs Checks] Empty lines * Fix [Docs Checks] Empty lines * Fix [Docs Checks] Empty lines * Fix [Docs Checks] Empty lines * Fix [Docs Checks] Empty lines * Fix [Docs Checks] Empty lines * Fix [Docs Checks] Empty lines

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 20, 2019

k8s-ci-robot requested review from amwat and munnerz February 20, 2019 12:39

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 20, 2019

k8s-ci-robot requested a review from BenTheElder February 20, 2019 12:48

neolit123 reviewed Feb 20, 2019

View reviewed changes

k8s-ci-robot added priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. kind/feature Categorizes issue or PR as related to a new feature. labels Feb 20, 2019

k8s-ci-robot assigned BenTheElder Feb 22, 2019

BenTheElder approved these changes Feb 23, 2019

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 23, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 23, 2019

BenTheElder reviewed Feb 23, 2019

View reviewed changes

k8s-ci-robot merged commit d31173b into kubernetes-sigs:master Feb 23, 2019

BenTheElder mentioned this pull request Feb 23, 2019

overlay network cannot be applied when host is behind a proxy #136

Closed

BenTheElder mentioned this pull request Apr 3, 2019

avoid intermediate file in Node.WriteFile #433

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

configure Docker daemon to use proxy #322

configure Docker daemon to use proxy #322

bart0sh commented Feb 20, 2019

bart0sh commented Feb 20, 2019

bart0sh commented Feb 20, 2019

bart0sh commented Feb 20, 2019

neolit123 left a comment

neolit123 Feb 20, 2019 •

edited

Loading

bart0sh Feb 20, 2019

neolit123 Feb 20, 2019

BenTheElder Feb 23, 2019 •

edited

Loading

BenTheElder Feb 23, 2019

neolit123 Feb 20, 2019

bart0sh Feb 20, 2019

neolit123 commented Feb 20, 2019 •

edited

Loading

bart0sh commented Feb 20, 2019

BenTheElder commented Feb 22, 2019

BenTheElder left a comment

BenTheElder Feb 23, 2019

bart0sh Feb 23, 2019

BenTheElder Feb 23, 2019

k8s-ci-robot commented Feb 23, 2019

BenTheElder Feb 23, 2019

configure Docker daemon to use proxy #322

configure Docker daemon to use proxy #322

Conversation

bart0sh commented Feb 20, 2019

bart0sh commented Feb 20, 2019

bart0sh commented Feb 20, 2019

bart0sh commented Feb 20, 2019

neolit123 left a comment

Choose a reason for hiding this comment

neolit123 Feb 20, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenTheElder Feb 23, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neolit123 commented Feb 20, 2019 • edited Loading

bart0sh commented Feb 20, 2019

BenTheElder commented Feb 22, 2019

BenTheElder left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-ci-robot commented Feb 23, 2019

Choose a reason for hiding this comment

neolit123 Feb 20, 2019 •

edited

Loading

BenTheElder Feb 23, 2019 •

edited

Loading

neolit123 commented Feb 20, 2019 •

edited

Loading