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

Jump to bottom

Bump kubebuilder version from v2 to v3 #40

Merged
merged 6 commits into from
Oct 29, 2021
Merged

Bump kubebuilder version from v2 to v3 #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3680ffe
Bump `kubebuilder` version from `v2` to `v3`
rustycl0ck Sep 7, 2021
abc08ba
Fixes for code review
rustycl0ck Sep 10, 2021
80d4bd9
Fix service account name in README
rustycl0ck Sep 13, 2021
6da13d4
Fix `.dockerignore` exclusion syntax
rustycl0ck Sep 22, 2021
cf93c7c
Add `kind` targets to Makefile
rustycl0ck Oct 21, 2021
fc6f132
Add migration warning for old verison
rustycl0ck Oct 20, 2021
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
24 changes: 15 additions & 9 deletions .dockerignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
bin/
config/
.git/
hack/
.gitignore
LICENSE
Makefile
PROJECT
README.md
Comment on lines -1 to -9
Copy link
Member

Choose a reason for hiding this comment

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

We do not need to remove them.

Copy link
Contributor Author

@rustycl0ck rustycl0ck Sep 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I just kept the default file in this case because I think it looked cleaner to have an allow-list instead have a block-list (block-list has to be updated whenever a new kind of file is added to the project). Let me know if you prefer to have both and I can add them back.

Copy link
Contributor

@tkuchiki tkuchiki Sep 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

The following configures seem to be correct.

$ cat .dockerignore
# More info: https://docs.docker.com/engine/reference/builder/#dockerignore-file
# Ignore all files
**

# Except go related files
!pkg
!cmd
!**/*.mod
!**/*.sum

You can check the following command.

$ docker image build -t build-context -f - . <<EOF
FROM busybox
COPY . /build-context
WORKDIR /build-context
CMD find .
EOF

$ docker container run --rm build-context
.
./go.sum
./cmd
./cmd/spanner-autoscaler
./cmd/spanner-autoscaler/main.go
./pkg
./pkg/pointer
./pkg/pointer/pointer.go
./pkg/pointer/pointer_test.go
./pkg/api
./pkg/api/v1alpha1
./pkg/api/v1alpha1/zz_generated.deepcopy.go
./pkg/api/v1alpha1/spannerautoscaler_types.go
./pkg/api/v1alpha1/groupversion_info.go
./pkg/metrics
./pkg/metrics/metrics.go
./pkg/metrics/fake.go
./pkg/controllers
./pkg/controllers/spannerautoscaler_controller.go
./pkg/controllers/spannerautoscaler_controller_test.go
./pkg/spanner
./pkg/spanner/spanner.go
./pkg/spanner/fake.go
./pkg/logging
./pkg/logging/context.go
./pkg/syncer
./pkg/syncer/fake
./pkg/syncer/fake/fake.go
./pkg/syncer/syncer_test.go
./pkg/syncer/syncer.go
./go.mod

