Skip to content

Harden NodeLocal LMCache allocation and UID-pool cleanup - #191

Merged
fredericsun merged 15 commits into
mainfrom
feature/lmcache-nodelocal-shm-hardening
Aug 24, 2026
Merged

Harden NodeLocal LMCache allocation and UID-pool cleanup#191
fredericsun merged 15 commits into
mainfrom
feature/lmcache-nodelocal-shm-hardening

Conversation

@fredericsun

@fredericsun fredericsun commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Pin PodLocal and NodeLocal LMCache MP servers to the supported CUDA allocation profile: lmcache_driven, non-lazy L1 allocation, and an empty shm_name. Startup gates, status, and lifecycle identity now require that exact live profile.
  • Treat l1Capacity as eagerly allocated private pinned host memory. Retain the exact /dev/shm/inference-cache/<uid> hostPath only for CUDA/PyTorch IPC lifetime objects, and document the trusted-node security and capacity boundary.
  • Reclaim idle or deleted NodeLocal UID pools with controller-owned, gated, one-shot cleanup Pods. Cleanup waits until no Pod mounts the exact UID hostPath, preserves the directory root, does not follow symlinks, and is covered by a CacheBackend finalizer.
  • Preserve availability by cancelling a still-gated cleanup when Engine demand returns; once cleanup starts, block Server recreation until it succeeds.
  • Align the API descriptions, generated CRD, samples, reference smoke checks, tests, and migration roadmap with the final contract.

Linked issues

Validation

  • make ci passed in the pre-push gate, including DCO, REUSE, naming, formatting, vet, lint, race tests, and builds.
  • make verify-samples passed: 27 admitted, one existing explicit skip, zero failures.
  • make cover-check passed at 90.1% logic coverage.
  • Focused unit and lifecycle tests cover the allocation profile, startup/status identity, cleanup gating, consumer quiescence, demand return, finalization, and symlink-safe directory cleanup.
  • This PR does not claim new focused GPU requalification beyond the live evidence recorded in the roadmap.

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—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.
  • The pre-push naming guard passed.

Quality

  • Every human-authored commit includes a matching DCO Signed-off-by: trailer.
  • make reuse-lint passes.
  • make build and tests pass locally.
  • Formatting, go vet, and lint are clean.
  • Generated CRD artifacts are committed with no drift.
  • New and changed behavior has unit tests.
  • The operator-facing reference smoke assertions are updated.
  • Remote CI is green.

Contracts

  • The implementation and migration roadmap describe the same NodeLocal contract.
  • Backward compatibility for v1alpha1 consumers was considered; no public field was removed or added.
  • Proto documentation is not applicable because this PR does not change proto/.
  • CRD API descriptions and design documentation are updated together.

Yue Sun added 3 commits August 22, 2026 21:24
Signed-off-by: Yue Sun <yue.s.sun@oracle.com>
Signed-off-by: Yue Sun <yue.s.sun@oracle.com>
Signed-off-by: Yue Sun <yue.s.sun@oracle.com>
@codecov

codecov Bot commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.66667% with 38 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ernal/controller/cachebackend_lmcache_nodelocal.go 85.77% 17 Missing and 18 partials ⚠️
internal/controller/cachebackend_reconciler.go 66.66% 2 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@github-actions

Copy link
Copy Markdown

Codex review

Files reviewed

  • api/v1alpha1/
    • [reviewed] cachebackend_types.go
  • config/crd/bases/
    • [skipped — generated] inferencecache.io_cachebackends.yaml
  • config/samples/
    • [reviewed] cachebackend-sglang-nodelocal-host-only.yaml
  • docs/concepts/
    • [reviewed] cachebackend-engine-binding.md
  • docs/design/
    • [reviewed] cachebackend-api.md
    • [reviewed] lmcache-multiprocess-migration-roadmap.md
    • [reviewed] sglang-lmcache-mp-mode.md
  • docs/reference-stack/
    • [reviewed] GPU-RUNBOOK.md
  • docs/reference-stack/manifests/sglang-lmcache/
    • [reviewed] deployment.yaml
  • docs/reference-stack/scripts/
    • [reviewed] default_install_smoke.sh
  • internal/adapters/builtin/runtime/
    • [reviewed] lmcache_mp_nodelocal.go
    • [reviewed] lmcache_mp_nodelocal_test.go
    • [reviewed] lmcache_mp_renderer.go
    • [reviewed] lmcache_mp_renderer_test.go
    • [reviewed] sglang_lmcache_test.go
    • [reviewed] vllm_lmcache_mp.go
    • [reviewed] vllm_lmcache_mp_test.go
  • internal/controller/
    • [reviewed] cachebackend_lmcache_mp_status.go
    • [reviewed] cachebackend_lmcache_mp_status_test.go
    • [reviewed] cachebackend_lmcache_nodelocal.go
    • [reviewed] cachebackend_mp_lifecycle_test.go
    • [reviewed] cachebackend_nodelocal_integration_test.go
    • [reviewed] cachebackend_reconciler.go
    • [reviewed] cachebackend_reconciler_test.go
  • internal/enginebinding/
    • [reviewed] metadata.go
  • internal/webhook/pod/
    • [reviewed] podinjector_test.go
  • internal/webhook/v1alpha1/
    • [reviewed] cachebackend_lmcache_mp_validation.go

Findings

Blocking

  • internal/controller/cachebackend_lmcache_nodelocal.go:447 — Terminally failed cleanup Pods are never replaced or retried. An eviction or other transition to PodFailed leaves the node permanently blocked and, during deletion, keeps the CacheBackend finalizer forever because only PodSucceeded is handled.
  • internal/controller/cachebackend_lmcache_nodelocal.go:333 — The finalizer survives a change away from NodeLocal, but finalization refuses to create cleanup intents once the current spec is no longer NodeLocal. If an injected UID-path consumer remains and no cleanup Pod already exists, nodeLocalShmConsumers keeps the finalizer forever with no path that can remove the consumer’s pool.

Should-fix

  • internal/controller/cachebackend_lmcache_nodelocal.go:443 — The deletion error for an invalid cleanup Pod is discarded and replaced with a synthetic error. Wrap and return the actual delete failure; otherwise repeated reconciliation cannot distinguish an expected retry from RBAC or API failures.
  • internal/adapters/builtin/runtime/lmcache_mp_nodelocal.go:297 — Cleanup Pods inherit pull secrets only from their immediate source Pod. The deletion fallback can use an engine Pod as the source even though the cleanup image comes from nodeLocal.server.image, so a private server image configured through nodeLocal.scheduling.imagePullSecrets can become unpullable and permanently hold the finalizer.

Nit

None.

Per-category coverage

  1. Vendor-neutral naming: Clean
  2. Contract & spec / docs sync: findings above at: internal/controller/cachebackend_lmcache_nodelocal.go:333, internal/controller/cachebackend_lmcache_nodelocal.go:447
  3. gRPC fail-open semantics: Clean
  4. Quality (tests / error wrapping / package placement / generated drift): findings above at: internal/controller/cachebackend_lmcache_nodelocal.go:443, internal/adapters/builtin/runtime/lmcache_mp_nodelocal.go:297

Verdict

changes-requested

Signed-off-by: Yue Sun <yue.s.sun@oracle.com>
@github-actions

Copy link
Copy Markdown

Codex review

