-
Notifications
You must be signed in to change notification settings - Fork 1.7k
chore: Migrate Skaffold presubmits to new Kokoro instance #9952
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
e3c20a2
78401b7
87e7bde
1cccda3
577f71e
c4fdbb0
552e556
3a2d5ee
e379cc8
07526b1
fe4e128
2e93229
e15e05e
161cd90
7c97cb8
2658f26
a865acf
2ac9fa6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,22 @@ GCP_ONLY ?= false | |
| GCP_PROJECT ?= k8s-skaffold | ||
| GKE_CLUSTER_NAME ?= integration-tests | ||
| GKE_ZONE ?= us-central1-a | ||
| GKE_REGION=us-central1 | ||
|
|
||
| # Set registry/auth/cluster location based on GCP_PROJECT | ||
| ifeq ($(GCP_PROJECT),skaffold-ci-cd) | ||
| # Presubmit environment: skaffold-ci-cd project with Artifact Registry | ||
| IMAGE_REPO_BASE := $(AR_REGION)-docker.pkg.dev/$(GCP_PROJECT) | ||
| GCLOUD_AUTH_CONFIG := $(AR_REGION)-docker.pkg.dev | ||
| GKE_LOCATION_FLAG := --region $(GKE_REGION) | ||
| $(info Using Artifact Registry config for project: $(GCP_PROJECT)) | ||
| else | ||
| # k8s-skaffold project with GCR | ||
| IMAGE_REPO_BASE := gcr.io/$(GCP_PROJECT) | ||
| GCLOUD_AUTH_CONFIG := gcr.io | ||
| GKE_LOCATION_FLAG := --zone $(GKE_ZONE) | ||
| $(info Using GCR config for project: $(GCP_PROJECT)) | ||
| endif | ||
|
|
||
| SUPPORTED_PLATFORMS = linux-amd64 darwin-amd64 windows-amd64.exe linux-arm64 darwin-arm64 | ||
| BUILD_PACKAGE = $(REPOPATH)/v2/cmd/skaffold | ||
|
|
@@ -142,9 +158,16 @@ integration-tests: | |
| ifeq ($(GCP_ONLY),true) | ||
| gcloud container clusters get-credentials \ | ||
| $(GKE_CLUSTER_NAME) \ | ||
| --zone $(GKE_ZONE) \ | ||
| $(GKE_LOCATION_FLAG) \ | ||
| --project $(GCP_PROJECT) | ||
| gcloud auth configure-docker us-central1-docker.pkg.dev | ||
|
|
||
| # Conditional Docker authentication: ONLY when GCR is used | ||
| ifneq ($(GCP_PROJECT),skaffold-ci-cd) | ||
| @echo "Configuring Docker for GCR: $(GCLOUD_AUTH_CONFIG)" | ||
| gcloud auth configure-docker $(GCLOUD_AUTH_CONFIG) -q | ||
| else | ||
| @echo "Docker auth is handled in the build script for skaffold-ci-cd" | ||
| endif | ||
| endif | ||
| @ GCP_ONLY=$(GCP_ONLY) GKE_CLUSTER_NAME=$(GKE_CLUSTER_NAME) ./hack/gotest.sh -v $(REPOPATH)/v2/integration -timeout 50m $(INTEGRATION_TEST_ARGS) | ||
|
|
||
|
|
@@ -157,17 +180,17 @@ release: $(BUILD_DIR)/VERSION | |
| --build-arg VERSION=$(VERSION) \ | ||
| -f deploy/skaffold/Dockerfile \ | ||
| --target release \ | ||
| -t gcr.io/$(GCP_PROJECT)/skaffold:$(VERSION) \ | ||
| -t gcr.io/$(GCP_PROJECT)/skaffold:latest \ | ||
| -t $(IMAGE_REPO_BASE)/skaffold:$(VERSION) \ | ||
| -t $(IMAGE_REPO_BASE)/skaffold:latest \ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line has two issues:
Please fix the indentation and add the line continuation character. |
||
| . | ||
|
|
||
| .PHONY: release-build | ||
| release-build: | ||
| docker build \ | ||
| -f deploy/skaffold/Dockerfile \ | ||
| --target release \ | ||
| -t gcr.io/$(GCP_PROJECT)/skaffold:edge \ | ||
| -t gcr.io/$(GCP_PROJECT)/skaffold:$(COMMIT) \ | ||
| -t $(IMAGE_REPO_BASE)/skaffold:edge \ | ||
| -t $(IMAGE_REPO_BASE)/skaffold:$(COMMIT) \ | ||
| . | ||
|
|
||
| .PHONY: release-lts | ||
|
|
@@ -176,53 +199,58 @@ release-lts: $(BUILD_DIR)/VERSION | |
| --build-arg VERSION=$(VERSION) \ | ||
| -f deploy/skaffold/Dockerfile.lts \ | ||
| --target release \ | ||
| -t gcr.io/$(GCP_PROJECT)/skaffold:lts \ | ||
| -t gcr.io/$(GCP_PROJECT)/skaffold:$(VERSION)-lts \ | ||
| -t gcr.io/$(GCP_PROJECT)/skaffold:$(SCANNING_MARKER)-lts \ | ||
| -t $(IMAGE_REPO_BASE)/skaffold:lts \ | ||
| -t $(IMAGE_REPO_BASE)/skaffold:$(VERSION)-lts \ | ||
| -t $(IMAGE_REPO_BASE)/skaffold:$(SCANNING_MARKER)-lts \ | ||
| . | ||
|
|
||
| .PHONY: release-lts-build | ||
| release-lts-build: | ||
| docker build \ | ||
| -f deploy/skaffold/Dockerfile.lts \ | ||
| --target release \ | ||
| -t gcr.io/$(GCP_PROJECT)/skaffold:edge-lts \ | ||
| -t gcr.io/$(GCP_PROJECT)/skaffold:$(COMMIT)-lts \ | ||
| -t $(IMAGE_REPO_BASE)/skaffold:edge-lts \ | ||
| -t $(IMAGE_REPO_BASE)/skaffold:$(COMMIT)-lts \ | ||
| . | ||
|
|
||
| .PHONY: clean | ||
| clean: | ||
| rm -rf $(BUILD_DIR) hack/bin $(EMBEDDED_FILES_CHECK) fs/assets/schemas_generated/ | ||
|
|
||
| # Runs a script to calculate a hash/digest of the build dependencies. Store it | ||
| # in DEPS_DIGEST. Then push the dependency image to GCR/AR. | ||
| .PHONY: build_deps | ||
| build_deps: | ||
| $(eval DEPS_DIGEST := $(shell ./hack/skaffold-deps-sha1.sh)) | ||
| docker build \ | ||
| -f deploy/skaffold/Dockerfile.deps \ | ||
| -t gcr.io/$(GCP_PROJECT)/build_deps:$(DEPS_DIGEST) \ | ||
| -t $(IMAGE_REPO_BASE)/build_deps:$(DEPS_DIGEST) \ | ||
| deploy/skaffold | ||
| docker push gcr.io/$(GCP_PROJECT)/build_deps:$(DEPS_DIGEST) | ||
| docker push $(IMAGE_REPO_BASE)/build_deps:$(DEPS_DIGEST) | ||
|
|
||
|
|
||
| skaffold-builder-ci: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
| docker build \ | ||
| --cache-from gcr.io/$(GCP_PROJECT)/build_deps \ | ||
| --cache-from $(IMAGE_REPO_BASE)/build_deps:$(DEPS_DIGEST) \ | ||
| -f deploy/skaffold/Dockerfile.deps \ | ||
| -t gcr.io/$(GCP_PROJECT)/build_deps \ | ||
| -t $(IMAGE_REPO_BASE)/build_deps \ | ||
| . | ||
| time docker build \ | ||
| -f deploy/skaffold/Dockerfile \ | ||
| --target builder \ | ||
| -t gcr.io/$(GCP_PROJECT)/skaffold-builder \ | ||
| --cache-from $(IMAGE_REPO_BASE)/build_deps \ | ||
| -t $(IMAGE_REPO_BASE)/skaffold-builder \ | ||
| . | ||
|
|
||
| .PHONY: skaffold-builder | ||
| skaffold-builder: | ||
| time docker build \ | ||
| -f deploy/skaffold/Dockerfile \ | ||
| --target builder \ | ||
| -t gcr.io/$(GCP_PROJECT)/skaffold-builder \ | ||
| -t $(IMAGE_REPO_BASE)/skaffold-builder \ | ||
| . | ||
|
|
||
| # Run integration tests within a local kind (Kubernetes IN Docker) cluster. | ||
| .PHONY: integration-in-kind | ||
| integration-in-kind: skaffold-builder | ||
| echo '{}' > /tmp/docker-config | ||
|
|
@@ -237,7 +265,7 @@ integration-in-kind: skaffold-builder | |
| -e INTEGRATION_TEST_ARGS=$(INTEGRATION_TEST_ARGS) \ | ||
| -e IT_PARTITION=$(IT_PARTITION) \ | ||
| --network kind \ | ||
| gcr.io/$(GCP_PROJECT)/skaffold-builder \ | ||
| $(IMAGE_REPO_BASE)/skaffold-builder \ | ||
| sh -eu -c ' \ | ||
| if ! kind get clusters | grep -q kind; then \ | ||
| trap "kind delete cluster" 0 1 2 15; \ | ||
|
|
@@ -262,7 +290,7 @@ integration-in-k3d: skaffold-builder | |
| -v $(CURDIR)/hack/maven/settings.xml:/root/.m2/settings.xml \ | ||
| -e INTEGRATION_TEST_ARGS=$(INTEGRATION_TEST_ARGS) \ | ||
| -e IT_PARTITION=$(IT_PARTITION) \ | ||
| gcr.io/$(GCP_PROJECT)/skaffold-builder \ | ||
| $(IMAGE_REPO_BASE)/skaffold-builder \ | ||
| sh -eu -c ' \ | ||
| if ! k3d cluster list | grep -q k3s-default; then \ | ||
| trap "k3d cluster delete" 0 1 2 15; \ | ||
|
|
@@ -276,27 +304,30 @@ integration-in-k3d: skaffold-builder | |
| make integration \ | ||
| ' | ||
|
|
||
| # The `gcloud auth configure-docker` below is needed this starts a separate | ||
| # container (us-central1-docker.pkg.dev/skaffold-ci-cd/skaffold-builder) to run | ||
| # the tests. This inner container doesn't inherit the Docker configuration from | ||
| # the host. | ||
| .PHONY: integration-in-docker | ||
| integration-in-docker: skaffold-builder-ci | ||
| docker run --rm \ | ||
| -v /var/run/docker.sock:/var/run/docker.sock \ | ||
| -v $(HOME)/.config/gcloud:/root/.config/gcloud \ | ||
| -v $(GOOGLE_APPLICATION_CREDENTIALS):$(GOOGLE_APPLICATION_CREDENTIALS) \ | ||
| -v $(CURDIR)/hack/maven/settings.xml:/root/.m2/settings.xml \ | ||
| -e GCP_ONLY=$(GCP_ONLY) \ | ||
| -e GCP_PROJECT=$(GCP_PROJECT) \ | ||
| -e GKE_CLUSTER_NAME=$(GKE_CLUSTER_NAME) \ | ||
| -e GKE_ZONE=$(GKE_ZONE) \ | ||
| -e DOCKER_CONFIG=/root/.docker \ | ||
| -e GOOGLE_APPLICATION_CREDENTIALS=$(GOOGLE_APPLICATION_CREDENTIALS) \ | ||
| -e INTEGRATION_TEST_ARGS=$(INTEGRATION_TEST_ARGS) \ | ||
| -e IT_PARTITION=$(IT_PARTITION) \ | ||
| gcr.io/$(GCP_PROJECT)/skaffold-builder \ | ||
| make integration-tests | ||
| $(IMAGE_REPO_BASE)/skaffold-builder \ | ||
| sh -c "gcloud auth configure-docker us-central1-docker.pkg.dev -q && make integration-tests" | ||
|
|
||
| .PHONY: submit-build-trigger | ||
| submit-build-trigger: | ||
| gcloud builds submit . \ | ||
| --project=$(GCP_PROJECT) \ | ||
| --config=deploy/cloudbuild.yaml \ | ||
| --substitutions="_RELEASE_BUCKET=$(RELEASE_BUCKET),COMMIT_SHA=$(COMMIT)" | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| FROM golang:1.19 as builder | ||
| FROM mirror.gcr.io/library/golang:1.23 as builder | ||
|
|
||
| WORKDIR /workspace | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new logic for
GCP_PROJECT=skaffold-ci-cdrelies onAR_REGIONandGKE_REGIONvariables. However, these variables are not defined with default values, nor are there any checks to ensure they are set. If they are not provided in the environment, they will be empty, leading to malformed values forIMAGE_REPO_BASEandGKE_LOCATION_FLAG, which can cause silent failures or hard-to-debug issues. It's safer to add checks to ensure these required variables are set.