Add StatefulSet CacheBackend support

[heymrbox]

Summary

Adds StatefulSet support for managed CacheBackend workloads. deploymentKind: StatefulSet now reconciles an owned StatefulSet, targets it from HPA, reports readiness from ready replicas, and uses volumeClaimTemplates for per-replica PVCs when spec.storage.pvc is set.

Also updates generated RBAC/CRD docs, samples, design docs, controller/unit/envtest coverage, and install-smoke assertions for the StatefulSet persistent-storage path.

Linked issues

CAC-115

Checklist

Vendor-neutral naming (required — see CONTRIBUTING.md)

• No oci / oracle / *.oci.com / oraclecloud.com in any API group, CRD group, proto package, gRPC service/package, Kubernetes namespace, image registry, Helm chart, or Go module path.
• Any cloud-specific (incl. OCI) integration lives in an isolated, optional adapter (pkg/adapters/.../) — never in core controllers, CRD types, the proto contract, or default config.
• No Oracle/OCI domain or namespace in sample manifests, README, or default values.
• Pre-commit naming guard passed (make install-hooks once, then it runs on every commit).

Quality

• make build and make test pass locally.
• make lint clean (gofmt + go vet).
• make manifests generate produces no drift (generated code committed).
• New/changed behavior has unit tests.
• Operator-facing change (CRD columns/fields, .status, CLI, gRPC/HTTP, install bundle/RBAC, samples)? If so, the install-smoke gate asserts it (see CONTRIBUTING.md).
• CI is green.

Contracts (only if touching CRDs or proto)

• Change matches the tech spec (or the spec is updated in the same PR).
• Backward compatibility considered for v1alpha1 consumers (engines, gateway clients).
• If proto/ changed, docs/design/grpc-contract.md is updated to match (the pre-commit hook enforces this).

[linear-code]

CAC-115

[github-actions]

Codex review

Files reviewed

• api/v1alpha1/
  • cachebackend_types.go [reviewed]
• config/crd/bases/
  • inferencecache.io_cachebackends.yaml [skipped — generated]
• config/rbac/
  • role.yaml [skipped — generated]
• config/samples/
  • README.md [reviewed]
  • cachebackend-lmcache.yaml [reviewed]
  • recipe-persistent-cache.yaml [reviewed]
• docs/design/
  • cachebackend-api.md [reviewed]
• docs/reference-stack/scripts/
  • default_install_smoke.sh [reviewed]
• internal/controller/
  • cachebackend_autoscaling_test.go [reviewed]
  • cachebackend_controller.go [reviewed]
  • cachebackend_controller_test.go [reviewed]
  • cachebackend_probe.go [reviewed]
  • cachebackend_storage_test.go [reviewed]
  • integration_test.go [reviewed]

Findings

Blocking

None.

Should-fix

internal/controller/cachebackend_controller.go:1026 — Verify: StatefulSet storage edits after creation are not reconciled or surfaced. The update path intentionally leaves volumeClaimTemplates untouched, but the reconcile path still changes the pod template when spec.storage.pvc is added/changed; that can either fail validation or silently leave storage shape drift while observedGeneration advances.

internal/controller/cachebackend_controller.go:767 — StatefulSet backends still get the Deployment restart-cascade polling cadence. The StatefulSet branch explicitly skips reconcileServerInstance, so this unconditional minServerRestartCascadeInterval requeue creates steady 30s reconciles for every StatefulSet backend with no cascade work to perform.

internal/controller/cachebackend_controller.go:1950 — StatefulSet readiness from ReadyReplicas is not directly tested. The PR claims readiness is reported from ready replicas, but the new tests only assert the initial RolloutInProgress shape; add a unit/envtest case for ReadyReplicas/UpdatedReplicas driving BackendReady and ReplicasUnavailable.

Nit

internal/controller/cachebackend_controller.go:1962 — The managedReadiness godoc still describes only Deployments. It now handles generic workload observations and StatefulSet readyReplicas, so the comment should not keep saying “Deployment” / “available replicas” as the sole contract.

Per-category coverage

1. Vendor-neutral naming: Clean
2. Contract & spec / docs sync: findings above at: internal/controller/cachebackend_controller.go:1026
3. gRPC fail-open semantics: Clean
4. Quality (tests / error wrapping / package placement / generated drift): findings above at: internal/controller/cachebackend_controller.go:767, internal/controller/cachebackend_controller.go:1950, internal/controller/cachebackend_controller.go:1962