Files reviewed

  • api/v1alpha1/
    • [reviewed] cachebackend_types.go
  • config/crd/bases/
    • [skipped — generated] inferencecache.io_cachebackends.yaml
  • config/samples/
    • [reviewed] cachebackend-sglang-nodelocal-host-only.yaml
  • docs/concepts/
    • [reviewed] cachebackend-engine-binding.md
  • docs/design/
    • [reviewed] cachebackend-api.md
    • [reviewed] lmcache-multiprocess-migration-roadmap.md
    • [reviewed] sglang-lmcache-mp-mode.md
  • docs/reference-stack/
    • [reviewed] GPU-RUNBOOK.md
  • docs/reference-stack/manifests/sglang-lmcache/
    • [reviewed] deployment.yaml
  • docs/reference-stack/scripts/
    • [reviewed] default_install_smoke.sh
  • internal/adapters/builtin/runtime/
    • [reviewed] lmcache_mp_nodelocal.go
    • [reviewed] lmcache_mp_nodelocal_test.go
    • [reviewed] lmcache_mp_renderer.go
    • [reviewed] lmcache_mp_renderer_test.go
    • [reviewed] sglang_lmcache_test.go
    • [reviewed] vllm_lmcache_mp.go
    • [reviewed] vllm_lmcache_mp_test.go
  • internal/controller/
    • [reviewed] cachebackend_lmcache_mp_status.go
    • [reviewed] cachebackend_lmcache_mp_status_test.go
    • [reviewed] cachebackend_lmcache_nodelocal.go
    • [reviewed] cachebackend_mp_lifecycle_test.go
    • [reviewed] cachebackend_nodelocal_integration_test.go
    • [reviewed] cachebackend_reconciler.go
    • [reviewed] cachebackend_reconciler_test.go
  • internal/enginebinding/
    • [reviewed] metadata.go
  • internal/webhook/pod/
    • [reviewed] podinjector_test.go
  • internal/webhook/v1alpha1/
    • [reviewed] cachebackend_lmcache_mp_validation.go

Findings

Blocking

  • internal/controller/cachebackend_lmcache_nodelocal.go:92 — Stale or pre-upgrade servers are deleted without creating a cleanup intent. Existing servers lack the newly required runtime profile, so an upgrade takes this branch and can leave their large POSIX-SHM L1 object permanently consuming /dev/shm, potentially preventing the replacement server’s pinned allocation.

Should-fix

  • docs/design/cachebackend-api.md:291 — The primary NodeLocal design contract describes idle deletion but omits the newly implemented gated cleanup, cancellation, server-recreation blocking, and deletion finalizer semantics. Those lifecycle guarantees appear only in the migration roadmap, leaving the normative API design out of sync with controller behavior.

Nit

  • internal/adapters/builtin/runtime/lmcache_mp_nodelocal.go:203 — The cleanup’s lstat followed by path-based scandir has a symlink-swap race, so “does not follow symlinks” is not strictly guaranteed under concurrent mutation. Descriptor-relative traversal with no-follow opens would make the stated property robust.

Per-category coverage

  1. Vendor-neutral naming: Clean
  2. Contract & spec / docs sync: findings above at: internal/controller/cachebackend_lmcache_nodelocal.go:92, docs/design/cachebackend-api.md:291
  3. gRPC fail-open semantics: Clean
  4. Quality (tests / error wrapping / package placement / generated drift): findings above at: internal/controller/cachebackend_lmcache_nodelocal.go:92, internal/adapters/builtin/runtime/lmcache_mp_nodelocal.go:203

Verdict

changes-requested

Signed-off-by: Yue Sun <yue.s.sun@oracle.com>
@github-actions

Copy link
Copy Markdown

Codex review

Files reviewed

.github/workflows

  • [reviewed] .github/workflows/ci.yml
  • [reviewed] .github/workflows/release-sbom.yml

Repository root

  • [reviewed] Makefile

api/v1alpha1

  • [reviewed] api/v1alpha1/cachebackend_types.go
  • [reviewed] api/v1alpha1/cachebackend_types_test.go

cmd/controller

  • [reviewed] cmd/controller/main.go
  • [reviewed] cmd/controller/main_test.go

cmd/node-local-shm-cleanup

  • [reviewed] cmd/node-local-shm-cleanup/main.go
  • [reviewed] cmd/node-local-shm-cleanup/main_test.go

config/crd/bases

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

config/manager

  • [reviewed] config/manager/manager.yaml

config/samples

  • [reviewed] config/samples/README.md
  • [reviewed] config/samples/cachebackend-sglang-nodelocal-host-only.yaml

dockerfiles

  • [reviewed] dockerfiles/Dockerfile

docs/concepts

  • [reviewed] docs/concepts/cachebackend-engine-binding.md

docs/design

  • [reviewed] docs/design/cachebackend-api.md
  • [reviewed] docs/design/lmcache-multiprocess-migration-roadmap.md
  • [reviewed] docs/design/sglang-lmcache-mp-mode.md

docs/operations

  • [reviewed] docs/operations/container-images.md
  • [reviewed] docs/operations/sbom.md

docs

  • [reviewed] docs/quickstart.md

docs/reference-stack

  • [reviewed] docs/reference-stack/GPU-RUNBOOK.md

docs/reference-stack/manifests/sglang-lmcache

  • [reviewed] docs/reference-stack/manifests/sglang-lmcache/deployment.yaml

docs/reference-stack/scripts

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

hack

  • [reviewed] hack/resolve-release-image-digests.sh
  • [reviewed] hack/resolve-release-image-digests_test.sh
  • [reviewed] hack/sbom-registry-smoke.sh
  • [reviewed] hack/verify-minimal-images.sh
  • [reviewed] hack/verify-minimal-images_test.sh

internal/adapters/builtin/runtime

  • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_nodelocal.go
  • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_nodelocal_test.go
  • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_renderer.go
  • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_renderer_test.go
  • [reviewed] internal/adapters/builtin/runtime/sglang_lmcache_test.go
  • [reviewed] internal/adapters/builtin/runtime/test_helpers_test.go
  • [reviewed] internal/adapters/builtin/runtime/vllm_lmcache_mp.go
  • [reviewed] internal/adapters/builtin/runtime/vllm_lmcache_mp_test.go

internal/controller

  • [reviewed] internal/controller/cachebackend_lmcache_mp_status.go
  • [reviewed] internal/controller/cachebackend_lmcache_mp_status_test.go
  • [reviewed] internal/controller/cachebackend_lmcache_nodelocal.go
  • [reviewed] internal/controller/cachebackend_mp_lifecycle_test.go
  • [reviewed] internal/controller/cachebackend_nodelocal_integration_test.go
  • [reviewed] internal/controller/cachebackend_reconciler.go
  • [reviewed] internal/controller/cachebackend_reconciler_test.go

internal/enginebinding

  • [reviewed] internal/enginebinding/metadata.go

internal/webhook/pod

  • [reviewed] internal/webhook/pod/podinjector_test.go

internal/webhook/v1alpha1

  • [reviewed] internal/webhook/v1alpha1/cachebackend_defaulter_envtest_test.go
  • [reviewed] internal/webhook/v1alpha1/cachebackend_lmcache_mp_validation.go

Findings

Blocking

  • internal/controller/cachebackend_lmcache_nodelocal.go:87 — Stale or legacy server Pods are deleted without first creating a cleanup intent. In particular, pre-upgrade Pods fail the new runtime-identity test, so an idle node can lose its only node-placement record and leave its UID directory permanently orphaned.
  • api/v1alpha1/cachebackend_types.go:481 — The new CRD transition rules make six previously mutable portions of the public v1alpha1 spec immutable. This is a breaking API change for existing clients and contradicts the PR’s backward-compatibility claim; it needs explicit versioning/migration rather than being introduced as cleanup hardening.

Should-fix

None.

Nit

None.

Per-category coverage

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

Verdict

changes-requested

