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

add support for building multi-CPU architecture images #1130

Closed
wants to merge 3 commits into from
Closed

add support for building multi-CPU architecture images #1130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
76b6f86
*: add support for building multi-CPU architecture images
paulfantom Apr 29, 2020
dc4ab90
*: remove containerized build process as golang is already locked in …
paulfantom Apr 29, 2020
fb1bb67
tests: fix selecting image for e2e tests
paulfantom Apr 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,6 @@ help.txt

# jsonnet dependency management
/scripts/vendor

# Image build tool
manifest-tool
60 changes: 36 additions & 24 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ GOLANGCI_VERSION := v1.25.0
HAS_GOLANGCI := $(shell which golangci-lint)

IMAGE = $(REGISTRY)/kube-state-metrics
MULTI_ARCH_IMG = $(IMAGE)-$(ARCH)

validate-modules:
@echo "- Verifying that the dependencies have expected content..."
Expand Down Expand Up @@ -59,13 +58,10 @@ doccheck: generate
@cd docs; for doc in *.md; do if [ "$$doc" != "README.md" ] && ! grep -q "$$doc" *.md; then echo "ERROR: No link to documentation file $${doc} detected"; exit 1; fi; done
@echo OK

build-local: clean
GOOS=$(shell uname -s | tr A-Z a-z) GOARCH=$(ARCH) CGO_ENABLED=0 go build -ldflags "-s -w -X ${PKG}/version.Release=${TAG} -X ${PKG}/version.Commit=${Commit} -X ${PKG}/version.BuildDate=${BuildDate}" -o kube-state-metrics

build: clean kube-state-metrics

kube-state-metrics:
${DOCKER_CLI} run --rm -v "${PWD}:/go/src/k8s.io/kube-state-metrics" -w /go/src/k8s.io/kube-state-metrics golang:${GO_VERSION} make build-local
GOOS=$(shell uname -s | tr A-Z a-z) GOARCH=$(ARCH) CGO_ENABLED=0 go build -ldflags "-s -w -X ${PKG}/version.Release=${TAG} -X ${PKG}/version.Commit=${Commit} -X ${PKG}/version.BuildDate=${BuildDate}" -o kube-state-metrics

test-unit: clean build
GOOS=$(shell uname -s | tr A-Z a-z) GOARCH=$(ARCH) $(TESTENVVAR) go test --race $(FLAGS) $(PKGS)
Expand All @@ -84,41 +80,57 @@ TEMP_DIR := $(shell mktemp -d)
all: all-container

sub-container-%:
$(MAKE) --no-print-directory ARCH=$* container
# 'clean' is necessary since build process needs to overwrite kube-state-metrics binary. Otherwise all images end up with binary made for one architecture
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not have clean as a pre-requisite to the sub-container-% target?

Copy link
Contributor Author

@paulfantom paulfantom May 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

That was the first thing I did, but it didn't work 🤷

$(MAKE) --no-print-directory ARCH=$* clean .container-$*

sub-push-%:
$(MAKE) --no-print-directory ARCH=$* push
# 'clean' is necessary since build process needs to overwrite kube-state-metrics binary. Otherwise all images end up with binary made for one architecture
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not have clean as a pre-requisite to the sub-push-% target?

$(MAKE) --no-print-directory ARCH=$* clean .quay-push-$*

all-container: $(addprefix sub-container-,$(ALL_ARCH))

all-push: $(addprefix sub-push-,$(ALL_ARCH))

container: .container-$(ARCH)
.container-$(ARCH): kube-state-metrics
cp -r * "${TEMP_DIR}"
${DOCKER_CLI} build -t $(MULTI_ARCH_IMG):$(TAG) "${TEMP_DIR}"
${DOCKER_CLI} tag $(MULTI_ARCH_IMG):$(TAG) $(MULTI_ARCH_IMG):latest
rm -rf "${TEMP_DIR}"
manifest-tool:
curl -fsSL https://github.com/estesp/manifest-tool/releases/download/v1.0.2/manifest-tool-linux-amd64 > ./manifest-tool
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's move the manifest-tool version to a variable

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's also have a bin existence check, so that we don't run this every time. Something like this:

HAS_GOLANGCI := $(shell which golangci-lint) which is used in the lint make target

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Simple checking for binary existence is a contrast to requiring a version of a binary. Consider having a pre-existing system-wide binary in version A, but specifying version B in Makefile. In such scenario you would need to add another check to validate globally installed binary version against version requested in Makefile. It is a question of code simplicity vs first execution speed.

chmod +x ./manifest-tool

