CAC-260: Report SGLang HiCache readiness from engine Pods

[fredericsun]

Summary

Report readiness for engine-local SGLang HiCache CacheBackends from the state of their selected engine Pods.

Ready=True means that every participating engine Pod:

• Was successfully injected from the current CacheBackend name, UID, and generation.
• Is currently Kubernetes Ready.

This confirms that the current HiCache configuration has reached serviceable engine Pods. It does not claim that a HiCache host-tier write/read round trip occurred, or distinguish a GPU hit from a host-tier read.

The controller remains observation-only for engine-local backends. It does not create, patch, restart, or automatically roll out the user-owned engine workload.

Readiness contract

Engine Pod state	Ready	Progressing	Degraded	Reason
No active matching Pods	False	True	False	AwaitingEnginePods
All active matching Pods explicitly skipped injection	True	False	False	AllEnginePodsSkipped
A participating Pod lacks a valid injection receipt	False	False	True	EnginePodsNotInjected
A receipt has another backend identity or a future generation	False	False	True	EnginePodsInjectionMismatch
A participating Pod carries an older backend generation	False	True	False	EnginePodsRolloutInProgress
All receipts are current, but a participating Pod is not Kubernetes Ready	False	False	True	EnginePodsUnavailable
All participating Pods have current receipts and are Kubernetes Ready	True	False	False	EnginePodsReady

Terminating and terminal (Succeeded or Failed) Pods are excluded.

An explicitly skipped Pod is excluded only when both the operator skip annotation and the webhook-verified skip receipt are present.

If the controller cannot list the live Pod set, it returns an error and preserves the previously published readiness verdict rather than replacing it with a potentially incorrect condition.

After a CacheBackend spec update, existing Pods retain their old injection generation. The backend therefore remains Ready=False/Progressing=True until the workload owner rolls those Pods.

This PR adds no cluster-wide Pod watch. A non-ready backend is polled every five seconds and returns to the existing matched-Pod reconciliation cadence after convergence.

Change breakdown

1. webhook: stamp CacheBackend injection generation

   Adds the inferencecache.io/injected-generation annotation to the existing injection receipt.

   Every successful Pod injection now records:

   • inferencecache.io/injected-by
   • inferencecache.io/injected-by-uid
   • inferencecache.io/injected-generation

   The generation identifies the exact CacheBackend spec rendered into the Pod. Fail-open and explicit-skip paths remove all three injection annotations so a Pod cannot retain a stale or partial receipt.

   Unit and envtest coverage verify successful stamping, cleanup, and skip behavior.

2. controller: report engine-local backend readiness

   Adds the engine-local readiness evaluator and routes SGLangHiCache reconciliation through it.

   The controller:

   • Lists selector-matched engine Pods in the CacheBackend namespace.
   • Excludes terminating, terminal, and webhook-verified skipped Pods.
   • Validates the backend name, UID, and generation receipt.
   • Checks Kubernetes Pod readiness.
   • Publishes Ready, Progressing, and Degraded with stable reason strings.
   • Keeps status.endpoint empty and removes server-backed health conditions.
   • Preserves the previous readiness verdict when the Pod list cannot be observed.
   • Continues to create no Deployment, Service, HPA, or engine workload.

   Table-driven tests cover no-Pod, all-skipped, missing receipt, mismatched identity, stale generation, unavailable Pod, converged, mixed-Pod, and Pod-list error cases.

3. test/docs: cover SGLang HiCache readiness contract

   Updates the API documentation, generated CRD descriptions, sample guidance, and engine-binding documentation to describe the new readiness semantics and data-plane boundary.

   The installed-cluster smoke gate now verifies:

   • The initial Ready=False/AwaitingEnginePods state.
   • Progressing=True and Degraded=False while no engine Pod exists.
   • No managed Deployment, Service, HPA, or endpoint is created.
   • Real Pod admission injects the complete HiCache CLI contract.
   • The admitted Pod receives the current backend name, UID, and generation receipt.
   • A persisted lightweight Pod becomes Kubernetes Ready.
   • The backend converges to Ready=True/EnginePodsReady.
   • Progressing=False, Degraded=False, and the matched Pod count is correct.

   The smoke fixture does not run SGLang or perform KV-cache I/O.

Out of scope

• HiCache host-tier or L3 data-plane probes.
• HiCache L3 storage configuration.
• Automatically rolling or restarting engine workloads.
• A cluster-wide Pod watch.

Linked issues

CAC-260

Validation