Signed-off-by: Yue Sun <yue.s.sun@oracle.com>
@github-actions

Copy link
Copy Markdown

Codex review

Files reviewed

  • .github/workflows/
    • [reviewed] .github/workflows/ci.yml
    • [reviewed] .github/workflows/release-sbom.yml
  • repository root
    • [reviewed] Makefile
  • api/v1alpha1/
    • [reviewed] cachebackend_types.go
    • [reviewed] cachebackend_types_test.go
  • cmd/controller/
    • [reviewed] main.go
    • [reviewed] main_test.go
  • cmd/node-local-shm-cleanup/
    • [reviewed] main.go
    • [reviewed] main_test.go
  • config/crd/bases/
    • [skipped — generated] inferencecache.io_cachebackends.yaml
  • config/manager/
    • [reviewed] manager.yaml
  • config/samples/
    • [reviewed] README.md
    • [reviewed] cachebackend-sglang-nodelocal-host-only.yaml
  • dockerfiles/
    • [reviewed] Dockerfile
  • docs/concepts/
    • [reviewed] cachebackend-engine-binding.md
  • docs/design/
    • [reviewed] cachebackend-api.md
    • [reviewed] lmcache-multiprocess-migration-roadmap.md
    • [reviewed] sglang-lmcache-mp-mode.md
  • docs/operations/
    • [reviewed] container-images.md
    • [reviewed] sbom.md
  • docs/
    • [reviewed] quickstart.md
  • docs/reference-stack/
    • [reviewed] GPU-RUNBOOK.md
  • docs/reference-stack/manifests/sglang-lmcache/
    • [reviewed] deployment.yaml
  • docs/reference-stack/scripts/
    • [reviewed] default_install_smoke.sh
  • hack/
    • [reviewed] resolve-release-image-digests.sh
    • [reviewed] resolve-release-image-digests_test.sh
    • [reviewed] sbom-registry-smoke.sh
    • [reviewed] verify-minimal-images.sh
    • [reviewed] verify-minimal-images_test.sh
  • internal/adapters/builtin/runtime/
    • [reviewed] lmcache_mp_nodelocal.go
    • [reviewed] lmcache_mp_nodelocal_test.go
    • [reviewed] lmcache_mp_renderer.go
    • [reviewed] lmcache_mp_renderer_test.go
    • [reviewed] sglang_lmcache_test.go
    • [reviewed] test_helpers_test.go
    • [reviewed] vllm_lmcache_mp.go
    • [reviewed] vllm_lmcache_mp_test.go
  • internal/controller/
    • [reviewed] cachebackend_lmcache_mp_status.go
    • [reviewed] cachebackend_lmcache_mp_status_test.go
    • [reviewed] cachebackend_lmcache_nodelocal.go
    • [reviewed] cachebackend_mp_lifecycle_test.go
    • [reviewed] cachebackend_nodelocal_integration_test.go
    • [reviewed] cachebackend_reconciler.go
    • [reviewed] cachebackend_reconciler_test.go
  • internal/enginebinding/
    • [reviewed] metadata.go
  • internal/webhook/pod/
    • [reviewed] podinjector_test.go
  • internal/webhook/v1alpha1/
    • [reviewed] cachebackend_defaulter_envtest_test.go
    • [reviewed] cachebackend_lmcache_mp_validation.go

Findings

Blocking

  • internal/controller/cachebackend_lmcache_nodelocal.go:568 — Releasing the cleanup Pod’s scheduling gate is not atomic with preventing new Engine consumers. An Engine can be admitted and scheduled onto the node after the consumer list at line 586 but before cleanup executes, allowing the helper to erase IPC objects while that new Pod mounts or uses the directory.

Should-fix

  • internal/adapters/builtin/runtime/lmcache_mp_nodelocal.go:316 — The nominally one-shot cleanup Pod uses RestartPolicyOnFailure. Ordinary helper failures therefore leave the Pod running in CrashLoopBackOff indefinitely, so the controller’s PodFailed replacement path is generally never reached; use Never or explicitly bound retries.

  • internal/controller/cachebackend_lmcache_nodelocal.go:586 — Verify: consumer quiescence is checked only in the CacheBackend namespace even though the hostPath is node-global. A Pod in another namespace mounting the exact UID path is invisible and can be cleaned underneath, contrary to the stated “after its consumers are gone” contract.

Nit

None.

Per-category coverage

  1. Vendor-neutral naming: Clean
  2. Contract & spec / docs sync: findings above at: internal/controller/cachebackend_lmcache_nodelocal.go:568, internal/controller/cachebackend_lmcache_nodelocal.go:586
  3. gRPC fail-open semantics: Clean
  4. Quality (tests / error wrapping / package placement / generated drift): findings above at: internal/adapters/builtin/runtime/lmcache_mp_nodelocal.go:316

Verdict

changes-requested

Signed-off-by: Yue Sun <yue.s.sun@oracle.com>
@github-actions

Copy link
Copy Markdown

Codex review

Files reviewed

  • .github/workflows/
    • [reviewed] .github/workflows/ci.yml
    • [reviewed] .github/workflows/release-sbom.yml
  • /
    • [reviewed] Makefile
  • api/v1alpha1/
    • [reviewed] cachebackend_types.go
    • [reviewed] cachebackend_types_test.go
  • cmd/controller/
    • [reviewed] main.go
    • [reviewed] main_test.go
  • cmd/node-local-shm-cleanup/
    • [reviewed] main.go
    • [reviewed] main_test.go
  • config/crd/bases/
    • [skipped — generated] inferencecache.io_cachebackends.yaml
  • config/manager/
    • [reviewed] manager.yaml
  • config/samples/
    • [reviewed] README.md
    • [reviewed] cachebackend-sglang-nodelocal-host-only.yaml
  • dockerfiles/
    • [reviewed] Dockerfile
  • docs/concepts/
    • [reviewed] cachebackend-engine-binding.md
  • docs/design/
    • [reviewed] cachebackend-api.md
    • [reviewed] lmcache-multiprocess-migration-roadmap.md
    • [reviewed] sglang-lmcache-mp-mode.md
  • docs/operations/
    • [reviewed] container-images.md
    • [reviewed] sbom.md
  • docs/
    • [reviewed] quickstart.md
  • docs/reference-stack/
    • [reviewed] GPU-RUNBOOK.md
  • docs/reference-stack/manifests/sglang-lmcache/
    • [reviewed] deployment.yaml
  • docs/reference-stack/scripts/
    • [reviewed] default_install_smoke.sh
  • hack/
    • [reviewed] resolve-release-image-digests.sh
    • [reviewed] resolve-release-image-digests_test.sh
    • [reviewed] sbom-registry-smoke.sh
    • [reviewed] verify-minimal-images.sh
    • [reviewed] verify-minimal-images_test.sh
  • internal/adapters/builtin/runtime/
    • [reviewed] lmcache_mp_nodelocal.go
    • [reviewed] lmcache_mp_nodelocal_test.go
    • [reviewed] lmcache_mp_renderer.go
    • [reviewed] lmcache_mp_renderer_test.go
    • [reviewed] sglang_lmcache_test.go
    • [reviewed] test_helpers_test.go
    • [reviewed] vllm_lmcache_mp.go
    • [reviewed] vllm_lmcache_mp_test.go
  • internal/controller/
    • [reviewed] cachebackend_lmcache_mp_status.go
    • [reviewed] cachebackend_lmcache_mp_status_test.go
    • [reviewed] cachebackend_lmcache_nodelocal.go
    • [reviewed] cachebackend_mp_lifecycle_test.go
    • [reviewed] cachebackend_nodelocal_integration_test.go
    • [reviewed] cachebackend_reconciler.go
    • [reviewed] cachebackend_reconciler_test.go
  • internal/enginebinding/
    • [reviewed] metadata.go
  • internal/webhook/pod/
    • [reviewed] podinjector_test.go
  • internal/webhook/v1alpha1/
    • [reviewed] cachebackend_defaulter_envtest_test.go
    • [reviewed] cachebackend_lmcache_mp_validation.go

