fix: cluster arch on ClusterIP access, and stranded password secret finalizer

[chideat]

Two independent bugs found while running a version-coverage check against a build of this operator. Both are long-standing on main and both are verified against a live cluster.

They're unrelated but share a discovery path and are each a one-file change, so they're bundled here — happy to split into two PRs if you'd prefer.

1. Cluster arch never starts when access.serviceType is ClusterIP

ensureService switches on Access.ServiceType with cases for NodePort and LoadBalancer only. ServiceType is a closed enum (+kubebuilder:validation:Enum=NodePort;LoadBalancer;ClusterIP) defaulted to ClusterIP, so the default path matches no case and ensureValkeyPodService never runs.

The per-pod service is therefore never created. init_cluster.sh unconditionally runs valkey-helper cluster expose, and Access() requires a service named after the pod to resolve its announce address — so it exits 1 and every node sits in Init:CrashLoopBackOff:

Expose  get service failed  {"target": "ns/drc-<name>-0-0", "error": "services \"drc-<name>-0-0\" not found"}
Expose  expose node port failed  {"error": "services \"drc-<name>-0-0\" not found"}

Net effect: cluster arch only ever comes up on NodePort or LoadBalancer. The default in-cluster ClusterIP mode has never worked. That looks unintended rather than unsupported — Access() has an explicit non-NodePort path that sets announceIp = pod.Status.PodIP.

Failover already gets this right — its ensureService sends everything except NodePort-with-ports to ensureValkeyPodService, which is why failover+ClusterIP works. This change aligns cluster with that behaviour.

cleanUselessService's LoadBalancer/NodePort guard is dropped for the same reason: all three service types now have pod binded services, so scale-down must reclaim them for ClusterIP too. fetchAllPodBindedServices already filters to services carrying a pod-name selector, so headless and instance services are unaffected — and failover's equivalent has no such guard either.

Why CI doesn't catch it: all three arch suites in test/e2e/e2e_test.go are commented // Generate test parameters for all supported versions with ClusterIP service type but then build their parameters with ServiceType: corev1.ServiceTypeNodePort. Only the working path is ever exercised. Worth fixing separately — it would also give cluster arch real ClusterIP coverage.

2. Password secret stranded, wedging namespace deletion

Asymmetric finalizer ordering in user_controller.go:

• the secret gets buf.red/user-finalizer at line 217, while merely reconciling the password secret
• the User gets its finalizer at line 259, only after Handler.Do succeeds

For an instance that never becomes ready these never balance. Handler.Do returns instance is not ready and requeues at line 240, so the secret carries a finalizer while the User carries none.

Deleting the namespace then strands the secret: the User has no finalizer, so it's removed immediately — taking with it the only controller that would ever run the cleanup path that drops the secret's finalizer. Nothing watches the secret afterward, so the namespace stays in Terminating forever:

NamespaceContentRemaining:   Some resources are remaining: secrets. has 1 resource instances
NamespaceFinalizersRemaining: buf.red/user-finalizer in 1 resource instances

The finalizer exists to keep the password readable until the user is dropped from the instance, so it isn't needed until that user exists. It's now added once the User reaches Ready, and only after the User itself is finalized.

That ordering is the invariant, and it's the load-bearing part of the fix: a secret finalizer set while the User has none is unreclaimable, whereas a User finalizer with no secret finalizer is safe — the cleanup path reclaims every secret it owns. So partial failure degrades to safe rather than wedged, and an instance that never becomes ready ends with no finalizers at all and is GC'd normally.

Also drops the ContainsFinalizer clause from the secret update condition — it existed to trigger the finalizer add, and with that moved out it would otherwise fire an r.Update on every reconcile once the finalizer was present.

Testing

go build ./..., go vet, and the unit suite (incl. envtest-backed controller suites) pass on this branch.

Both fixes verified live on a 4-node cluster:

• ClusterIP cluster: a 3-shard instance crash-looped in init on the unfixed build; with this change the same spec reaches Ready in ~90s with cluster_state:ok, 16384 slots assigned, 3 nodes announcing their pod IPs, and the three drc-* per-pod services created by the operator itself.
• Finalizer: an instance left permanently unschedulable produces its acl secret with finalizers=[] (previously ["buf.red/user-finalizer"]), and deleting the namespace now completes in ~60s instead of hanging indefinitely.
• No regression: 7.2 / 8.0 / 8.1 / 9.0 / 9.1 all still reach Ready on failover arch.

Note

Secrets already stranded by an earlier operator still need the finalizer removed by hand — nothing watches them once their User is gone:

kubectl -n <ns> patch secret <name> --type=merge -p '{"metadata":{"finalizers":null}}'

🤖 Generated with Claude Code