Verdict

changes-requested

[heymrbox]

Addressed in a560a88.

• StatefulSet storage edits now surface explicit immutable drift: when the desired volumeClaimTemplates shape differs from the owned live StatefulSet, the controller leaves the StatefulSet untouched, clears the advertised endpoint, and publishes Ready=False / Progressing=False with reason ImmutableStatefulSetStorage.
• The Deployment restart-cascade polling cadence now stays on Deployment-managed backends only, so StatefulSet backends do not self-requeue every 30s for cascade work they cannot perform.
• Added direct tests for StatefulSet ReadyReplicas readiness, StatefulSet no-cascade requeue behavior, and immutable storage drift; updated the CacheBackend design doc with the new status reason.

Verification:

• go test -count=1 ./internal/controller -run "TestReconcileStatefulSet(StorageEditSurfacesImmutableDrift|DoesNotScheduleDeploymentCascadePoll)|TestManagedReadinessForStatefulSetUsesReadyReplicas"
• go test -count=1 ./internal/controller
• make pre-pr
• pre-push make ci gate passed

[github-actions]

Codex review

Files reviewed

• api/v1alpha1

  • [reviewed] cachebackend_types.go

• config/crd/bases

  • [skipped — generated] inferencecache.io_cachebackends.yaml

• config/rbac

  • [skipped — generated] role.yaml

• config/samples

  • [reviewed] README.md
  • [reviewed] cachebackend-lmcache.yaml
  • [reviewed] recipe-persistent-cache.yaml

• docs/design

  • [reviewed] cachebackend-api.md

• docs/reference-stack/scripts

  • [reviewed] default_install_smoke.sh

• internal/controller

  • [reviewed] cachebackend_autoscaling_test.go
  • [reviewed] cachebackend_controller.go
  • [reviewed] cachebackend_controller_test.go
  • [reviewed] cachebackend_probe.go
  • [reviewed] cachebackend_storage_test.go
  • [reviewed] integration_test.go

Findings

Blocking

None.

Should-fix

internal/controller/cachebackend_controller.go:353 — Deployment-to-StatefulSet persistent kind switches silently strand the old shared PVC. Because warnRetainedPVC only runs when spec.storage.pvc is absent, a CR that keeps storage enabled while switching to deploymentKind: StatefulSet leaves <name>-data owner-referenced but unused, with no OrphanedPVCRetained-style visibility or migration signal.

internal/controller/cachebackend_controller.go:1420 — reconcileImmutableStatefulSetStorage clears probe-related status without clearing the probe rate-limit slot. If the operator fixes the immutable-storage drift within the prior probe window, the next managed reconcile can skip /probe with no FunctionalProbeOK condition present and briefly publish Ready from the upstream gates, which is the same stale-rate-limit re-entry hazard the other lifecycle exits explicitly avoid.

Nit

config/samples/recipe-persistent-cache.yaml:49 — The inline comment still says “Single replica is required for a PVC-backed (ReadWriteOnce) backend.” In this sample it is true because deploymentKind: Deployment, but after this PR the general statement is no longer true for StatefulSet-backed PVCs.

Per-category coverage

1. Vendor-neutral naming: Clean
2. Contract & spec / docs sync: findings above at: config/samples/recipe-persistent-cache.yaml:49
3. gRPC fail-open semantics: Clean
4. Quality (tests / error wrapping / package placement / generated drift): findings above at: internal/controller/cachebackend_controller.go:353, internal/controller/cachebackend_controller.go:1420

Verdict

changes-requested

[heymrbox]

Addressed in f5fe047.

• Added a SharedPVCRetained warning when a persistent Deployment backend switches to StatefulSet and the prior shared Deployment PVC is retained but no longer mounted.
• Immutable StatefulSet storage drift now clears the probe rate-limit slot before removing FunctionalProbeOK, so returning to the managed path gets a fresh probe instead of reusing a stale limiter entry.
• Scoped the persistent-cache sample comment to Deployment-backed ReadWriteOnce PVCs and documented the SharedPVCRetained event in the CacheBackend design notes.

Verification:

• go test -count=1 ./internal/controller -run "TestReconcile(StorageKeptViaStatefulSetSwitchWarnsAndKeepsDeploymentPVC|ImmutableStatefulSetStorageClearsProbeRateLimiter)"
• go test -count=1 ./internal/controller
• make pre-pr
• pre-push make ci gate passed

