support compile binaries with kubeedge/build-tools images #3756

gy95 · 2022-03-31T02:06:22Z

What type of PR is this?

What this PR does / why we need it:
support compile source code with images,but not depend on local machine golang.
Which issue(s) this PR fixes:

Fixes #3687

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

fisherxu · 2022-03-31T09:40:50Z

Makefile

@@ -51,6 +51,9 @@ all:
 	KUBEEDGE_OUTPUT_SUBPATH=$(OUT_DIR) hack/make-rules/build.sh $(WHAT)
 endif

+.PHONY: compile


It should be part of make with export BUILD_WITH_CONTAINER=true
make all or make is also do the compilation work

fisherxu · 2022-03-31T09:42:36Z

hack/lib/golang.sh

+    if [ ${KUBEEDGE_COMPILE_WITH_IMAGE} = true ]; then
+      set -x
+      docker run --rm -e CGO_ENABLED=0 -v ${KUBEEDGE_ROOT}:${MOUNTPATH} -w ${MOUNTPATH} ${GOLANG_IMAGE} \
+          /usr/local/go/bin/go build -o ${MOUNTPATH}/_output/local/bin/${name} -gcflags="${gogcflags:-}" -ldflags "${goldflags:-}" $bin


it's better to use this image: kubeedge/build-tools, so all the compilation work can in one env.

fisherxu · 2022-03-31T12:11:05Z

hack/make-rules/compile.sh

+
+source "${KUBEEDGE_ROOT}/hack/lib/init.sh"
+
+kubeedge::golang::build_binaries "$@"


This script seems no needed?

gy95 · 2022-04-01T01:28:28Z

Makefile

+else ifeq ($(KUBEEDGE_COMPILE_WITH_IMAGE),true)
+all:
+	hack/make-rules/build.sh $(WHAT)


can also delete this block directly, or emphasize the env KUBEEDGE_COMPILE_WITH_IMAGE to make it separated from using local go. ?

BUILD_WITH_CONTAINER will be a better name, in container, not image.

We can set BUILD_WITH_CONTAINER as a env value like istio? so no need to add it to cmd everytime.

Looks like adding another script for container is fine.

yes, we can use BUILD_WITH_CONTAINER as an env like below.

root@ecs-8f95:~/go/src/github.com/kubeedge/kubeedge# export BUILD_WITH_CONTAINER=true root@ecs-8f95:~/go/src/github.com/kubeedge/kubeedge# make all WHAT=keadm hack/make-rules/build_with_container.sh keadm building github.com/kubeedge/kubeedge/keadm/cmd/keadm + docker run --rm -v /root/go/src/github.com/kubeedge/kubeedge:/kubeedge -w /kubeedge kubeedge/build-tools /usr/local/go/bin/go build -o /kubeedge/_output/local/bin/keadm -gcflags= -ldflags '-s -w -buildid= -X github.com/kubeedge/kubeedge/pkg/version.buildDate=2022-04-02T00:58:49Z -X github.com/kubeedge/kubeedge/pkg/version.gitCommit=e0d10fd6800e0fb02a9c307d98c23eac3d589121 -X github.com/kubeedge/kubeedge/pkg/version.gitTreeState=dirty -X github.com/kubeedge/kubeedge/pkg/version.gitVersion=v1.5.0-beta.0.1359+e0d10fd6800e0f-dirty -X github.com/kubeedge/kubeedge/pkg/version.gitMajor=1 -X github.com/kubeedge/kubeedge/pkg/version.gitMinor=5+' github.com/kubeedge/kubeedge/keadm/cmd/keadm + set +x root@ecs-8f95:~/go/src/github.com/kubeedge/kubeedge# unset BUILD_WITH_CONTAINER root@ecs-8f95:~/go/src/github.com/kubeedge/kubeedge# make all WHAT=keadm KUBEEDGE_OUTPUT_SUBPATH=_output/local hack/make-rules/build.sh keadm go detail version: go version go1.16.15 linux/amd64 go version: 1.16.15 building github.com/kubeedge/kubeedge/keadm/cmd/keadm + go build -o /root/go/src/github.com/kubeedge/kubeedge/_output/local/bin/keadm -gcflags= -ldflags '-s -w -buildid= -X github.com/kubeedge/kubeedge/pkg/version.buildDate=2022-04-02T01:00:08Z -X github.com/kubeedge/kubeedge/pkg/version.gitCommit=e0d10fd6800e0fb02a9c307d98c23eac3d589121 -X github.com/kubeedge/kubeedge/pkg/version.gitTreeState=dirty -X github.com/kubeedge/kubeedge/pkg/version.gitVersion=v1.5.0-beta.0.1359+e0d10fd6800e0f-dirty -X github.com/kubeedge/kubeedge/pkg/version.gitMajor=1 -X github.com/kubeedge/kubeedge/pkg/version.gitMinor=5+' github.com/kubeedge/kubeedge/keadm/cmd/keadm + set +x

And can also specify the BUILD_WITH_CONTAINER env in make command, just like this:

