Merge pull request #157 from red-hat-storage/sync_us--main

Syncing latest changes from upstream main for ramen
red-hat-storage · Dec 12, 2023 · 169e764 · 169e764
2 parents 2bedeb0 + fe846a6
commit 169e764
Show file tree

Hide file tree

Showing 65 changed files with 957 additions and 401 deletions.
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
@@ -27,6 +27,7 @@ env:
   IMAGE_REGISTRY: "quay.io"
   IMAGE_TAG: "ci"
   DOCKERCMD: "podman"
+  DRIVER: "container"
 defaults:
   run:
     shell: bash
@@ -79,7 +80,7 @@ jobs:
       - name: GolangCI Lint
         uses: golangci/golangci-lint-action@v3
         with:
-          version: v1.49.0
+          version: v1.55.2
 
   unit-test:
     name: Unit tests
@@ -116,7 +117,6 @@ jobs:
           curl -LO https://storage.googleapis.com/minikube/releases/latest/minikube-linux-amd64
           sudo install minikube-linux-amd64 /usr/local/bin/minikube
           minikube version
-          mkdir "$HOME/.minikube/profiles"
 
       - name: Install kubectl
         run: |
@@ -145,6 +145,10 @@ jobs:
         run: make black
         working-directory: test
 
+      - name: Start test cluster
+        run: make cluster
+        working-directory: test
+
       - name: Run tests
         run: make test
         working-directory: test
@@ -153,6 +157,10 @@ jobs:
         run: make coverage
         working-directory: test
 
+      - name: Clean up
+        run: make clean
+        working-directory: test
+
   ramenctl-test:
     name: ramenctl tests
     runs-on: ubuntu-22.04

diff --git a/.gitignore b/.gitignore
@@ -4,8 +4,8 @@
 *.dll
 *.so
 *.dylib