[github-actions]

Codex review

Files reviewed

• api/v1alpha1/
  • cachebackend_types.go [reviewed]
• config/crd/bases/
  • inferencecache.io_cachebackends.yaml [skipped — generated]
• config/rbac/
  • role.yaml [skipped — generated]
• config/samples/
  • README.md [reviewed]
  • cachebackend-lmcache.yaml [reviewed]
  • recipe-persistent-cache.yaml [reviewed]
• docs/design/
  • cachebackend-api.md [reviewed]
• docs/reference-stack/scripts/
  • default_install_smoke.sh [reviewed]
• internal/controller/
  • cachebackend_autoscaling_test.go [reviewed]
  • cachebackend_controller.go [reviewed]
  • cachebackend_controller_test.go [reviewed]
  • cachebackend_probe.go [reviewed]
  • cachebackend_storage_test.go [reviewed]
  • integration_test.go [reviewed]

Findings

Blocking

internal/controller/cachebackend_controller.go:1088 — StatefulSet PVC template drift compares live defaulted PVC specs against un-defaulted desired specs — A real apiserver defaults fields inside volumeClaimTemplates such as volumeMode, so the next reconcile after create can falsely detect immutable storage drift and clear status.endpoint / Ready for an otherwise valid persistent StatefulSet backend.

Should-fix

internal/controller/cachebackend_controller.go:869 — Verify: per-replica StatefulSet PVC retention is intentional and documented — The StatefulSet spec does not set persistentVolumeClaimRetentionPolicy, so template-created PVCs default to Retain on StatefulSet deletion/scale-down; because the CacheBackend owns the StatefulSet, not those PVCs, deleting or kind-switching the CacheBackend can leave storage behind without the Deployment path’s owner-ref lifecycle.

docs/reference-stack/scripts/default_install_smoke.sh:1691 — StatefulSet storage smoke stops before asserting the CacheBackend status surface — The new smoke proves the StatefulSet and PVCs exist, but not that status.endpoint remains published or that Ready reflects StatefulSet readiness after subsequent reconciles, which would miss the false immutable-drift regression above.

Nit

None.

Per-category coverage

1. Vendor-neutral naming: Clean
2. Contract & spec / docs sync: findings above at: internal/controller/cachebackend_controller.go:869
3. gRPC fail-open semantics: Clean
4. Quality (tests / error wrapping / package placement / generated drift): findings above at: internal/controller/cachebackend_controller.go:1088, docs/reference-stack/scripts/default_install_smoke.sh:1691

Verdict

changes-requested

[heymrbox]

Addressed in f9bfbc1.

• Normalized StatefulSet volumeClaimTemplate drift comparison so apiserver-defaulted volumeMode=Filesystem does not trip ImmutableStatefulSetStorage.
• Made per-replica StatefulSet PVC retention explicit as Retain/Retain and documented the retention/reclaim behavior.
• Extended install-smoke to force a follow-up reconcile and assert persistent-cache-sts status.endpoint plus Ready=True/BackendReady stay published.

Verification:

• go test -count=1 ./internal/controller -run "Test(ReconcileStatefulSetPersistentStorageUsesVolumeClaimTemplates|StatefulSetVolumeClaimTemplateComparisonIgnoresAPIServerDefaults)"
• go test -count=1 ./internal/controller
• bash -n docs/reference-stack/scripts/default_install_smoke.sh
• make pre-pr
• pre-push make ci gate passed

[github-actions]

Codex review

Files reviewed

• api/v1alpha1/
  • [reviewed] cachebackend_types.go
• config/crd/bases/
  • [skipped — generated] inferencecache.io_cachebackends.yaml
• config/rbac/
  • [skipped — generated] role.yaml
• config/samples/
  • [reviewed] README.md
  • [reviewed] cachebackend-lmcache.yaml
  • [reviewed] recipe-persistent-cache.yaml
• docs/design/
  • [reviewed] cachebackend-api.md
• docs/reference-stack/scripts/
  • [reviewed] default_install_smoke.sh
• internal/controller/
  • [reviewed] cachebackend_autoscaling_test.go
  • [reviewed] cachebackend_controller.go
  • [reviewed] cachebackend_controller_test.go
  • [reviewed] cachebackend_probe.go
  • [reviewed] cachebackend_storage_test.go
  • [reviewed] integration_test.go

