add golangci-lint check

.golangci.yml

@@ -0,0 +1,40 @@
+version: "2"
+run:
+  timeout: 5m
+  allow-parallel-runners: true
+linters:
+  settings:
+    staticcheck:
+      dot-import-whitelist: # allow dot imports for the following packages
+        - github.com/onsi/gomega
+        - github.com/onsi/ginkgo/v2
+  default: none
+  enable:
+    - copyloopvar
+    - dupl
+    - errcheck
+    - goconst
+    - gocyclo
+    - govet
+    - ineffassign
+    - lll
+    - misspell
+    - nakedret
+    - prealloc
+    - staticcheck
+    - unconvert
+    - unparam
+    - unused
+  exclusions:
+    rules:
+      - path: "api/*"
+        linters:
+          - lll
+      - path: "internal/controller/pie/*"
+        linters:
+          - dupl
+          - lll
+formatters:
+  enable:
+    - gofmt
+    - goimports

Makefile

@@ -68,6 +68,14 @@ vet: ## Run go vet against code.
 test: manifests generate fmt vet ## Run tests.
 	PIE_ENVTEST_VERSION=$(ENVTEST_K8S_VERSION) PIE_ENVTEST_ASSETS_DIR=$(LOCALBIN) go test ./... -race -coverprofile cover.out 
 
+.PHONY: lint
+lint: golangci-lint ## Run golangci-lint.
+	$(GOLANGCI_LINT) run
+
+.PHONY: lint-fix
+lint-fix: golangci-lint ## Run golangci-lint and perform fixes.
+	$(GOLANGCI_LINT) run --fix
+
 ##@ Build
 
 .PHONY: build
@@ -152,6 +160,7 @@ $(LOCALBIN):
 KUBECTL ?= kubectl
 KUSTOMIZE ?= $(LOCALBIN)/kustomize
 CONTROLLER_GEN ?= $(LOCALBIN)/controller-gen
+GOLANGCI_LINT ?= $(LOCALBIN)/golangci-lint
 
 .PHONY: kustomize
 kustomize: $(KUSTOMIZE) ## Download kustomize locally if necessary. If wrong version is installed, it will be removed before downloading.
@@ -167,3 +176,9 @@ controller-gen: $(CONTROLLER_GEN) ## Download controller-gen locally if necessar
 $(CONTROLLER_GEN): $(LOCALBIN)
 	test -s $(LOCALBIN)/controller-gen && $(LOCALBIN)/controller-gen --version | grep -q $(CONTROLLER_TOOLS_VERSION) || \
 	GOBIN=$(LOCALBIN) go install sigs.k8s.io/controller-tools/cmd/controller-gen@$(CONTROLLER_TOOLS_VERSION)
+
+.PHONY: golangci-lint
+golangci-lint: $(GOLANGCI_LINT) ## Download golangci-lint locally if necessary.
+$(GOLANGCI_LINT): $(LOCALBIN)
+	test -s $(GOLANGCI_LINT) && $(GOLANGCI_LINT) version | grep -q $(GOLANGCI_LINT_VERSION) || \
+	GOBIN=$(LOCALBIN) go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION)

cmd/controller.go