Passed locally:

• make test
• make lint
• make manifests generate — no generated drift
• make verify-docs-sync DOCS_SYNC_BASE=origin/main
• Pre-push make ci
  • Vendor-neutral naming checks
  • gofmt
  • go vet
  • golangci-lint
  • Prometheus rule validation
  • Documentation synchronization tests
  • Race tests
  • Go binary builds
  • Generated-code checks

verify-golden-vectors was explicitly skipped by make ci because the local Python environment does not have xxhash.

The full installed-cluster smoke was not run locally for this branch. The smoke gate is updated, but this PR does not claim a real SGLang runtime or HiCache data-plane test.

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 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.

Quality

• make build and make test pass locally.
• make lint clean.
• make manifests generate produces no drift.
• New and changed behavior has unit tests.
• The install-smoke gate asserts the operator-facing status and admission behavior.
• CI is green.

Contracts

• The CacheBackend design documentation is updated with the readiness contract.
• Backward compatibility was considered for v1alpha1 consumers. The CRD shape remains compatible; this begins publishing standard readiness conditions for SGLangHiCache.
• proto/ is unchanged, so docs/design/grpc-contract.md does not require an update.

[linear-code]

CAC-260

[github-actions]

Codex review

Files reviewed

• api/
  • [reviewed] api/v1alpha1/cachebackend_types.go
• config/crd/
  • [skipped — generated] config/crd/bases/inferencecache.io_cachebackends.yaml
• config/samples/
  • [reviewed] config/samples/README.md
• docs/concepts/
  • [reviewed] docs/concepts/cachebackend-engine-binding.md
• 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_controller.go
  • [reviewed] internal/controller/cachebackend_controller_test.go
  • [reviewed] internal/controller/cachebackend_engine_local_readiness.go
  • [reviewed] internal/controller/cachebackend_engine_local_readiness_test.go
• internal/webhook/pod/
  • [reviewed] internal/webhook/pod/envtest_integration_test.go
  • [reviewed] internal/webhook/pod/podinjector.go
  • [reviewed] internal/webhook/pod/podinjector_test.go

Findings

Blocking

None.

Should-fix

None.

Nit

• internal/webhook/pod/podinjector.go:393 — Update the comment to include injected-generation in the receipt. The implementation now stamps and clears three injection annotations, while this explanation still names only the original pair.
• internal/webhook/pod/podinjector.go:509 — Refresh the failOpen documentation to describe the complete name/UID/generation receipt. Its current wording says only AnnotationInjectedBy is stripped, which no longer accurately documents the helper’s contract.

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/webhook/pod/podinjector.go:393, internal/webhook/pod/podinjector.go:509

Verdict

approve-with-nits

[github-actions]

Codex review

Files reviewed

• api/
  • [reviewed] api/v1alpha1/cachebackend_types.go
• config/
  • [skipped — generated] config/crd/bases/inferencecache.io_cachebackends.yaml
  • [reviewed] config/samples/README.md
• docs/
  • [reviewed] docs/concepts/cachebackend-engine-binding.md
  • [reviewed] docs/design/cachebackend-api.md
  • [reviewed] docs/reference-stack/scripts/default_install_smoke.sh
• internal/controller/
  • [reviewed] internal/controller/cachebackend_controller.go
  • [reviewed] internal/controller/cachebackend_controller_test.go
  • [reviewed] internal/controller/cachebackend_engine_local_readiness.go
  • [reviewed] internal/controller/cachebackend_engine_local_readiness_test.go
• internal/webhook/pod/
  • [reviewed] internal/webhook/pod/envtest_integration_test.go
  • [reviewed] internal/webhook/pod/podinjector.go
  • [reviewed] internal/webhook/pod/podinjector_test.go

Findings

Blocking

None.

Should-fix

• internal/controller/cachebackend_engine_local_readiness.go:181 — Mixed invalid receipts use the wrong documented precedence — when one Pod lacks a receipt and another has a mismatched receipt, this branch reports EnginePodsInjectionMismatch, although the contract at docs/design/cachebackend-api.md:266 says any incomplete receipt yields EnginePodsNotInjected. Reorder the missing/mismatched branches or explicitly document and test the intended precedence.

Nit

None.

Per-category coverage

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

Verdict

changes-requested

[github-actions]

Codex review

Files reviewed

• api/v1alpha1/
  • [reviewed] cachebackend_types.go
• config/crd/bases/
  • [skipped — generated] inferencecache.io_cachebackends.yaml