Findings

Blocking

None.

Should-fix

internal/controller/cachebackend_controller.go:1042 — StatefulSet updates do not reconcile PersistentVolumeClaimRetentionPolicy. buildStatefulSet always wants Retain/Retain, but the update branch only carries replicas, serviceName, labels, and pod spec forward; if this mutable field drifts or was created without it, the controller can leave destructive PVC retention semantics in place despite the documented Retain contract.

internal/controller/cachebackend_controller.go:1505 — A single storage-retained-warned annotation suppresses both OrphanedPVCRetained and SharedPVCRetained. A PVC warned for one retention mode will not emit the other warning on a later storage-removal/kind-switch sequence, which conflicts with the documented operator-facing event behavior.

Nit

internal/controller/cachebackend_controller.go:1884 — Comment still says “Deployment that is not yet Available” inside the workload-generic readiness helper. This is stale after adding StatefulSet support and can confuse future readiness changes.

Per-category coverage

1. Vendor-neutral naming: Clean
2. Contract & spec / docs sync: findings above at: internal/controller/cachebackend_controller.go:1042, internal/controller/cachebackend_controller.go:1505
3. gRPC fail-open semantics: Clean
4. Quality (tests / error wrapping / package placement / generated drift): findings above at: internal/controller/cachebackend_controller.go:1042, internal/controller/cachebackend_controller.go:1505, internal/controller/cachebackend_controller.go:1884

Verdict

changes-requested

[heymrbox]

Addressed in ed1b9e3.

• StatefulSet updates now reconcile persistentVolumeClaimRetentionPolicy back to the documented Retain/Retain contract if it drifts or was created before the field was set.
• Split retained-PVC warning guards by reason, so OrphanedPVCRetained and SharedPVCRetained can both fire once for the same retained PVC when it moves through both operator-visible states.
• Updated the stale KV-event gate comment to describe managed workloads instead of only Deployments.

Verification:

• go test -count=1 ./internal/controller -run "TestReconcile(StatefulSetRepairsPVCRetentionPolicy|RetainedPVCWarningsAreReasonScoped)"
• go test -count=1 ./internal/controller
• make pre-pr
• pre-push make ci gate passed

[github-actions]

Codex review

Files reviewed

• api/v1alpha1/
  • cachebackend_types.go [reviewed]
• config/crd/bases/
  • inferencecache.io_cachebackends.yaml [skipped — generated]
• config/rbac/
  • role.yaml [skipped — generated]
• config/samples/
  • README.md [reviewed]
  • cachebackend-lmcache.yaml [reviewed]
  • recipe-persistent-cache.yaml [reviewed]
• docs/design/
  • cachebackend-api.md [reviewed]
• docs/reference-stack/scripts/
  • default_install_smoke.sh [reviewed]
• internal/controller/
  • cachebackend_autoscaling_test.go [reviewed]
  • cachebackend_controller.go [reviewed]
  • cachebackend_controller_test.go [reviewed]
  • cachebackend_probe.go [reviewed]
  • cachebackend_storage_test.go [reviewed]
  • integration_test.go [reviewed]

Findings

Blocking

internal/controller/cachebackend_controller.go:627 — Switching to StatefulSet deletes the owned Deployment before the StatefulSet replacement has been successfully applied; if applyStatefulSet fails, this path can return before updateManagedStatus and leave the previous endpoint/Ready status published for a workload it just deleted. The reverse path at internal/controller/cachebackend_controller.go:663 has the same issue for StatefulSet→Deployment; clear status on replacement failure or stage the replacement so status never advertises a missing/foreign workload.

Should-fix

None.

Nit

internal/controller/cachebackend_controller.go:259 — The Reconcile godoc still says managed backends are wrapped into a Deployment + Service. With StatefulSet now supported, this should say Deployment or StatefulSet to keep the controller contract comment current.

docs/design/cachebackend-api.md:197 — The KV-event readiness section still says “Once the managed cache-backend Deployment is Available”. The surrounding text now supports StatefulSet readyReplicas, so this should be generalized to the managed workload becoming serving/ready.

Per-category coverage

1. Vendor-neutral naming: Clean
2. Contract & spec / docs sync: findings above at: internal/controller/cachebackend_controller.go:259, docs/design/cachebackend-api.md:197
3. gRPC fail-open semantics: Clean
4. Quality (tests / error wrapping / package placement / generated drift): findings above at: internal/controller/cachebackend_controller.go:627