!**/*.go doesn't work, so I choose !pkg and !cmd.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Excellent catch!! Thank you 🙇‍♂️ , I was not aware that this is an open bug in docker. I have now fixed it as per your recommendation since ! does not work with wild cards.

References:

# More info: https://docs.docker.com/engine/reference/builder/#dockerignore-file

# NOTE: The exclusion syntax using `!` does not work as expected
# Ref:
# - https://github.com/moby/moby/issues/30018
# - https://github.com/kubernetes-sigs/kubebuilder/issues/2073

# Ignore all files
**

# Only include Go files
!pkg
!cmd
!go.mod
!go.sum
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ vendor/
*.so
*.dylib
bin
testbin/*

# Test binary, build with `go test -c`
*.test
Expand Down
16 changes: 11 additions & 5 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,26 @@
# Build the manager binary
FROM golang:1.14.0-buster AS builder
FROM golang:1.16 AS builder

WORKDIR /workspace

# Copy the Go Modules manifests
COPY go.mod go.mod
COPY go.sum go.sum
# cache deps before building and copying source so that we don't need to re-download as much
# and so that source changes don't invalidate our downloaded layer
RUN go mod download

# Copy the go source
COPY pkg/ pkg/
COPY cmd/ cmd/
RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 GO111MODULE=on go build -a -o manager cmd/spanner-autoscaler/main.go

FROM gcr.io/distroless/static:nonroot
# Build
RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -a -o manager cmd/spanner-autoscaler/main.go

# Use distroless as minimal base image to package the manager binary
# Refer to https://github.com/GoogleContainerTools/distroless for more details
FROM gcr.io/distroless/static:nonroot
WORKDIR /
COPY --from=builder /workspace/manager .
USER nonroot:nonroot
USER 65532:65532
tkuchiki marked this conversation as resolved.
Show resolved Hide resolved

ENTRYPOINT ["/manager"]
222 changes: 110 additions & 112 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,132 +1,130 @@
GOOS := $(shell go env GOOS)
GOARCH := $(shell go env GOARCH)

# Image URL to use all building/pushing image targets
IMG ?= mercari/spanner-autoscaler:latest
rustycl0ck marked this conversation as resolved.
Show resolved Hide resolved
# Produce CRDs that work back to Kubernetes 1.11 (no version conversion)
CRD_OPTIONS ?= "crd:trivialVersions=true,preserveUnknownFields=false"

# Get the currently used golang install path (in GOPATH/bin, unless GOBIN is set)
ifeq (,$(shell go env GOBIN))
GOBIN=$(shell go env GOPATH)/bin
else
GOBIN=$(shell go env GOBIN)
endif

# Setting SHELL to bash allows bash commands to be executed by recipes.
# This is a requirement for 'setup-envtest.sh' in the test target.
# Options are set to exit when a recipe line exits non-zero or a piped command fails.
SHELL = /usr/bin/env bash -o pipefail
.SHELLFLAGS = -ec

all: build

##@ General

# The help target prints out all targets with their descriptions organized
# beneath their categories. The categories are represented by '##@' and the
# target descriptions by '##'. The awk commands is responsible for reading the
# entire set of makefiles included in this invocation, looking for lines of the
# file as xyz: ## something, and then pretty-format the target and help. Then,
# if there's a line with ##@ something, that gets pretty-printed as a category.
# More info on the usage of ANSI control characters for terminal formatting:
# https://en.wikipedia.org/wiki/ANSI_escape_code#SGR_parameters
# More info on the awk command:
# http://linuxcommand.org/lc3_adv_awk.php

KUBEBUILDER_VERSION ?= 2.1.0
help: ## Display this help.
@awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n make \033[36m<target>\033[0m\n"} /^[a-zA-Z_0-9-]+:.*?##/ { printf " \033[36m%-15s\033[0m %s\n", $$1, $$2 } /^##@/ { printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST)

BIN_DIR := $(shell pwd)/bin
##@ Development

KUBEBUILDER_ASSETS_DIR := ${BIN_DIR}/kubebuilder
KUBEBUILDER_ASSETS := ${KUBEBUILDER_ASSETS_DIR}/bin
manifests: controller-gen ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects.
$(CONTROLLER_GEN) $(CRD_OPTIONS) rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases

generate: controller-gen ## Generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations.
$(CONTROLLER_GEN) object:headerFile="hack/boilerplate.go.txt" paths="./..."

fmt: ## Run go fmt against code.
go fmt ./...

vet: ## Run go vet against code.
go vet ./...

CONTROLLER_GEN := bin/controller-gen
KIND := bin/kind
KUBECTL := bin/kubectl
SKAFFOLD := bin/skaffold
KPT := bin/kpt
ENVTEST_ASSETS_DIR=$(shell pwd)/testbin
test: manifests generate fmt vet ## Run tests.
mkdir -p ${ENVTEST_ASSETS_DIR}
test -f ${ENVTEST_ASSETS_DIR}/setup-envtest.sh || curl -sSLo ${ENVTEST_ASSETS_DIR}/setup-envtest.sh https://raw.githubusercontent.com/kubernetes-sigs/controller-runtime/v0.8.3/hack/setup-envtest.sh
source ${ENVTEST_ASSETS_DIR}/setup-envtest.sh; fetch_envtest_tools $(ENVTEST_ASSETS_DIR); setup_envtest_env $(ENVTEST_ASSETS_DIR); go test ./... -coverprofile cover.out

KIND_NODE_VERSION := 1.18.2
kind-cluster: kind-cluster-create ## Deploy RBAC resources to kindd cluster for development
kubectl apply -f kind
kubectl apply -f kind/rbac

GO_TEST ?= go test
KIND_CLUSTER_NAME = spanner-autoscaler
kind-cluster-create: kind ## Create a kind cluster for development
@if [ -z $(shell $(KIND) get clusters | grep $(KIND_CLUSTER_NAME)) ]; then \
$(KIND) create cluster --name $(KIND_CLUSTER_NAME); \
fi
kubectl config use-context kind-$(KIND_CLUSTER_NAME)

all: manager
kind-cluster-delete: kind ## Delete the kind cluster created for development
@$(KIND) delete cluster --name $(KIND_CLUSTER_NAME)

.PHONY: test
test: kubebuilder
@KUBEBUILDER_ASSETS=${KUBEBUILDER_ASSETS} $(GO_TEST) -v -race ./...
kind-cluster-reset: kind kind-cluster-delete kind-cluster-create kind-cluster ## Recreate the kind cluster for development

# Run tests
coverage: generate fmt vet manifests
@KUBEBUILDER_ASSETS=${KUBEBUILDER_ASSETS} $(GO_TEST) -v -race ./... -covermode=atomic -coverprofile=cover.out
##@ Build

# Build manager binary
manager: generate fmt vet
build: generate fmt vet ## Build manager binary.
go build -o bin/manager cmd/spanner-autoscaler/main.go

# Install CRDs into a cluster
install: manifests
kustomize build config/crd | kubectl apply -f -
run: manifests generate fmt vet ## Run a controller from your host.
go run ./main.go

# Deploy controller in the configured Kubernetes cluster in ~/.kube/config
deploy: manifests
cd config/manager && kustomize edit set image controller=${IMG}
kustomize build config/default | kubectl apply -f -
docker-build: test ## Build docker image with the manager.
docker build -t ${IMG} .

.PHONY: fmt
fmt: ## Run go fmt against code
go fmt ./...
docker-push: ## Push docker image with the manager.
docker push ${IMG}

.PHONY: vet
vet: ## Run go vet against code
go vet ./...
##@ Deployment

.PHONY: lint
lint: lint/golangci-lint ## Run all linters.
install: manifests kustomize ## Install CRDs into the K8s cluster specified in ~/.kube/config.
$(KUSTOMIZE) build config/crd | kubectl apply -f -

.PHONY: tools/golangci-lint
tools/golangci-lint: # go get 'golangci-lint' binary
ifeq (, $(shell which golangci-lint))
GO111MODULE=off go get -u github.com/golangci/golangci-lint/cmd/golangci-lint
endif
uninstall: manifests kustomize ## Uninstall CRDs from the K8s cluster specified in ~/.kube/config.
$(KUSTOMIZE) build config/crd | kubectl delete -f -

deploy: manifests kustomize ## Deploy controller to the K8s cluster specified in ~/.kube/config.
cd config/manager && $(KUSTOMIZE) edit set image controller=${IMG}
$(KUSTOMIZE) build config/default | kubectl apply -f -

undeploy: ## Undeploy controller from the K8s cluster specified in ~/.kube/config.
$(KUSTOMIZE) build config/default | kubectl delete -f -


CONTROLLER_GEN = $(shell pwd)/bin/controller-gen
controller-gen: ## Download controller-gen locally if necessary.
$(call go-get-tool,$(CONTROLLER_GEN),sigs.k8s.io/controller-tools/cmd/controller-gen@v0.4.1)

KUSTOMIZE = $(shell pwd)/bin/kustomize
kustomize: ## Download kustomize locally if necessary.
$(call go-get-tool,$(KUSTOMIZE),sigs.k8s.io/kustomize/kustomize/v3@v3.8.7)

KIND = $(shell pwd)/bin/kind
.PHONY: kind
kind: ## Downlaod 'kind' locally if necessary
$(call go-get-tool,$(KIND),sigs.k8s.io/kind@v0.11.1)


# go-get-tool will 'go get' any package $2 and install it to $1.
PROJECT_DIR := $(shell dirname $(abspath $(lastword $(MAKEFILE_LIST))))
define go-get-tool
@[ -f $(1) ] || { \
set -e ;\
TMP_DIR=$$(mktemp -d) ;\
cd $$TMP_DIR ;\
go mod init tmp ;\
echo "Downloading $(2)" ;\
GOBIN=$(PROJECT_DIR)/bin go get $(2) ;\
rm -rf $$TMP_DIR ;\
}
endef

.PHONY: lint/golangci-lint
lint/golangci-lint: tools/golangci-lint .golangci.yml ## Run golangci-lint.
golangci-lint run

# Build the docker image
docker-build:
docker image build . -t ${IMG}

# Push the docker image
docker-push:
docker image push ${IMG}

kubebuilder: ${KUBEBUILDER_ASSETS}/kubebuilder
${KUBEBUILDER_ASSETS}/kubebuilder:
@{ \
set -e ;\
TMP_DIR=./tmp/kubebuilder ;\
mkdir -p $$TMP_DIR ;\
curl -sSL https://github.com/kubernetes-sigs/kubebuilder/releases/download/v${KUBEBUILDER_VERSION}/kubebuilder_${KUBEBUILDER_VERSION}_${GOOS}_${GOARCH}.tar.gz | tar -xz -C $$TMP_DIR ;\
mv $$TMP_DIR/kubebuilder_${KUBEBUILDER_VERSION}_${GOOS}_${GOARCH} ${KUBEBUILDER_ASSETS_DIR} ;\
rm -rf $$TMP_DIR ;\
}

.PHONY: run
run: $(SKAFFOLD)
# NOTE: since skaffold uses kind executable from PATH directly, so this overrides PATH to use project local executable.
@PATH=$${PWD}/bin:$${PATH} $(SKAFFOLD) dev --filename=./skaffold/skaffold.yaml

.PHONY: manifests
manifests: $(CONTROLLER_GEN)
@$(CONTROLLER_GEN) crd:trivialVersions=true rbac:roleName=manager-role webhook paths="./pkg/api/..." output:crd:artifacts:config=config/crd/bases

.PHONY: deepcopy
deepcopy: $(CONTROLLER_GEN)
@$(CONTROLLER_GEN) object:headerFile=./hack/boilerplate.go.txt paths="./..."

controlller-gen: $(CONTROLLER_GEN)
$(CONTROLLER_GEN): go.sum
@go build -o $(CONTROLLER_GEN) sigs.k8s.io/controller-tools/cmd/controller-gen

kind: $(KIND)
$(KIND): go.sum
@go build -o ./bin/kind sigs.k8s.io/kind

kubectl: $(KUBECTL)
$(KUBECTL): .kubectl-version
@curl -Lso $(KUBECTL) https://storage.googleapis.com/kubernetes-release/release/$(shell cat .kubectl-version)/bin/$(GOOS)/$(GOARCH)/kubectl
@chmod +x $(KUBECTL)

skaffold: $(SKAFFOLD)
$(SKAFFOLD): .skaffold-version
@curl -Lso $(SKAFFOLD) https://storage.googleapis.com/skaffold/releases/$(shell cat .skaffold-version)/skaffold-$(GOOS)-$(GOARCH)
@chmod +x $(SKAFFOLD)

kpt: $(KPT)
$(KPT): go.sum
@go build -o $(KPT) github.com/GoogleContainerTools/kpt

.PHONY: kind-cluster
kind-cluster: $(KIND) $(KUBECTL)
@$(KIND) delete cluster --name spanner-autoscaler
@$(KIND) create cluster --name spanner-autoscaler --image kindest/node:v${KIND_NODE_VERSION}
@$(KUBECTL) apply -f ./config/crd/bases/spanner.mercari.com_spannerautoscalers.yaml
@$(KUBECTL) apply -f ./kind/namespace.yaml
@$(KUBECTL) apply -f ./kind/rbac

.PHONY: kpt/crd
kpt/crd: manifests kpt
@kustomize build config/crd > ./kpt/crd/spannerautoscalers.spanner.mercari.com.yaml
Comment on lines -64 to -132
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason why you removed these make tasks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ref
This is the current status of the Makefile:
Screen Shot 2021-10-13 at 18 25 01

  • The current kpt package was anyways broken (Upgrade and fix broken kpt package files #41), so I didn't bother on porting it over yet. I will fix it along with kpt upgrade in Kpt package upgrade #43.

  • As for others:

    • kubectl: I think we don't need to install it on the developer's system because this is a basic tool for interaction with k8s and is expected to be present already.
    • kind: Same as above. I think any k8s developer would already have access to some kind of development k8s environment (be it kind or any other k8s setup of their choice).
    • kubebuilder: Same as above. Anyone building or modifying operators, would already have it on their system I think. We shouldn't burden ourselves with maintaining more code/components than what is/are necessary.
    • skaffold: I'm not sure about this one. Is this really used by any of the current code owners for development? If so, then I can port it.

Let me know what you think and which (or all) targets should be ported over.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks!

  • kubectl
    • Yes, it may not be necessary.
  • kind
    • I agree that installation is not necessary. However, I think it would be more developer-friendly to not leave out make tasks that make it easier to build a local environment using kind.
  • kubebuilder
    • Yes, it may not be necessary.
  • skaffold
    • I have never used it. Does anyone use it? @mercari/spanner-autoscaler

The current kpt package was anyways broken (Upgrade and fix broken kpt package files #41), so I didn't bother on porting it over yet. I will fix it along with kpt upgrade in Kpt package upgrade #43.

OK!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added the kind target now. Please have a look. Thanks!

11 changes: 10 additions & 1 deletion PROJECT
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,16 @@
version: "2"
version: "3"
domain: mercari.com
layout:
- go.kubebuilder.io/v3
projectName: spanner-autoscaler
repo: github.com/mercari/spanner-autoscaler
resources:
- group: spanner
version: v1alpha1
kind: SpannerAutoscaler
controller: true
domain: mercari.com
path: github.com/mercari/spanner-autoscaler/api/v1alpha1
api:
crdVersion: v1
namespaced: true
17 changes: 12 additions & 5 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,13 @@ The installation has 3 steps:
$ make install
```

> :warning: **Migration from `v0.1.5`:** Names of some resources (`Deployment`, `serviceAccount`,`Role` etc) have changed since version `0.1.5`. Thus, you must first uninstall the old version before installing the new version. To uninstall the old version:
> ```console
> $ git checkout v0.1.5
> $ kustomize build config/default | kubectl delete -f -
> ```
> Specifically, the kubernetes service account used for running the spanner-autoscaler has changed from `default` to `spanner-autoscaler-controller-manager`. Please keep this in mind. It is recommended to follow the below configuration steps and re-create any resources if needed.

### 2. Deploy operator to cluster

```
Expand Down Expand Up @@ -94,17 +101,17 @@ You can configure the controller(`spanner-autoscaler-manager`) to use GKE Worklo

1. Make cluster [to use Workload Identity](https://cloud.google.com/kubernetes-engine/docs/how-to/workload-identity?hl=en#enable_on_cluster).
2. Create a GCP Service Account for the controller.
3. Configure Workload Identity between Kubernetes service account of the controller(`spanner-autoscaler/default`) and the GCP service account created in step 2.
3. Configure Workload Identity between Kubernetes service account of the controller(`spanner-autoscaler/spanner-autoscaler-controller-manager`) and the GCP service account created in step 2.
1. Allow Kubernetes service account to impersonate the GCP service account by creating an IAM Policy binding

```sh
$ gcloud iam service-accounts add-iam-policy-binding --role roles/iam.workloadIdentityUser --member "serviceAccount:PROJECT_ID.svc.id.goog[spanner-autoscaler/default]" GSA_NAME@PROJECT_ID.iam.gserviceaccount.com`
$ gcloud iam service-accounts add-iam-policy-binding --role roles/iam.workloadIdentityUser --member "serviceAccount:PROJECT_ID.svc.id.goog[spanner-autoscaler/spanner-autoscaler-controller-manager]" GSA_NAME@PROJECT_ID.iam.gserviceaccount.com`
```

2. Add annotation

```sh
$ kubectl annotate serviceaccount --namespace spanner-autoscaler default iam.gke.io/gcp-service-account=GSA_NAME@PROJECT_ID.iam.gserviceaccount.com`
$ kubectl annotate serviceaccount --namespace spanner-autoscaler spanner-autoscaler-controller-manager iam.gke.io/gcp-service-account=GSA_NAME@PROJECT_ID.iam.gserviceaccount.com`
```

## Configuration
Expand Down Expand Up @@ -164,7 +171,7 @@ There are possible choices of GCP Service Account configurations.
name: spanner-autoscaler-service-account-reader
subjects:
- kind: ServiceAccount
name: default
name: spanner-autoscaler-controller-manager
namespace: spanner-autoscaler
```

Expand Down Expand Up @@ -218,7 +225,7 @@ roleRef:
name: spanner-autoscaler-event-publisher
subjects:
- kind: ServiceAccount
name: default
name: spanner-autoscaler-controller-manager
namespace: spanner-autoscaler
```

Expand Down
Loading