Dockerfile: go mod cache

[7sunarni]

cache go modules, so improve building speed
https://docs.docker.com/language/golang/build-images/#create-a-dockerfile-for-the-application

[volcano-sh-bot]

Welcome @7sunarni!

It looks like this is your first PR to volcano-sh/volcano 馃帀.

Thank you, and welcome to Volcano. 😃

[7sunarni]

assign @Thor-wl

[7sunarni]

Hi~ Can you take minutes review this, it could speed up CI stage
@william-wang

[hwdef]

please make the DCO CI happy first

[7sunarni]

please make the DCO CI happy first

Done

[hwdef]

/lgtm

[7sunarni]

It seems both of reviewers not active any more, how about assign new reviewer?

[7sunarni]

Update: this commit use alpine mirror to speed up building webhook-manager docker image. I think most of Volcano's user in mainland. This mirror could be useful in improving building container image.

[7sunarni]

@Monokaix Hi~ Sorry to tag you. It looks like that two reviewers assigned not active in this project. Do you know how and who to assign new reviewers?

[Monokaix]

Update: this commit use alpine mirror to speed up building webhook-manager docker image. I think most of Volcano's user in mainland. This mirror could be useful in improving building container image.

It's a good intention here. But volcano is used all over the world, we should not specify cdn url in a hard code way, my opinion is that set a parameter like env to let user specify custom cdn url or just disable it: )

[Monokaix]

Welcome! A little doubt here. Does this cache optimization you mentioned only take effect when building multiple binaries? Seems that when we execute make vc-controller-manager, the go mod download will be executed, and after your pr, this step has been moved to the front, so what's the difference here?

[7sunarni]

Does this cache optimization you mentioned only take effect when building multiple binaries?

I think when your change not including go.mod and go.sum file and just including some go file, in docker building stage, this will not download dependency. Not only in building multiple binaries, and in local develop/test stage building image.

Seems that when we execute make vc-controller-manager, the go mod download will be executed, and after your pr, this step has been moved to the front, so what's the difference here?

As I know, docker will use the cache if the file don't change. In most case, go.mod and go.sum will not change, so docker will use cache in step3 and step4 . This is also what Docker document recommend us to do. And if we just copy .volcano to the build, this step has file change, docker will not use cache from this.

[Monokaix]

I think we should also add --build-args includes this env in docker build command: )

[7sunarni]

You are right.

[Monokaix]

Does this cache optimization you mentioned only take effect when building multiple binaries?

I think when your change not including go.mod and go.sum file and just including some go file, in docker building stage, this will not download dependency. Not only in building multiple binaries, and in local develop/test stage building image.

Seems that when we execute make vc-controller-manager, the go mod download will be executed, and after your pr, this step has been moved to the front, so what's the difference here?

As I know, docker will use the cache if the file don't change. In most case, go.mod and go.sum will not change, so docker will use cache in step3 and step4 . This is also what Docker document recommend us to do. And if we just copy .volcano to the build, this step has file change, docker will not use cache from this.

Got that! And please rebase and make a clean pr.

[7sunarni]

Update: this commit use alpine mirror to speed up building webhook-manager docker image. I think most of Volcano's user in mainland. This mirror could be useful in improving building container image.

It's a good intention here. But volcano is used all over the world, we should not specify cdn url in a hard code way, my opinion is that set a parameter like env to let user specify custom cdn url or just disable it: )

You are right. I add a argument like --build-arg APK_MIRROR="https://mirrors.huaweicloud.com" to let users do their choice, default will not do anything.