Verdict

changes-requested

[heymrbox]

Addressed in e6b4535.

• Kind-switch replacement failures now still run the managed status pass with an empty endpoint and non-Ready workload observation, so a deleted old workload is not advertised if the replacement apply fails.
• Added regression coverage for both Deployment→StatefulSet and StatefulSet→Deployment replacement failures.
• Updated the controller and CacheBackend API docs to say managed workloads can be Deployment or StatefulSet.

Verification:

• go test -count=1 ./internal/controller -run "TestReconcile(DeploymentToStatefulSetApplyFailureClearsStatus|StatefulSetToDeploymentApplyFailureClearsStatus)"
• go test -count=1 ./internal/controller
• make pre-pr
• pre-push make ci gate passed

[github-actions]

Codex review

Files reviewed

api/v1alpha1/

• cachebackend_types.go [reviewed]

config/crd/bases/

• inferencecache.io_cachebackends.yaml [skipped — generated]

config/rbac/

• role.yaml [skipped — generated]

config/samples/

• README.md [reviewed]
• cachebackend-lmcache.yaml [reviewed]
• recipe-persistent-cache.yaml [reviewed]

docs/design/

• cachebackend-api.md [reviewed]

docs/reference-stack/scripts/

• default_install_smoke.sh [reviewed]

internal/controller/

• cachebackend_autoscaling_test.go [reviewed]
• cachebackend_controller.go [reviewed]
• cachebackend_controller_test.go [reviewed]
• cachebackend_probe.go [reviewed]
• cachebackend_storage_test.go [reviewed]
• integration_test.go [reviewed]

Findings

Blocking

None.

Should-fix

api/v1alpha1/cachebackend_types.go:567 — status.capacity godoc still says it reports the bound PVC capacity whenever spec.storage.pvc is set, but the new StatefulSet path intentionally leaves capacity empty. This feeds the generated CRD/operator docs, so the API contract should match the new Deployment-only capacity behavior.

api/v1alpha1/cachebackend_types.go:637 — status.firstAvailableAt still describes a workload first reporting Available and later says a recreated managed Deployment keeps the prior anchor. StatefulSet readiness now uses ready replicas, and the design doc updated this to “serving”; the API field comment should not remain Deployment-specific.

api/v1alpha1/cachebackend_types.go:645 — status.observedServerInstance is still described as the controller’s cascade baseline generally, while the implementation clears it for StatefulSet backends. The comment’s inert list also omits StatefulSet, so generated API docs imply this recovery signal applies where it no longer does.

internal/controller/cachebackend_controller.go:745 — StatefulSet backends skip the cache-server restart cascade entirely. Since this PR makes StatefulSet a managed backend that engines can be injected against, a StatefulSet pod restart/replacement leaves existing LMCache engine clients with the same stale socket/EPIPE failure the Deployment cascade was added to repair.

Nit

None.

Per-category coverage

1. Vendor-neutral naming: Clean
2. Contract & spec / docs sync: findings above at: api/v1alpha1/cachebackend_types.go:567, api/v1alpha1/cachebackend_types.go:637, api/v1alpha1/cachebackend_types.go:645
3. gRPC fail-open semantics: Clean
4. Quality (tests / error wrapping / package placement / generated drift): findings above at: internal/controller/cachebackend_controller.go:745

Verdict

changes-requested

[heymrbox]

Addressed in 2484b08.

• StatefulSet-managed backend pods now participate in the same server-instance cascade path as Deployment-managed backends. The observer authenticates StatefulSet pods through their direct ownerRef, tracks their <pod-uid>:<restart-sum> identity, and still rolls injected engine Deployments on a real cache-server replacement.
• Kept the periodic server-instance poll for StatefulSet backends so in-place cache-server container restarts are observable there too.
• Updated the API/design docs and regenerated the CacheBackend CRD descriptions for capacity, firstAvailableAt, and observedServerInstance.
• Added regression coverage for a StatefulSet cache-server pod replacement cascading the injected engine Deployment, and updated the old StatefulSet skip/clear expectations.

Verification:

• go test -count=1 ./internal/controller -run "TestReconcileServerInstance_StatefulSetPodReplacementCascadesEngineDeployment|TestReconcileStatefulSetSchedulesServerInstancePoll|TestReconcileSwitchToStatefulSetShedsDeployment"
• go test -count=1 ./internal/controller
• make pre-pr
• pre-push make ci gate passed

