fix(nvca): persist workload config after Helm updates

[rohithb-hub]

TL;DR

The Helm-update path in doUpdateWorkload discarded the WorkloadConfig decoded
from the nvcf-workload-config control ConfigMap. A successful revision could
apply new workload objects while MiniService.spec.workloadConfig kept describing
the prior revision. Status reconciliation then used stale config, for example
leaving StatusByWorkerReadiness disabled after a revision that enabled it.

Fix: prepareUpdateWorkload now returns the decoded WorkloadConfig, and
doUpdateWorkload calls saveWorkloadConfig after a successful apply. The existing
targeted SSA contract from #626 is preserved: miniservice-controller owns only
spec.workloadConfig and unrelated MiniService spec fields are untouched.

Additional Details

The root cause is a single discarded return value. decodeObjects has always
returned a *v1alpha1.WorkloadConfig as its third value. The install path
(doInstall) captured it and called saveWorkloadConfig. The update path
(prepareUpdateWorkload) assigned it to _, so the decoded config was never
propagated.

Key behavior:

• Absent nvcf-workload-config in the rendered output clears spec.workloadConfig,
  consistent with how saveWorkloadConfig handles a nil desired value via SSA.
• Render, validation, or apply failures return before saveWorkloadConfig is
  called, so the prior config stays aligned with the workload revision still serving.
• saveWorkloadConfig is called unchanged, so the targeted SSA payload introduced
  in fix(nvca): preserve MiniService spec when saving workload config #626 continues to restrict ownership to spec.workloadConfig only.

For the Reviewer

reconcile.go: two mechanical changes only. prepareUpdateWorkload return
signature gets a new *v1alpha1.WorkloadConfig slot; all early returns updated.
doUpdateWorkload unpacks the new return value and calls saveWorkloadConfig
after applySSAWorkload.

reconcile_update_test.go: four new tests and two setup helpers, all
self-contained. No existing tests were modified.

For QA

Ran:

go test ./internal/miniservice/... \
  -ldflags '-X github.com/NVIDIA/k8s-dra-driver-gpu/internal/info.version=v25.8.0' \
  -count=1

All tests pass. The only failure in the package is TestController, which is
pre-existing and unrelated to this change. QA not needed: the fix is contained
to the in-process reconcile path with full unit coverage added.

Issues

Closes #651

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

• Bug Fixes
  • Workload configuration is now correctly saved when workload updates succeed.
  • Updated configurations replace previous values, while removing the configuration clears it as expected.
  • Existing workload configuration is preserved when an update fails.
  • Unrelated MiniService settings remain unchanged during workload updates.
  • Workload configuration handling now behaves consistently across supported update scenarios.

[coderabbitai]

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: 5569ca40-b492-422e-ac37-8f0a6edf6c72

📥 Commits

Reviewing files that changed from the base of the PR and between bdd82dc and f737c51.

📒 Files selected for processing (1)

• src/compute-plane-services/nvca/internal/miniservice/reconcile_update_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• src/compute-plane-services/nvca/internal/miniservice/reconcile_update_test.go

---

📝 Walkthrough

Walkthrough

The update reconciliation flow retains decoded WorkloadConfig data, persists it after successful workload application, clears it when absent, and preserves the previous value when application fails. Tests cover these scenarios and unrelated spec fields.

Changes

Workload configuration persistence

Layer / File(s)	Summary
Update preparation and persistence src/compute-plane-services/nvca/internal/miniservice/reconcile.go	prepareUpdateWorkload returns the decoded WorkloadConfig. doUpdateWorkload saves it after workload objects apply successfully.
Update scenario coverage src/compute-plane-services/nvca/internal/miniservice/reconcile_update_test.go	Tests cover configuration creation, replacement, clearing, unrelated spec-field preservation, and retention after failed workload application.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant RenderedWorkload
  participant prepareUpdateWorkload
  participant doUpdateWorkload
  participant MiniService
  RenderedWorkload->>prepareUpdateWorkload: Render workload objects and WorkloadConfig
  prepareUpdateWorkload->>doUpdateWorkload: Return decoded WorkloadConfig
  doUpdateWorkload->>MiniService: Apply workload objects
  doUpdateWorkload->>MiniService: Persist WorkloadConfig

Loading

Possibly related PRs

• NVIDIA/nvcf#626: Adds the targeted workload-config persistence flow used by this update path.

Suggested reviewers: vrv3814

🚥 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 Conventional Commits format and accurately identifies the workload-config persistence bug fix.
Linked Issues check	✅ Passed	The changes carry, persist, replace, clear, and preserve WorkloadConfig as required by issue #651, with targeted regression tests.
Out of Scope Changes check	✅ Passed	All production and test changes directly support workload-config persistence during Helm updates and the linked issue objectives.
Docstring Coverage	✅ Passed	Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/nvca-persist-workload-config-helm-update

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="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

---

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

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

src/compute-plane-services/nvca/internal/miniservice/reconcile_update_test.go (1)

619-665: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use a table-driven test for the successful transitions.

These tests share the same setup and doUpdateWorkload call. Put the enabled and disabled configurations in table cases with t.Run.

As per coding guidelines, tests with multiple scenarios must use table-driven tests.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/compute-plane-services/nvca/internal/miniservice/reconcile_update_test.go`
around lines 619 - 665, Refactor the successful workload-config transition tests
around TestDoUpdateWorkload_PersistsWorkloadConfig_NoConfigToEnabled and
TestDoUpdateWorkload_PersistsWorkloadConfig_EnabledToDisabled into one
table-driven test with t.Run cases. Parameterize the prior config, rendered
feature-flag value, and expected enabled state while retaining the shared setup,
doUpdateWorkload invocation, status assertions, and unrelated-spec checks where
applicable.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@src/compute-plane-services/nvca/internal/miniservice/reconcile_update_test.go`:
- Line 677: Update the comment describing the empty configYAML case to replace
the non-ASCII arrow with ASCII text, while preserving its meaning and leaving
the test logic unchanged.

In `@src/compute-plane-services/nvca/internal/miniservice/reconcile.go`:
- Around line 1032-1035: Update the saveWorkloadConfig flow around the
NewInstrumentedCRClient.Patch call to wrap the Kubernetes patch operation with
nvcaotel.InvokeWithSpan. Within the span callback, record patch failures using
RecordError and SetStatus, while preserving the existing error propagation from
saveWorkloadConfig.

---

Nitpick comments:
In
`@src/compute-plane-services/nvca/internal/miniservice/reconcile_update_test.go`:
- Around line 619-665: Refactor the successful workload-config transition tests
around TestDoUpdateWorkload_PersistsWorkloadConfig_NoConfigToEnabled and
TestDoUpdateWorkload_PersistsWorkloadConfig_EnabledToDisabled into one
table-driven test with t.Run cases. Parameterize the prior config, rendered
feature-flag value, and expected enabled state while retaining the shared setup,
doUpdateWorkload invocation, status assertions, and unrelated-spec checks where
applicable.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c608f8ca-9c86-4d4f-84c1-cbd73b6218a5

📥 Commits

Reviewing files that changed from the base of the PR and between 2c6475b and bdd82dc.

📒 Files selected for processing (2)

• src/compute-plane-services/nvca/internal/miniservice/reconcile.go
• src/compute-plane-services/nvca/internal/miniservice/reconcile_update_test.go