• config/samples/
  • [reviewed] README.md
• docs/concepts/
  • [reviewed] cachebackend-engine-binding.md
• docs/design/
  • [reviewed] cachebackend-api.md
• docs/reference-stack/scripts/
  • [reviewed] default_install_smoke.sh
• internal/controller/
  • [reviewed] cachebackend_controller.go
  • [reviewed] cachebackend_controller_test.go
  • [reviewed] cachebackend_engine_local_readiness.go
  • [reviewed] cachebackend_engine_local_readiness_test.go
• internal/webhook/pod/
  • [reviewed] envtest_integration_test.go
  • [reviewed] podinjector.go
  • [reviewed] podinjector_test.go

Findings

Blocking

• internal/controller/cachebackend_engine_local_readiness.go:162 — The controller treats user-writable name/UID/generation annotations as proof that injection succeeded. Because the webhook has failurePolicy=Ignore and all three values are readable, a Pod creator can prepopulate a current receipt while the webhook is unavailable, yielding Ready=True/EnginePodsReady without any HiCache configuration being injected.
• internal/controller/cachebackend_engine_local_readiness.go:140 — The “webhook-verified” skip receipt is likewise forgeable during webhook failure. A matching Pod carrying the two public skip annotations is excluded, and an all-forged-skipped set produces Ready=True/AllEnginePodsSkipped, so the published verdict does not satisfy the stated verification contract.

Should-fix

• internal/controller/cachebackend_engine_local_readiness_test.go:23 — Tests cover malformed and mismatched receipts but not Pods that carry a syntactically current forged receipt or forged skip pair after bypassing admission. Add coverage for the webhook-unavailable path once the receipt verification mechanism is corrected.

Nit

None.

Per-category coverage

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

Verdict

changes-requested

[github-actions]

Codex review

Files reviewed

• api/v1alpha1/
  • [reviewed] cachebackend_types.go
• config/crd/bases/
  • [skipped — generated] inferencecache.io_cachebackends.yaml
• config/samples/
  • [reviewed] README.md
• docs/concepts/
  • [reviewed] cachebackend-engine-binding.md
• docs/design/
  • [reviewed] cachebackend-api.md
• docs/reference-stack/scripts/
  • [reviewed] default_install_smoke.sh
• internal/controller/
  • [reviewed] cachebackend_controller.go
  • [reviewed] cachebackend_controller_test.go
  • [reviewed] cachebackend_engine_local_readiness.go
  • [reviewed] cachebackend_engine_local_readiness_test.go
• internal/webhook/pod/
  • [reviewed] envtest_integration_test.go
  • [reviewed] podinjector.go
  • [reviewed] podinjector_test.go

Findings

Blocking

• internal/controller/cachebackend_engine_local_readiness.go:146 — A Pod is excluded whenever its user-controlled skip-inject annotation is truthy, without requiring the webhook-verified inject-skipped: skip-inject-annotation receipt. This violates the stated readiness contract and lets an uninjected or unavailable matching Pod produce Ready=True/AllEnginePodsSkipped.

Should-fix

• docs/design/cachebackend-api.md:273 — The design documentation explicitly describes an unverified skip annotation as sufficient, contradicting the PR’s contract that both the operator annotation and webhook skip receipt are required. Update the table and explanatory text to match the implemented contract after fixing the evaluator.
• internal/controller/cachebackend_engine_local_readiness_test.go:96 — This test positively requires the incorrect behavior by expecting a Pod without the verified skip receipt to be excluded. It should instead expect EnginePodsNotInjected, preserving coverage of the required two-annotation rule.

Nit

None.

Per-category coverage

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

Verdict

changes-requested

[fredericsun]

#171 (comment)
skip-inject is the operator-authoritative opt-out, so an opted-out Pod is intentionally excluded from engine-local cache readiness. inject-skipped is operational metadata used for the SkippedByOperator Event; it is not webhook-authenticated and remains forgeable under failurePolicy=Ignore. Requiring the pair would not close the bypass and would contradict the documented trust model. The readiness contract was intentionally updated in 5d4671e: all explicitly opted-out Pods yield AllEnginePodsSkipped, while participating Pods must carry a current receipt, converge under the HiCache adapter, and be Kubernetes Ready.

[github-actions]

Codex review

Files reviewed

• api/v1alpha1/
  • [reviewed] cachebackend_types.go
• config/crd/bases/
  • [skipped — generated] inferencecache.io_cachebackends.yaml