Findings

Blocking

None.

Should-fix

  • config/manager/manager.yaml:30 — Verify: the shipped manager uses an all-zero cleanup-image digest, but no changed install/release path substitutes the published digest. A default NodeLocal deployment would create unpullable cleanup Pods and can leave CacheBackend deletion permanently blocked by the finalizer.
  • docs/design/cachebackend-api.md:358/dev/shm is still described as “the L1 tier,” contradicting the new private pinned-memory contract documented earlier in the same file. This leaves the normative API design internally inconsistent.
  • docs/design/sglang-lmcache-mp-mode.md:388 — The design still says L1 lives in /dev/shm and that the tmpfs must be sized for it. Under the newly enforced non-lazy, empty-shm_name profile, /dev/shm is an IPC surface rather than the L1 store.

Nit

  • docs/reference-stack/manifests/sglang-lmcache/deployment.yaml:224 — The comment says “The MP L1 tier lives here,” immediately before explaining that L1 is private pinned memory. Remove the stale first clause to avoid a direct contradiction.

Per-category coverage

  1. Vendor-neutral naming: Clean
  2. Contract & spec / docs sync: findings above at: config/manager/manager.yaml:30, docs/design/cachebackend-api.md:358, docs/design/sglang-lmcache-mp-mode.md:388, docs/reference-stack/manifests/sglang-lmcache/deployment.yaml:224
  3. gRPC fail-open semantics: Clean
  4. Quality (tests / error wrapping / package placement / generated drift): Clean

Verdict

changes-requested

Signed-off-by: Yue Sun <yue.s.sun@oracle.com>
@github-actions

Copy link
Copy Markdown

Codex review

Files reviewed

.github/workflows

  • [reviewed] .github/workflows/ci.yml
  • [reviewed] .github/workflows/release-sbom.yml

Root

  • [reviewed] Makefile

api/v1alpha1

  • [reviewed] api/v1alpha1/cachebackend_types.go
  • [reviewed] api/v1alpha1/cachebackend_types_test.go

cmd/controller

  • [reviewed] cmd/controller/main.go
  • [reviewed] cmd/controller/main_test.go

cmd/node-local-shm-cleanup

  • [reviewed] cmd/node-local-shm-cleanup/main.go
  • [reviewed] cmd/node-local-shm-cleanup/main_test.go

config/crd/bases

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

config/manager

  • [reviewed] config/manager/manager.yaml

config/samples

  • [reviewed] config/samples/README.md
  • [reviewed] config/samples/cachebackend-sglang-nodelocal-host-only.yaml

dockerfiles

  • [reviewed] dockerfiles/Dockerfile

docs/concepts

  • [reviewed] docs/concepts/cachebackend-engine-binding.md

docs/design

  • [reviewed] docs/design/cachebackend-api.md
  • [reviewed] docs/design/lmcache-multiprocess-migration-roadmap.md
  • [reviewed] docs/design/sglang-lmcache-mp-mode.md

docs/operations

  • [reviewed] docs/operations/container-images.md
  • [reviewed] docs/operations/sbom.md

docs

  • [reviewed] docs/quickstart.md

docs/reference-stack

  • [reviewed] docs/reference-stack/GPU-RUNBOOK.md
  • [reviewed] docs/reference-stack/manifests/sglang-lmcache/deployment.yaml
  • [reviewed] docs/reference-stack/scripts/default_install_smoke.sh

hack

  • [reviewed] hack/render-release-install.sh
  • [reviewed] hack/render-release-install_test.sh
  • [reviewed] hack/resolve-release-image-digests.sh
  • [reviewed] hack/resolve-release-image-digests_test.sh
  • [reviewed] hack/sbom-registry-smoke.sh
  • [reviewed] hack/verify-minimal-images.sh
  • [reviewed] hack/verify-minimal-images_test.sh

internal/adapters/builtin/runtime

  • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_nodelocal.go
  • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_nodelocal_test.go
  • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_renderer.go
  • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_renderer_test.go
  • [reviewed] internal/adapters/builtin/runtime/sglang_lmcache_test.go
  • [reviewed] internal/adapters/builtin/runtime/test_helpers_test.go
  • [reviewed] internal/adapters/builtin/runtime/vllm_lmcache_mp.go
  • [reviewed] internal/adapters/builtin/runtime/vllm_lmcache_mp_test.go

internal/controller

  • [reviewed] internal/controller/cachebackend_lmcache_mp_status.go
  • [reviewed] internal/controller/cachebackend_lmcache_mp_status_test.go
  • [reviewed] internal/controller/cachebackend_lmcache_nodelocal.go
  • [reviewed] internal/controller/cachebackend_mp_lifecycle_test.go
  • [reviewed] internal/controller/cachebackend_nodelocal_integration_test.go
  • [reviewed] internal/controller/cachebackend_reconciler.go
  • [reviewed] internal/controller/cachebackend_reconciler_test.go

internal/enginebinding

  • [reviewed] internal/enginebinding/metadata.go

internal/webhook/pod

  • [reviewed] internal/webhook/pod/podinjector_test.go

internal/webhook/v1alpha1

  • [reviewed] internal/webhook/v1alpha1/cachebackend_defaulter_envtest_test.go
  • [reviewed] internal/webhook/v1alpha1/cachebackend_lmcache_mp_validation.go

Findings

Blocking

None.

Should-fix

internal/controller/cachebackend_lmcache_nodelocal.go:587 — Cleanup identity validation checks only the name, owner UID, node, and hostPath volume, not the helper container, mount, command, image, or successful termination. A same-namespace actor can create a Pod with the backend owner reference and expected metadata, mark an unrelated container successful, and make the controller accept cleanup without clearing the directory.

config/manager/manager.yaml:30 — The ordinary checked-in installation passes an all-zero cleanup digest, which satisfies validateOptions but can never produce a runnable cleanup Pod. Verify that every supported non-release installation path replaces this placeholder; otherwise NodeLocal cleanup and backend finalization can become permanently stuck in normal make deploy/kustomize installations.

Nit

cmd/controller/main.go:253 — The digest validation regex accepts syntactically invalid image names and the all-zero placeholder because it only excludes whitespace and @. Reuse a container-reference parser or the release renderer’s stricter validation so invalid configuration fails at controller startup rather than when Kubernetes admits or pulls the cleanup Pod.

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_lmcache_nodelocal.go:587, config/manager/manager.yaml:30, cmd/controller/main.go:253

Verdict

changes-requested

Signed-off-by: Yue Sun <yue.s.sun@oracle.com>
@github-actions

Copy link
Copy Markdown

Codex review

Files reviewed

.github/workflows

  • [reviewed] .github/workflows/ci.yml
  • [reviewed] .github/workflows/release-sbom.yml

Repository root

  • [reviewed] Makefile
  • [reviewed] README.md

api/v1alpha1

  • [reviewed] api/v1alpha1/cachebackend_types.go
  • [reviewed] api/v1alpha1/cachebackend_types_test.go

cmd/controller

  • [reviewed] cmd/controller/main.go
  • [reviewed] cmd/controller/main_test.go

cmd/node-local-shm-cleanup

  • [reviewed] cmd/node-local-shm-cleanup/main.go
  • [reviewed] cmd/node-local-shm-cleanup/main_test.go