-bin
-testbin/*
+/bin
+/testbin/*
 
 # Test binary, build with `go test -c`
 *.test
@@ -37,9 +37,14 @@ venv
 # Test enviromemnt generated files
 test/.coverage
 test/.coverage.*
+test/addons/kubevirt/vm/id_rsa.pub
 
 # Python generated files
 *.egg-info/
 __pycache__/
 test/build
 ramenctl/build
+
+# Generated disk images
+*.qcow2
+*.iso
diff --git a/.golangci.yaml b/.golangci.yaml
@@ -33,34 +33,162 @@ linters-settings:
   wsl:
     allow-trailing-comment: true
     enforce-err-cuddling: true
+  revive:
+    ignore-generated-header: false
+    severity: error
+    rules:
+      - name: blank-imports
+      - name: context-as-argument
+      - name: context-keys-type
+      - name: dot-imports
+      - name: error-return
+      - name: error-strings
+      - name: error-naming
+      - name: exported
+      - name: if-return
+      - name: increment-decrement
+      - name: var-naming
+      - name: var-declaration
+      - name: package-comments
+      - name: range
+      - name: receiver-naming
+      - name: time-naming
+      - name: unexported-return
+      - name: indent-error-flow
+      - name: errorf
+      - name: empty-block
+      - name: superfluous-else
+      - name: unreachable-code
+      - name: redefines-builtin-id
+      - name: atomic
+      - name: constant-logical-expr
+      - name: unnecessary-stmt
+      - name: get-return
+      - name: modifies-parameter
+      - name: modifies-value-receiver
+      - name: range-val-in-closure
+      - name: waitgroup-by-value
+      - name: call-to-gc
+      - name: duplicated-imports
+      - name: unhandled-error
+      # - name: flag-parameter
+      # - name: unused-receiver
+      # - name: unused-parameter
+      # - name: confusing-naming
+      # - name: import-shadowing
+      # - name: confusing-results
+      # - name: bool-literal-in-expr
+
+issues:
+  exclude-rules:
+    # Allow dot imports for ginkgo and gomega
+    - source: ginkgo|gomega
+      linters:
+        - revive
+      text: "should not use dot imports"
+    - source: "^func Test"
+      linters:
+        - funlen
+
 
 linters:
-  enable-all: true
-  disable:
-    - exhaustivestruct
-    - gochecknoglobals
-    - gochecknoinits
-    - godot
-    - godox
-    - paralleltest
-    - goerr113  # TODO: Need to introduce error definition and bring this back
-    - goheader  # TODO: Introduce back post fixing linter errors
-    - gci
-    - interfacer  # interfacer linter is archived and deprecated (https://github.com/mvdan/interfacer)
-    # TODO: fix folloing linter errors.
-    - exhaustruct
-    - tagliatelle
-    - gomoddirectives
-    - goimports
-    - wrapcheck
-    - varnamelen
-    - staticcheck
-    - nosnakecase
-    - ireturn
-    - nilnil
-    - containedctx
-    - nonamedreturns
-    - forcetypeassert
-    - promlinter
-    - contextcheck
-    - errname
+  disable-all: true
+  enable:
+    - asasalint
+    - asciicheck
+    - bidichk
+    - bodyclose
+    - cyclop
+    - deadcode
+    - decorder
+    #  - depguard
+    - dogsled
+    - dupl
+    - durationcheck
+    - errcheck
+    - errchkjson
+    #  - errorlint
+    - execinquery
+    - exhaustive
+    - exportloopref
+    - forbidigo
+    - funlen
+    - gocognit
+    #  - goconst
+    - gocritic
+    - gocyclo
+    - gofmt
+    - gofumpt
+    - golint
+    - gomnd
+    - gomodguard
+    - goprintffuncname
+    - gosec
+    - gosimple
+    - govet
+    - grouper
+    - ifshort
+    - importas
+    - ineffassign
+    - interfacebloat
+    - lll
+    - logrlint
+    - maintidx
+    - makezero
+    - maligned
+    - misspell
+    #  - nakedret
+    - nestif
+    - nilerr
+    - nlreturn
+    - noctx
+    #  - nolintlint
+    - nosprintfhostport
+    - prealloc
+    - predeclared
+    - reassign
+    - revive
+    - rowserrcheck
+    - scopelint
+    - sqlclosecheck
+    - structcheck
+    - stylecheck
+    - tenv
+    - testpackage
+    - thelper
+    - tparallel
+    - typecheck
+    - unconvert
+    - unparam
+    - unused
+    - usestdlibvars
+    - varcheck
+    #  - wastedassign
+    - whitespace
+    - wsl
+    #  - exhaustivestruct
+    #  - gochecknoglobals
+    #  - gochecknoinits
+    #  - godot
+    #  - godox
+    #  - paralleltest
+    #  - goerr113  # TODO: Need to introduce error definition and bring this back
+    #  - goheader  # TODO: Introduce back post fixing linter errors
+    #  - gci
+    #  - interfacer  # interfacer linter is archived and deprecated (https://github.com/mvdan/interfacer)
+    #  - exhaustruct
+    #  - tagliatelle
+    #  - gomoddirectives
+    #  - goimports
+    #  - wrapcheck
+    #  - varnamelen
+    #  - staticcheck
+    #  - nosnakecase
+    #  - ireturn
+    #  - nilnil
+    #  - containedctx
+    #  - nonamedreturns
+    #  - forcetypeassert
+    #  - promlinter
+    #  - contextcheck
+    #  - errname
diff --git a/Makefile b/Makefile
@@ -124,17 +124,9 @@ manifests: controller-gen ## Generate WebhookConfiguration, ClusterRole and Cust
 generate: controller-gen ## Generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations.
 	$(CONTROLLER_GEN) object:headerFile="hack/boilerplate.go.txt" paths="./..."
 
-GOLANGCI_URL := https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh
-GOLANGCI_VERSION := 1.49.0
-GOLANGCI_INSTALLED_VER := $(shell testbin/golangci-lint version --format=short 2>&1)
 .PHONY: golangci-bin
-golangci-bin: ## Download and install goloanci-lint locally if necessary.
-ifeq (,$(GOLANGCI_INSTALLED_VER))
-	$(info Installing golangci-lint (version: $(GOLANGCI_VERSION)) into testbin)
-	curl -sSfL $(GOLANGCI_URL) | sh -s -- -b testbin v$(GOLANGCI_VERSION)
-else ifneq ($(GOLANGCI_VERSION),$(GOLANGCI_INSTALLED_VER))
-	$(error Incorrect version ($(GOLANGCI_INSTALLED_VER)) for golangci-lint found, expecting $(GOLANGCI_VERSION))
-endif
+golangci-bin:
+	hack/install-golangci-lint.sh
 
 .PHONY: lint
 lint: golangci-bin ## Run configured golangci-lint and pre-commit.sh linters against the code.
@@ -415,15 +407,15 @@ catalog-push: ## Push a catalog image.
 .PHONY: docker-buildx
 docker-buildx: # Build and push docker image for the manager for cross-platform support
 ifeq ($(DOCKERCMD),docker)
-	# copy existing Dockerfile and insert --platform=${BUILDPLATFORM} and 
+	# copy existing Dockerfile and insert --platform=${BUILDPLATFORM} and
 	# replace GOARCH value to ${TARGETARCH} into Dockerfile.cross, and preserve the original Dockerfile
 	$(eval PLATFORMS="linux/arm64,linux/amd64,linux/s390x,linux/ppc64le")
 	$(SED_CMD) \
 		-e '1 s/\(^FROM\)/FROM --platform=\$$\{BUILDPLATFORM\}/; t' \
 		-e ' 1,// s//FROM --platform=\$$\{BUILDPLATFORM\}/' \
 		Dockerfile > Dockerfile.cross
 	$(SED_CMD) -e 's/GOARCH=amd64/GOARCH=$${TARGETARCH}/' -i Dockerfile.cross
-	- $(DOCKERCMD) buildx create --name $(IMAGE_NAME)-builder --bootstrap --use 
+	- $(DOCKERCMD) buildx create --name $(IMAGE_NAME)-builder --bootstrap --use
 	- $(DOCKERCMD) buildx build --push --platform="${PLATFORMS}" --tag ${IMG} -f Dockerfile.cross .
 	- $(DOCKERCMD) buildx rm $(IMAGE_NAME)-builder
 	rm Dockerfile.cross

diff --git a/controllers/drplacementcontrol.go b/controllers/drplacementcontrol.go
@@ -535,20 +535,23 @@ func (d *DRPCInstance) checkMetroFailoverPrerequisites(curHomeCluster string) (b
 func (d *DRPCInstance) checkRegionalFailoverPrerequisites() bool {
 	d.setProgression(rmn.ProgressionWaitForStorageMaintenanceActivation)
 
-	if required, activationsRequired := requiresRegionalFailoverPrerequisites(
-		d.ctx,
-		d.reconciler.APIReader,
-		rmnutil.DRPolicyS3Profiles(d.drPolicy, d.drClusters).List(),
-		d.instance.GetName(), d.instance.GetNamespace(),
-		d.vrgs, d.instance.Spec.FailoverCluster,
-		d.reconciler.ObjStoreGetter, d.log); required {
-		for _, drCluster := range d.drClusters {
-			if drCluster.Name != d.instance.Spec.FailoverCluster {
-				continue
-			}
+	for _, drCluster := range d.drClusters {
+		if drCluster.Name != d.instance.Spec.FailoverCluster {
+			continue
+		}
 
+		// we want to work with failover cluster only, because the previous primary cluster might be unreachable
+		if required, activationsRequired := requiresRegionalFailoverPrerequisites(
+			d.ctx,
+			d.reconciler.APIReader,
+			[]string{drCluster.Spec.S3ProfileName},
+			d.instance.GetName(), d.instance.GetNamespace(),
+			d.vrgs, d.instance.Spec.FailoverCluster,
+			d.reconciler.ObjStoreGetter, d.log); required {
 			return checkFailoverMaintenanceActivations(drCluster, activationsRequired, d.log)
 		}
+
+		break
 	}
 
 	return true
@@ -1204,7 +1207,7 @@ func (d *DRPCInstance) switchToCluster(targetCluster, targetClusterNamespace str
 		vrgState = vrg.Status.State
 	}
 
-	d.log.Info(fmt.Sprintf("PVs/PVCs have been Restored? %v and VRG Primary %v", restored, vrgState))
+	d.log.Info(fmt.Sprintf("PVs/PVCs have been Restored? '%v' and VRG Primary='%v'", restored, vrgState))
 
 	if !restored || vrg.Status.State != rmn.PrimaryState {
 		d.setProgression(rmn.ProgressionWaitingForResourceRestore)
@@ -1388,9 +1391,11 @@ func (d *DRPCInstance) updateUserPlacementRule(homeCluster, reason string) error
 	d.log.Info(fmt.Sprintf("Updating user Placement %s homeCluster %s",
 		d.userPlacement.GetName(), homeCluster))
 
-	err := d.addAnnotation(LastAppDeploymentCluster, homeCluster)
-	if err != nil {
-		return err
+	added := rmnutil.AddAnnotation(d.instance, LastAppDeploymentCluster, homeCluster)
+	if added {
+		if err := d.reconciler.Update(d.ctx, d.instance); err != nil {
+			return err
+		}
 	}
 
 	newPD := &clrapiv1beta1.ClusterDecision{
@@ -1401,19 +1406,6 @@ func (d *DRPCInstance) updateUserPlacementRule(homeCluster, reason string) error
 	return d.reconciler.updateUserPlacementStatusDecision(d.ctx, d.userPlacement, newPD)
 }
 
-func (d *DRPCInstance) addAnnotation(key, value string) error {
-	annotations := d.instance.GetAnnotations()
-
-	if annotations == nil {
-		annotations = map[string]string{}
-	}
-
-	annotations[key] = value
-	d.instance.SetAnnotations(annotations)
-
-	return d.reconciler.Update(d.ctx, d.instance)
-}
-
 func (d *DRPCInstance) clearUserPlacementRuleStatus() error {
 	d.log.Info("Clearing user Placement", "name", d.userPlacement.GetName())
 
@@ -1748,7 +1740,7 @@ func (d *DRPCInstance) cleanupForVolSync(clusterToSkip string) error {
 
 			// Recreate the VRG ManifestWork for the secondary. This typically happens during Hub Recovery.
 			if errors.IsNotFound(err) {
-				err := d.createVolSyncDestManifestWork(clusterName)
+				err := d.createVolSyncDestManifestWork(clusterToSkip)
 				if err != nil {
 					return err
 				}
@@ -1998,7 +1990,7 @@ func (d *DRPCInstance) ensureVRGDeleted(clusterName string) bool {
 }
 
 func (d *DRPCInstance) updateVRGState(clusterName string, state rmn.ReplicationState) (bool, error) {
-	d.log.Info(fmt.Sprintf("Updating VRG ReplicationState to secondary for cluster %s", clusterName))
+	d.log.Info(fmt.Sprintf("Updating VRG ReplicationState to %s for cluster %s", state, clusterName))
 
 	vrg, err := d.getVRGFromManifestWork(clusterName)
 	if err != nil {