By the way, it seems you are from Huawei. And the usage in this official article seems have a little mistake. The config command should be sed -i "s@https://dl-cdn.alpinelinux.org/@https://repo.huaweicloud.com/@g" /etc/apk/repositories . The comments in this article had point out this mistake, but not correct yet. :-(

[Monokaix]

Update: this commit use alpine mirror to speed up building webhook-manager docker image. I think most of Volcano's user in mainland. This mirror could be useful in improving building container image.

It's a good intention here. But volcano is used all over the world, we should not specify cdn url in a hard code way, my opinion is that set a parameter like env to let user specify custom cdn url or just disable it: )

You are right. I add a argument like --build-arg APK_MIRROR="https://mirrors.huaweicloud.com" to let users do their choice, default will not do anything.

By the way, it seems you are from Huawei. And the usage in this official article seems have a little mistake. The config command should be sed -i "s@https://dl-cdn.alpinelinux.org/@https://repo.huaweicloud.com/@g" /etc/apk/repositories . The comments in this article had point out this mistake, but not correct yet. :-(

Thanks, I will convey the mistake but I'm not responsible for that article actually: )

[Monokaix]

Update: this commit use alpine mirror to speed up building webhook-manager docker image. I think most of Volcano's user in mainland. This mirror could be useful in improving building container image.

It's a good intention here. But volcano is used all over the world, we should not specify cdn url in a hard code way, my opinion is that set a parameter like env to let user specify custom cdn url or just disable it: )

You are right. I add a argument like --build-arg APK_MIRROR="https://mirrors.huaweicloud.com" to let users do their choice, default will not do anything.

By the way, it seems you are from Huawei. And the usage in this official article seems have a little mistake. The config command should be sed -i "s@https://dl-cdn.alpinelinux.org/@https://repo.huaweicloud.com/@g" /etc/apk/repositories . The comments in this article had point out this mistake, but not correct yet. :-(

You can also make suggestions in the forum https://bbs.huaweicloud.com/suggestion/new directly.

[7sunarni]

Does this cache optimization you mentioned only take effect when building multiple binaries?

I think when your change not including go.mod and go.sum file and just including some go file, in docker building stage, this will not download dependency. Not only in building multiple binaries, and in local develop/test stage building image.

Seems that when we execute make vc-controller-manager, the go mod download will be executed, and after your pr, this step has been moved to the front, so what's the difference here?

As I know, docker will use the cache if the file don't change. In most case, go.mod and go.sum will not change, so docker will use cache in step3 and step4 . This is also what Docker document recommend us to do. And if we just copy .volcano to the build, this step has file change, docker will not use cache from this.

Got that! And please rebase and make a clean pr.

Done

[Monokaix]

--build-arg APK_MIRROR="https://mirrors.huaweicloud.com"

Is "--build-arg APK_MIRROR="https://mirrors.huaweicloud.com" updated? I didn't see that yet: )

[william-wang]

retrigger the ci.

[7sunarni]

--build-arg APK_MIRROR="https://mirrors.huaweicloud.com"

Is "--build-arg APK_MIRROR="https://mirrors.huaweicloud.com" updated? I didn't see that yet: )

I don't add this parameter in Makefile. If this parameter is missing, it will not change the alpine mirror.

[Monokaix]

--build-arg APK_MIRROR="https://mirrors.huaweicloud.com"

Is "--build-arg APK_MIRROR="https://mirrors.huaweicloud.com" updated? I didn't see that yet: )

I don't add this parameter in Makefile. If this parameter is missing, it will not change the alpine mirror.

Can the env transfer to docker image context？Seems that the env should be transferred through build-arg.

[7sunarni]

--build-arg APK_MIRROR="https://mirrors.huaweicloud.com"

Is "--build-arg APK_MIRROR="https://mirrors.huaweicloud.com" updated? I didn't see that yet: )

I don't add this parameter in Makefile. If this parameter is missing, it will not change the alpine mirror.

Can the env transfer to docker image context？Seems that the env should be transferred through build-arg.

It can. I add parameter to the Makefile.

[Monokaix]

/lgtm

[william-wang]

/approve

[volcano-sh-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: william-wang

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 [william-wang]

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

[7sunarni]

@volcano-sh-bot CI