root@ecs-8f95:~/go/src/github.com/kubeedge/kubeedge# make all WHAT=keadm BUILD_WITH_CONTAINER=true hack/make-rules/build_with_container.sh keadm building github.com/kubeedge/kubeedge/keadm/cmd/keadm + docker run --rm -v /root/go/src/github.com/kubeedge/kubeedge:/kubeedge -w /kubeedge kubeedge/build-tools /usr/local/go/bin/go build -o /kubeedge/_output/local/bin/keadm -gcflags= -ldflags '-s -w -buildid= -X github.com/kubeedge/kubeedge/pkg/version.buildDate=2022-04-02T01:22:49Z -X github.com/kubeedge/kubeedge/pkg/version.gitCommit=b216deb58ac6de7f1e5bcab9c3ec136286ae3647 -X github.com/kubeedge/kubeedge/pkg/version.gitTreeState=clean -X github.com/kubeedge/kubeedge/pkg/version.gitVersion=v1.5.0-beta.0.1357+b216deb58ac6de -X github.com/kubeedge/kubeedge/pkg/version.gitMajor=1 -X github.com/kubeedge/kubeedge/pkg/version.gitMinor=5+' github.com/kubeedge/kubeedge/keadm/cmd/keadm + set +x root@ecs-8f95:~/go/src/github.com/kubeedge/kubeedge# make all WHAT=keadm KUBEEDGE_OUTPUT_SUBPATH=_output/local hack/make-rules/build.sh keadm go detail version: go version go1.16.15 linux/amd64 go version: 1.16.15 building github.com/kubeedge/kubeedge/keadm/cmd/keadm + go build -o /root/go/src/github.com/kubeedge/kubeedge/_output/local/bin/keadm -gcflags= -ldflags '-s -w -buildid= -X github.com/kubeedge/kubeedge/pkg/version.buildDate=2022-04-02T01:23:51Z -X github.com/kubeedge/kubeedge/pkg/version.gitCommit=b216deb58ac6de7f1e5bcab9c3ec136286ae3647 -X github.com/kubeedge/kubeedge/pkg/version.gitTreeState=clean -X github.com/kubeedge/kubeedge/pkg/version.gitVersion=v1.5.0-beta.0.1357+b216deb58ac6de -X github.com/kubeedge/kubeedge/pkg/version.gitMajor=1 -X github.com/kubeedge/kubeedge/pkg/version.gitMinor=5+' github.com/kubeedge/kubeedge/keadm/cmd/keadm + set +x

fisherxu · 2022-04-02T03:51:49Z

Makefile

@@ -35,6 +35,7 @@ define ALL_HELP_INFO
 #   make all
 #   make all HELP=y
 #   make all WHAT=cloudcore
+#   make all WHAT=cloudcore BUILD_WITH_CONTAINER=true


Need some comments for this

fisherxu · 2022-04-02T03:52:41Z

hack/lib/golang.sh

+      KUBEEDGE_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd -P)"
+      MOUNTPATH="/kubeedge"
+      KUBEEDGE_BUILD_IMAGE="kubeedge/build-tools"


Is it possible to place them to build_with_container.sh?

fixed, PTAL 😄

fisherxu · 2022-04-02T07:17:01Z

hack/lib/golang.sh

-    go build -o ${KUBEEDGE_OUTPUT_BINPATH}/${name} -gcflags="${gogcflags:-}" -ldflags "${goldflags:-}" $bin
-    set +x
+    # if env BUILD_WITH_CONTAINER exist, we build with docker images
+    set +u


Remove the set +/-u?

cannot, if want to check the variable exist or not using if [ ! -z ${BUILD_WITH_CONTAINER} ], we must set +u, or will report the error BUILD_WITH_CONTAINER: unbound variable
and there're lots of set -o nounset in the scripts, this will prevent we check the env exist or not.

refactored, and more common, PTAL

fisherxu · 2022-04-06T01:35:04Z

hack/make-rules/build_with_container.sh

+
+KUBEEDGE_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd -P)"
+MOUNTPATH="${MOUNTPATH:-/kubeedge}"
+KUBEEDGE_BUILD_IMAGE="kubeedge/build-tools"


Suggested change

KUBEEDGE_BUILD_IMAGE="kubeedge/build-tools"

MOUNTPATH=${MOUNTPATH:-"/kubeedge"}

KUBEEDGE_BUILD_IMAGE=${KUBEEDGE_BUILD_IMAGE:-"kubeedge/build-tools"}

fisherxu · 2022-04-06T02:09:49Z

Makefile

+#   If we want to build KubeEdge in image container mode, there're two ways to implement the function.
+#       1) make all WHAT=cloudcore BUILD_WITH_CONTAINER=true
+#       2) export BUILD_WITH_CONTAINER=true && make all WHAT=cloudcore


Suggested change

# If we want to build KubeEdge in image container mode, there're two ways to implement the function.

# 1) make all WHAT=cloudcore BUILD_WITH_CONTAINER=true

# 2) export BUILD_WITH_CONTAINER=true && make all WHAT=cloudcore