[github-actions]

Codex review

Files reviewed

• api/v1alpha1
  • [reviewed] api/v1alpha1/cachebackend_types.go
• config/crd/bases
  • [skipped — generated] config/crd/bases/inferencecache.io_cachebackends.yaml
• config/rbac
  • [skipped — generated] config/rbac/role.yaml
• config/samples
  • [reviewed] config/samples/README.md
  • [reviewed] config/samples/cachebackend-lmcache.yaml
  • [reviewed] config/samples/recipe-persistent-cache.yaml
• docs/design
  • [reviewed] docs/design/cachebackend-api.md
• docs/reference-stack/scripts
  • [reviewed] docs/reference-stack/scripts/default_install_smoke.sh
• internal/controller
  • [reviewed] internal/controller/cachebackend_autoscaling_test.go
  • [reviewed] internal/controller/cachebackend_controller.go
  • [reviewed] internal/controller/cachebackend_controller_test.go
  • [reviewed] internal/controller/cachebackend_probe.go
  • [reviewed] internal/controller/cachebackend_server_restart.go
  • [reviewed] internal/controller/cachebackend_server_restart_test.go
  • [reviewed] internal/controller/cachebackend_storage_test.go
  • [reviewed] internal/controller/integration_test.go

Findings

Blocking

None.

Should-fix

docs/design/cachebackend-api.md:28 — The spec table still says storage.pvc.size is “Mutable upward only” without qualifying that this only applies to Deployment-backed shared PVCs. For StatefulSet backends, the new controller path treats volumeClaimTemplate shape changes, including size changes, as immutable drift and reports Ready=False/ImmutableStatefulSetStorage, so this table contradicts the behavior documented later.

docs/design/cachebackend-api.md:29 — The storageClassName row says later edits are ignored/logged, which is only true for the Deployment shared-PVC path. On StatefulSet backends, changing the storage class changes the desired volumeClaimTemplate spec and trips the immutable-storage condition instead.

Nit

None.

Per-category coverage

1. Vendor-neutral naming: Clean
2. Contract & spec / docs sync: findings above at: docs/design/cachebackend-api.md:28, docs/design/cachebackend-api.md:29
3. gRPC fail-open semantics: Clean
4. Quality (tests / error wrapping / package placement / generated drift): Clean

Verdict

changes-requested

[heymrbox]

Addressed in 49c82e5.

• Updated the CacheBackend API spec table so storage.pvc.size and storage.pvc.storageClassName describe the two managed storage paths separately.
• Deployment-backed shared PVCs still document the upward-size patch / immutable StorageClass behavior.
• StatefulSet-backed per-replica PVCs now explicitly document that size or StorageClass changes alter volumeClaimTemplates and surface as Ready=False/ImmutableStatefulSetStorage instead of being applied in place.

Verification:

• make pre-pr
• pre-push make ci gate passed

[github-actions]

Codex review

Files reviewed

• api/v1alpha1
  • api/v1alpha1/cachebackend_types.go [reviewed]
• config/crd/bases
  • config/crd/bases/inferencecache.io_cachebackends.yaml [skipped — generated]
• config/rbac
  • config/rbac/role.yaml [skipped — generated]
• config/samples
  • config/samples/README.md [reviewed]
  • config/samples/cachebackend-lmcache.yaml [reviewed]
  • config/samples/recipe-persistent-cache.yaml [reviewed]
• docs/design
  • docs/design/cachebackend-api.md [reviewed]
• docs/reference-stack/scripts
  • docs/reference-stack/scripts/default_install_smoke.sh [reviewed]
• internal/controller
  • internal/controller/cachebackend_autoscaling_test.go [reviewed]
  • internal/controller/cachebackend_controller.go [reviewed]
  • internal/controller/cachebackend_controller_test.go [reviewed]
  • internal/controller/cachebackend_probe.go [reviewed]
  • internal/controller/cachebackend_server_restart.go [reviewed]
  • internal/controller/cachebackend_server_restart_test.go [reviewed]
  • internal/controller/cachebackend_storage_test.go [reviewed]
  • internal/controller/integration_test.go [reviewed]

Findings

Blocking

None.

Should-fix

internal/controller/cachebackend_controller.go:636 — Immutable StatefulSet storage drift returns before HPA reconciliation. If the same edit removes or changes spec.autoscaling, the old owned HPA remains active and can keep scaling the retained StatefulSet even though the controller has published a terminal storage-drift condition.

