Refactor: Tidy some imports and tests

.golangci.yaml

@@ -40,6 +40,12 @@ linters-settings:
           - pkg: github.com/crunchydata/postgres-operator/internal/testing/*
             desc: The "internal/testing" packages should be used only in tests.
 
+      tests:
+        files: ['$test']
+        deny:
+          - pkg: github.com/pkg/errors
+            desc: Use the "errors" package unless you are interacting with stack traces.
+
   errchkjson:
     check-error-free-encoding: true
 

Makefile

@@ -9,9 +9,6 @@ PGMONITOR_DIR ?= hack/tools/pgmonitor
 PGMONITOR_VERSION ?= v5.1.1
 QUERIES_CONFIG_DIR ?= hack/tools/queries
 
-EXTERNAL_SNAPSHOTTER_DIR ?= hack/tools/external-snapshotter
-EXTERNAL_SNAPSHOTTER_VERSION ?= v8.0.1
-
 # Buildah's "build" used to be "bud". Use the alias to be compatible for a while.
 BUILDAH_BUILD ?= buildah bud
 
@@ -55,12 +52,6 @@ get-pgmonitor:
 	cp -r '$(PGMONITOR_DIR)/postgres_exporter/common/.' '${QUERIES_CONFIG_DIR}'
 	cp '$(PGMONITOR_DIR)/postgres_exporter/linux/queries_backrest.yml' '${QUERIES_CONFIG_DIR}'
 
-.PHONY: get-external-snapshotter
-get-external-snapshotter:
-	git -C '$(dir $(EXTERNAL_SNAPSHOTTER_DIR))' clone https://github.com/kubernetes-csi/external-snapshotter.git || git -C '$(EXTERNAL_SNAPSHOTTER_DIR)' fetch origin
-	@git -C '$(EXTERNAL_SNAPSHOTTER_DIR)' checkout '$(EXTERNAL_SNAPSHOTTER_VERSION)'
-	@git -C '$(EXTERNAL_SNAPSHOTTER_DIR)' config pull.ff only
-
 .PHONY: clean
 clean: ## Clean resources
 clean: clean-deprecated
@@ -203,7 +194,7 @@ check: get-pgmonitor
 check-envtest: ## Run check using envtest and a mock kube api
 check-envtest: ENVTEST_USE = $(ENVTEST) --bin-dir=$(CURDIR)/hack/tools/envtest use $(ENVTEST_K8S_VERSION)
 check-envtest: SHELL = bash
-check-envtest: get-pgmonitor tools/setup-envtest get-external-snapshotter
+check-envtest: get-pgmonitor tools/setup-envtest
 	@$(ENVTEST_USE) --print=overview && echo
 	source <($(ENVTEST_USE) --print=env) && PGO_NAMESPACE="postgres-operator" QUERIES_CONFIG_DIR="$(CURDIR)/${QUERIES_CONFIG_DIR}" \
 		$(GO_TEST) -count=1 -cover ./...
@@ -214,7 +205,7 @@ check-envtest: get-pgmonitor tools/setup-envtest get-external-snapshotter
 # make check-envtest-existing PGO_TEST_TIMEOUT_SCALE=1.2
 .PHONY: check-envtest-existing
 check-envtest-existing: ## Run check using envtest and an existing kube api
-check-envtest-existing: get-pgmonitor get-external-snapshotter
+check-envtest-existing: get-pgmonitor
 check-envtest-existing: createnamespaces
 	kubectl apply --server-side -k ./config/dev
 	USE_EXISTING_CLUSTER=true PGO_NAMESPACE="postgres-operator" QUERIES_CONFIG_DIR="$(CURDIR)/${QUERIES_CONFIG_DIR}" \

go.mod

@@ -22,6 +22,7 @@ require (
 	go.opentelemetry.io/otel/sdk v1.27.0
 	go.opentelemetry.io/otel/trace v1.27.0
 	golang.org/x/crypto v0.27.0
+	golang.org/x/tools v0.22.0
 	gotest.tools/v3 v3.1.0
 	k8s.io/api v0.30.2
 	k8s.io/apimachinery v0.30.2
@@ -72,13 +73,14 @@ require (
 	go.opentelemetry.io/otel/metric v1.27.0 // indirect
 	go.opentelemetry.io/proto/otlp v1.3.1 // indirect
 	golang.org/x/exp v0.0.0-20240604190554-fc45aab8b7f8 // indirect
+	golang.org/x/mod v0.18.0 // indirect
 	golang.org/x/net v0.29.0 // indirect
 	golang.org/x/oauth2 v0.21.0 // indirect
+	golang.org/x/sync v0.8.0 // indirect
 	golang.org/x/sys v0.25.0 // indirect
 	golang.org/x/term v0.24.0 // indirect
 	golang.org/x/text v0.18.0 // indirect
 	golang.org/x/time v0.5.0 // indirect
-	golang.org/x/tools v0.22.0 // indirect
 	gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect
 	google.golang.org/genproto/googleapis/api v0.0.0-20240610135401-a8a62080eff3 // indirect
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20240903143218-8af14fe29dc1 // indirect

go.sum

@@ -161,6 +161,8 @@ golang.org/x/exp v0.0.0-20240604190554-fc45aab8b7f8 h1:LoYXNGAShUG3m/ehNk4iFctuh
 golang.org/x/exp v0.0.0-20240604190554-fc45aab8b7f8/go.mod h1:jj3sYF3dwk5D+ghuXyeI3r5MFf+NT2An6/9dOA95KSI=
 golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
+golang.org/x/mod v0.18.0 h1:5+9lSbEzPSdWkH32vYPBwEpX8KwDbM52Ud9xBUvNlb0=
+golang.org/x/mod v0.18.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c=
 golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
 golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
@@ -172,6 +174,8 @@ golang.org/x/oauth2 v0.21.0/go.mod h1:XYTD2NtWslqkgxebSiOHnXEap4TF09sJSc7H1sXbht
 golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
+golang.org/x/sync v0.8.0 h1:3NFvSEYkUoMifnESzZl15y791HH1qU2xm6eCJU5ZPXQ=
+golang.org/x/sync v0.8.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
 golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=

internal/controller/pgupgrade/world_test.go

@@ -13,8 +13,8 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 
+	"github.com/crunchydata/postgres-operator/internal/controller/runtime"
 	"github.com/crunchydata/postgres-operator/internal/initialize"
 	"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
 )
@@ -34,7 +34,7 @@ func TestPopulateCluster(t *testing.T) {
 
 	t.Run("NotFound", func(t *testing.T) {
 		cluster := v1beta1.NewPostgresCluster()
-		expected := apierrors.NewNotFound(schema.GroupResource{}, "name")
+		expected := apierrors.NewNotFound(runtime.GR{}, "name")
 
 		world := NewWorld()
 		err := world.populateCluster(cluster, expected)

internal/controller/postgrescluster/cluster_test.go

@@ -8,7 +8,6 @@ import (
 	"context"
 	"testing"
 
-	"github.com/pkg/errors"
 	"go.opentelemetry.io/otel"
 	"gotest.tools/v3/assert"
 	appsv1 "k8s.io/api/apps/v1"
@@ -17,20 +16,19 @@ import (
 	rbacv1 "k8s.io/api/rbac/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
+	"github.com/crunchydata/postgres-operator/internal/controller/runtime"
 	"github.com/crunchydata/postgres-operator/internal/initialize"
 	"github.com/crunchydata/postgres-operator/internal/naming"
 	"github.com/crunchydata/postgres-operator/internal/testing/cmp"
 	"github.com/crunchydata/postgres-operator/internal/testing/require"
 	"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
 )
 
-var gvks = []schema.GroupVersionKind{{
+var gvks = []runtime.GVK{{
 	Group:   corev1.SchemeGroupVersion.Group,
 	Version: corev1.SchemeGroupVersion.Version,
 	Kind:    "ConfigMapList",
@@ -91,7 +89,7 @@ func TestCustomLabels(t *testing.T) {
 	ns := setupNamespace(t, cc)
 
 	reconcileTestCluster := func(cluster *v1beta1.PostgresCluster) {
-		assert.NilError(t, errors.WithStack(reconciler.Client.Create(ctx, cluster)))
+		assert.NilError(t, reconciler.Client.Create(ctx, cluster))
 		t.Cleanup(func() {
 			// Remove finalizers, if any, so the namespace can terminate.
 			assert.Check(t, client.IgnoreNotFound(
@@ -107,36 +105,33 @@ func TestCustomLabels(t *testing.T) {
 		assert.Assert(t, result.Requeue == false)
 	}
 
-	getUnstructuredLabels := func(cluster v1beta1.PostgresCluster, u unstructured.Unstructured) (map[string]map[string]string, error) {
-		var err error
+	getUnstructuredLabels := func(t *testing.T, cluster *v1beta1.PostgresCluster, u *unstructured.Unstructured) map[string]map[string]string {
+		t.Helper()
 		labels := map[string]map[string]string{}
 
-		if metav1.IsControlledBy(&u, &cluster) {
+		if metav1.IsControlledBy(u, cluster) {
 			switch u.GetKind() {
 			case "StatefulSet":
-				var resource appsv1.StatefulSet
-				err = runtime.DefaultUnstructuredConverter.
-					FromUnstructured(u.UnstructuredContent(), &resource)
+				resource, err := runtime.FromUnstructuredObject[appsv1.StatefulSet](u)
+				assert.NilError(t, err)
 				labels["resource"] = resource.GetLabels()
 				labels["podTemplate"] = resource.Spec.Template.GetLabels()
 			case "Deployment":
-				var resource appsv1.Deployment
-				err = runtime.DefaultUnstructuredConverter.
-					FromUnstructured(u.UnstructuredContent(), &resource)
+				resource, err := runtime.FromUnstructuredObject[appsv1.Deployment](u)
+				assert.NilError(t, err)
 				labels["resource"] = resource.GetLabels()
 				labels["podTemplate"] = resource.Spec.Template.GetLabels()
 			case "CronJob":
-				var resource batchv1.CronJob
-				err = runtime.DefaultUnstructuredConverter.
-					FromUnstructured(u.UnstructuredContent(), &resource)
+				resource, err := runtime.FromUnstructuredObject[batchv1.CronJob](u)
+				assert.NilError(t, err)
 				labels["resource"] = resource.GetLabels()
 				labels["jobTemplate"] = resource.Spec.JobTemplate.GetLabels()
 				labels["jobPodTemplate"] = resource.Spec.JobTemplate.Spec.Template.GetLabels()
 			default:
 				labels["resource"] = u.GetLabels()
 			}
 		}
-		return labels, err
+		return labels
 	}
 
 	t.Run("Cluster", func(t *testing.T) {
@@ -176,10 +171,8 @@ func TestCustomLabels(t *testing.T) {
 				client.InNamespace(cluster.Namespace),
 				client.MatchingLabelsSelector{Selector: selector}))
 
-			for i := range uList.Items {
-				u := uList.Items[i]
-				labels, err := getUnstructuredLabels(*cluster, u)
-				assert.NilError(t, err)
+			for _, u := range uList.Items {
+				labels := getUnstructuredLabels(t, cluster, &u)
 				for resourceType, resourceLabels := range labels {
 					t.Run(u.GetKind()+"/"+u.GetName()+"/"+resourceType, func(t *testing.T) {
 						assert.Equal(t, resourceLabels["my.cluster.label"], "daisy")
@@ -226,11 +219,8 @@ func TestCustomLabels(t *testing.T) {
 						client.InNamespace(cluster.Namespace),
 						client.MatchingLabelsSelector{Selector: selector}))
 
-					for i := range uList.Items {
-						u := uList.Items[i]
-
-						labels, err := getUnstructuredLabels(*cluster, u)
-						assert.NilError(t, err)
+					for _, u := range uList.Items {
+						labels := getUnstructuredLabels(t, cluster, &u)
 						for resourceType, resourceLabels := range labels {
 							t.Run(u.GetKind()+"/"+u.GetName()+"/"+resourceType, func(t *testing.T) {
 								assert.Equal(t, resourceLabels["my.instance.label"], set.Metadata.Labels["my.instance.label"])
@@ -276,11 +266,8 @@ func TestCustomLabels(t *testing.T) {
 				client.InNamespace(cluster.Namespace),
 				client.MatchingLabelsSelector{Selector: selector}))
 
-			for i := range uList.Items {
-				u := uList.Items[i]
-
-				labels, err := getUnstructuredLabels(*cluster, u)
-				assert.NilError(t, err)
+			for _, u := range uList.Items {
+				labels := getUnstructuredLabels(t, cluster, &u)
 				for resourceType, resourceLabels := range labels {
 					t.Run(u.GetKind()+"/"+u.GetName()+"/"+resourceType, func(t *testing.T) {
 						assert.Equal(t, resourceLabels["my.pgbackrest.label"], "lucy")
@@ -314,11 +301,8 @@ func TestCustomLabels(t *testing.T) {
 				client.InNamespace(cluster.Namespace),
 				client.MatchingLabelsSelector{Selector: selector}))
 
-			for i := range uList.Items {
-				u := uList.Items[i]
-
-				labels, err := getUnstructuredLabels(*cluster, u)
-				assert.NilError(t, err)
+			for _, u := range uList.Items {
+				labels := getUnstructuredLabels(t, cluster, &u)
 				for resourceType, resourceLabels := range labels {
 					t.Run(u.GetKind()+"/"+u.GetName()+"/"+resourceType, func(t *testing.T) {
 						assert.Equal(t, resourceLabels["my.pgbouncer.label"], "lucy")
@@ -344,7 +328,7 @@ func TestCustomAnnotations(t *testing.T) {
 	ns := setupNamespace(t, cc)
 
 	reconcileTestCluster := func(cluster *v1beta1.PostgresCluster) {
-		assert.NilError(t, errors.WithStack(reconciler.Client.Create(ctx, cluster)))
+		assert.NilError(t, reconciler.Client.Create(ctx, cluster))
 		t.Cleanup(func() {
 			// Remove finalizers, if any, so the namespace can terminate.
 			assert.Check(t, client.IgnoreNotFound(
@@ -360,36 +344,33 @@ func TestCustomAnnotations(t *testing.T) {
 		assert.Assert(t, result.Requeue == false)
 	}
 
-	getUnstructuredAnnotations := func(cluster v1beta1.PostgresCluster, u unstructured.Unstructured) (map[string]map[string]string, error) {
-		var err error
+	getUnstructuredAnnotations := func(t *testing.T, cluster *v1beta1.PostgresCluster, u *unstructured.Unstructured) map[string]map[string]string {
+		t.Helper()
 		annotations := map[string]map[string]string{}
 
-		if metav1.IsControlledBy(&u, &cluster) {
+		if metav1.IsControlledBy(u, cluster) {
 			switch u.GetKind() {
 			case "StatefulSet":
-				var resource appsv1.StatefulSet
-				err = runtime.DefaultUnstructuredConverter.
-					FromUnstructured(u.UnstructuredContent(), &resource)
+				resource, err := runtime.FromUnstructuredObject[appsv1.StatefulSet](u)
+				assert.NilError(t, err)
 				annotations["resource"] = resource.GetAnnotations()
 				annotations["podTemplate"] = resource.Spec.Template.GetAnnotations()
 			case "Deployment":
-				var resource appsv1.Deployment
-				err = runtime.DefaultUnstructuredConverter.
-					FromUnstructured(u.UnstructuredContent(), &resource)
+				resource, err := runtime.FromUnstructuredObject[appsv1.Deployment](u)
+				assert.NilError(t, err)
 				annotations["resource"] = resource.GetAnnotations()
 				annotations["podTemplate"] = resource.Spec.Template.GetAnnotations()
 			case "CronJob":
-				var resource batchv1.CronJob
-				err = runtime.DefaultUnstructuredConverter.
-					FromUnstructured(u.UnstructuredContent(), &resource)
+				resource, err := runtime.FromUnstructuredObject[batchv1.CronJob](u)
+				assert.NilError(t, err)
 				annotations["resource"] = resource.GetAnnotations()
 				annotations["jobTemplate"] = resource.Spec.JobTemplate.GetAnnotations()
 				annotations["jobPodTemplate"] = resource.Spec.JobTemplate.Spec.Template.GetAnnotations()
 			default:
 				annotations["resource"] = u.GetAnnotations()
 			}
 		}
-		return annotations, err
+		return annotations
 	}
 
 	t.Run("Cluster", func(t *testing.T) {
@@ -430,10 +411,8 @@ func TestCustomAnnotations(t *testing.T) {
 				client.InNamespace(cluster.Namespace),
 				client.MatchingLabelsSelector{Selector: selector}))
 
-			for i := range uList.Items {
-				u := uList.Items[i]
-				annotations, err := getUnstructuredAnnotations(*cluster, u)
-				assert.NilError(t, err)
+			for _, u := range uList.Items {
+				annotations := getUnstructuredAnnotations(t, cluster, &u)
 				for resourceType, resourceAnnotations := range annotations {
 					t.Run(u.GetKind()+"/"+u.GetName()+"/"+resourceType, func(t *testing.T) {
 						assert.Equal(t, resourceAnnotations["my.cluster.annotation"], "daisy")
@@ -480,11 +459,8 @@ func TestCustomAnnotations(t *testing.T) {
 						client.InNamespace(cluster.Namespace),
 						client.MatchingLabelsSelector{Selector: selector}))
 
-					for i := range uList.Items {
-						u := uList.Items[i]
-
-						annotations, err := getUnstructuredAnnotations(*cluster, u)
-						assert.NilError(t, err)
+					for _, u := range uList.Items {
+						annotations := getUnstructuredAnnotations(t, cluster, &u)
 						for resourceType, resourceAnnotations := range annotations {
 							t.Run(u.GetKind()+"/"+u.GetName()+"/"+resourceType, func(t *testing.T) {
 								assert.Equal(t, resourceAnnotations["my.instance.annotation"], set.Metadata.Annotations["my.instance.annotation"])
@@ -530,11 +506,8 @@ func TestCustomAnnotations(t *testing.T) {
 				client.InNamespace(cluster.Namespace),
 				client.MatchingLabelsSelector{Selector: selector}))
 
-			for i := range uList.Items {
-				u := uList.Items[i]
-
-				annotations, err := getUnstructuredAnnotations(*cluster, u)
-				assert.NilError(t, err)
+			for _, u := range uList.Items {
+				annotations := getUnstructuredAnnotations(t, cluster, &u)
 				for resourceType, resourceAnnotations := range annotations {
 					t.Run(u.GetKind()+"/"+u.GetName()+"/"+resourceType, func(t *testing.T) {
 						assert.Equal(t, resourceAnnotations["my.pgbackrest.annotation"], "lucy")
@@ -568,11 +541,8 @@ func TestCustomAnnotations(t *testing.T) {
 				client.InNamespace(cluster.Namespace),
 				client.MatchingLabelsSelector{Selector: selector}))
 
-			for i := range uList.Items {
-				u := uList.Items[i]
-
-				annotations, err := getUnstructuredAnnotations(*cluster, u)
-				assert.NilError(t, err)
+			for _, u := range uList.Items {
+				annotations := getUnstructuredAnnotations(t, cluster, &u)
 				for resourceType, resourceAnnotations := range annotations {
 					t.Run(u.GetKind()+"/"+u.GetName()+"/"+resourceType, func(t *testing.T) {
 						assert.Equal(t, resourceAnnotations["my.pgbouncer.annotation"], "lucy")

internal/controller/postgrescluster/controller_test.go

@@ -13,7 +13,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	. "github.com/onsi/gomega/gstruct"
-	"github.com/pkg/errors"
+	"github.com/pkg/errors" //nolint:depguard // This legacy test covers so much code, it logs the origin of unexpected errors.
 
 	"go.opentelemetry.io/otel"
 	"gotest.tools/v3/assert"