Pass proxy environment variables to the building container when building node images #3480

Zumium · 2024-01-14T11:36:03Z

When used in a restricted network environment, KIND may not be able to download necessary resources directly from the Internet during the building process, which makes the building fail. Adding --http-proxy option to the "build node-image" command helps to solve the problem by setting up the building container with http_proxy and https_proxy environment variables and making the downloading to be able to use the provided proxy.

linux-foundation-easycla · 2024-01-14T11:36:07Z

The committers listed above are authorized under a signed CLA.

✅ login: Zumium / name: martinzhou (1bb0060)

k8s-ci-robot · 2024-01-14T11:36:12Z

Welcome @Zumium!

It looks like this is your first PR to kubernetes-sigs/kind 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kind has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2024-01-14T11:36:12Z

Hi @Zumium. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

aojea · 2024-01-14T12:13:41Z

we inherit proxy variables from the environment, is this not the case with building ?

https://kind.sigs.k8s.io/docs/user/quick-start/#configure-kind-to-use-a-proxy

I don't expect to add flags for this

aojea · 2024-01-14T12:15:55Z

pkg/build/nodeimage/buildcontext.go

@@ -135,6 +136,9 @@ func (c *buildContext) buildImage(bits kube.Bits) error {
 		"docker", "commit",
 		// we need to put this back after changing it when running the image
 		"--change", `ENTRYPOINT [ "/usr/local/bin/entrypoint", "/sbin/init" ]`,
+		// remove http proxy setting


why remove?

It's because during my testing, I found that the control panel wasn't able to start up with my built node image, and the built image was carrying the proxy settings. When I removed them, it worked.

this does not gives much confidence on the solution proposed :/

aojea · 2024-01-14T12:16:05Z

pkg/build/nodeimage/buildcontext.go

+		"--security-opt", "seccomp=unconfined", // ignore seccomp
+		"--network=host",
+	}
+	if c.httpProxy != "" {


this can be obtained from

kind/pkg/cluster/internal/providers/common/proxy.go

Lines 35 to 37 in 40c81f1

// GetProxyEnvs returns a map of proxy environment variables to their values

// If proxy settings are set, NO_PROXY is modified to include the cluster subnets

func GetProxyEnvs(cfg *config.Cluster) map[string]string {

It seems that this GetProxyEnvs function is for using in provisioning clusters, because it requires a cluster config as its parameter, which we don't have when building node images. I'll just read from env vars directly.

aojea · 2024-01-14T12:16:19Z

pkg/cmd/kind/build/nodeimage/nodeimage.go

@@ -86,6 +87,12 @@ func NewCommand(logger log.Logger, streams cmd.IOStreams) *cobra.Command {
 		"",
 		"architecture to build for, defaults to the host architecture",
 	)
+	cmd.Flags().StringVar(


Zumium · 2024-01-14T13:46:38Z

@aojea Hi, aojea. The "http_proxy" and "https_proxy" env vars are not working with building yet because they are not passed to the building container, which is what happened when I tested it. If no flags is the preferred way, I'll adjust my code to read proxy settings from env vars instead of a flag.

aojea · 2024-01-14T15:50:09Z

pkg/build/nodeimage/buildcontext.go

+		"--name=" + id,
+		"--platform=" + dockerBuildOsAndArch(c.arch),
+		"--security-opt", "seccomp=unconfined", // ignore seccomp
+		"--network=host",


this was not here before, was it?

The line --network=host is added by me, the others were there before. The purpose of --network=host is to be able to use local host proxy like "http://127.0.0.1:1080".

you can not submit a PR with multiple changes without informing the reviewers, this is a change in behavior, it requires additional discussion, and in this case it seems that is only to fit your personal workflow

please do only one change at a time

The purpose of --network=host is to be able to use local host proxy like "http://127.0.0.1:1080/".

@Zumium
This is not necessary for the current PR, you can still use the proxy even without the host network, doesn't it?

@liangyuanpeng Well, I'm afraid it's necessary especially for a proxy running on localhost. Imagine your proxy server is running locally on host, for example http://127.0.0.1:1080, which I believe is a common case. If the address is directly set to the container and not using host network, "127.0.0.1" will refer to the container's localhost, and the programs inside the container will fail to connect to proxy because they're not connecting to the right place.

@BenTheElder I believe that the core of the problem is the meaning of "localhost" changes inside & outside of the container, and proxies usually bind on localhost for safety reason, preventing a process inside container from connecting to it directly. If host network is not accepted, we may have to add some kind of relay-er, and it will be a more complex solution.

you can use the IP of your host, proxies use to listen in all host ip addresses, is easier to modify your proxy than force this pod to run with hostnetwork

It's also ok to just keep KIND simple and leave the worry to users. I'll modify my code later.

I don't disagree, but this was this way for years with zero problems, and everytime we change something that sounds simple, it always have consequences

pkg/build/nodeimage/buildcontext.go

aojea · 2024-01-14T15:53:10Z

/assign @BenTheElder

BenTheElder · 2024-02-01T23:59:57Z

sorry for the delay, we should still make one of the requested changes re: host network

aojea · 2024-02-07T10:06:30Z

/ok-to-test

aojea · 2024-02-07T21:11:26Z

can you squash all commits?

…ntainer when building node images

Zumium · 2024-02-08T01:29:20Z

can you squash all commits?

Done.

Zumium · 2024-02-09T04:48:33Z

/retest

aojea · 2024-02-09T11:53:39Z

/lgtm
/approve

k8s-ci-robot · 2024-02-09T11:53:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aojea, Zumium

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~~ [aojea]

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

Zumium · 2024-02-09T14:03:01Z

Thank everyone for helping! What a coincidence that today happens to be 除夕(chu xi) festival, which marks the beginning of a new Chinese year. Again, thank you everyone and happy Chinese new year!

liangyuanpeng · 2024-02-09T14:41:37Z

happy new year :)

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Jan 14, 2024

k8s-ci-robot requested review from aojea and neolit123 January 14, 2024 11:36

aojea reviewed Jan 14, 2024

View reviewed changes

Zumium changed the title ~~Add "--http-proxy" option to "kind build node-image" command~~ Pass proxy environment variables to the building container when building node images Jan 14, 2024

aojea reviewed Jan 14, 2024

View reviewed changes

pkg/build/nodeimage/buildcontext.go Show resolved Hide resolved

k8s-ci-robot assigned BenTheElder Jan 14, 2024

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 7, 2024

pass HTTP_PROXY, HTTPS_PROXY and NO_PROXY env vars to the building co…

1bb0060

…ntainer when building node images

Zumium force-pushed the main branch from eb65b7f to 1bb0060 Compare February 8, 2024 01:22

k8s-ci-robot assigned aojea Feb 9, 2024

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

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

k8s-ci-robot merged commit 8d6a3c2 into kubernetes-sigs:main Feb 9, 2024
27 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pass proxy environment variables to the building container when building node images #3480

Pass proxy environment variables to the building container when building node images #3480

Zumium commented Jan 14, 2024

linux-foundation-easycla bot commented Jan 14, 2024 •

edited

Loading

k8s-ci-robot commented Jan 14, 2024

k8s-ci-robot commented Jan 14, 2024

aojea commented Jan 14, 2024

aojea Jan 14, 2024

Zumium Jan 14, 2024

aojea Jan 14, 2024

aojea Jan 14, 2024

Zumium Jan 14, 2024

aojea Jan 14, 2024

Zumium commented Jan 14, 2024

aojea Jan 14, 2024

Zumium Jan 14, 2024

aojea Jan 14, 2024

aojea Jan 14, 2024

liangyuanpeng Jan 28, 2024

Zumium Feb 5, 2024

Zumium Feb 5, 2024

aojea Feb 7, 2024

Zumium Feb 7, 2024

aojea Feb 7, 2024 •

edited

Loading

aojea commented Jan 14, 2024

BenTheElder commented Feb 1, 2024

aojea commented Feb 7, 2024

aojea commented Feb 7, 2024

Zumium commented Feb 8, 2024

Zumium commented Feb 9, 2024

aojea commented Feb 9, 2024

k8s-ci-robot commented Feb 9, 2024

Zumium commented Feb 9, 2024

liangyuanpeng commented Feb 9, 2024

	// GetProxyEnvs returns a map of proxy environment variables to their values
	// If proxy settings are set, NO_PROXY is modified to include the cluster subnets
	func GetProxyEnvs(cfg *config.Cluster) map[string]string {

Pass proxy environment variables to the building container when building node images #3480

Pass proxy environment variables to the building container when building node images #3480

Conversation

Zumium commented Jan 14, 2024

linux-foundation-easycla bot commented Jan 14, 2024 • edited Loading

k8s-ci-robot commented Jan 14, 2024

k8s-ci-robot commented Jan 14, 2024

aojea commented Jan 14, 2024

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zumium commented Jan 14, 2024

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

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

Choose a reason for hiding this comment

aojea Feb 7, 2024 • edited Loading

Choose a reason for hiding this comment

aojea commented Jan 14, 2024

BenTheElder commented Feb 1, 2024

aojea commented Feb 7, 2024

aojea commented Feb 7, 2024

Zumium commented Feb 8, 2024

Zumium commented Feb 9, 2024

aojea commented Feb 9, 2024

k8s-ci-robot commented Feb 9, 2024

Zumium commented Feb 9, 2024

liangyuanpeng commented Feb 9, 2024

linux-foundation-easycla bot commented Jan 14, 2024 •

edited

Loading

aojea Feb 7, 2024 •

edited

Loading