internal/controller/cachebackend_controller.go:1444 — The immutable-storage path clears status.observedServerInstance while leaving the live StatefulSet running, and the caller returns before reconcileServerInstance can run. Existing injected engine pods can still be pointed at that Service, so a cache-server pod replacement during this drift state will not trigger the restart cascade that normally recovers stale LMCache sockets.

Nit

None.

Per-category coverage

1. Vendor-neutral naming: Clean
2. Contract & spec / docs sync: findings above at: internal/controller/cachebackend_controller.go:1444
3. gRPC fail-open semantics: Clean
4. Quality (tests / error wrapping / package placement / generated drift): findings above at: internal/controller/cachebackend_controller.go:636, internal/controller/cachebackend_controller.go:1444

Verdict

changes-requested

[heymrbox]

Addressed in 9564625.

• Immutable StatefulSet storage drift now still reconciles the owned HPA, so clearing or changing spec.autoscaling is not skipped while the controller is publishing Ready=False/ImmutableStatefulSetStorage.
• The drift path no longer clears the server-instance latch/shadow for a live StatefulSet. It keeps the normal server-instance observer and periodic poll active, so cache-server restarts during drift can still cascade to injected engine Deployments.
• Added regressions for both cases: HPA cleanup during drift, and server restart cascade during drift.

Verification:

• go test -count=1 ./internal/controller -run "TestReconcile(StatefulSetImmutableStorageDriftStillCleansUpHPA|ServerInstance_ImmutableStatefulSetStorageDriftStillCascades)"
• go test -count=1 ./internal/controller
• make pre-pr
• pre-push make ci gate passed

[github-actions]

Codex review

Files reviewed

• api/v1alpha1

  • cachebackend_types.go [reviewed]

• config/crd/bases

  • inferencecache.io_cachebackends.yaml [skipped — generated]

• config/rbac

  • role.yaml [skipped — generated]

• config/samples

  • README.md [reviewed]
  • cachebackend-lmcache.yaml [reviewed]
  • recipe-persistent-cache.yaml [reviewed]

• docs/design

  • cachebackend-api.md [reviewed]

• docs/reference-stack/scripts

  • default_install_smoke.sh [reviewed]

• internal/controller

  • cachebackend_autoscaling_test.go [reviewed]
  • cachebackend_controller.go [reviewed]
  • cachebackend_controller_test.go [reviewed]
  • cachebackend_probe.go [reviewed]
  • cachebackend_server_restart.go [reviewed]
  • cachebackend_server_restart_test.go [reviewed]
  • cachebackend_storage_test.go [reviewed]
  • integration_test.go [reviewed]

Findings

Blocking

None.

Should-fix

internal/controller/cachebackend_controller.go:1326 — StatefulSet PVC name length is not validated for the new volumeClaimTemplates path. The existing admission guard only accounts for the Deployment PVC name <cachebackend>-data, but StatefulSet PVCs are generated as <template-name>-<statefulset-name>-<ordinal>; a name that passes today can still make per-replica PVC creation fail and leave the StatefulSet stuck.

Nit

None.

Per-category coverage

1. Vendor-neutral naming: Clean
2. Contract & spec / docs sync: Clean
3. gRPC fail-open semantics: Clean
4. Quality (tests / error wrapping / package placement / generated drift): findings above at: internal/controller/cachebackend_controller.go:1326

Verdict

changes-requested

[heymrbox]

Addressed in 88509f4.

• Added adapter-aware admission validation for persistent StatefulSet backends: the validator now checks the actual generated per-replica PVC name shape, <volumeClaimTemplate>-<statefulset>-<ordinal>, against the Kubernetes 253-character object-name limit.
• The check uses the selected runtime adapter’s declared DataVolume name instead of hard-coding the current LMCache cache-data template name.
• Added a regression where the CacheBackend name passes the Deployment shared-PVC check but would make the StatefulSet PVC name too long.

Verification:

• go test -count=1 ./internal/webhook/v1alpha1 -run TestValidator_StorageWithOverlongNameRejected
• go test -count=1 ./internal/webhook/v1alpha1
• make pre-pr
• pre-push make ci gate passed

[github-actions]

Codex review

Files reviewed

• api/v1alpha1

  • [reviewed] api/v1alpha1/cachebackend_types.go

