fix(bundler): honor bundlers query param; driver-managed cleanup#1606
Conversation
Recipe evidence checkNo leaf overlays affected by this PR. This gate is warning-only and never blocks merge. |
|
@CodeRabbit full review |
✅ Action performedFull review finished. |
Coverage Report ✅
Coverage BadgeMerging this branch will increase overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds a Estimated code review effort: 3 (Moderate) | ~25 minutes Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Handler as ParseQueryParams
participant Config
participant Bundler as filterEnabledComponents
Client->>Handler: POST /v1/bundle?bundlers=a,b
Handler->>Handler: split, trim, drop empty names
Handler->>Config: WithBundlers(names)
Config-->>Bundler: Bundlers() filter set
Bundler->>Bundler: validate names declared & enabled
alt unknown or disabled name
Bundler-->>Client: ErrCodeInvalidRequest (HTTP 400)
else valid names
Bundler->>Bundler: prune enabledRefs and enabledSet
Bundler-->>Client: bundle with requested subset
end
Related issues: Related PRs: None identified. Suggested labels: documentation, enhancement, bundler, tools Suggested reviewers: Reviewers familiar with the bundler package and cleanup tooling. Poem Names in a list, trimmed and neat, 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
api/aicr/v1/server.yaml (2)
1557-1565: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDeprecated
BundleRequest.bundlersschema still describes old semantics.Enum still lists a fixed 6-value "bundler type" set and describes "bundler types to execute", contradicting the new dynamic recipe component-name filter. Since this schema is deprecated and no longer referenced by the actual
/v1/bundlerequestBody, low priority, but worth a note so it doesn't confuse readers of the deprecated path.As per coding guidelines, "Keep the OpenAPI contract in sync with user-visible API changes, including new endpoints, query parameters, and enum values."
🤖 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 `@api/aicr/v1/server.yaml` around lines 1557 - 1565, The deprecated BundleRequest.bundlers schema still reflects the old fixed bundler-type semantics, which no longer match the dynamic recipe component-name filtering. Update the deprecated schema in api/aicr/v1/server.yaml so its description and example clearly align with the current behavior, and remove or revise the hardcoded enum values in the BundleRequest definition to avoid exposing outdated API contract details. Keep the changes localized to the deprecated BundleRequest schema and related OpenAPI metadata.Source: Coding guidelines
1026-1055: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winStale
invalidBundler400 example.The
invalidBundlerexample underPOST /v1/bundlestill documents the old "bundler type" validation error ("Invalid bundler type", "unsupported bundle type: ..."), not the new component-name filter's actual error format (unknown component %q in bundlers filter; recipe declares: %sor the "disabled" variant fromfilterEnabledComponents). This directly contradicts the semantics just documented for thebundlersparameter above (lines 745-750).As per coding guidelines, "Keep the OpenAPI contract in sync with user-visible API changes, including new endpoints, query parameters, and enum values."
📝 Proposed fix
invalidBundler: - summary: Invalid bundler type + summary: Unknown component in bundlers filter value: code: INVALID_REQUEST - message: "Invalid bundler type" + message: "unknown component \"invalid-bundler\" in bundlers filter; recipe declares: gpu-operator, network-operator" details: - bundler: "invalid-bundler" - error: "unsupported bundle type: invalid-bundler" - valid: ["gpu-operator", "network-operator", "nodewright-operator", "nvsentinel", "cert-manager", "nvidia-dra-driver-gpu"] requestId: "550e8400-e29b-41d4-a716-446655440000" timestamp: "2025-01-15T10:30:00Z" retryable: false🤖 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 `@api/aicr/v1/server.yaml` around lines 1026 - 1055, The `invalidBundler` example for `POST /v1/bundle` is stale and still shows the old bundler-type validation message. Update the OpenAPI example in `server.yaml` to match the current `bundlers` filter behavior and the actual errors produced by `filterEnabledComponents` / the bundlers validation path, including the `unknown component %q in bundlers filter; recipe declares: %s` and disabled-component variant as appropriate. Keep the example payload aligned with the `Error` schema and the user-visible behavior documented for the `bundlers` query parameter.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.
Outside diff comments:
In `@api/aicr/v1/server.yaml`:
- Around line 1557-1565: The deprecated BundleRequest.bundlers schema still
reflects the old fixed bundler-type semantics, which no longer match the dynamic
recipe component-name filtering. Update the deprecated schema in
api/aicr/v1/server.yaml so its description and example clearly align with the
current behavior, and remove or revise the hardcoded enum values in the
BundleRequest definition to avoid exposing outdated API contract details. Keep
the changes localized to the deprecated BundleRequest schema and related OpenAPI
metadata.
- Around line 1026-1055: The `invalidBundler` example for `POST /v1/bundle` is
stale and still shows the old bundler-type validation message. Update the
OpenAPI example in `server.yaml` to match the current `bundlers` filter behavior
and the actual errors produced by `filterEnabledComponents` / the bundlers
validation path, including the `unknown component %q in bundlers filter; recipe
declares: %s` and disabled-component variant as appropriate. Keep the example
payload aligned with the `Error` schema and the user-visible behavior documented
for the `bundlers` query parameter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 2d1d8dc3-cfcc-4dbf-9bcc-821260aa22e9
📒 Files selected for processing (12)
DEVELOPMENT.mdapi/aicr/v1/server.yamldocs/integrator/index.mddocs/user/api-reference.mdpkg/bundler/bundler.gopkg/bundler/bundler_test.gopkg/bundler/config/config.gopkg/bundler/config/config_test.gopkg/bundler/handler.gopkg/bundler/handler_test.gopkg/server/bundle_handler.gotools/cleanup
mchmarny
force-pushed
the
fix/bundlers-filter-cleanup-uvm
branch
from
41c9430 to
972648c
Compare
|
@CodeRabbit full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/aicr/v1/server.yaml (1)
1026-1055: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winStale
invalidBundler400 example doesn't match new filter error semantics.The
bundlersparameter now rejects unknown/disabled component names with messages like "unknown component ... in bundlers filter" or "component ... is disabled and cannot be selected via the bundlers filter" (perfilterEnabledComponents), but theinvalidBundlerexample still documents the old "Invalid bundler type" / "unsupported bundle type" shape with a fixedvalidenum list. Update the example (and consider renaming it) to reflect the new unknown-name/disabled-name rejection.As per coding guidelines:
api/aicr/v1/server.yaml: "Keep the OpenAPI contract in sync with user-visible API changes, including new endpoints, query parameters, and enum values."📝 Proposed fix
invalidBundler: - summary: Invalid bundler type + summary: Unknown component in bundlers filter value: code: INVALID_REQUEST - message: "Invalid bundler type" + message: "unknown component \"invalid-bundler\" in bundlers filter; recipe declares: gpu-operator, network-operator" details: - bundler: "invalid-bundler" - error: "unsupported bundle type: invalid-bundler" - valid: ["gpu-operator", "network-operator", "nodewright-operator", "nvsentinel", "cert-manager", "nvidia-dra-driver-gpu"] requestId: "550e8400-e29b-41d4-a716-446655440000" timestamp: "2025-01-15T10:30:00Z" retryable: false🤖 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 `@api/aicr/v1/server.yaml` around lines 1026 - 1055, The 400 response example for bundler filtering is outdated and no longer matches the error shapes produced by filterEnabledComponents. Update the OpenAPI example under the invalidBundler example to reflect the new user-visible failures for unknown or disabled component names in the bundlers filter, including the new message text and a details payload consistent with those cases. If the current example name no longer fits, rename it to something that clearly describes unknown/disabled bundler selection rather than “Invalid bundler type,” and remove the stale fixed valid enum list.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 `@pkg/bundler/handler.go`:
- Around line 269-284: The bundlers query parsing in handler.go cannot
distinguish an absent parameter from an explicitly empty one because
Query.Get("bundlers") returns the same value for both, causing ?bundlers= to
bypass validation and fall back to bundling everything. Update the bundler
parsing logic in the request handler to detect parameter presence separately
from value (for example, using the query map’s key presence) and treat an
explicitly empty bundlers value the same as other unusable inputs by returning
the existing invalid-request error from the bundlers parsing block.
In `@tools/cleanup`:
- Around line 118-149: The driver-managed detection in detect_driver_managed is
hardcoded to the gpu-operator namespace, so it can miss Talos installs in
privileged-gpu-operator and fail to set DRIVER_MANAGED. Update the kubectl get
ds and helm get values probes to resolve the actual release/daemonset namespace
dynamically or search across namespaces, matching the approach used by
pkg/bundler/deployer/helm/templates/deploy.sh.tmpl. Keep the existing fail-open
behavior, but make the namespace lookup robust so the warning is emitted
whenever gpu-operator is actually managing the driver.
---
Outside diff comments:
In `@api/aicr/v1/server.yaml`:
- Around line 1026-1055: The 400 response example for bundler filtering is
outdated and no longer matches the error shapes produced by
filterEnabledComponents. Update the OpenAPI example under the invalidBundler
example to reflect the new user-visible failures for unknown or disabled
component names in the bundlers filter, including the new message text and a
details payload consistent with those cases. If the current example name no
longer fits, rename it to something that clearly describes unknown/disabled
bundler selection rather than “Invalid bundler type,” and remove the stale fixed
valid enum list.
🪄 Autofix (Beta)
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: ASSERTIVE
Plan: Enterprise
Run ID: 6ad8663d-1a17-40eb-ab46-2bed1f36c022
📒 Files selected for processing (12)
DEVELOPMENT.mdapi/aicr/v1/server.yamldocs/integrator/index.mddocs/user/api-reference.mdpkg/bundler/bundler.gopkg/bundler/bundler_test.gopkg/bundler/config/config.gopkg/bundler/config/config_test.gopkg/bundler/handler.gopkg/bundler/handler_test.gopkg/server/bundle_handler.gotools/cleanup
mchmarny
force-pushed
the
fix/bundlers-filter-cleanup-uvm
branch
from
972648c to
661a0e2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@api/aicr/v1/server.yaml`:
- Around line 1565-1570: The deprecated schema description for the recipe
component list is too broad about what the bundlers query parameter accepts.
Update the OpenAPI text near the bundled components field in
api/aicr/v1/server.yaml to match the filtering behavior in
filterEnabledComponents, stating that only enabled recipe components are
accepted and that recipe-declared but disabled components are rejected with HTTP
400. Keep the wording aligned with the existing bundler contract so the
server.yaml description stays in sync with user-visible behavior.
- Around line 745-750: The bundlers documentation is missing the rejection case
for components disabled via the set query parameter. Update the OpenAPI
descriptions in server.yaml for both the bundlers query parameter and the
deprecated BundleRequest.bundlers field so they explicitly say that names the
recipe does not declare, or the recipe/--set disables, are rejected with HTTP
400. Use the existing bundlers filtering wording and keep it aligned with the
bundle-time validation in the server bundle request flow.
🪄 Autofix (Beta)
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: ASSERTIVE
Plan: Enterprise
Run ID: dcc97eea-123f-4fea-a94e-fa8433aaaaa4
📒 Files selected for processing (12)
DEVELOPMENT.mdapi/aicr/v1/server.yamldocs/integrator/index.mddocs/user/api-reference.mdpkg/bundler/bundler.gopkg/bundler/bundler_test.gopkg/bundler/config/config.gopkg/bundler/config/config_test.gopkg/bundler/handler.gopkg/bundler/handler_test.gopkg/server/bundle_handler.gotools/cleanup
mchmarny
force-pushed
the
fix/bundlers-filter-cleanup-uvm
branch
from
661a0e2 to
b7dc449
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tools/cleanup`:
- Around line 124-163: The cleanup detection pipelines in
detect_driver_managed() can still terminate the script under set -euo pipefail
when kubectl or helm returns non-zero, which breaks the intended fail-open
behavior. Update the kubectl get daemonset ... | awk and helm list -A ... | awk
paths so their exit status is neutralized (for example by appending || true or
equivalent handling) while keeping the existing DRIVER_MANAGED logic intact.
🪄 Autofix (Beta)
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: ASSERTIVE
Plan: Enterprise
Run ID: caedd1c0-68b2-4e17-b089-fc7a29ee9992
📒 Files selected for processing (12)
DEVELOPMENT.mdapi/aicr/v1/server.yamldocs/integrator/index.mddocs/user/api-reference.mdpkg/bundler/bundler.gopkg/bundler/bundler_test.gopkg/bundler/config/config.gopkg/bundler/config/config_test.gopkg/bundler/handler.gopkg/bundler/handler_test.gopkg/server/bundle_handler.gotools/cleanup
mchmarny
force-pushed
the
fix/bundlers-filter-cleanup-uvm
branch
from
b7dc449 to
f822d6f
Compare
|
@CodeRabbit full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
♻️ Duplicate comments (2)
api/aicr/v1/server.yaml (2)
744-750: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winBundlers doc still omits
--set-disabled rejection case.The current description only says a name "the recipe disables" is rejected; it doesn't mention that a component disabled via the
setquery parameter (--set component:enabled=false) is also rejected with the samecomponent "X" is disabled and cannot be selected via the bundlers filtererror, perfilterEnabledComponentsinpkg/bundler/bundler.go. This was flagged in a prior review round and marked "addressed", but the text shown here still matches the original, unfixed wording.📝 Suggested doc fix
- Comma-delimited list of recipe component names to bundle. - If not specified, all enabled components are bundled. - Components not listed are skipped as if disabled (their - dependency edges are treated as satisfied externally). - A name the recipe does not declare, or one the recipe - disables, is rejected with HTTP 400. + Comma-delimited list of recipe component names to bundle. + If not specified, all enabled components are bundled. + Components not listed are skipped as if disabled (their + dependency edges are treated as satisfied externally). + A name the recipe does not declare, or one disabled by the + recipe or a `set` override, is rejected with HTTP 400.As per coding guidelines,
api/aicr/v1/server.yaml: "Keep the OpenAPI contract in sync with user-visible API changes, including new endpoints, query parameters, and enum values."🤖 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 `@api/aicr/v1/server.yaml` around lines 744 - 750, Update the Bundlers API description in the server.yaml OpenAPI contract to explicitly mention that a component disabled via the set query parameter (--set component:enabled=false) is also rejected by the bundlers filter. Keep the wording aligned with filterEnabledComponents in pkg/bundler/bundler.go and make clear that both recipe-disabled and set-disabled components return the same HTTP 400 rejection.Source: Coding guidelines
1565-1569: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDeprecated field description still overstates accepted names.
The text "accepts any component name the recipe declares" is inaccurate — a declared-but-disabled component is rejected with HTTP 400 by
filterEnabledComponents, not accepted. This was flagged previously and marked "addressed", but the wording shown here is unchanged from the original.📝 Suggested doc fix
- Optional list of recipe component names to bundle. - If not specified, all enabled components are bundled. - Superseded by the "bundlers" query parameter, which accepts - any component name the recipe declares. + Optional list of recipe component names to bundle. + If not specified, all enabled components are bundled. + Superseded by the "bundlers" query parameter, which only + accepts enabled recipe components; a name the recipe does + not declare, or one disabled by the recipe or a `set` + override, is rejected with HTTP 400.As per coding guidelines,
api/aicr/v1/server.yaml: "Keep the OpenAPI contract in sync with user-visible API changes."🤖 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 `@api/aicr/v1/server.yaml` around lines 1565 - 1569, Update the deprecated bundler description in server.yaml so it no longer claims the query parameter accepts any declared component name. Align the wording with filterEnabledComponents and the bundlers parameter behavior by stating that only enabled component names are accepted and declared-but-disabled components are rejected with HTTP 400. Keep the rest of the OpenAPI text unchanged while correcting the user-visible contract language.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.
Duplicate comments:
In `@api/aicr/v1/server.yaml`:
- Around line 744-750: Update the Bundlers API description in the server.yaml
OpenAPI contract to explicitly mention that a component disabled via the set
query parameter (--set component:enabled=false) is also rejected by the bundlers
filter. Keep the wording aligned with filterEnabledComponents in
pkg/bundler/bundler.go and make clear that both recipe-disabled and set-disabled
components return the same HTTP 400 rejection.
- Around line 1565-1569: Update the deprecated bundler description in
server.yaml so it no longer claims the query parameter accepts any declared
component name. Align the wording with filterEnabledComponents and the bundlers
parameter behavior by stating that only enabled component names are accepted and
declared-but-disabled components are rejected with HTTP 400. Keep the rest of
the OpenAPI text unchanged while correcting the user-visible contract language.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: d247f95d-66a8-4082-b38c-1d2a23e96e1d
📒 Files selected for processing (12)
DEVELOPMENT.mdapi/aicr/v1/server.yamldocs/integrator/index.mddocs/user/api-reference.mdpkg/bundler/bundler.gopkg/bundler/bundler_test.gopkg/bundler/config/config.gopkg/bundler/config/config_test.gopkg/bundler/handler.gopkg/bundler/handler_test.gopkg/server/bundle_handler.gotools/cleanup
Implement the documented POST /v1/bundle 'bundlers' query parameter as a positive filter on recipe component names: unlisted enabled components are skipped like disabled ones (dependency edges pruned as satisfied externally); unknown or disabled names are rejected with 400. tools/cleanup now detects driver-managed clusters (gpu-operator driver.enabled=true) and prints cordon/reboot/uncordon guidance before the confirmation prompt and after completion, since uninstalling gpu-operator can leave nvidia_uvm wedged mid-unload and break the next install. DEVELOPMENT.md documents the redeploy workaround. Fixes: #1531 Fixes: #1553 Signed-off-by: Mark Chmarny <[email protected]>
mchmarny
force-pushed
the
fix/bundlers-filter-cleanup-uvm
branch
from
82badaf to
733f74e
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Re the two duplicate comments in review 4626014951: that review ran against |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
DEVELOPMENT.md (1)
549-581: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider linking the new subsection from the Common Issues table.
The table above (Line 553-560) lists quick-reference issues but doesn't point readers to this new subsection for driver-managed redeploy failures.
📝 Optional addition to the Common Issues table
| Build failures | Run `make tidy` to update dependencies | | K8s connection fails | Check `~/.kube/config` or `KUBECONFIG` env | +| Reinstall fails after `make cleanup` on a driver-managed cluster | See [Redeploying on Driver-Managed Clusters](`#redeploying-on-driver-managed-clusters`) |🤖 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 `@DEVELOPMENT.md` around lines 549 - 581, Add a reference from the Common Issues table to the new “Redeploying on Driver-Managed Clusters” subsection so readers can jump directly to the driver-managed GPU-node reboot guidance. Update the table entry set in DEVELOPMENT.md near the existing “Build failures” and “K8s connection fails” rows, and point it at the new subsection that describes `make cleanup`, `driver.enabled`, and `nvidia-driver-daemonset` behavior.
🤖 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.
Outside diff comments:
In `@DEVELOPMENT.md`:
- Around line 549-581: Add a reference from the Common Issues table to the new
“Redeploying on Driver-Managed Clusters” subsection so readers can jump directly
to the driver-managed GPU-node reboot guidance. Update the table entry set in
DEVELOPMENT.md near the existing “Build failures” and “K8s connection fails”
rows, and point it at the new subsection that describes `make cleanup`,
`driver.enabled`, and `nvidia-driver-daemonset` behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: e6a2e78e-1210-43ac-8ccc-707d341e98a7
📒 Files selected for processing (12)
DEVELOPMENT.mdapi/aicr/v1/server.yamldocs/integrator/index.mddocs/user/api-reference.mdpkg/bundler/bundler.gopkg/bundler/bundler_test.gopkg/bundler/config/config.gopkg/bundler/config/config_test.gopkg/bundler/handler.gopkg/bundler/handler_test.gopkg/server/bundle_handler.gotools/cleanup
1 participant
Summary
Implements the documented
POST /v1/bundlebundlersquery parameter as a positive component-name filter, and makestools/cleanupdetect driver-managed clusters and print the GPU-node reboot guidance needed to avoid a wedgednvidia_uvmon redeploy.Motivation / Context
bundlerswas documented in the OpenAPI spec and API reference but never read by the server — every request bundled all components. Separately,tools/cleanupon driver-managed clusters (gpu-operatordriver.enabled=true, e.g. EKS) can leavenvidia_uvmwedged mid-unload, breaking the next install until the GPU nodes are rebooted; the script gave no warning.Fixes: #1531
Fixes: #1553
Type of Change
Component(s) Affected
cmd/aicrd,pkg/server)pkg/bundler,pkg/component/*)docs/,examples/)tools/cleanup,DEVELOPMENT.mdImplementation Notes
filterEnabledComponentsafter recipe-levelenabledand--settoggles: unlisted enabled components are skipped exactly like disabled ones, so the existing dependency-edge pruning treats them as satisfied externally (no false circular-dependency in helmfile). An unknown name, or a name the recipe disables, is rejected withErrCodeInvalidRequest(HTTP 400) rather than silently ignored.tools/cleanupdetects driver-managed mode via thenvidia-driver-daemonset(primary; robust to release-name drift) with ahelm get valuesdriver.enabledfallback (yq, or a 2-space-indent-anchored awk that cannot false-positive on nested keys likedriver.rdma.enabled). Detection is read-only, fail-open, and prints cordon → reboot → uncordon guidance both before the confirmation prompt and after completion. Automated drain/reboot is cloud-specific and out of scope per the issue.Testing
make qualifypassed locally (unit tests with-race, golangci-lint + yamllint, e2e incl. chainsaw 23/23, grype scan clean, license headers).shellcheck -x tools/cleanupclean. New table-driven tests:TestMake_BundlersFilter,TestMake_BundlersFilterDependencyPruned,TestParseBundleConfig_Bundlers,TestWithBundlers.Coverage:
pkg/bundler/...78.3% → 79.3% (+1.0%).Risk Assessment
Rollout notes: No behavior change for requests that omit
bundlers(previously-ignored values now take effect or return 400, which matches the documented contract). Cleanup-script change is output-only.Checklist
make testwith-race)make lint)git commit -S) — GPG signing info