fix: ensure plugin removal during Helm uninstall (#111)

* feat: optimize Dockerfile Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com> * feat: ensure plugin removal during helm uninstall Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com> * test: simplify redundant Reconcile logic; add edge case test for coverage Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com> --------- Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
validator-labs · Nov 16, 2023 · 0917418 · 0917418
1 parent 855e70e
commit 0917418
Show file tree

Hide file tree

Showing 5 changed files with 134 additions and 26 deletions.
diff --git a/Dockerfile b/Dockerfile
@@ -6,6 +6,11 @@ ARG TARGETARCH
 RUN apk add --no-cache curl
 
 WORKDIR /workspace
+
+# Get Helm
+RUN curl -s https://get.helm.sh/helm-v3.10.1-linux-amd64.tar.gz | tar -xzf - && \
+    mv linux-amd64/helm . && rm -rf linux-amd64
+
 # Copy the Go Modules manifests
 COPY go.mod go.mod
 COPY go.sum go.sum
@@ -19,10 +24,6 @@ COPY api/ api/
 COPY internal/ internal/
 COPY pkg/ pkg/
 
-# Get Helm
-RUN curl -s https://get.helm.sh/helm-v3.10.1-linux-amd64.tar.gz | tar -xzf - && \
-    mv linux-amd64/helm . && rm -rf linux-amd64
-
 # Build
 # the GOARCH has not a default value to allow the binary be built according to the host where the command
 # was called. For example, if we call make docker-build in a local env which has the Apple Silicon M1 SO

diff --git a/chart/validator/templates/cleanup.yaml b/chart/validator/templates/cleanup.yaml
@@ -0,0 +1,115 @@
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: spectro-cleanup-role
+rules:
+- apiGroups:
+  - ""
+  resources:
+  - configmaps
+  - serviceaccounts
+  verbs:
+  - '*'
+- apiGroups:
+  - batch
+  resources:
+  - jobs
+  verbs:
+  - '*'
+- apiGroups:
+  - rbac.authorization.k8s.io
+  resources:
+  - rolebindings
+  - roles
+  verbs:
+  - '*'
+- apiGroups:
+  - validation.spectrocloud.labs
+  resources:
+  - '*'
+  verbs:
+  - '*'
+---
+kind: RoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: spectro-cleanup-rolebinding
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: spectro-cleanup-role
+subjects:
+  - kind: ServiceAccount
+    name: spectro-cleanup
+    namespace: {{ .Release.Namespace }}
+---
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: spectro-cleanup
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: validator-cleanup-config
+data:
+  resource-config.json: |-
+    [
+      {
+        "group": "validation.spectrocloud.labs",
+        "version": "v1alpha1",
+        "resource": "validatorconfigs",
+        "name": "validator-config",
+        "namespace": "{{ .Release.Namespace }}"
+      },
+      {
+        "group": "",
+        "version": "v1",
+        "resource": "configmaps",
+        "name": "validator-cleanup-config",
+        "namespace": "{{ .Release.Namespace }}"
+      },
+      {
+        "group": "batch",
+        "version": "v1",
+        "resource": "jobs",
+        "name": "validator-cleanup",
+        "namespace": "{{ .Release.Namespace }}"
+      }
+    ]
+---
+apiVersion: batch/v1
+kind: Job
+metadata:
+  name: validator-cleanup
+  annotations:
+    "helm.sh/hook": pre-delete
+spec:
+  template:
+    spec:
+      restartPolicy: Never
+      serviceAccountName: spectro-cleanup
+      containers:
+      - name: validator-cleanup
+        image: gcr.io/spectro-images-public/release/spectro-cleanup:1.1.0
+        command: ["/cleanup"]
+        env:
+        - name: CLEANUP_DELAY_SECONDS
+          value: "10"
+        resources:
+          requests:
+            cpu: "10m"
+            memory: "25Mi"
+          limits:
+            cpu: "100m"
+            memory: "50Mi"
+        volumeMounts:
+        - name: validator-cleanup-config
+          mountPath: /tmp/spectro-cleanup
+      volumes:
+        - name: validator-cleanup-config
+          configMap:
+            name: validator-cleanup-config
+            items:
+            - key: resource-config.json
+              path: resource-config.json
diff --git a/internal/controller/validationresult_controller.go b/internal/controller/validationresult_controller.go
@@ -22,7 +22,6 @@ import (
 
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
-	apierrs "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -65,23 +64,15 @@ func (r *ValidationResultReconciler) Reconcile(ctx context.Context, req ctrl.Req
 	vc := &v1alpha1.ValidatorConfig{}
 	vcKey := types.NamespacedName{Namespace: r.Namespace, Name: constants.ValidatorConfig}
 	if err := r.Get(ctx, vcKey, vc); err != nil {
-		// ignore not-found errors, since they can't be fixed by an immediate requeue
-		if apierrs.IsNotFound(err) {
-			return ctrl.Result{}, nil
-		}
-		r.Log.Error(err, "failed to fetch ValidatorConfig")
+		r.Log.Error(err, "failed to fetch ValidatorConfig", "key", vcKey)
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
 	vr = &v1alpha1.ValidationResult{}
 	vrKey = req.NamespacedName
 
 	if err := r.Get(ctx, req.NamespacedName, vr); err != nil {
-		// ignore not-found errors, since they can't be fixed by an immediate requeue
-		if apierrs.IsNotFound(err) {
-			return ctrl.Result{}, nil
-		}
-		r.Log.Error(err, "failed to fetch ValidationResult")
+		r.Log.Error(err, "failed to fetch ValidationResult", "key", req)
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 

diff --git a/internal/controller/validationresult_controller_test.go b/internal/controller/validationresult_controller_test.go
@@ -17,13 +17,9 @@ import (
 	//+kubebuilder:scaffold:imports
 )
 
-const (
-	validationResultName = "validator-plugin-aws-service-quota"
-)
+const validationResultName = "validator-plugin-aws-service-quota"
 
-var (
-	vrServiceQuota = filepath.Join("testdata", "vr-aws-service-quota.yaml")
-)
+var vrServiceQuota = filepath.Join("testdata", "vr-aws-service-quota.yaml")
 
 var _ = Describe("ValidationResult controller", Ordered, func() {
 
@@ -59,6 +55,15 @@ var _ = Describe("ValidationResult controller", Ordered, func() {
 	}
 	vrKey := types.NamespacedName{Name: validationResultName, Namespace: validatorNamespace}
 
+	It("Should create a ValidationResult before the ValidatorConfig is created and fail initially", func() {
+		By("By creating a ValidationResult before creating the ValidatorConfig")
+		ctx := context.Background()
+
+		vrPre := vr.DeepCopy()
+		vrPre.Name = "vr-copy"
+		Expect(k8sClient.Create(ctx, vrPre)).Should(Succeed())
+	})
+
 	It("Should hash the ValidationResult and update its Status once a ValidationResult is created", func() {
 		By("By creating a new ValidationResult")
 		ctx := context.Background()

diff --git a/internal/controller/validatorconfig_controller.go b/internal/controller/validatorconfig_controller.go
@@ -64,11 +64,7 @@ func (r *ValidatorConfigReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 
 	vc := &v1alpha1.ValidatorConfig{}
 	if err := r.Get(ctx, req.NamespacedName, vc); err != nil {
-		// ignore not-found errors, since they can't be fixed by an immediate requeue
-		if apierrs.IsNotFound(err) {
-			return ctrl.Result{}, nil
-		}
-		r.Log.Error(err, "failed to fetch ValidatorConfig")
+		r.Log.Error(err, "failed to fetch ValidatorConfig", "key", req)
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}