Refactor patch logic to use openapiv3 instead of v2/swagger by tstirrat15 · Pull Request #446 · authzed/spicedb-operator

tstirrat15 · 2026-08-31T16:53:13Z

Alternative to #445

Description

Under the old logic, the controller had to load the entirety of the API's swagger to construct the Strategic Patch Merges. This changes things to use the new OpenAPIv3 APIs provided by k8s, which reduces memory usage significantly, bringing down peak RSS from ~250mb to ~80:

Current numbers

phase            retained    +retained        churn      max rss
----------------------------------------------------------------
baseline          4.2 MiB            -            -     85.8 MiB
discovery         4.3 MiB    +90.2 KiB    531.9 KiB     85.8 MiB
controller        8.7 MiB     +4.4 MiB     12.1 MiB     85.8 MiB
openapi          16.1 MiB     +7.4 MiB     38.7 MiB     85.8 MiB

(as measured using the new script)

Changes

Add a patchmeta file that describes a resolver for various GVKs used by the controller
Make ApplyPatch look up the descriptions of the objects using that resolver
Stand up a resolver on startup instead of loading the entire openAPI description

Testing

Review. See that tests pass and are sane.

codecov-commenter · 2026-08-31T16:54:57Z

Codecov Report

❌ Patch coverage is 78.20513% with 17 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
pkg/config/patchmeta.go	83.01%	7 Missing and 2 partials ⚠️
pkg/cmd/run/run.go	0.00%	5 Missing ⚠️
pkg/config/patch.go	50.00%	2 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

tstirrat15

See comments

tstirrat15 · 2026-09-01T16:07:23Z

              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.


Drive-by cleanup

tstirrat15 · 2026-09-01T16:07:36Z

+            # mysql needs ~100mb for startup; this leaves some headroom.
+            storage: 500Mi


This seems to make the tests more stable

tstirrat15 · 2026-09-01T16:08:02Z

+	// 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 {


Some extra defensiveness

tstirrat15 · 2026-09-01T16:08:23Z

+		corev1.SchemeGroupVersion.WithResource("events"),
+		corev1.SchemeGroupVersion.WithResource("persistentvolumeclaims"),


Extra debug output that goes into artifacts

tstirrat15 · 2026-09-01T16:09:08Z

 		}
 	}

-	resources, err := f.OpenAPISchema()


This was responsible for ~100mb of memory usage

tstirrat15 · 2026-09-01T16:09:23Z

+	if err != nil {
+		return err
+	}
+	patchMetaResolver := config.NewV3PatchMetaResolver(openapi3.NewRoot(openAPIV3Client))


We pass this resolver down so that we can fetch the objects as needed

tstirrat15 · 2026-09-01T16:09:49Z

This file is what does the work of fetching those objects from the k8s API

tstirrat15 · 2026-09-01T16:10:02Z

+				// Resolved here, at the only point that needs it, and with the
+				// GVK in hand -- which is what lets the v3 resolver fetch a
+				// single group-version instead of the whole cluster's schema.
+				lookupPatchMeta, err := resolver.LookupPatchMeta(gv.WithKind(*typeMeta.Kind))


And this is where we actually do the lazy fetching

tstirrat15 · 2026-09-01T17:08:12Z

+            - "--innodb-use-native-aio=0"
+            - "--innodb-flush-method=fsync"


This also seems to be salient

we haven't changed any of this in a long time, i'm unclear on why this is needed now. if the explanation here were true, the tests would never have passed?

My assumption is that it's highly dependent on what the backing storage is/does, and therefore sensitive to exactly how CI is set up.

ecordell · 2026-09-01T20:59:08Z

+            - "--innodb-use-native-aio=0"
+            - "--innodb-flush-method=fsync"


we haven't changed any of this in a long time, i'm unclear on why this is needed now. if the explanation here were true, the tests would never have passed?

ecordell · 2026-09-01T21:02:55Z

+// AssertDeploymentPatchedFunc 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 AssertDeploymentPatchedFunc(ctx context.Context, namespace string, kclient kubernetes.Interface) func(owner string, labels map[string]string, envName, envValue string) {


nit: isn't this really AssertDeploymentEnvVar?

ecordell · 2026-09-01T21:04:41Z