comma:= ,
empty:=
space:= $(empty) $(empty)
manifest-push: manifest-tool
./manifest-tool push from-args --platforms $(subst $(space),$(comma),$(addprefix linux/,$(ALL_ARCH))) --template $(IMAGE):$(TAG)-ARCH --target $(IMAGE):$(TAG)
./manifest-tool push from-args --platforms $(subst $(space),$(comma),$(addprefix linux/,$(ALL_ARCH))) --template $(IMAGE):latest-ARCH --target $(IMAGE):latest

release-images: all-push manifest-push

container: .container-$(ARCH)
ifeq ($(ARCH), amd64)
# Adding check for amd64
${DOCKER_CLI} tag $(MULTI_ARCH_IMG):$(TAG) $(IMAGE):$(TAG)
${DOCKER_CLI} tag $(MULTI_ARCH_IMG):$(TAG) $(IMAGE):latest
${DOCKER_CLI} tag $(IMAGE):$(TAG)-$(ARCH) $(IMAGE):$(TAG)
${DOCKER_CLI} tag $(IMAGE):$(TAG)-$(ARCH) $(IMAGE):latest
endif

.container-%: kube-state-metrics
cp -r * "${TEMP_DIR}"
${DOCKER_CLI} build -t $(IMAGE):$(TAG)-$* "${TEMP_DIR}"
${DOCKER_CLI} tag $(IMAGE):$(TAG)-$* $(IMAGE):latest-$*
rm -rf "${TEMP_DIR}"

quay-push: .quay-push-$(ARCH)
.quay-push-$(ARCH): .container-$(ARCH)
${DOCKER_CLI} push $(MULTI_ARCH_IMG):$(TAG)
${DOCKER_CLI} push $(MULTI_ARCH_IMG):latest
ifeq ($(ARCH), amd64)
${DOCKER_CLI} push $(IMAGE):$(TAG)
${DOCKER_CLI} push $(IMAGE):latest
endif

push: .push-$(ARCH)
.push-$(ARCH): .container-$(ARCH)
gcloud docker -- push $(MULTI_ARCH_IMG):$(TAG)
gcloud docker -- push $(MULTI_ARCH_IMG):latest
.quay-push-%: .container-%
${DOCKER_CLI} push $(IMAGE):$(TAG)-$*
${DOCKER_CLI} push $(IMAGE):latest-$*

push: .push-%
.push-%: .container-%
gcloud docker -- push $(IMAGE):$(TAG)-$*
gcloud docker -- push $(IMAGE):latest-$*
ifeq ($(ARCH), amd64)
gcloud docker -- push $(IMAGE):$(TAG)
gcloud docker -- push $(IMAGE):latest
Expand All @@ -131,7 +143,7 @@ clean:
e2e:
./tests/e2e.sh

generate: build-local
generate: build
@echo ">> generating docs"
@./scripts/generate-help-text.sh
@$(GOPATH)/bin/embedmd -w `find . -path ./vendor -prune -o -name "*.md" -print`
Expand Down Expand Up @@ -164,4 +176,4 @@ install-tools:
@echo Installing tools from tools.go
@cat tools/tools.go | grep _ | awk -F'"' '{print $$2}' | xargs -tI % go install %

.PHONY: all build build-local all-push all-container test-unit test-benchmark-compare container push quay-push clean e2e validate-modules shellcheck licensecheck lint generate embedmd
.PHONY: all build all-push all-container all-manifest test-unit test-benchmark-compare container push quay-push manifest-push clean e2e validate-modules shellcheck licensecheck lint generate embedmd
2 changes: 1 addition & 1 deletion RELEASE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ Maintaining the release branches for older minor releases happens on a best effo
* Cut the new release branch, i.e., `release-1.2`, or merge/cherry-pick changes onto the minor release branch you intend to tag the release on
* Cut the new release tag, i.e., `v1.2.0-rc.0`
* Create issue requesting for image release to `k8s.gcr.io` (or to `staging-k8s.gcr.io` in case of release candidates) and assign it to one of Googlers (@serathius, @loburm)
* Build and push newest image to `quay.io`(@brancz)
* Build and push newest image to `quay.io` - `make release-images` (@brancz)

## Stable release

Expand Down
2 changes: 1 addition & 1 deletion tests/e2e.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ make build
# query kube-state-metrics image tag
make container
docker images -a
KUBE_STATE_METRICS_IMAGE_TAG=$(docker images -a|grep 'quay.io/coreos/kube-state-metrics'|grep -v 'latest'|awk '{print $2}'|sort -u)
KUBE_STATE_METRICS_IMAGE_TAG=$(docker images -a|grep 'quay.io/coreos/kube-state-metrics'|grep -v 'latest'|awk '{print $2}'|sort -u| head -n1)
echo "local kube-state-metrics image tag: $KUBE_STATE_METRICS_IMAGE_TAG"

# update kube-state-metrics image tag in deployment.yaml
Expand Down