Refactor patch logic to use openapiv3 instead of v2/swagger

e2e/assertions_test.go

@@ -112,6 +112,36 @@ func AssertHealthySpiceDBClusterFunc(ctx context.Context, namespace string, kcli
 	}
 }
 
+// AssertDeploymentEnvVar asserts that a strategic merge patch actually
+// reached the SpiceDB deployment, rather than merely failing to error.
+// The container check is the primary part of the assertion.
+func AssertDeploymentEnvVar(ctx context.Context, namespace string, kclient kubernetes.Interface) func(owner string, labels map[string]string, envName, envValue string) {
+	return func(owner string, labels map[string]string, envName, envValue string) {
+		ctx, cancel := context.WithCancel(ctx)
+		DeferCleanup(cancel)
+
+		Eventually(func(g Gomega) {
+			deps, err := kclient.AppsV1().Deployments(namespace).List(ctx, metav1.ListOptions{
+				LabelSelector: fmt.Sprintf("%s=%s,%s=%s", metadata.ComponentLabelKey, metadata.ComponentSpiceDBLabelValue, metadata.OwnerLabelKey, owner),
+			})
+			g.Expect(err).To(Succeed())
+			g.Expect(len(deps.Items)).To(Equal(1))
+			deployment := deps.Items[0]
+
+			for k, v := range labels {
+				g.Expect(deployment.GetLabels()).To(HaveKeyWithValue(k, v))
+			}
+
+			containers := deployment.Spec.Template.Spec.Containers
+			g.Expect(len(containers)).To(Equal(1))
+			g.Expect(containers[0].Name).To(Equal("spicedb"))
+			g.Expect(containers[0].Image).ToNot(BeEmpty(),
+				"image was dropped, so containers were replaced instead of merged by name")
+			g.Expect(containers[0].Env).To(ContainElement(corev1.EnvVar{Name: envName, Value: envValue}))
+		}).Should(Succeed())
+	}
+}
+
 func AssertDependentResourceCleanupFunc(ctx context.Context, namespace string, kclient kubernetes.Interface) func(owner, secretName string) {
 	return func(owner, secretName string) {
 		ctx, cancel := context.WithCancel(ctx)

e2e/cluster_test.go

@@ -64,6 +64,7 @@ var _ = Describe("SpiceDBClusters", func() {
 		AssertMigrationJobCleanup      func(owner string)
 		AssertServiceAccount           func(name string, annotations map[string]string, owner string)
 		AssertPDB                      func(name, owner string)
+		AssertDeploymentPatched        func(owner string, labels map[string]string, envName, envValue string)
 		AssertHealthySpiceDBCluster    func(image, owner string, logMatcher types.GomegaMatcher)
 		AssertDependentResourceCleanup func(owner, secretName string)
 		AssertMigrationsCompleted      func(image, migration, phase, name, datastoreEngine string)
@@ -129,6 +130,7 @@ var _ = Describe("SpiceDBClusters", func() {
 		AssertMigrationJobCleanup = AssertMigrationJobCleanupFunc(ctx, testNamespace, kclient)
 		AssertServiceAccount = AssertServiceAccountFunc(ctx, testNamespace, kclient)
 		AssertPDB = AssertPDBFunc(ctx, testNamespace, kclient)
+		AssertDeploymentPatched = AssertDeploymentEnvVar(ctx, testNamespace, kclient)
 		AssertHealthySpiceDBCluster = AssertHealthySpiceDBClusterFunc(ctx, testNamespace, kclient)
 		AssertDependentResourceCleanup = AssertDependentResourceCleanupFunc(ctx, testNamespace, kclient)
 		AssertMigrationsCompleted = AssertMigrationsCompletedFunc(ctx, testNamespace, kclient, client)
@@ -351,6 +353,19 @@ var _ = Describe("SpiceDBClusters", func() {
 								"labels": {
 								  "added": "via-patch"
 								}
+							  },
+							  "spec": {
+								"template": {
+								  "spec": {
+									"containers": [{
+									  "name": "spicedb",
+									  "env": [{
+										"name": "ADDED_VIA_PATCH",
+										"value": "true"
+									  }]
+									}]
+								  }
+								}
 							  }
 							}`),
 						}}
@@ -380,6 +395,11 @@ var _ = Describe("SpiceDBClusters", func() {
 						By("creating the serviceaccount")
 						AssertServiceAccount("spicedb-non-default", map[string]string{"authzed.com/e2e": "true"}, cluster.Name)
 						AssertPDB(cluster.Name+"-spicedb", cluster.Name)
+
+						By("applying the strategic merge patch to the deployment")
+						AssertDeploymentPatched(cluster.Name,
+							map[string]string{"added": "via-patch"},
+							"ADDED_VIA_PATCH", "true")
 					})
 				})
 			})

e2e/databases/cockroach.go

@@ -114,9 +114,17 @@ func (p *CockroachProvider) running(ctx context.Context) error {
 		return err
 	}
 
-	if _, err := p.kclient.AppsV1().StatefulSets(p.namespace).Get(ctx, "cockroachdb", metav1.GetOptions{}); err != nil {
+	statefulSet, err := p.kclient.AppsV1().StatefulSets(p.namespace).Get(ctx, "cockroachdb", metav1.GetOptions{})
+	if err != nil {
 		return err
 	}
 
+	// The StatefulSet existing says nothing about the database accepting
+	// connections. Waiting on a ready replica means a pod that never starts
+	// surfaces here, instead of 5 minutes later as a connection refused.
+	if statefulSet.Status.ReadyReplicas < 1 {
+		return fmt.Errorf("statefulset %s/cockroachdb has no ready replicas", p.namespace)
+	}
+
 	return nil
 }

e2e/databases/database.go

@@ -6,16 +6,20 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"strings"
 	"time"
 
 	"github.com/fluxcd/cli-utils/pkg/kstatus/polling"
 	"github.com/fluxcd/pkg/ssa"
 	//revive:disable:dot-imports convention is dot-import
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/util/yaml"
+	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
 	crClient "sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -64,10 +68,111 @@ func CreateFromManifests(ctx context.Context, namespace, engine string, restConf
 	_, err = resourceManager.ApplyAll(ctx, objs, ssa.DefaultApplyOptions())
 	Expect(err).To(Succeed())
 	By(fmt.Sprintf("waiting for %s to start..", engine))
+	stopWatching := watchForEarlyContainerFailures(ctx, namespace, restConfig)
+	defer stopWatching()
+
 	err = resourceManager.Wait(objs, ssa.WaitOptions{
 		Interval: 1 * time.Second,
 		Timeout:  120 * time.Second,
 	})
 	Expect(err).To(Succeed())
 	By(fmt.Sprintf("%s running", engine))
 }
+
+// earlyFailurePollInterval has to be short relative to the kubelet's restart
+// backoff, which starts around 10s: a container that dies on startup is only
+// observable as its first instance for a few seconds.
+const earlyFailurePollInterval = 2 * time.Second
+
+// watchForEarlyContainerFailures reports the log of a container's *first*
+// instance as soon as that container restarts, and returns a function that
+// stops the watch.
+//
+// The first instance is usually the only one that shows the original cause.
+// Later instances routinely fail for downstream reasons -- a half-written data
+// directory, say -- and by the time a spec fails and the suite dumps state the
+// container has restarted several times, leaving only the derived symptom.
+// Kubernetes retains just the current and previous instance's logs, so the
+// first one has to be captured while it is still the previous.
+func watchForEarlyContainerFailures(ctx context.Context, namespace string, restConfig *rest.Config) func() {
+	k, err := kubernetes.NewForConfig(restConfig)
+	if err != nil {
+		GinkgoWriter.Println("could not create client to watch for early container failures", err)
+		return func() {}
+	}
+
+	pollCtx, cancel := context.WithCancel(ctx)
+	done := make(chan struct{})
+
+	go func() {
+		// This runs outside a spec goroutine, so an unrecovered panic here
+		// would take down the whole suite with an unrelated-looking failure.
+		defer GinkgoRecover()
+		defer close(done)
+
+		reported := map[string]bool{}
+		ticker := time.NewTicker(earlyFailurePollInterval)
+		defer ticker.Stop()
+
+		for {
+			select {
+			case <-pollCtx.Done():
+				return
+			case <-ticker.C:
+			}
+
+			pods, err := k.CoreV1().Pods(namespace).List(pollCtx, metav1.ListOptions{})
+			if err != nil {
+				continue
+			}
+
+			for _, pod := range pods.Items {
+				statuses := append(append([]corev1.ContainerStatus{},
+					pod.Status.InitContainerStatuses...), pod.Status.ContainerStatuses...)
+
+				for _, status := range statuses {
+					key := pod.Name + "/" + status.Name
+					if status.RestartCount == 0 || reported[key] {
+						continue
+					}
+					reported[key] = true
+					reportFirstInstanceLog(pollCtx, k, pod.Namespace, pod.Name, status)
+				}
+			}
+		}
+	}()
+
+	return func() {
+		cancel()
+		<-done
+	}
+}
+
+func reportFirstInstanceLog(ctx context.Context, k kubernetes.Interface, namespace, pod string, status corev1.ContainerStatus) {
+	GinkgoWriter.Printf("container %s/%s restarted (restarts: %d); capturing the log of its first instance\n",
+		namespace, pod+"/"+status.Name, status.RestartCount)
+
+	if t := status.LastTerminationState.Terminated; t != nil {
+		GinkgoWriter.Printf("  first instance: %s (exit %d), ran for %s\n",
+			t.Reason, t.ExitCode, t.FinishedAt.Sub(t.StartedAt.Time))
+	}
+
+	logs, err := k.CoreV1().Pods(namespace).GetLogs(pod, &corev1.PodLogOptions{
+		Container: status.Name,
+		Previous:  true,
+	}).DoRaw(ctx)
+	if err != nil {
+		GinkgoWriter.Printf("  could not read the first instance's log: %v\n", err)
+		return
+	}
+
+	GinkgoWriter.Printf("  first instance log:\n%s\n", indentLines(string(logs), "    "))
+}
+
+func indentLines(s, indent string) string {
+	lines := strings.Split(strings.TrimRight(s, "\n"), "\n")
+	for i, line := range lines {
+		lines[i] = indent + line
+	}
+	return strings.Join(lines, "\n")
+}

e2e/databases/manifests/cockroachdb.yaml

@@ -93,13 +93,6 @@ spec:
               name: grpc
             - containerPort: 8080
               name: http
-          # We recommend that you do not configure a liveness probe on a production environment, as this can impact the availability of production databases.
-          #       livenessProbe:
-          #         httpGet:
-          #           path: "/health"
-          #           port: http
-          #         initialDelaySeconds: 30
-          #         periodSeconds: 5
           readinessProbe:
             httpGet:
               path: "/health?ready=1"

e2e/databases/manifests/mysql.yaml

@@ -16,12 +16,27 @@ spec:
       containers:
         - image: mysql:oracle # Support for arm64 and amd64
           name: mysql
+          # InnoDB's default direct/async I/O fails with EIO against the
+          # local-path volume, which lives on the kind node's overlayfs:
+          # initialization dies on the first sized write to ibdata1, leaving a
+          # half-written data directory that mysqld then refuses to reuse, so
+          # the pod crash-loops forever. Plain buffered I/O avoids it.
+          args:
+            - "--innodb-use-native-aio=0"
+            - "--innodb-flush-method=fsync"
           env:
             - name: MYSQL_ROOT_PASSWORD
               value: password
           ports:
             - containerPort: 3306
               name: mysql
+          # Catch container failures early
+          readinessProbe:
+            tcpSocket:
+              port: mysql
+            initialDelaySeconds: 10
+            periodSeconds: 5
+            failureThreshold: 30
           volumeMounts:
             - name: mysql-disk
               mountPath: /var/lib/mysql
@@ -32,7 +47,10 @@ spec:
         accessModes: ["ReadWriteOnce"]
         resources:
           requests:
-            storage: 100Mi
+            # A freshly initialized datadir is ~105MiB, so this leaves headroom.
+            # Note the provisioner does not enforce this, so it is a statement
+            # of intent rather than a limit.
+            storage: 500Mi
 ---
 apiVersion: v1
 kind: Service

e2e/databases/mysql.go

@@ -121,9 +121,17 @@ func (p *MySQLProvider) running(ctx context.Context) error {
 		return err
 	}
 
-	if _, err := p.kclient.AppsV1().StatefulSets(p.namespace).Get(ctx, "mysql", metav1.GetOptions{}); err != nil {
+	statefulSet, err := p.kclient.AppsV1().StatefulSets(p.namespace).Get(ctx, "mysql", metav1.GetOptions{})
+	if err != nil {
 		return err
 	}
 
+	// The StatefulSet existing says nothing about the database accepting
+	// connections. Waiting on a ready replica means a pod that never starts
+	// surfaces here, instead of 5 minutes later as a connection refused.
+	if statefulSet.Status.ReadyReplicas < 1 {
+		return fmt.Errorf("statefulset %s/mysql has no ready replicas", p.namespace)
+	}
+
 	return nil
 }

e2e/databases/postgres.go

@@ -116,9 +116,17 @@ func (p *PostgresProvider) running(ctx context.Context) error {
 		return err
 	}
 
-	if _, err := p.kclient.AppsV1().StatefulSets(p.namespace).Get(ctx, "postgresql-db", metav1.GetOptions{}); err != nil {
+	statefulSet, err := p.kclient.AppsV1().StatefulSets(p.namespace).Get(ctx, "postgresql-db", metav1.GetOptions{})
+	if err != nil {
 		return err
 	}
 
+	// The StatefulSet existing says nothing about the database accepting
+	// connections. Waiting on a ready replica means a pod that never starts
+	// surfaces here, instead of 5 minutes later as a connection refused.
+	if statefulSet.Status.ReadyReplicas < 1 {
+		return fmt.Errorf("statefulset %s/postgresql-db has no ready replicas", p.namespace)
+	}
+
 	return nil
 }