# Set the environment variable or arg BUILD_WITH_CONTAINER to build inside container,

# only docker and make are required in this mode.

# 1) make all WHAT=cloudcore BUILD_WITH_CONTAINER=true

# 2) export BUILD_WITH_CONTAINER=true && make all WHAT=cloudcore

Let's move them below L47

fisherxu · 2022-04-06T02:11:23Z

Makefile

@@ -22,6 +22,8 @@ COMPONENTS=cloud \
 .EXPORT_ALL_VARIABLES:
 OUT_DIR ?= _output/local


Set a default value for BUILD_WITH_CONTAINER here? Let's build in container by default.

yes, but seems that we need more fix to adjust it, will open another PR to set build in container default
https://github.com/kubeedge/kubeedge/runs/5844143228?check_suite_focus=true

Ok, if there're some actions build in container originally, we can disable BUILD_WITH_CONTAINER.

Signed-off-by: gy95 <guoyao17@huawei.com>

fisherxu

/lgtm
/approve

kubeedge-bot · 2022-04-06T03:20:34Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fisherxu

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

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

kubeedge-bot requested review from daixiang0 and subpathdev March 31, 2022 02:06

kubeedge-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 31, 2022

gy95 force-pushed the binary branch from 99380a3 to 992dda8 Compare March 31, 2022 02:48

fisherxu reviewed Mar 31, 2022

View reviewed changes

gy95 force-pushed the binary branch from 992dda8 to 4369b1e Compare March 31, 2022 11:58

fisherxu reviewed Mar 31, 2022

View reviewed changes

gy95 force-pushed the binary branch 3 times, most recently from be785f0 to 52e1d86 Compare April 1, 2022 01:20

gy95 commented Apr 1, 2022

View reviewed changes

gy95 force-pushed the binary branch from e0d10fd to b216deb Compare April 2, 2022 01:01

fisherxu reviewed Apr 2, 2022

View reviewed changes

gy95 force-pushed the binary branch from b216deb to 8ef20db Compare April 2, 2022 06:21

fisherxu reviewed Apr 2, 2022

View reviewed changes

gy95 force-pushed the binary branch from 8ef20db to 0a6518b Compare April 6, 2022 01:10

fisherxu reviewed Apr 6, 2022

View reviewed changes

gy95 force-pushed the binary branch from 0a6518b to 2cd2feb Compare April 6, 2022 02:31

support compile binaries with golang images

0bd2d50

Signed-off-by: gy95 <guoyao17@huawei.com>

gy95 force-pushed the binary branch from 2cd2feb to 0bd2d50 Compare April 6, 2022 02:59

fisherxu reviewed Apr 6, 2022

View reviewed changes

kubeedge-bot added the lgtm Indicates that a PR is ready to be merged. label Apr 6, 2022

kubeedge-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 6, 2022

kubeedge-bot merged commit 595c970 into kubeedge:master Apr 6, 2022

gy95 deleted the binary branch April 6, 2022 03:21

gy95 changed the title ~~support compile binaries with golang images~~ support compile binaries with kubeedge/build-tools images Jun 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support compile binaries with kubeedge/build-tools images #3756

support compile binaries with kubeedge/build-tools images #3756

gy95 commented Mar 31, 2022

fisherxu Mar 31, 2022

fisherxu Mar 31, 2022

fisherxu Mar 31, 2022

gy95 Apr 1, 2022

fisherxu Apr 1, 2022

fisherxu Apr 1, 2022

fisherxu Apr 1, 2022

gy95 Apr 2, 2022 •

edited

gy95 Apr 2, 2022

fisherxu Apr 2, 2022

gy95 Apr 2, 2022

fisherxu Apr 2, 2022

gy95 Apr 2, 2022

fisherxu Apr 2, 2022

gy95 Apr 2, 2022

gy95 Apr 6, 2022

fisherxu Apr 6, 2022

gy95 Apr 6, 2022

fisherxu Apr 6, 2022

fisherxu Apr 6, 2022

gy95 Apr 6, 2022

fisherxu Apr 6, 2022

fisherxu left a comment

kubeedge-bot commented Apr 6, 2022


		source "${KUBEEDGE_ROOT}/hack/lib/init.sh"

		kubeedge::golang::build_binaries "$@"

	KUBEEDGE_BUILD_IMAGE="kubeedge/build-tools"
	MOUNTPATH=${MOUNTPATH:-"/kubeedge"}
	KUBEEDGE_BUILD_IMAGE=${KUBEEDGE_BUILD_IMAGE:-"kubeedge/build-tools"}

		@@ -22,6 +22,8 @@ COMPONENTS=cloud \
		.EXPORT_ALL_VARIABLES:
		OUT_DIR ?= _output/local

support compile binaries with kubeedge/build-tools images #3756

support compile binaries with kubeedge/build-tools images #3756

Conversation

gy95 commented Mar 31, 2022

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

gy95 Apr 2, 2022 • edited

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

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

fisherxu left a comment

Choose a reason for hiding this comment

kubeedge-bot commented Apr 6, 2022

gy95 Apr 2, 2022 •

edited