• config/crd/bases

  • [skipped — generated] config/crd/bases/inferencecache.io_cachebackends.yaml

• config/rbac

  • [skipped — generated] config/rbac/role.yaml

• config/samples

  • [reviewed] config/samples/README.md
  • [reviewed] config/samples/cachebackend-lmcache.yaml
  • [reviewed] config/samples/recipe-persistent-cache.yaml

• docs/design

  • [reviewed] docs/design/cachebackend-api.md

• docs/reference-stack/scripts

  • [reviewed] docs/reference-stack/scripts/default_install_smoke.sh

• internal/controller

  • [reviewed] internal/controller/cachebackend_autoscaling_test.go
  • [reviewed] internal/controller/cachebackend_controller.go
  • [reviewed] internal/controller/cachebackend_controller_test.go
  • [reviewed] internal/controller/cachebackend_probe.go
  • [reviewed] internal/controller/cachebackend_server_restart.go
  • [reviewed] internal/controller/cachebackend_server_restart_test.go
  • [reviewed] internal/controller/cachebackend_storage_test.go
  • [reviewed] internal/controller/integration_test.go

• internal/webhook/v1alpha1

  • [reviewed] internal/webhook/v1alpha1/cachebackend_webhook.go
  • [reviewed] internal/webhook/v1alpha1/cachebackend_webhook_test.go

Findings

Blocking

None.

Should-fix

internal/webhook/v1alpha1/cachebackend_webhook.go:913 — Document the new StatefulSet PVC-name admission rule — This adds a user-visible validation failure for persistent StatefulSet backends, but docs/design/cachebackend-api.md’s validating rules table has no row for it. Operators reading the design doc will not see why a schema-valid StatefulSet storage spec can be rejected for generated PVC name length.

Nit

None.

Per-category coverage

1. Vendor-neutral naming: Clean
2. Contract & spec / docs sync: findings above at: internal/webhook/v1alpha1/cachebackend_webhook.go:913
3. gRPC fail-open semantics: Clean
4. Quality (tests / error wrapping / package placement / generated drift): Clean

Verdict

changes-requested

[heymrbox]

Addressed in b0e082a.

• Added the StatefulSet generated-PVC-name validation to the CacheBackend design doc’s validating rules table.
• The row documents both child PVC name shapes: Deployment shared PVCs as <cachebackend-name>-data, and StatefulSet per-replica PVCs as <volumeClaimTemplate>-<statefulset-name>-<ordinal> using the selected runtime adapter’s DataVolume name.

Verification:

• make pre-pr
• pre-push make ci gate passed

[github-actions]

Codex review

Files reviewed

• api/v1alpha1
  • [reviewed] api/v1alpha1/cachebackend_types.go
• config/crd/bases
  • [skipped — generated] config/crd/bases/inferencecache.io_cachebackends.yaml
• config/rbac
  • [skipped — generated] config/rbac/role.yaml
• config/samples
  • [reviewed] config/samples/README.md
  • [reviewed] config/samples/cachebackend-lmcache.yaml
  • [reviewed] config/samples/recipe-persistent-cache.yaml
• docs/design
  • [reviewed] docs/design/cachebackend-api.md
• docs/reference-stack/scripts
  • [reviewed] docs/reference-stack/scripts/default_install_smoke.sh
• internal/controller
  • [reviewed] internal/controller/cachebackend_autoscaling_test.go
  • [reviewed] internal/controller/cachebackend_controller.go
  • [reviewed] internal/controller/cachebackend_controller_test.go
  • [reviewed] internal/controller/cachebackend_probe.go
  • [reviewed] internal/controller/cachebackend_server_restart.go
  • [reviewed] internal/controller/cachebackend_server_restart_test.go
  • [reviewed] internal/controller/cachebackend_storage_test.go
  • [reviewed] internal/controller/integration_test.go
• internal/webhook/v1alpha1
  • [reviewed] internal/webhook/v1alpha1/cachebackend_webhook.go
  • [reviewed] internal/webhook/v1alpha1/cachebackend_webhook_test.go

Findings

Blocking

None.

Should-fix

None.

Nit

None.

Per-category coverage

1. Vendor-neutral naming: Clean
2. Contract & spec / docs sync: Clean
3. gRPC fail-open semantics: Clean
4. Quality (tests / error wrapping / package placement / generated drift): Clean

Verdict

approve