@@ -51,7 +51,12 @@ var (
 func init() {
 	flags := controllerCmd.Flags()
 	flags.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
-	flags.StringVar(&healthProbeAddr, "health-probe-bind-address", ":8081", "The address the health probe endpoint binds to.")
+	flags.StringVar(
+		&healthProbeAddr,
+		"health-probe-bind-address",
+		":8081",
+		"The address the health probe endpoint binds to.",
+	)
 	flags.BoolVar(&enableLeaderElection, "leader-elect", true,
 		"Enable leader election for controller manager. "+
 			"Enabling this will ensure there is only one active controller manager.")
@@ -177,7 +182,9 @@ func makeReceiveRunner(exporter metrics.MetricsExporter) manager.Runnable {
 
 		go func() {
 			<-ctx.Done()
-			s.Close()
+			if err := s.Close(); err != nil && !errors.Is(err, http.ErrServerClosed) {
+				setupLog.Error(err, "failed to close receiver server")
+			}
 		}()
 
 		return s.ListenAndServe()

cmd/probe.go

@@ -45,7 +45,12 @@ var provisionProbeCmd = &cobra.Command{
 
 func init() {
 	fs := probeCmd.Flags()
-	fs.StringVar(&probeConfig.controllerAddr, "destination-address", "http://localhost:8080", "metrics aggregator's address")
+	fs.StringVar(
+		&probeConfig.controllerAddr,
+		"destination-address",
+		"http://localhost:8080",
+		"metrics aggregator's address",
+	)
 	fs.StringVar(&probeConfig.storageClass, "storage-class", "", "target StorageClass name")
 	fs.StringVar(&probeConfig.fioFilename, "path", "/test", "target I/O test directory path")
 	fs.StringVar(&probeConfig.nodeName, "node-name", "", "node name")

internal/controller/pie/pieprobe_controller.go

@@ -114,7 +114,10 @@ func (r *PieProbeReconciler) reconcileMountProbes(ctx context.Context, pieProbe
 		return err
 	}
 	allNodeList := corev1.NodeList{}
-	r.client.List(ctx, &allNodeList)
+	err = r.client.List(ctx, &allNodeList)
+	if err != nil {
+		return err
+	}
 	availableNodeList := []corev1.Node{}
 	for _, node := range allNodeList.Items {
 		if !nodeSelector.Match(&node) {
@@ -258,13 +261,16 @@ func (r *PieProbeReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Complete(r)
 }
 
-func getPVCName(nodeName string, pieProbe *piev1alpha1.PieProbe) string {
+func getPVCName(nodeName string, pieProbe *piev1alpha1.PieProbe) (string, error) {
 	pieProbeName := pieProbe.Name
 	storageClass := pieProbe.Spec.MonitoringStorageClass
 
-	sha1 := sha1.New()
-	io.WriteString(sha1, pieProbeName+"\000"+nodeName+"\000"+storageClass)
-	hashedName := hex.EncodeToString(sha1.Sum(nil))
+	sha1Hash := sha1.New()
+	_, err := io.WriteString(sha1Hash, pieProbeName+"\000"+nodeName+"\000"+storageClass)
+	if err != nil {
+		return "", fmt.Errorf("failed to hash pvc name: %w", err)
+	}
+	hashedName := hex.EncodeToString(sha1Hash.Sum(nil))
 
 	if len(pieProbeName) > 10 {
 		pieProbeName = pieProbeName[:10]
@@ -275,7 +281,7 @@ func getPVCName(nodeName string, pieProbe *piev1alpha1.PieProbe) string {
 	if len(storageClass) > 14 {
 		storageClass = storageClass[:14]
 	}
-	return fmt.Sprintf("%s-%s-%s-%s-%s", constants.PVCNamePrefix, pieProbeName, nodeName, storageClass, hashedName[:6])
+	return fmt.Sprintf("%s-%s-%s-%s-%s", constants.PVCNamePrefix, pieProbeName, nodeName, storageClass, hashedName[:6]), nil
 }
 
 func (r *PieProbeReconciler) createOrUpdatePVC(
@@ -285,7 +291,10 @@ func (r *PieProbeReconciler) createOrUpdatePVC(
 ) error {
 	logger := log.FromContext(ctx)
 
-	pvcName := getPVCName(nodeName, pieProbe)
+	pvcName, err := getPVCName(nodeName, pieProbe)
+	if err != nil {
+		return err
+	}
 	storageClass := pieProbe.Spec.MonitoringStorageClass
 
 	pvc := &corev1.PersistentVolumeClaim{}
@@ -308,7 +317,9 @@ func (r *PieProbeReconciler) createOrUpdatePVC(
 		}
 		pvc.Spec.Resources.Requests[corev1.ResourceStorage] = *pieProbe.Spec.PVCCapacity
 
-		ctrl.SetControllerReference(pieProbe, pvc, r.client.Scheme())
+		if err := ctrl.SetControllerReference(pieProbe, pvc, r.client.Scheme()); err != nil {
+			return err
+		}
 
 		return nil
 	})
@@ -354,14 +365,18 @@ func (r *PieProbeReconciler) createOrUpdateJob(
 	nodeName *string,
 ) error {
 	_ = log.FromContext(ctx)
+	cronJobName, err := getCronJobName(kind, nodeName, pieProbe)
+	if err != nil {
+		return err
+	}
 
 	cronjob := &batchv1.CronJob{}
 	cronjob.SetNamespace(pieProbe.GetNamespace())
-	cronjob.SetName(getCronJobName(kind, nodeName, pieProbe))
+	cronjob.SetName(cronJobName)
 
 	storageClass := pieProbe.Spec.MonitoringStorageClass
 
-	_, err := ctrl.CreateOrUpdate(ctx, r.client, cronjob, func() error {
+	_, err = ctrl.CreateOrUpdate(ctx, r.client, cronjob, func() error {
 		label := map[string]string{
 			constants.ProbeStorageClassLabelKey: storageClass,
 			constants.ProbePieProbeLabelKey:     pieProbe.GetName(),
@@ -470,25 +485,31 @@ func (r *PieProbeReconciler) createOrUpdateJob(
 					},
 				},
 			}
+			pvcName, err := getPVCName(*nodeName, pieProbe)
+			if err != nil {
+				return err
+			}
 			cronjob.Spec.JobTemplate.Spec.Template.Spec.Volumes = []corev1.Volume{
 				{
 					Name: volumeName,
 					VolumeSource: corev1.VolumeSource{
 						PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
-							ClaimName: getPVCName(*nodeName, pieProbe),
+							ClaimName: pvcName,
 						},
 					},
 				},
 			}
 		}
 
-		ctrl.SetControllerReference(pieProbe, cronjob, r.client.Scheme())
+		if err := ctrl.SetControllerReference(pieProbe, cronjob, r.client.Scheme()); err != nil {
+			return err
+		}
 
 		return nil
 	})
 
 	if err != nil {
-		return fmt.Errorf("failed to create CronJob: %s", getCronJobName(kind, nodeName, pieProbe))
+		return fmt.Errorf("failed to create CronJob: %s", cronJobName)
 	}
 
 	return nil
@@ -501,7 +522,7 @@ func (r *PieProbeReconciler) createOrUpdateJob(
 // However, if the node and StorageClass names are too long, the CronJob name will not fit in 52 characters.
 // So we cut off the node and StorageClass names to an appropriate length and added a hash value at the end
 // to balance readability and uniqueness.
-func getCronJobName(kind int, nodeNamePtr *string, pieProbe *piev1alpha1.PieProbe) string {
+func getCronJobName(kind int, nodeNamePtr *string, pieProbe *piev1alpha1.PieProbe) (string, error) {
 	nodeName := ""
 	if nodeNamePtr != nil {
 		nodeName = *nodeNamePtr
@@ -510,9 +531,12 @@ func getCronJobName(kind int, nodeNamePtr *string, pieProbe *piev1alpha1.PieProb
 	pieProbeName := pieProbe.Name
 	storageClass := pieProbe.Spec.MonitoringStorageClass
 
-	sha1 := sha1.New()
-	io.WriteString(sha1, pieProbeName+"\000"+nodeName+"\000"+storageClass)
-	hashedName := hex.EncodeToString(sha1.Sum(nil))
+	sha1Hash := sha1.New()
+	_, err := io.WriteString(sha1Hash, pieProbeName+"\000"+nodeName+"\000"+storageClass)
+	if err != nil {
+		return "", fmt.Errorf("failed to hash cronjob name: %w", err)
+	}
+	hashedName := hex.EncodeToString(sha1Hash.Sum(nil))
 
 	if len(pieProbeName) > 10 {
 		pieProbeName = pieProbeName[:10]
@@ -525,9 +549,9 @@ func getCronJobName(kind int, nodeNamePtr *string, pieProbe *piev1alpha1.PieProb
 	}
 
 	if kind == ProvisionProbe {
-		return fmt.Sprintf("%s-%s-%s-%s", constants.ProvisionProbeNamePrefix, pieProbeName, storageClass, hashedName[:6])
+		return fmt.Sprintf("%s-%s-%s-%s", constants.ProvisionProbeNamePrefix, pieProbeName, storageClass, hashedName[:6]), nil
 	} else { // kind == MountProbe
-		return fmt.Sprintf("%s-%s-%s-%s-%s", constants.MountProbeNamePrefix, pieProbeName, nodeName, storageClass, hashedName[:6])
+		return fmt.Sprintf("%s-%s-%s-%s-%s", constants.MountProbeNamePrefix, pieProbeName, nodeName, storageClass, hashedName[:6]), nil
 	}
 }
 

internal/controller/pie/pieprobe_controller_test.go

@@ -2,6 +2,7 @@ package pie
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"strings"
 	"time"
@@ -98,36 +99,36 @@ func prepareObjects(ctx context.Context) error {
 
 func deletePieProbeAndReferencingResources(ctx context.Context, pieProbe *piev1alpha1.PieProbe) error {
 	if err := k8sClient.Delete(ctx, pieProbe); err != nil {
-		return err
+		return fmt.Errorf("failed to delete PieProbe %s: %w", pieProbe.Name, err)
 	}
 
 	var pvcList corev1.PersistentVolumeClaimList
 	if err := k8sClient.List(ctx, &pvcList, client.MatchingLabels(map[string]string{
 		"storage-class": pieProbe.Spec.MonitoringStorageClass,
 	})); err != nil {
-		return err
+		return fmt.Errorf("failed to list PVCs: %w", err)
 	}
 
 	for _, pvc := range pvcList.Items {
-		pvc.ObjectMeta.Finalizers = []string{}
+		pvc.Finalizers = []string{}
 		if err := k8sClient.Update(ctx, &pvc); err != nil {
-			return err
+			return fmt.Errorf("failed to update PVC %s: %w", pvc.Name, err)
 		}
-		if err := k8sClient.Delete(ctx, &pvc); err != nil {
-			return err
+		if err := k8sClient.Delete(ctx, &pvc); err != nil && !apierrors.IsNotFound(err) {
+			return fmt.Errorf("failed to delete PVC %s: %w", pvc.Name, err)
 		}
 	}
 
 	var cronjobList batchv1.CronJobList
 	if err := k8sClient.List(ctx, &cronjobList, client.MatchingLabels(map[string]string{
 		"storage-class": pieProbe.Spec.MonitoringStorageClass,
 	})); err != nil {
-		return err
+		return fmt.Errorf("failed to list CronJobs: %w", err)
 	}
 
 	for _, cronjob := range cronjobList.Items {
-		if err := k8sClient.Delete(ctx, &cronjob); err != nil {
-			return err
+		if err := k8sClient.Delete(ctx, &cronjob); err != nil && !apierrors.IsNotFound(err) {
+			return fmt.Errorf("failed to delete CronJob %s: %w", cronjob.Name, err)
 		}
 	}
 
@@ -210,7 +211,10 @@ var _ = Describe("PieProbe controller for specifying resources", func() {
 			}
 			_, err := ctrl.CreateOrUpdate(ctx, k8sClient, pieProbe, func() error { return nil })
 			Expect(err).NotTo(HaveOccurred())
-			defer deletePieProbeAndReferencingResources(ctx, pieProbe)
+			defer func() {
+				err := deletePieProbeAndReferencingResources(ctx, pieProbe)
+				Expect(err).NotTo(HaveOccurred())
+			}()
 
 			By("checking the CronJob's resource specified")
 			Eventually(func(g Gomega) {

internal/controller/probe_pod_controller.go

@@ -105,7 +105,9 @@ func (r *ProbePodReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 
 // SetupWithManager sets up the controller with the Manager.
 func (r *ProbePodReconciler) SetupWithManager(mgr ctrl.Manager) error {
-	mgr.Add(r.po)
+	if err := mgr.Add(r.po); err != nil {
+		return err
+	}
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&corev1.Pod{}).
 		Complete(r)

internal/controller/provision_observer.go

@@ -98,7 +98,10 @@ func (p *provisionObserver) deleteEventTime(namespace, podName string) {
 	delete(p.podPieProbeName, namespacePod{namespace, podName})
 }
 
-func (p *provisionObserver) getNodeNameAndStorageClass(ctx context.Context, namespace, podName string) (string, string, error) {
+func (p *provisionObserver) getNodeNameAndStorageClass(
+	ctx context.Context,
+	namespace, podName string,
+) (string, string, error) {
 	var pod corev1.Pod
 	err := p.client.Get(ctx, client.ObjectKey{Namespace: namespace, Name: podName}, &pod)
 	if err != nil {
@@ -109,7 +112,9 @@ func (p *provisionObserver) getNodeNameAndStorageClass(ctx context.Context, name
 }
 
 func isProbeJob2(o metav1.OwnerReference) bool {
-	return o.Kind == "Job" && (strings.HasPrefix(o.Name, constants.MountProbeNamePrefix) || strings.HasPrefix(o.Name, constants.ProvisionProbeNamePrefix))
+	return o.Kind == "Job" &&
+		(strings.HasPrefix(o.Name, constants.MountProbeNamePrefix) ||
+			strings.HasPrefix(o.Name, constants.ProvisionProbeNamePrefix))
 }
 
 func (p *provisionObserver) deleteOwnerJobOfPod(ctx context.Context, namespace, podName string) error {