+// failure. The database namespaces matter as much as the per-spec ones: a
+// database that never becomes ready fails specs in BeforeEach, and nothing in
+// the test namespaces explains why.
+var dumpNamespacePrefixes = []string{"test", "postgres-", "mysql-", "cockroachdb-"}


Suggested change

var dumpNamespacePrefixes = []string{"test", "postgres-", "mysql-", "cockroachdb-"}

var dumpNamespacePrefixes = []string{"test", "postgres", "mysql", "cockroachdb"}

super nit

ecordell · 2026-09-01T21:08:06Z

+					name = pod.Name + "-" + container.Name + "-previous.log"
+				}
+
+				logs, err := k.CoreV1().Pods(namespace).GetLogs(pod.Name, &corev1.PodLogOptions{


I thought I had written a Ginkgo helper somewhere to tail pod logs we could re-use

Tail in util_test.go? It seems like it's designed for asserting things about the logs on the side, rather than taking log output and directing it somewhere. I could use it but it's awkward.

ecordell · 2026-09-01T21:10:22Z

+	resolver := newTestPatchMetaResolver()
+
+	for _, gvk := range patchedKinds {
+		t.Run(gvk.Kind, func(t *testing.T) {


t.Parallel?

tstirrat15 added 2 commits August 31, 2026 09:30

chore: diagnose high memory usage

2814d64

chore: make apiv3 fix instead

b2c8cb4

tstirrat15 force-pushed the tstirrat/refactor-to-use-openapi-v3 branch 2 times, most recently from 3c0f6f0 to f1fb6ac Compare August 31, 2026 18:48

chore: clean up references to v2

cfe9c0e

tstirrat15 force-pushed the tstirrat/refactor-to-use-openapi-v3 branch 2 times, most recently from bc1b14d to 6a7bfdf Compare August 31, 2026 19:36

tstirrat15 marked this pull request as ready for review August 31, 2026 19:38

tstirrat15 force-pushed the tstirrat/refactor-to-use-openapi-v3 branch 2 times, most recently from 95a3276 to 8105dfd Compare August 31, 2026 20:41

chore: fix builds by moving memprofile

b2651db

tstirrat15 force-pushed the tstirrat/refactor-to-use-openapi-v3 branch 6 times, most recently from 805259a to 7821663 Compare September 1, 2026 16:07

tstirrat15 commented Sep 1, 2026

View reviewed changes

tstirrat15 force-pushed the tstirrat/refactor-to-use-openapi-v3 branch from 7821663 to 2c1a206 Compare September 1, 2026 16:20

chore: fix mysql in CI"

cb33119

tstirrat15 force-pushed the tstirrat/refactor-to-use-openapi-v3 branch from 2c1a206 to cb33119 Compare September 1, 2026 16:59

tstirrat15 commented Sep 1, 2026

View reviewed changes

ecordell reviewed Sep 1, 2026

View reviewed changes

tstirrat15 mentioned this pull request Sep 2, 2026

test: fix e2e #447

Closed

chore: address feedback

18f26e2

ecordell approved these changes Sep 2, 2026

View reviewed changes

tstirrat15 added this pull request to the merge queue Sep 2, 2026

Merged via the queue into main with commit 258dbf9 Sep 2, 2026
10 of 11 checks passed

tstirrat15 deleted the tstirrat/refactor-to-use-openapi-v3 branch September 2, 2026 23:49

github-actions Bot locked and limited conversation to collaborators Sep 2, 2026

		# mysql needs ~100mb for startup; this leaves some headroom.
		storage: 500Mi

		corev1.SchemeGroupVersion.WithResource("events"),
		corev1.SchemeGroupVersion.WithResource("persistentvolumeclaims"),

		- "--innodb-use-native-aio=0"
		- "--innodb-flush-method=fsync"

	var dumpNamespacePrefixes = []string{"test", "postgres-", "mysql-", "cockroachdb-"}
	var dumpNamespacePrefixes = []string{"test", "postgres", "mysql", "cockroachdb"}

Uh oh!

Conversation

tstirrat15 commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Testing

Uh oh!

codecov-commenter commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tstirrat15 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tstirrat15 commented Aug 31, 2026 •

edited

Loading

codecov-commenter commented Aug 31, 2026 •

edited

Loading