config/crd/bases

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

config/manager

  • [reviewed] config/manager/manager.yaml

config/samples

  • [reviewed] config/samples/README.md
  • [reviewed] config/samples/cachebackend-sglang-nodelocal-host-only.yaml

dockerfiles

  • [reviewed] dockerfiles/Dockerfile

docs/cli

  • [reviewed] docs/cli/doctor.md

docs/concepts

  • [reviewed] docs/concepts/cachebackend-engine-binding.md

docs/design

  • [reviewed] docs/design/cachebackend-api.md
  • [reviewed] docs/design/grpc-tls.md
  • [reviewed] docs/design/lmcache-multiprocess-migration-roadmap.md
  • [reviewed] docs/design/sglang-lmcache-mp-mode.md

docs/operations

  • [reviewed] docs/operations/container-images.md
  • [reviewed] docs/operations/sbom.md

docs

  • [reviewed] docs/quickstart.md

docs/reference-stack

  • [reviewed] docs/reference-stack/GPU-RUNBOOK.md
  • [reviewed] docs/reference-stack/README.md

docs/reference-stack/manifests/sglang-lmcache

  • [reviewed] docs/reference-stack/manifests/sglang-lmcache/deployment.yaml

docs/reference-stack/scripts

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

hack

  • [reviewed] hack/render-release-install.sh
  • [reviewed] hack/render-release-install_test.sh
  • [reviewed] hack/resolve-release-image-digests.sh
  • [reviewed] hack/resolve-release-image-digests_test.sh
  • [reviewed] hack/sbom-registry-smoke.sh
  • [reviewed] hack/verify-minimal-images.sh
  • [reviewed] hack/verify-minimal-images_test.sh

internal/adapters/builtin/runtime

  • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_nodelocal.go
  • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_nodelocal_test.go
  • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_renderer.go
  • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_renderer_test.go
  • [reviewed] internal/adapters/builtin/runtime/sglang_lmcache_test.go
  • [reviewed] internal/adapters/builtin/runtime/test_helpers_test.go
  • [reviewed] internal/adapters/builtin/runtime/vllm_lmcache_mp.go
  • [reviewed] internal/adapters/builtin/runtime/vllm_lmcache_mp_test.go

internal/controller

  • [reviewed] internal/controller/cachebackend_lmcache_mp_status.go
  • [reviewed] internal/controller/cachebackend_lmcache_mp_status_test.go
  • [reviewed] internal/controller/cachebackend_lmcache_nodelocal.go
  • [reviewed] internal/controller/cachebackend_mp_lifecycle_test.go
  • [reviewed] internal/controller/cachebackend_nodelocal_integration_test.go
  • [reviewed] internal/controller/cachebackend_reconciler.go
  • [reviewed] internal/controller/cachebackend_reconciler_test.go

internal/enginebinding

  • [reviewed] internal/enginebinding/metadata.go

internal/webhook/pod

  • [reviewed] internal/webhook/pod/podinjector_test.go

internal/webhook/v1alpha1

  • [reviewed] internal/webhook/v1alpha1/cachebackend_defaulter_envtest_test.go
  • [reviewed] internal/webhook/v1alpha1/cachebackend_lmcache_mp_validation.go

Findings

Blocking

None.

Should-fix

internal/controller/cachebackend_lmcache_nodelocal.go:520 — Failed cleanup Pods are immediately replaced with deterministically chained retries without any backoff or retry limit. A persistent helper failure can therefore create and delete Pods continuously, producing unbounded API churn while the CacheBackend finalizer remains blocked.

Nit

cmd/node-local-shm-cleanup/main.go:18emptyDirectory returns RemoveAll errors without identifying the entry that failed. Wrap the error with the child path so cleanup failures are actionable rather than reporting only empty <root>: permission denied.

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_lmcache_nodelocal.go:520, cmd/node-local-shm-cleanup/main.go:18

Verdict

changes-requested

Signed-off-by: Yue Sun <yue.s.sun@oracle.com>
@github-actions

Copy link
Copy Markdown

Codex review

Files reviewed

.github/workflows/

  • [reviewed] .github/workflows/ci.yml
  • [reviewed] .github/workflows/release-sbom.yml

Root

  • [reviewed] Makefile
  • [reviewed] README.md

api/v1alpha1/

  • [reviewed] api/v1alpha1/cachebackend_types.go
  • [reviewed] api/v1alpha1/cachebackend_types_test.go

cmd/controller/

  • [reviewed] cmd/controller/main.go
  • [reviewed] cmd/controller/main_test.go

cmd/node-local-shm-cleanup/

  • [reviewed] cmd/node-local-shm-cleanup/main.go
  • [reviewed] cmd/node-local-shm-cleanup/main_test.go

config/crd/bases/

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

config/manager/

  • [reviewed] config/manager/manager.yaml

config/samples/

  • [reviewed] config/samples/README.md
  • [reviewed] config/samples/cachebackend-sglang-nodelocal-host-only.yaml

dockerfiles/

  • [reviewed] dockerfiles/Dockerfile

docs/cli/

  • [reviewed] docs/cli/doctor.md

docs/concepts/

  • [reviewed] docs/concepts/cachebackend-engine-binding.md

docs/design/

  • [reviewed] docs/design/cachebackend-api.md
  • [reviewed] docs/design/grpc-tls.md
  • [reviewed] docs/design/lmcache-multiprocess-migration-roadmap.md
  • [reviewed] docs/design/sglang-lmcache-mp-mode.md

docs/operations/

  • [reviewed] docs/operations/container-images.md
  • [reviewed] docs/operations/sbom.md

docs/

  • [reviewed] docs/quickstart.md

docs/reference-stack/

  • [reviewed] docs/reference-stack/GPU-RUNBOOK.md
  • [reviewed] docs/reference-stack/README.md

docs/reference-stack/manifests/sglang-lmcache/

  • [reviewed] docs/reference-stack/manifests/sglang-lmcache/deployment.yaml

docs/reference-stack/scripts/

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

hack/

  • [reviewed] hack/render-release-install.sh
  • [reviewed] hack/render-release-install_test.sh
  • [reviewed] hack/resolve-release-image-digests.sh
  • [reviewed] hack/resolve-release-image-digests_test.sh
  • [reviewed] hack/sbom-registry-smoke.sh
  • [reviewed] hack/verify-minimal-images.sh
  • [reviewed] hack/verify-minimal-images_test.sh

internal/adapters/builtin/runtime/

  • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_nodelocal.go
  • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_nodelocal_test.go
  • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_renderer.go
  • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_renderer_test.go
  • [reviewed] internal/adapters/builtin/runtime/sglang_lmcache_test.go
  • [reviewed] internal/adapters/builtin/runtime/test_helpers_test.go
  • [reviewed] internal/adapters/builtin/runtime/vllm_lmcache_mp.go
  • [reviewed] internal/adapters/builtin/runtime/vllm_lmcache_mp_test.go

internal/controller/

  • [reviewed] internal/controller/cachebackend_lmcache_mp_status.go
  • [reviewed] internal/controller/cachebackend_lmcache_mp_status_test.go
  • [reviewed] internal/controller/cachebackend_lmcache_nodelocal.go
  • [reviewed] internal/controller/cachebackend_mp_lifecycle_test.go
  • [reviewed] internal/controller/cachebackend_nodelocal_integration_test.go
  • [reviewed] internal/controller/cachebackend_reconciler.go
  • [reviewed] internal/controller/cachebackend_reconciler_test.go

internal/enginebinding/

  • [reviewed] internal/enginebinding/metadata.go