• config/samples/
  • [reviewed] README.md
• docs/concepts/
  • [reviewed] cachebackend-engine-binding.md
• docs/design/
  • [reviewed] cachebackend-api.md
• docs/reference-stack/scripts/
  • [reviewed] default_install_smoke.sh
• internal/controller/
  • [reviewed] cachebackend_controller.go
  • [reviewed] cachebackend_controller_test.go
  • [reviewed] cachebackend_engine_local_readiness.go
  • [reviewed] cachebackend_engine_local_readiness_test.go
• internal/webhook/pod/
  • [reviewed] envtest_integration_test.go
  • [reviewed] podinjector.go
  • [reviewed] podinjector_test.go

Findings

Blocking

• internal/controller/cachebackend_engine_local_readiness.go:148 — Pods are excluded based solely on the user-controlled skip annotation, without requiring the webhook-verified inject-skipped receipt. This contradicts the PR’s readiness contract and lets a matching Pod added or modified without successful webhook admission make the backend report AllEnginePodsSkipped or cease blocking readiness.

Should-fix

• internal/controller/cachebackend_engine_local_readiness.go:234 — Configuration convergence reapplies only the adapter’s canonical injection and does not verify spec.integration.engineOverrides, although the webhook applies those overrides before stamping the receipt. A Pod with forged current annotations and canonical HiCache flags—but missing current override args or environment—can therefore report Ready.
• internal/controller/cachebackend_engine_local_readiness_test.go:97 — The test explicitly codifies that a skip needs no webhook-authored receipt, reinforcing behavior contrary to the stated PR contract. Replace this case with coverage requiring both annotations and treating an unverified skip as a participating, uninjected Pod.
• docs/design/cachebackend-api.md:460 — The design documentation calls the skip annotation a public, unauthenticated opt-out, contradicting the PR contract that exclusion requires both the operator annotation and webhook-verified skip receipt. Documentation and implementation need one consistent readiness contract.

Nit

• docs/concepts/cachebackend-engine-binding.md:52 — The concise HiCache readiness description omits the additional adapter-configuration convergence requirement documented elsewhere. Mentioning it here would prevent readers from interpreting receipt plus Kubernetes readiness as sufficient.

Per-category coverage

1. Vendor-neutral naming: Clean
2. Contract & spec / docs sync: findings above at: internal/controller/cachebackend_engine_local_readiness.go:148, internal/controller/cachebackend_engine_local_readiness.go:234, internal/controller/cachebackend_engine_local_readiness_test.go:97, docs/design/cachebackend-api.md:460, docs/concepts/cachebackend-engine-binding.md:52
3. gRPC fail-open semantics: Clean
4. Quality (tests / error wrapping / package placement / generated drift): findings above at: internal/controller/cachebackend_engine_local_readiness_test.go:97

Verdict

changes-requested

[github-actions]

Codex review

Files reviewed

• api/v1alpha1/
  • [reviewed] cachebackend_types.go
• config/crd/bases/
  • [skipped — generated] inferencecache.io_cachebackends.yaml
• config/samples/
  • [reviewed] README.md
• docs/concepts/
  • [reviewed] cachebackend-engine-binding.md
• docs/design/
  • [reviewed] cachebackend-api.md
• docs/reference-stack/scripts/
  • [reviewed] default_install_smoke.sh
• internal/controller/
  • [reviewed] cachebackend_controller.go
  • [reviewed] cachebackend_controller_test.go
  • [reviewed] cachebackend_engine_local_readiness.go
  • [reviewed] cachebackend_engine_local_readiness_test.go
• internal/webhook/pod/
  • [reviewed] envtest_integration_test.go
  • [reviewed] overrides.go
  • [reviewed] podinjector.go
  • [reviewed] podinjector_test.go

Findings

Blocking

• internal/controller/cachebackend_engine_local_readiness.go:148 — Skipped Pods are excluded using only the user-supplied skip-inject annotation, contrary to the PR contract requiring both the operator annotation and webhook-verified skip receipt. If admission fails open or is bypassed, any matching Pod can consequently avoid configuration and readiness validation and even make the backend Ready=True/AllEnginePodsSkipped.

Should-fix

• internal/controller/cachebackend_engine_local_readiness.go:186 — The idempotent replay does not reliably verify engineOverrides values already present on a Pod. ApplyEngineConfigWithOverrides classifies pre-existing noncanonical args/env as user-owned, so an incorrect value matching an override key remains unchanged and reflect.DeepEqual can report convergence despite the current override never reaching the Pod.

