Remove server-side apply tests for old Kubernetes

Kubernetes 1.22 and OpenShift 4.8 have been out of commission for some
time now.

Author: cbandy

internal/controller/postgrescluster/apply.go

@@ -54,8 +54,7 @@ func (r *Reconciler) apply(ctx context.Context, object client.Object) error {
 func applyServiceSpec(
 	patch *kubeapi.JSON6902, actual, intent corev1.ServiceSpec, path ...string,
 ) {
-	// Service.Spec.Selector is not +mapType=atomic until Kubernetes 1.22.
-	// - https://issue.k8s.io/97970
+	// Service.Spec.Selector cannot be unset; perhaps https://issue.k8s.io/117447
 	if !equality.Semantic.DeepEqual(actual.Selector, intent.Selector) {
 		patch.Replace(append(path, "selector")...)(intent.Selector)
 	}

internal/controller/postgrescluster/apply_test.go

@@ -14,7 +14,6 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"gotest.tools/v3/assert"
-	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -144,41 +143,6 @@ func TestServerSideApply(t *testing.T) {
 		)
 	})
 
-	t.Run("StatefulSetStatus", func(t *testing.T) {
-		constructor := func(name string) *appsv1.StatefulSet {
-			var sts appsv1.StatefulSet
-			sts.SetGroupVersionKind(appsv1.SchemeGroupVersion.WithKind("StatefulSet"))
-			sts.Namespace, sts.Name = ns.Name, name
-			sts.Spec.Selector = &metav1.LabelSelector{
-				MatchLabels: map[string]string{"select": name},
-			}
-			sts.Spec.Template.Labels = map[string]string{"select": name}
-			sts.Spec.Template.Spec.Containers = []corev1.Container{{Name: "test", Image: "test"}}
-			return &sts
-		}
-
-		cc := client.WithFieldOwner(cc, t.Name())
-		reconciler := Reconciler{Writer: cc}
-		upstream := constructor("status-upstream")
-
-		// The structs defined in "k8s.io/api/apps/v1" marshal empty status fields.
-		switch {
-		case serverVersion.LessThan(version.MustParseGeneric("1.22")):
-			assert.ErrorContains(t,
-				cc.Patch(ctx, upstream, client.Apply, client.ForceOwnership),
-				"field not declared in schema",
-				"expected https://issue.k8s.io/109210")
-
-		default:
-			assert.NilError(t,
-				cc.Patch(ctx, upstream, client.Apply, client.ForceOwnership))
-		}
-
-		// Our apply method generates the correct apply-patch.
-		again := constructor("status-local")
-		assert.NilError(t, reconciler.apply(ctx, again))
-	})
-
 	t.Run("ServiceSelector", func(t *testing.T) {
 		constructor := func(name string) *corev1.Service {
 			var service corev1.Service
@@ -190,61 +154,6 @@ func TestServerSideApply(t *testing.T) {
 			return &service
 		}
 
-		t.Run("wrong-keys", func(t *testing.T) {
-			cc := client.WithFieldOwner(cc, t.Name())
-			reconciler := Reconciler{Writer: cc}
-
-			intent := constructor("some-selector")
-			intent.Spec.Selector = map[string]string{"k1": "v1"}
-
-			// Create the Service.
-			before := intent.DeepCopy()
-			assert.NilError(t,
-				cc.Patch(ctx, before, client.Apply, client.ForceOwnership))
-
-			// Something external mucks it up.
-			assert.NilError(t,
-				cc.Patch(ctx, before,
-					client.RawPatch(client.Merge.Type(), []byte(`{"spec":{"selector":{"bad":"v2"}}}`)),
-					client.FieldOwner("wrong")))
-
-			// client.Apply cannot correct it in old versions of Kubernetes.
-			after := intent.DeepCopy()
-			assert.NilError(t,
-				cc.Patch(ctx, after, client.Apply, client.ForceOwnership))
-
-			switch {
-			case serverVersion.LessThan(version.MustParseGeneric("1.22")):
-
-				assert.Assert(t, len(after.Spec.Selector) != len(intent.Spec.Selector),
-					"expected https://issue.k8s.io/97970, got %v", after.Spec.Selector)
-
-			default:
-				assert.DeepEqual(t, after.Spec.Selector, intent.Spec.Selector)
-			}
-
-			// Our apply method corrects it.
-			again := intent.DeepCopy()
-			assert.NilError(t, reconciler.apply(ctx, again))
-			assert.DeepEqual(t, again.Spec.Selector, intent.Spec.Selector)
-
-			var count int
-			var managed *metav1.ManagedFieldsEntry
-			for i := range again.ManagedFields {
-				if again.ManagedFields[i].Manager == t.Name() {
-					count++
-					managed = &again.ManagedFields[i]
-				}
-			}
-
-			assert.Equal(t, count, 1, "expected manager once in %v", again.ManagedFields)
-			assert.Equal(t, managed.Operation, metav1.ManagedFieldsOperationApply)
-
-			assert.Assert(t, managed.FieldsV1 != nil)
-			assert.Assert(t, strings.Contains(string(managed.FieldsV1.Raw), `"f:selector":{`),
-				"expected f:selector in %s", managed.FieldsV1.Raw)
-		})
-
 		for _, tt := range []struct {
 			name     string
 			selector map[string]string
@@ -275,6 +184,9 @@ func TestServerSideApply(t *testing.T) {
 				assert.NilError(t,
 					cc.Patch(ctx, after, client.Apply, client.ForceOwnership))
 
+				// Perhaps one of:
+				// - https://issue.k8s.io/117447
+				// - https://github.com/kubernetes-sigs/structured-merge-diff/issues/259
 				assert.Assert(t, len(after.Spec.Selector) != len(intent.Spec.Selector),
 					"got %v", after.Spec.Selector)
 

internal/controller/postgrescluster/controller_test.go

@@ -21,7 +21,6 @@ import (
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/rand"
-	"k8s.io/apimachinery/pkg/util/version"
 	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -340,59 +339,32 @@ spec:
 			//
 			// The "metadata.finalizers" field is also okay.
 			// - https://book.kubebuilder.io/reference/using-finalizers.html
-			//
-			// NOTE(cbandy): Kubernetes prior to v1.16.10 and v1.17.6 does not track
-			// managed fields on the status subresource: https://issue.k8s.io/88901
-			switch {
-			case suite.ServerVersion.LessThan(version.MustParseGeneric("1.22")):
-
-				// Kubernetes 1.22 began tracking subresources in managed fields.
-				// - https://pr.k8s.io/100970
-				Expect(existing.ManagedFields).To(ContainElement(
-					MatchFields(IgnoreExtras, Fields{
-						"Manager": Equal(test.Owner),
-						"FieldsV1": PointTo(MatchAllFields(Fields{
-							"Raw": WithTransform(func(in []byte) (out map[string]any) {
-								Expect(yaml.Unmarshal(in, &out)).To(Succeed())
-								return out
-							}, MatchAllKeys(Keys{
-								"f:metadata": MatchAllKeys(Keys{
-									"f:finalizers": Not(BeZero()),
-								}),
-								"f:status": Not(BeZero()),
-							})),
-						})),
-					}),
-				), `controller should manage only "finalizers" and "status"`)
-
-			default:
-				Expect(existing.ManagedFields).To(ContainElements(
-					MatchFields(IgnoreExtras, Fields{
-						"Manager": Equal(test.Owner),
-						"FieldsV1": PointTo(MatchAllFields(Fields{
-							"Raw": WithTransform(func(in []byte) (out map[string]any) {
-								Expect(yaml.Unmarshal(in, &out)).To(Succeed())
-								return out
-							}, MatchAllKeys(Keys{
-								"f:metadata": MatchAllKeys(Keys{
-									"f:finalizers": Not(BeZero()),
-								}),
-							})),
+			Expect(existing.ManagedFields).To(ContainElements(
+				MatchFields(IgnoreExtras, Fields{
+					"Manager": Equal(test.Owner),
+					"FieldsV1": PointTo(MatchAllFields(Fields{
+						"Raw": WithTransform(func(in []byte) (out map[string]any) {
+							Expect(yaml.Unmarshal(in, &out)).To(Succeed())
+							return out
+						}, MatchAllKeys(Keys{
+							"f:metadata": MatchAllKeys(Keys{
+								"f:finalizers": Not(BeZero()),
+							}),
 						})),
-					}),
-					MatchFields(IgnoreExtras, Fields{
-						"Manager": Equal(test.Owner),
-						"FieldsV1": PointTo(MatchAllFields(Fields{
-							"Raw": WithTransform(func(in []byte) (out map[string]any) {
-								Expect(yaml.Unmarshal(in, &out)).To(Succeed())
-								return out
-							}, MatchAllKeys(Keys{
-								"f:status": Not(BeZero()),
-							})),
+					})),
+				}),
+				MatchFields(IgnoreExtras, Fields{
+					"Manager": Equal(test.Owner),
+					"FieldsV1": PointTo(MatchAllFields(Fields{
+						"Raw": WithTransform(func(in []byte) (out map[string]any) {
+							Expect(yaml.Unmarshal(in, &out)).To(Succeed())
+							return out
+						}, MatchAllKeys(Keys{
+							"f:status": Not(BeZero()),
 						})),
-					}),
-				), `controller should manage only "finalizers" and "status"`)
-			}
+					})),
+				}),
+			), `controller should manage only "finalizers" and "status"`)
 		})
 
 		Specify("Patroni Distributed Configuration", func() {

internal/controller/postgrescluster/suite_test.go

@@ -6,30 +6,19 @@ package postgrescluster
 
 import (
 	"context"
-	"os"
-	"strings"
 	"testing"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
-	"k8s.io/apimachinery/pkg/util/version"
-	"k8s.io/client-go/discovery"
-	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log"
-	"sigs.k8s.io/controller-runtime/pkg/manager"
 
 	"github.com/crunchydata/postgres-operator/internal/logging"
 	"github.com/crunchydata/postgres-operator/internal/testing/require"
 )
 
 var suite struct {
 	Client client.Client
-	Config *rest.Config
-
-	ServerVersion *version.Version
-
-	Manager manager.Manager
 }
 
 func TestAPIs(t *testing.T) {
@@ -39,24 +28,10 @@ func TestAPIs(t *testing.T) {
 }
 
 var _ = BeforeSuite(func() {
-	if os.Getenv("KUBEBUILDER_ASSETS") == "" && !strings.EqualFold(os.Getenv("USE_EXISTING_CLUSTER"), "true") {
-		Skip("skipping")
-	}
+	suite.Client = require.Kubernetes(GinkgoT())
 
 	logging.SetLogSink(logging.Logrus(GinkgoWriter, "test", 1, 1))
 	log.SetLogger(logging.FromContext(context.Background()))
-
-	By("bootstrapping test environment")
-	suite.Config, suite.Client = require.Kubernetes2(GinkgoT())
-
-	dc, err := discovery.NewDiscoveryClientForConfig(suite.Config)
-	Expect(err).ToNot(HaveOccurred())
-
-	server, err := dc.ServerVersion()
-	Expect(err).ToNot(HaveOccurred())
-
-	suite.ServerVersion, err = version.ParseGeneric(server.GitVersion)
-	Expect(err).ToNot(HaveOccurred())
 })
 
 var _ = AfterSuite(func() {