internal/webhook/pod/

  • [reviewed] internal/webhook/pod/podinjector_test.go

internal/webhook/v1alpha1/

  • [reviewed] internal/webhook/v1alpha1/cachebackend_defaulter_envtest_test.go
  • [reviewed] internal/webhook/v1alpha1/cachebackend_lmcache_mp_validation.go

Findings

Blocking

  • internal/controller/cachebackend_lmcache_nodelocal.go:531 — Exhausting the retry limit silently abandons cleanup. The terminal Pod is still deleted at line 557 without a replacement, so the next reconciliation can recreate the server or remove the deletion finalizer despite the UID directory never having been cleaned; retain a blocking failure state or continue retrying until cleanup succeeds.

Should-fix

None.

Nit

None.

Per-category coverage

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

Verdict

changes-requested

Signed-off-by: Yue Sun <yue.s.sun@oracle.com>
@github-actions

Copy link
Copy Markdown

Codex review

Files reviewed

.github/workflows

  • [reviewed] .github/workflows/ci.yml
  • [reviewed] .github/workflows/release-sbom.yml

Root

  • [reviewed] Makefile
  • [reviewed] README.md

api/v1alpha1

  • [reviewed] api/v1alpha1/cachebackend_types.go
  • [reviewed] api/v1alpha1/cachebackend_types_test.go

cmd/controller

  • [reviewed] cmd/controller/main.go
  • [reviewed] cmd/controller/main_test.go

cmd/node-local-shm-cleanup

  • [reviewed] cmd/node-local-shm-cleanup/main.go
  • [reviewed] cmd/node-local-shm-cleanup/main_test.go

config/crd/bases

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

config/manager

  • [reviewed] config/manager/manager.yaml

config/samples

  • [reviewed] config/samples/README.md
  • [reviewed] config/samples/cachebackend-sglang-nodelocal-host-only.yaml

dockerfiles

  • [reviewed] dockerfiles/Dockerfile

docs/cli

  • [reviewed] docs/cli/doctor.md

docs/concepts

  • [reviewed] docs/concepts/cachebackend-engine-binding.md

docs/design

  • [reviewed] docs/design/cachebackend-api.md
  • [reviewed] docs/design/grpc-tls.md
  • [reviewed] docs/design/lmcache-multiprocess-migration-roadmap.md
  • [reviewed] docs/design/sglang-lmcache-mp-mode.md

docs/operations

  • [reviewed] docs/operations/container-images.md
  • [reviewed] docs/operations/sbom.md

docs

  • [reviewed] docs/quickstart.md

docs/reference-stack

  • [reviewed] docs/reference-stack/GPU-RUNBOOK.md
  • [reviewed] docs/reference-stack/README.md

docs/reference-stack/manifests/sglang-lmcache

  • [reviewed] docs/reference-stack/manifests/sglang-lmcache/deployment.yaml

docs/reference-stack/scripts

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

hack

  • [reviewed] hack/render-release-install.sh
  • [reviewed] hack/render-release-install_test.sh
  • [reviewed] hack/resolve-release-image-digests.sh
  • [reviewed] hack/resolve-release-image-digests_test.sh
  • [reviewed] hack/sbom-registry-smoke.sh
  • [reviewed] hack/verify-minimal-images.sh
  • [reviewed] hack/verify-minimal-images_test.sh

internal/adapters/builtin/runtime

  • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_nodelocal.go
  • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_nodelocal_test.go
  • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_renderer.go
  • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_renderer_test.go
  • [reviewed] internal/adapters/builtin/runtime/sglang_lmcache_test.go
  • [reviewed] internal/adapters/builtin/runtime/test_helpers_test.go
  • [reviewed] internal/adapters/builtin/runtime/vllm_lmcache_mp.go
  • [reviewed] internal/adapters/builtin/runtime/vllm_lmcache_mp_test.go

internal/controller

  • [reviewed] internal/controller/cachebackend_lmcache_mp_status.go
  • [reviewed] internal/controller/cachebackend_lmcache_mp_status_test.go
  • [reviewed] internal/controller/cachebackend_lmcache_nodelocal.go
  • [reviewed] internal/controller/cachebackend_mp_lifecycle_test.go
  • [reviewed] internal/controller/cachebackend_nodelocal_integration_test.go
  • [reviewed] internal/controller/cachebackend_reconciler.go
  • [reviewed] internal/controller/cachebackend_reconciler_test.go

internal/enginebinding

  • [reviewed] internal/enginebinding/metadata.go

internal/webhook/pod

  • [reviewed] internal/webhook/pod/podinjector_test.go

internal/webhook/v1alpha1

  • [reviewed] internal/webhook/v1alpha1/cachebackend_defaulter_envtest_test.go
  • [reviewed] internal/webhook/v1alpha1/cachebackend_lmcache_mp_validation.go

Findings

Blocking

None.

Should-fix

  • internal/controller/cachebackend_lmcache_nodelocal.go:617 — Consumer detection treats every Pod declaring the hostPath as a live mount, including terminal Pods and, during finalization, unscheduled Pods. Retained completed Pods can therefore prevent cleanup indefinitely, while an unscheduled Pod can permanently strand the CacheBackend finalizer despite never mounting the directory.

Nit

None.

Per-category coverage

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

Verdict

changes-requested

Signed-off-by: Yue Sun <yue.s.sun@oracle.com>
@github-actions

Copy link
Copy Markdown

Codex review

Files reviewed

  • .github/workflows/
    • [reviewed] .github/workflows/ci.yml
    • [reviewed] .github/workflows/release-sbom.yml
  • Repository root
    • [reviewed] Makefile
    • [reviewed] README.md
  • api/v1alpha1/
    • [reviewed] cachebackend_types.go
    • [reviewed] cachebackend_types_test.go
  • cmd/controller/
    • [reviewed] main.go
    • [reviewed] main_test.go
  • cmd/node-local-shm-cleanup/
    • [reviewed] main.go
    • [reviewed] main_test.go
  • config/crd/bases/
    • [skipped — generated] inferencecache.io_cachebackends.yaml
  • config/manager/
    • [reviewed] manager.yaml
  • config/samples/
    • [reviewed] README.md
    • [reviewed] cachebackend-sglang-nodelocal-host-only.yaml
  • dockerfiles/
    • [reviewed] Dockerfile
  • docs/cli/
    • [reviewed] doctor.md
  • docs/concepts/
    • [reviewed] cachebackend-engine-binding.md
  • docs/design/
    • [reviewed] cachebackend-api.md
    • [reviewed] grpc-tls.md
    • [reviewed] lmcache-multiprocess-migration-roadmap.md
    • [reviewed] sglang-lmcache-mp-mode.md
  • docs/operations/
    • [reviewed] container-images.md
    • [reviewed] sbom.md
  • docs/
    • [reviewed] quickstart.md
  • docs/reference-stack/
    • [reviewed] GPU-RUNBOOK.md
    • [reviewed] README.md
  • docs/reference-stack/manifests/sglang-lmcache/
    • [reviewed] deployment.yaml
  • docs/reference-stack/scripts/
    • [reviewed] default_install_smoke.sh
  • hack/
    • [reviewed] render-release-install.sh
    • [reviewed] render-release-install_test.sh
    • [reviewed] resolve-release-image-digests.sh
    • [reviewed] resolve-release-image-digests_test.sh
    • [reviewed] sbom-registry-smoke.sh
    • [reviewed] verify-minimal-images.sh
    • [reviewed] verify-minimal-images_test.sh
  • internal/adapters/builtin/runtime/
    • [reviewed] lmcache_mp_nodelocal.go
    • [reviewed] lmcache_mp_nodelocal_test.go
    • [reviewed] lmcache_mp_renderer.go
    • [reviewed] lmcache_mp_renderer_test.go
    • [reviewed] sglang_lmcache_test.go
    • [reviewed] test_helpers_test.go
    • [reviewed] vllm_lmcache_mp.go
    • [reviewed] vllm_lmcache_mp_test.go
  • internal/controller/
    • [reviewed] cachebackend_lmcache_mp_status.go
    • [reviewed] cachebackend_lmcache_mp_status_test.go
    • [reviewed] cachebackend_lmcache_nodelocal.go
    • [reviewed] cachebackend_mp_lifecycle_test.go
    • [reviewed] cachebackend_nodelocal_integration_test.go
    • [reviewed] cachebackend_reconciler.go
    • [reviewed] cachebackend_reconciler_test.go
  • internal/enginebinding/
    • [reviewed] metadata.go
  • internal/webhook/pod/
    • [reviewed] podinjector_test.go
  • internal/webhook/v1alpha1/
    • [reviewed] cachebackend_defaulter_envtest_test.go
    • [reviewed] cachebackend_lmcache_mp_validation.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

