Merge pull request #250 from microsoft/user/pavanneerudu/azurestackhc…

…imachine-del-bugfix AzureStackHCIVirtualMachine Deletion Bug fix, tests, mocks and fixes for issues including linting, codeql, tsa, etc
microsoft · Jan 25, 2024 · 8034ac1 · 8034ac1
2 parents 482ec88 + 8cc25f8
commit 8034ac1
Show file tree

Hide file tree

Showing 53 changed files with 918 additions and 2,456 deletions.
diff --git a/.gdn/.gdntsa b/.gdn/.gdntsa
@@ -10,7 +10,7 @@
     ],
     "instanceUrl": "https://microsoft.visualstudio.com/",
     "projectName": "OS",
-    "areaPath": "OS\\Core\\STACK\\CoreNet\\MSK8S",
+    "areaPath": "OS\\Core\\Edge2Cloud\\Kubernetes",
     "iterationPath": "OS\\Future",
     "allTools": true
 }
diff --git a/.gitignore b/.gitignore
@@ -42,7 +42,7 @@ kubeconfig
 
 # Ignore output manifests
 config/samples
-manager_image_patch.yaml-e
+config/default/manager_image_patch.yaml
 
 # Bazel
 bazel-*
@@ -53,9 +53,11 @@ bazel-*
 # e2e output
 test/e2e/junit.e2e_suite.*.xml
 test/e2e/logs/*
+test/mocks/gomock*
+
 
 # generated template yamls
 templates/cluster-template*.yaml
 
 # Hack files
-/hack/config/manager_image_patch.yaml
+/hack/config/manager_image_patch_template.yaml
diff --git a/.pipelines/build.yaml b/.pipelines/build.yaml
@@ -13,6 +13,7 @@ jobs:
 
     steps:
       - checkout: self
+
       - task: DownloadBuildArtifacts@0
         inputs:
           buildType: "specific"
@@ -22,28 +23,56 @@ jobs:
           branchName: "refs/heads/master"
           downloadType: "specific"
           downloadPath: "$(System.DefaultWorkingDirectory)"
+
       - task: InstallSSHKey@0
         inputs:
           knownHostsEntry: "$(KNOWN_HOST)"
           sshPublicKey: "$(SSH_PUBLIC_KEY)"
           sshKeySecureFile: "azure-pipelines-ssh-key-new"
+        displayName: "Install SSH Key🔑"
+
+      - script: |
+          git config --global url.ssh://[email protected]/.insteadOf https://github.com/
+          go env -w GOPRIVATE="github.com/microsoft"
+        displayName: "Configure git to use SSH ⚙️" 
+
       - task: DockerInstaller@0
         inputs:
           dockerVersion: "17.09.0-ce"
+
       - task: Docker@2
         displayName: Login to ACR
         inputs:
           command: login
           containerRegistry: mocimages-connection
+
       - task: GoTool@0
         inputs:
           version: "1.19.9"
+        displayName: "Install Go"
+
+      - script: |-
+          set -xe
+          make test
+        workingDirectory: "$(System.DefaultWorkingDirectory)"
+        displayName: "Run Unit Tests🧪"
+
+      - script: |-
+          [[ -z "$(git diff --cached)" ]] || {
+            echo "Changed files:";
+            git diff --cached --name-only;
+            git diff --cached --color | cat;
+            echo "Source changes detected after running make. Make sure to build both the product and test code locally (and run unit tests) before pushing changes";
+            exit 1;
+          }
+        workingDirectory: "$(System.DefaultWorkingDirectory)"
+        displayName: "Check for uncommitted changes"
 
       - script: |
           set -e
           set -x
           mkdir manifest
-          git config --global url.ssh://[email protected]/.insteadOf https://github.com/
+          
           chmod +x hack/ensure-kustomize.sh
           chmod +x hack/gen-flavors.sh
           chmod +x hack/for-pipeline.sh
@@ -81,7 +110,7 @@ jobs:
           mkdir -p out/
           cp deployment/cloud-operator.yaml out/cloud-operator.yaml
         workingDirectory: "$(System.DefaultWorkingDirectory)"
-        displayName: "Build CAPH"
+        displayName: "Build CAPH🚀"
 
       - task: AzureArtifacts.manifest-generator-task.manifest-generator-task.ManifestGeneratorTask@0
         displayName: "SBOM Generation"
@@ -95,4 +124,4 @@ jobs:
         artifact: templates
 
       - publish: $(System.DefaultWorkingDirectory)/manifest
-        artifact: manifest
+        artifact: manifest
diff --git a/Dockerfile b/Dockerfile
@@ -13,42 +13,11 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# Build the manager binary
-#FROM golang:1.12.9 as builder
-# WORKDIR /workspace
-
-# Run this with docker build --build_arg $(go env GOPROXY) to override the goproxy
-#ARG goproxy=https://proxy.golang.org
-#ENV GOPROXY=$goproxy
-
-#ENV GOPRIVATE="github.com/microsoft"
-#RUN go env GOPRIVATE=github.com/microsoft
-
-# 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 sources
-#COPY ./ ./
-#COPY ./bin/manager ./
-
-# Build
-#ARG ARCH
-#RUN CGO_ENABLED=0 GOOS=linux GOARCH=${ARCH} GO111MODULE=on  \
-#    go build -a -ldflags '-extldflags "-static"' \
-#    -o manager .
-
-
-# NOTE: Approach above is not used while we still have a couple of private git repo's.
-# Can be uncommented later.
-
-# Copy the controller-manager into a thin image
-#FROM alpine:3.11
-FROM gcr.io/distroless/static:latest
+# Use distroless as minimal base image to package the manager binary
+FROM  mcr.microsoft.com/cbl-mariner/distroless/debug:2.0
 WORKDIR /
+
 COPY bin/manager ./
-USER nobody
+USER 65532:65532
+
 ENTRYPOINT ["/manager"]
diff --git a/Makefile b/Makefile
@@ -58,6 +58,7 @@ ROOT_DIR:=$(shell dirname $(realpath $(firstword $(MAKEFILE_LIST))))
 TOOLS_DIR := hack/tools
 TOOLS_BIN_DIR := $(abspath $(TOOLS_DIR)/bin)
 BIN_DIR := bin
+export PATH := $(TOOLS_BIN_DIR):$(PATH)
 
 # set --output-base used for conversion-gen which needs to be different for in GOPATH and outside GOPATH dev
 OUTPUT_BASE := --output-base=$(ROOT_DIR)
@@ -66,8 +67,6 @@ OUTPUT_BASE := --output-base=$(ROOT_DIR)
 CAPI_VERSION := v1.4.2
 
 # Binaries.
-KUBE_APISERVER=$(TOOLS_BIN_DIR)/kube-apiserver
-ETCD=$(TOOLS_BIN_DIR)/etcd
 GO_INSTALL = ./scripts/go_install.sh
 
 # Binaries.
@@ -87,11 +86,11 @@ ENVSUBST_VER := v2.0.0-20210730161058-179042472c46
 ENVSUBST_BIN := envsubst
 ENVSUBST := $(TOOLS_BIN_DIR)/$(ENVSUBST_BIN)-$(ENVSUBST_VER)
 
-GOLANGCI_LINT_VER := v1.48.0
+GOLANGCI_LINT_VER := v1.54.1
 GOLANGCI_LINT_BIN := golangci-lint
 GOLANGCI_LINT := $(TOOLS_BIN_DIR)/$(GOLANGCI_LINT_BIN)-$(GOLANGCI_LINT_VER)
 
-KUSTOMIZE_VER := v4.5.2
+KUSTOMIZE_VER := v5.0.3
 KUSTOMIZE_BIN := kustomize
 KUSTOMIZE := $(TOOLS_BIN_DIR)/$(KUSTOMIZE_BIN)-$(KUSTOMIZE_VER)
 
@@ -107,14 +106,28 @@ GO_APIDIFF_VER := v0.6.0
 GO_APIDIFF_BIN := go-apidiff
 GO_APIDIFF := $(TOOLS_BIN_DIR)/$(GO_APIDIFF_BIN)
 
-GINKGO_VER := v2.9.7
+GINKGO_VER := v2.9.2
 GINKGO_BIN := ginkgo
 GINKGO := $(TOOLS_BIN_DIR)/$(GINKGO_BIN)-$(GINKGO_VER)
 
-KUBECTL_VER := v1.25.11
+KUBECTL_VER := v1.26.6
 KUBECTL_BIN := kubectl
 KUBECTL := $(TOOLS_BIN_DIR)/$(KUBECTL_BIN)-$(KUBECTL_VER)
 
+# ENVTEST is used for running controller tests.
+SETUP_ENVTEST_VER := 116a1b831fffe7ccc3c8145306c3e1a3b1b14ffa # Note: this matches the commit ID of the dependent controller-runtime module.
+SETUP_ENVTEST_BIN := setup-envtest
+SETUP_ENVTEST := $(abspath $(TOOLS_BIN_DIR)/$(SETUP_ENVTEST_BIN)-$(SETUP_ENVTEST_VER))
+
+#
+# Kubebuilder 
+#
+export KUBEBUILDER_ENVTEST_KUBERNETES_VERSION ?= 1.26.0
+export KUBEBUILDER_CONTROLPLANE_START_TIMEOUT ?= 60s
+export KUBEBUILDER_CONTROLPLANE_STOP_TIMEOUT ?= 60s
+
+KUBEBUILDER_ASSETS ?= $(shell $(SETUP_ENVTEST) use --use-env -p path $(KUBEBUILDER_ENVTEST_KUBERNETES_VERSION))
+
 # Version
 MAJOR_VER ?= 1
 MINOR_VER ?= 1
@@ -153,27 +166,10 @@ help:  ## Display this help
 ## Testing
 ## --------------------------------------
 
-test: export TEST_ASSET_KUBECTL = $(ROOT_DIR)/$(KUBECTL)
-test: export TEST_ASSET_KUBE_APISERVER = $(ROOT_DIR)/$(KUBE_APISERVER)
-test: export TEST_ASSET_ETCD = $(ROOT_DIR)/$(ETCD)
-
 .PHONY: test
-test: $(KUBECTL) $(KUBE_APISERVER) $(ETCD) generate lint ## Run tests
-	go test ./...
-
-
-.PHONY: test-integration
-test-integration: ## Run integration tests
-	go test -v -tags=integration ./test/integration/...
-
-.PHONY: test-e2e
-test-e2e: ## Run e2e tests
-	PULL_POLICY=IfNotPresent $(MAKE) docker-build
-	MANAGER_IMAGE=$(CONTROLLER_IMG)-$(ARCH):$(TAG) \
-	go test ./test/e2e -v -tags=e2e -ginkgo.v -ginkgo.trace -count=1 -timeout=90m
-
-$(KUBE_APISERVER) $(ETCD): ## install test asset kube-apiserver, etcd
-	source ./scripts/fetch_ext_bins.sh && fetch_tools
+test: generate lint fmt modules vet $(SETUP_ENVTEST) $(GINKGO) ## Run tests
+	KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" \
+	ginkgo -r -v -cover -coverprofile cover.out ./...
 
 ## --------------------------------------
 ## Binaries
@@ -182,8 +178,9 @@ $(KUBE_APISERVER) $(ETCD): ## install test asset kube-apiserver, etcd
 .PHONY: binaries
 binaries: manager ## Builds and installs all binaries
 
-.PHONY: manager
-manager: ## Build manager binary.
+ ## Build manager binary.
+.PHONY: manager 
+manager: generate lint fmt modules vet ## Build manager binary.
 	CGO_ENABLED=0 GOOS=linux go build -a -ldflags '-extldflags "-static"' -o bin/manager cmd/manager/main.go
 
 ## --------------------------------------
@@ -203,6 +200,7 @@ $(CLUSTERCTL_BIN): $(CLUSTERCTL)
 $(CONTROLLER_GEN): ## Build controller-gen from tools folder.
 	GOBIN=$(TOOLS_BIN_DIR) $(GO_INSTALL) sigs.k8s.io/controller-tools/cmd/controller-gen $(CONTROLLER_GEN_BIN) $(CONTROLLER_GEN_VER)
 
+
 $(CONVERSION_GEN): ## Build conversion-gen.
 	GOBIN=$(TOOLS_BIN_DIR) $(GO_INSTALL) k8s.io/code-generator/cmd/conversion-gen $(CONVERSION_GEN_BIN) $(CONVERSION_GEN_VER)
 
@@ -212,11 +210,21 @@ $(ENVSUBST): ## Build envsubst from tools folder.
 .PHONY: $(ENVSUBST_BIN)
 $(ENVSUBST_BIN): $(ENVSUBST)
 
+.PHONY: $(SETUP_ENVTEST_BIN)
+$(SETUP_ENVTEST_BIN): $(SETUP_ENVTEST) ## Build a local copy of setup-envtest.
+
+.PHONY: setup-envtest
+setup-envtest: $(SETUP_ENVTEST) ## Set up envtest (download kubebuilder assets)
+	@echo KUBEBUILDER_ASSETS=$(KUBEBUILDER_ASSETS)
+
+.PHONY: mockgen
+mockgen: $(MOCKGEN) ## Generate mocks using mockgen.
+
 $(GOLANGCI_LINT): ## Build golangci-lint from tools folder.
 	GOBIN=$(TOOLS_BIN_DIR) $(GO_INSTALL) github.com/golangci/golangci-lint/cmd/golangci-lint $(GOLANGCI_LINT_BIN) $(GOLANGCI_LINT_VER)
 
 $(KUSTOMIZE): ## Build kustomize from tools folder.
-	GOBIN=$(TOOLS_BIN_DIR) $(GO_INSTALL) sigs.k8s.io/kustomize/kustomize/v4 $(KUSTOMIZE_BIN) $(KUSTOMIZE_VER)
+	GOBIN=$(TOOLS_BIN_DIR) $(GO_INSTALL) sigs.k8s.io/kustomize/kustomize/v5 $(KUSTOMIZE_BIN) $(KUSTOMIZE_VER)
 
 $(MOCKGEN): ## Build mockgen from tools folder.
 	GOBIN=$(TOOLS_BIN_DIR) $(GO_INSTALL) github.com/golang/mock/mockgen $(MOCKGEN_BIN) $(MOCKGEN_VER)
@@ -228,7 +236,7 @@ $(GO_APIDIFF): ## Build go-apidiff.
 	GOBIN=$(TOOLS_BIN_DIR) $(GO_INSTALL) github.com/joelanford/go-apidiff $(GO_APIDIFF_BIN) $(GO_APIDIFF_VER)
 
 $(GINKGO): ## Build ginkgo.
-	GOBIN=$(TOOLS_BIN_DIR) $(GO_INSTALL) github.com/onsi/ginkgo/ginkgo $(GINKGO_BIN) $(GINKGO_VER)
+	GOBIN=$(TOOLS_BIN_DIR) $(GO_INSTALL) github.com/onsi/ginkgo/v2/ginkgo $(GINKGO_BIN) $(GINKGO_VER)
 
 $(KUBECTL): ## Get kubectl
 	mkdir -p $(TOOLS_BIN_DIR)
@@ -240,6 +248,10 @@ $(KUBECTL): ## Get kubectl
 .PHONY: $(KUBECTL_BIN)
 $(KUBECTL_BIN): $(KUBECTL)
 
+$(SETUP_ENVTEST): # Build setup-envtest.
+	GOBIN=$(TOOLS_BIN_DIR) $(GO_INSTALL) sigs.k8s.io/controller-runtime/tools/setup-envtest $(SETUP_ENVTEST_BIN) $(SETUP_ENVTEST_VER)
+	@echo "🛠  setup-envtest installed to $(TOOLS_BIN_DIR)/$(ENVTEST_BIN)"
+
 ## --------------------------------------
 ## Linting
 ## --------------------------------------
@@ -260,6 +272,15 @@ modules: ## Runs go mod to ensure proper vendoring.
 	go mod tidy
 	cd $(TOOLS_DIR); go mod tidy
 
+# Run go fmt against code
+.PHONY: fmt
+fmt: 
+	go fmt ./... 
+
+# Run go vet against code
+vet:
+	go vet ./...
+
 .PHONY: generate
 generate: ## Generate code
 	$(MAKE) generate-go
@@ -270,19 +291,13 @@ generate: ## Generate code
 generate-go: $(CONTROLLER_GEN) $(MOCKGEN) $(CONVERSION_GEN) ## Runs Go related generate targets
 	go generate ./...
 	$(CONTROLLER_GEN) \
-		paths=./api/... \
+		paths=./api/v1beta1 \
 		object:headerFile=./hack/boilerplate/boilerplate.generatego.txt
-
-	$(CONVERSION_GEN) \
-		--input-dirs=./api/v1alpha3 \
-		--input-dirs=./api/v1alpha4 \
-		--output-file-base=zz_generated.conversion $(OUTPUT_BASE) \
-		--go-header-file=./hack/boilerplate/boilerplate.generatego.txt
 
 .PHONY: generate-manifests
 generate-manifests: $(CONTROLLER_GEN) ## Generate manifests e.g. CRD, RBAC etc.
 	$(CONTROLLER_GEN) \
-		paths=./api/... \
+		paths=./api/v1beta1 \
 		crd:crdVersions=v1 \
 		rbac:roleName=manager-role \
 		output:crd:dir=$(CRD_ROOT) \
@@ -523,4 +538,4 @@ verify-modules: modules
 verify-gen: generate
 	@if !(git diff --quiet HEAD); then \
 		echo "generated files are out of date, run make generate"; exit 1; \
-	fi
+	fi
diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go
@@ -106,12 +106,14 @@ func (s Subnets) ToMap() map[string]*SubnetSpec {
 
 type IPAllocationMethod int32
 
+// nolint: golint
 const (
 	IPAllocationMethod_Invalid IPAllocationMethod = 0
 	IPAllocationMethod_Dynamic IPAllocationMethod = 1
 	IPAllocationMethod_Static  IPAllocationMethod = 2
 )
 
+// nolint: golint
 type IpConfigurationSpec struct {
 	Name string `json:"name,omitempty"`
 	// +optional
@@ -128,6 +130,8 @@ type IpConfigurationSpec struct {
 	// +optional
 	Gateway string `json:"gateway,omitempty"`
 }
+
+// nolint: golint
 type IpConfigurations []*IpConfigurationSpec
 
 type NetworkInterfaceSpec struct {

diff --git a/cloud/defaults.go b/cloud/defaults.go
@@ -59,6 +59,7 @@ const (
 	LatestVersion = "latest"
 )
 
+// nolint: golint
 const (
 	// Role is used to distinguish api server lb from other lbs.
 	// used by SDN integration