• internal/controller/cachebackend_engine_local_readiness.go:60 — reconcileEngineLocal dereferences spec.engineSelector without guarding nil. An admission-bypassed or previously persisted malformed SGLangHiCache resource can panic the controller rather than receiving an unmanaged/degraded verdict.

Nit

None.

Per-category coverage

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

Verdict

changes-requested

[EdHasNoLife]

Review — direction is solid; one reachable panic to fix before merge

Read the full diff. The Ready/Progressing/Degraded contract for engine-local SGLangHiCache, the injected-generation receipt, and especially the "run the idempotent adapter against an in-memory copy and require no change" convergence check (which defends against a forged current-gen receipt rather than trusting the annotation) are all well done. Test coverage is strong and docs/samples/smoke are updated in lockstep. One blocking correctness bug, two minors.

🔴 Blocking — nil-pointer panic when a SGLangHiCache backend has no engineSelector

internal/controller/cachebackend_engine_local_readiness.go:

selector := labels.SelectorFromSet(backend.Spec.EngineSelector.MatchLabels)

EngineSelector is +optional (json:"engineSelector,omitempty") and there is no admission rule requiring it for any type — so {runtime: SGLang, type: SGLangHiCache} with no engineSelector is admitted, reaches reconcileEngineLocal via dispatch, and dereferences a nil pointer → panic. controller-runtime recovers per-reconcile, so it becomes a hot panic-loop on that CR that never publishes status, rather than a clean Degraded.

Notably every other consumer guards this — podinjector.go, cacheindex_controller.go, podaudit.go, checks/cachebackend.go, and Reconcile itself (if backend.Spec.EngineSelector != nil && len(...) > 0) all check for nil/empty first. The new evaluator is the sole exception.

Fix: treat a nil/empty selector as an empty participant set → AwaitingEnginePods (matches the repo pattern and the "no active Pods" contract row), or add an admission rule requiring engineSelector for SGLangHiCache. The guard is the smaller, more consistent fix. Please add a nil-selector test either way.

🟡 Minor — "future generation" ⇒ Degraded can transiently flap under informer lag

A participant with injected-generation > backend.Generation is bucketed as mismatched → Degraded=True. In steady state that's only forgery (fine), but right after a spec bump N→N+1, if the workload owner rolls immediately and a new-gen Pod appears before the controller's cached backend reflects N+1, the controller briefly sees pod-gen > cached-gen and flips Degraded (a terminal-sounding condition that may page). Narrow window, self-heals within the 5s poll, and generation is monotonic so rollback can't trigger it — but consider classifying future-generation as Progressing/RolloutInProgress rather than Degraded, or tolerating it for one poll.

🟡 Minor — PR description contradicts the implemented skip semantics

The PR body says a skipped Pod is excluded "only when both the operator skip annotation and the webhook-verified skip receipt are present." The code excludes on the operator skip-inject annotation alone (SkipAnnotationOptsOut(pod.Annotations[AnnotationSkip])), and the test explicit skip does not require a webhook-authored receipt asserts exactly that. The in-repo doc (cachebackend-api.md: "a public operator control, not a webhook-authenticated decision") matches the code — only the PR body oversells it. Worth correcting so the posture (any pod-create actor can self-exclude; all-skipped ⇒ Ready=True/AllEnginePodsSkipped) isn't misread.

Non-blocking notes

• engineConfigConverged relies on the adapter being not just idempotent but reflect.DeepEqual-stable on its own output; a future nil-vs-empty-slice normalization inside injection would produce a false EnginePodsNotInjected/Degraded. Holds today (the current() fixture proves it); a one-line comment pinning that requirement would help the next adapter author.
• Nice: deterministic status messages via sort.Strings + truncateMessage (avoids resourceVersion churn), fail-safe verdict preservation on Pod-list error (with a dedicated interceptor test), and receipt cleanup applied consistently across all three annotations on every success/skip/fail-open path.

Test coverage

Strong — 14-case table covering every contract row incl. terminal/terminating exclusion, missing-vs-mismatched precedence, forged-receipt-without-config, and stale-beats-unavailable; preserve-on-list-error; envtest asserting the generation annotation + its stripping; and a full AwaitingEnginePods → EnginePodsReady smoke transition with a real Pod. The one gap is the nil-engineSelector case above.

Verdict: fix the panic (guard + test) before merge; the two minors are worth addressing but not blocking. Everything else is solid.