Signed-off-by: Yue Sun <yue.s.sun@oracle.com>
@github-actions

Copy link
Copy Markdown

Codex review

Files reviewed

.github/workflows

  • [reviewed] .github/workflows/ci.yml
  • [reviewed] .github/workflows/release-sbom.yml

Repository root

  • [reviewed] Makefile
  • [reviewed] README.md

api/v1alpha1

  • [reviewed] api/v1alpha1/cachebackend_types.go
  • [reviewed] api/v1alpha1/cachebackend_types_test.go

cmd/controller

  • [reviewed] cmd/controller/main.go
  • [reviewed] cmd/controller/main_test.go

cmd/node-local-shm-cleanup

  • [reviewed] cmd/node-local-shm-cleanup/main.go
  • [reviewed] cmd/node-local-shm-cleanup/main_test.go

config/crd/bases

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

config/manager

  • [reviewed] config/manager/manager.yaml

config/samples

  • [reviewed] config/samples/README.md
  • [reviewed] config/samples/cachebackend-sglang-nodelocal-host-only.yaml

dockerfiles

  • [reviewed] dockerfiles/Dockerfile

docs/cli

  • [reviewed] docs/cli/doctor.md

docs/concepts

  • [reviewed] docs/concepts/cachebackend-engine-binding.md

docs/design

  • [reviewed] docs/design/cachebackend-api.md
  • [reviewed] docs/design/grpc-tls.md
  • [reviewed] docs/design/lmcache-multiprocess-migration-roadmap.md
  • [reviewed] docs/design/sglang-lmcache-mp-mode.md

docs/operations

  • [reviewed] docs/operations/container-images.md
  • [reviewed] docs/operations/sbom.md

docs

  • [reviewed] docs/quickstart.md

docs/reference-stack

  • [reviewed] docs/reference-stack/GPU-RUNBOOK.md
  • [reviewed] docs/reference-stack/README.md

docs/reference-stack/manifests/sglang-lmcache

  • [reviewed] docs/reference-stack/manifests/sglang-lmcache/deployment.yaml

docs/reference-stack/scripts

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

hack

  • [reviewed] hack/render-release-install.sh
  • [reviewed] hack/render-release-install_test.sh
  • [reviewed] hack/resolve-release-image-digests.sh
  • [reviewed] hack/resolve-release-image-digests_test.sh
  • [reviewed] hack/sbom-registry-smoke.sh
  • [reviewed] hack/verify-minimal-images.sh
  • [reviewed] hack/verify-minimal-images_test.sh

internal/adapters/builtin/runtime

  • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_nodelocal.go
  • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_nodelocal_test.go
  • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_renderer.go
  • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_renderer_test.go
  • [reviewed] internal/adapters/builtin/runtime/sglang_lmcache_test.go
  • [reviewed] internal/adapters/builtin/runtime/test_helpers_test.go
  • [reviewed] internal/adapters/builtin/runtime/vllm_lmcache_mp.go
  • [reviewed] internal/adapters/builtin/runtime/vllm_lmcache_mp_test.go

internal/controller

  • [reviewed] internal/controller/cachebackend_lmcache_mp_status.go
  • [reviewed] internal/controller/cachebackend_lmcache_mp_status_test.go
  • [reviewed] internal/controller/cachebackend_lmcache_nodelocal.go
  • [reviewed] internal/controller/cachebackend_mp_lifecycle_test.go
  • [reviewed] internal/controller/cachebackend_nodelocal_integration_test.go
  • [reviewed] internal/controller/cachebackend_reconciler.go
  • [reviewed] internal/controller/cachebackend_reconciler_test.go

internal/enginebinding

  • [reviewed] internal/enginebinding/metadata.go

internal/webhook/pod

  • [reviewed] internal/webhook/pod/podinjector_test.go

internal/webhook/v1alpha1

  • [reviewed] internal/webhook/v1alpha1/cachebackend_defaulter_envtest_test.go
  • [reviewed] internal/webhook/v1alpha1/cachebackend_lmcache_mp_validation.go
  • [reviewed] internal/webhook/v1alpha1/cachebackend_lmcache_mp_validation_test.go

Findings

Blocking

None.

Should-fix

internal/controller/cachebackend_lmcache_nodelocal.go:531 — Verify: exhausting the retry count leaves the terminal cleanup Pod permanently present with no condition, Event, or documented recovery mechanism. This indefinitely blocks server recreation and CacheBackend finalization; operators need an observable, supported recovery path that cannot accidentally bypass required cleanup.

Nit

None.

Per-category coverage

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

Verdict

changes-requested

Signed-off-by: Yue Sun <yue.s.sun@oracle.com>
@github-actions

Copy link
Copy Markdown

Codex review

Files reviewed

  • .github/workflows/
    • [reviewed] .github/workflows/ci.yml
    • [reviewed] .github/workflows/release-sbom.yml
  • repository root
    • [reviewed] Makefile
    • [reviewed] README.md
  • api/v1alpha1/
    • [reviewed] cachebackend_types.go
    • [reviewed] cachebackend_types_test.go
  • cmd/controller/
    • [reviewed] main.go
    • [reviewed] main_test.go
  • cmd/node-local-shm-cleanup/
    • [reviewed] main.go
    • [reviewed] main_test.go
  • config/crd/bases/
    • [skipped — generated] inferencecache.io_cachebackends.yaml
  • config/manager/
    • [reviewed] manager.yaml
  • config/samples/
    • [reviewed] README.md
    • [reviewed] cachebackend-sglang-nodelocal-host-only.yaml
  • dockerfiles/
    • [reviewed] Dockerfile
  • docs/cli/
    • [reviewed] doctor.md
  • docs/concepts/
    • [reviewed] cachebackend-engine-binding.md
  • docs/design/
    • [reviewed] cachebackend-api.md
    • [reviewed] grpc-tls.md
    • [reviewed] lmcache-multiprocess-migration-roadmap.md
    • [reviewed] sglang-lmcache-mp-mode.md
  • docs/operations/
    • [reviewed] container-images.md
    • [reviewed] sbom.md
  • docs/
    • [reviewed] quickstart.md
  • docs/reference-stack/
    • [reviewed] GPU-RUNBOOK.md
    • [reviewed] README.md
  • docs/reference-stack/manifests/sglang-lmcache/
    • [reviewed] deployment.yaml
  • docs/reference-stack/scripts/
    • [reviewed] default_install_smoke.sh
  • hack/
    • [reviewed] render-release-install.sh
    • [reviewed] render-release-install_test.sh
    • [reviewed] resolve-release-image-digests.sh
    • [reviewed] resolve-release-image-digests_test.sh
    • [reviewed] sbom-registry-smoke.sh
    • [reviewed] verify-minimal-images.sh
    • [reviewed] verify-minimal-images_test.sh
  • internal/adapters/builtin/runtime/
    • [reviewed] lmcache_mp_nodelocal.go
    • [reviewed] lmcache_mp_nodelocal_test.go
    • [reviewed] lmcache_mp_renderer.go
    • [reviewed] lmcache_mp_renderer_test.go
    • [reviewed] sglang_lmcache_test.go
    • [reviewed] test_helpers_test.go
    • [reviewed] vllm_lmcache_mp.go
    • [reviewed] vllm_lmcache_mp_test.go
  • internal/controller/
    • [reviewed] cachebackend_lmcache_mp_status.go
    • [reviewed] cachebackend_lmcache_mp_status_test.go
    • [reviewed] cachebackend_lmcache_nodelocal.go
    • [reviewed] cachebackend_mp_lifecycle_test.go
    • [reviewed] cachebackend_nodelocal_integration_test.go
    • [reviewed] cachebackend_reconciler.go
    • [reviewed] cachebackend_reconciler_test.go
  • internal/enginebinding/
    • [reviewed] metadata.go
  • internal/webhook/pod/
    • [reviewed] podinjector_test.go
  • internal/webhook/v1alpha1/
    • [reviewed] cachebackend_defaulter_envtest_test.go
    • [reviewed] cachebackend_lmcache_mp_validation.go
    • [reviewed] cachebackend_lmcache_mp_validation_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

