Skip to content

fix(nvca): inject transport trust into Helm LLM workers - #900

Merged
mikeyrcamp merged 2 commits into
mainfrom
mcamp/fix/helm-llm-transport-trust
Aug 17, 2026
Merged

fix(nvca): inject transport trust into Helm LLM workers#900
mikeyrcamp merged 2 commits into
mainfrom
mcamp/fix/helm-llm-transport-trust

Conversation

@mikeyrcamp

@mikeyrcamp mikeyrcamp commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

TL;DR

Inject configured transport trust into the utility pod generated for Helm-based LLM functions so the LLM worker can use the merged certificate bundle.

Additional Details

Helm LLM translation places the llm-worker container in a utility pod that reconciliation separates from the normal workload object collection. Transport trust mutation previously ran only on that workload collection, so the utility pod was appended without the trust ConfigMap volume, merged-certificate volume, installer init container, certificate mount, or STARGATE_TLS_CERT_PATH.

This change:

  • Includes the utility pod in transport trust preparation while preserving the existing llm-worker filter.
  • Copies the LLM worker resource requirements to the injected installer init container so resource validation continues to pass.
  • Adds controller-level regression coverage for the real Helm LLM lifecycle.
  • Adds focused coverage for installer resource inheritance.

Dependencies: none. License and NOTICE impact: none.

For the Reviewer

Please focus on the utility-pod inclusion in internal/miniservice/reconcile.go and resource inheritance in internal/transporttls/transport_tls.go.

For QA

Verified with:

  • Go tests for the transport TLS and MiniService packages.
  • Bazel tests for the same packages.
  • Scoped Go linting.
  • A k3d API-server round trip confirming the installer resources, trust volumes, merged certificate mount, and certificate-path environment variable.

Additional QA is not required.

Issues

Fixes #606

Relates to #19

Checklist

  • I am familiar with the Contributing Guidelines.
  • I have signed off my commits for Developer Certificate of Origin (DCO) compliance.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

Summary by CodeRabbit

  • Enhancements

    • Transport TLS setup now applies to utility workloads as well as worker workloads.
    • TLS installation containers inherit the worker’s CPU and memory requests and limits.
    • Helm resource-limit enforcement is covered for LLM workloads, including generated workers and TLS initialization containers.
  • Tests

    • Expanded integration coverage for TLS certificates, trust bundles, mounts, volumes, and resource settings.

@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 18696d68-40d3-4f9b-b3f2-de6bca44fbc0

📥 Commits

Reviewing files that changed from the base of the PR and between 1be91b8 and cc81336.

📒 Files selected for processing (6)
  • src/compute-plane-services/nvca/internal/miniservice/BUILD.bazel
  • src/compute-plane-services/nvca/internal/miniservice/controller_test.go
  • src/compute-plane-services/nvca/internal/miniservice/reconcile.go
  • src/compute-plane-services/nvca/internal/transporttls/BUILD.bazel
  • src/compute-plane-services/nvca/internal/transporttls/transport_tls.go
  • src/compute-plane-services/nvca/internal/transporttls/transport_tls_test.go

Included review availability: Your plan includes up to 12 reviews per rolling hour; 11 remain after this review.


📝 Walkthrough

Walkthrough

The change applies transport TLS preparation to Helm LLM utility pods and propagates LLM worker resource requirements to the TLS installer init container. Integration tests verify TLS resources, mounts, environment variables, and trust-bundle data.

Changes

Helm LLM transport TLS

Layer / File(s) Summary
Transport TLS resource propagation
src/compute-plane-services/nvca/internal/transporttls/...
InjectIntoPodSpec passes the LLM worker resource requirements to the generated install init container. Unit tests verify CPU and memory requests and limits.
Helm utility pod wiring and regression coverage
src/compute-plane-services/nvca/internal/miniservice/...
MiniService reconciliation includes utilsPod in transport TLS preparation. The integration test verifies the LLM worker, TLS init container, volumes, mounts, environment, and trust-bundle ConfigMap.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to cc813

The change injects transport trust into Helm LLM utility pods and preserves installer resource validation; no actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: apartha-nv

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title uses the required fix(nvca) format and accurately describes the transport trust injection bug fix.
Linked Issues check ✅ Passed The changes address issue #606 by mutating the Helm utils pod, preserving worker filtering, inheriting resources, and adding lifecycle regression coverage.
Out of Scope Changes check ✅ Passed All code, test, and Bazel changes support transport trust injection and installer resource inheritance for Helm LLM utility pods.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch mcamp/fix/helm-llm-transport-trust

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="Running error: context loading failed: failed to load packages: failed to load packages: failed to load with go/packages: err: exit status 1: stderr: go: inconsistent vendoring in /src/compute-plane-services/nvca:\n\tgithub.com/NVIDIA/KAI-scheduler@v0.12.6: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/NVIDIA/k8s-dra-driver-gpu@v0.0.0-20251017125642-cfe35ffd3d2c: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/NVIDIA/nvcf/src/libraries/go/lib@v0.0.0-20260722095202-f5e2792f5630: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/aws/aws-sdk-go@v1.55.5: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/bombsimon/logrusr/v4@v4.1.0: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/evanphx/json-patch/v5@v5.9.11: is explicitly required in

... [truncated 21721 characters] ...

i: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/apiextensions-apiserver: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/apimachinery: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/client-go: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/component-base: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tsigs.k8s.io/controller-runtime: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tgolang.org/x/crypto: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\n\tTo ignore the vendor directory, use -mod=readonly or -mod=mod.\n\tTo sync the vendor directory, run:\n\t\tgo mod vendor\n"


Comment @coderabbitai help to get the list of available commands.

@mikeyrcamp
mikeyrcamp marked this pull request as ready for review August 17, 2026 16:29
@mikeyrcamp
mikeyrcamp requested a review from a team as a code owner August 17, 2026 16:29
@mikeyrcamp
mikeyrcamp requested a review from shobham-nv August 17, 2026 16:29
Comment thread src/compute-plane-services/nvca/internal/miniservice/controller_test.go Outdated
@mikeyrcamp
mikeyrcamp enabled auto-merge August 17, 2026 17:21
Route the translator-created Helm utility pod through the existing transport TLS preparation path. Copy the LLM worker resource requirements onto the trust installer so enforced resource validation continues to pass.

Fixes #606

Signed-off-by: Mike Camp <mcamp@nvidia.com>
Add a distinct Helm LLM transport trust controller case while keeping the existing default-function scenario intact.
@mikeyrcamp
mikeyrcamp force-pushed the mcamp/fix/helm-llm-transport-trust branch from 4d6cf15 to 2da252b Compare August 17, 2026 17:23
@mikeyrcamp
mikeyrcamp added this pull request to the merge queue Aug 17, 2026
Merged via the queue into main with commit 824774d Aug 17, 2026
19 checks passed
@mikeyrcamp
mikeyrcamp deleted the mcamp/fix/helm-llm-transport-trust branch August 17, 2026 17:42
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.

fix(nvca): inject transport trust into Helm LLM utility pods

2 participants