@linear-code

linear-code Bot commented Aug 24, 2026

Copy link
Copy Markdown

CAC-268

CAC-271

@github-actions

Copy link
Copy Markdown

Codex review

Files reviewed

  • .github/workflows/
    • [reviewed] .github/workflows/ci.yml
    • [reviewed] .github/workflows/release-sbom.yml
  • repository root
    • [reviewed] Makefile
    • [reviewed] README.md
  • api/v1alpha1/
    • [reviewed] api/v1alpha1/cachebackend_types.go
    • [reviewed] api/v1alpha1/cachebackend_types_test.go
  • cmd/controller/
    • [reviewed] cmd/controller/main.go
    • [reviewed] cmd/controller/main_test.go
  • cmd/node-local-shm-cleanup/
    • [reviewed] cmd/node-local-shm-cleanup/main.go
    • [reviewed] cmd/node-local-shm-cleanup/main_test.go
  • config/crd/bases/
    • [skipped — generated] config/crd/bases/inferencecache.io_cachebackends.yaml
  • config/manager/
    • [reviewed] config/manager/manager.yaml
  • config/samples/
    • [reviewed] config/samples/README.md
    • [reviewed] config/samples/cachebackend-sglang-nodelocal-host-only.yaml
  • dockerfiles/
    • [reviewed] dockerfiles/Dockerfile
  • docs/cli/
    • [reviewed] docs/cli/doctor.md
  • docs/concepts/
    • [reviewed] docs/concepts/cachebackend-engine-binding.md
  • docs/design/
    • [reviewed] docs/design/cachebackend-api.md
    • [reviewed] docs/design/grpc-tls.md
    • [reviewed] docs/design/lmcache-multiprocess-migration-roadmap.md
    • [reviewed] docs/design/sglang-lmcache-mp-mode.md
  • docs/operations/
    • [reviewed] docs/operations/container-images.md
    • [reviewed] docs/operations/sbom.md
  • docs/
    • [reviewed] docs/quickstart.md
  • docs/reference-stack/
    • [reviewed] docs/reference-stack/GPU-RUNBOOK.md
    • [reviewed] docs/reference-stack/README.md
  • docs/reference-stack/manifests/sglang-lmcache/
    • [reviewed] docs/reference-stack/manifests/sglang-lmcache/deployment.yaml
  • docs/reference-stack/scripts/
    • [reviewed] docs/reference-stack/scripts/default_install_smoke.sh
  • hack/
    • [reviewed] hack/render-release-install.sh
    • [reviewed] hack/render-release-install_test.sh
    • [reviewed] hack/resolve-release-image-digests.sh
    • [reviewed] hack/resolve-release-image-digests_test.sh
    • [reviewed] hack/sbom-registry-smoke.sh
    • [reviewed] hack/verify-minimal-images.sh
    • [reviewed] hack/verify-minimal-images_test.sh
  • internal/adapters/builtin/runtime/
    • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_nodelocal.go
    • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_nodelocal_test.go
    • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_renderer.go
    • [reviewed] internal/adapters/builtin/runtime/lmcache_mp_renderer_test.go
    • [reviewed] internal/adapters/builtin/runtime/sglang_lmcache_test.go
    • [reviewed] internal/adapters/builtin/runtime/test_helpers_test.go
    • [reviewed] internal/adapters/builtin/runtime/vllm_lmcache_mp.go
    • [reviewed] internal/adapters/builtin/runtime/vllm_lmcache_mp_test.go
  • internal/controller/
    • [reviewed] internal/controller/cachebackend_lmcache_mp_status.go
    • [reviewed] internal/controller/cachebackend_lmcache_mp_status_test.go
    • [reviewed] internal/controller/cachebackend_lmcache_nodelocal.go
    • [reviewed] internal/controller/cachebackend_mp_lifecycle_test.go
    • [reviewed] internal/controller/cachebackend_nodelocal_integration_test.go
    • [reviewed] internal/controller/cachebackend_reconciler.go
    • [reviewed] internal/controller/cachebackend_reconciler_test.go
  • internal/enginebinding/
    • [reviewed] internal/enginebinding/metadata.go
  • internal/webhook/pod/
    • [reviewed] internal/webhook/pod/podinjector_test.go
  • internal/webhook/v1alpha1/
    • [reviewed] internal/webhook/v1alpha1/cachebackend_defaulter_envtest_test.go
    • [reviewed] internal/webhook/v1alpha1/cachebackend_lmcache_mp_validation.go
    • [reviewed] internal/webhook/v1alpha1/cachebackend_lmcache_mp_validation_test.go

Findings

Blocking

None.

Should-fix

  • internal/controller/cachebackend_lmcache_nodelocal.go:423 — Finalization cannot clean a pool when both its server and injected engine Pods disappeared before the controller recorded a cleanup intent. No durable node history remains, so the finalizer sees no consumers or cleanup Pods and removes itself while the UID directory and IPC files remain on the former node.
  • internal/adapters/builtin/runtime/lmcache_mp_nodelocal.go:209 — Cleanup Pods explicitly run as UID 0 and therefore are rejected in namespaces enforcing the Kubernetes Restricted Pod Security Standard. Because cleanup runs in the CacheBackend namespace, such clusters can permanently block idle cleanup and CacheBackend deletion; either support the policy or make this prerequisite and recovery path explicit in the API/design documentation and tests.

Nit

None.

Per-category coverage

  1. Vendor-neutral naming: Clean
  2. Contract & spec / docs sync: findings above at: internal/controller/cachebackend_lmcache_nodelocal.go:423, internal/adapters/builtin/runtime/lmcache_mp_nodelocal.go:209
  3. gRPC fail-open semantics: Clean
  4. Quality (tests / error wrapping / package placement / generated drift): findings above at: internal/controller/cachebackend_lmcache_nodelocal.go:423, internal/adapters/builtin/runtime/lmcache_mp_nodelocal.go:209

Verdict

changes-requested

@fredericsun
fredericsun merged commit 1ec8fb3 into main Aug 24, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants