feat(helm): split gateway and workspace charts - #2643
Conversation
|
This pull request has had no activity for 14 days and is now marked stale. It may be closed in 7 days if there is no further activity. |
0fb5d1f to
025c11b
Compare
johntmyers
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Thanks @johntmyers—I reviewed the refreshed head you invited for review. Thanks @dhirajsb as well; I checked the rebase, chart split, ownership test, release action, and documentation updates described in your issue follow-up. The linked issue makes this project-valid, and the docs cover the direct Kubernetes workflow, but two rendering and upgrade defects need an author update before CI dispatch.
Action required: @dhirajsb, fix GATOR-025c11b8-01 and GATOR-025c11b8-02, add the requested regressions, and push a new head for follow-up review.
Blocking findings:
GATOR-025c11b8-01: the new workspace chart renders required values into each manifest and losesapiVersion.GATOR-025c11b8-02: older gateway releases upgraded with--reuse-valuescan fail on the absentworkspaceResourcesmap.
Carried findings:
- None
Non-blocking suggestions:
GATOR-025c11b8-03: update the release-canary skill and consider smoke coverage for the newly published workspace OCI chart.
Gator metadata
- Validation: Project-valid because PR #2643 implements the chart boundary and acceptance criteria in linked issue #2485, and maintainer @johntmyers invited the refreshed PR for review.
- Docs: Fern Kubernetes setup documentation and related architecture/chart/debug guidance are updated.
- Checks: DCO and currently published informational checks are green; required Branch Checks and Helm Lint are pending/not dispatched for this head.
- E2E:
test:e2eis required for the Kubernetes Helm deployment path, but dispatch waits until blocking review feedback is resolved. - Head SHA:
025c11b8249276b870f110b6b8becdd00b205ede - Base SHA:
65745a06ef7b4aafc00600fd7f2d16ef7da49c26 - Merge base SHA:
65745a06ef7b4aafc00600fd7f2d16ef7da49c26 - Patch ID:
e7519f47ed7ec635339375868e1b3fd4fa9f020a - Gator payload:
7 - Review mode:
initial - Previous reviewed SHA: none
- Review budget exhausted: no
- Maintainer decision required: no
- Next state:
gator:in-review
|
Label |
|
/ok to test 53df022 |
johntmyers
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Thanks @dhirajsb. I checked your current-head fixes for both prior findings: the workspace validation helper no longer emits required values into rendered manifests, and the gateway helper safely defaults a missing legacy workspaceResources map to enabled while preserving explicit false. The added render regressions cover both obligations, and the follow-up review found no new blocking issue in the author delta.
Blocking findings:
- No blocking findings remain
Carried findings:
- None;
GATOR-025c11b8-01andGATOR-025c11b8-02are resolved by this head
Gator metadata
- Validation: Project-valid because PR #2643 implements the accepted chart boundary in linked issue #2485.
- Docs: Fern Kubernetes setup documentation and chart/architecture guidance are updated.
- Checks: Current-head Branch Checks and Helm Lint are running on the mirrored branch.
- E2E:
test:e2eapplied;/ok to test 53df022d656727dfbb4743a7103b4c94732ad1c2created the current-head mirror, and Branch E2E Checks are running. - Head SHA:
53df022d656727dfbb4743a7103b4c94732ad1c2 - Base SHA:
65745a06ef7b4aafc00600fd7f2d16ef7da49c26 - Merge base SHA:
65745a06ef7b4aafc00600fd7f2d16ef7da49c26 - Patch ID:
2616509eab62e5eca7b9fb2e734377b3f0414154 - Gator payload:
7 - Review mode:
follow_up - Previous reviewed SHA:
025c11b8249276b870f110b6b8becdd00b205ede - Review budget exhausted: no
- Maintainer decision required: no
- Next state:
gator:watch-pipeline
|
The two blocking Gator findings are addressed in |
|
Label |
|
@derekwaynecarr @mrunalp can you please also review and approve? 🙏 |
ca67ab5 to
d0576b2
Compare
|
/ok to test d0576b2 |
johntmyers
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
Thanks @johntmyers. I checked the rebase and conflict resolution you requested. Thanks @dhirajsb as well; I verified that the three unchanged author commits remain patch-equivalent and reviewed the VM-runtime CI conflict resolution against current main. The required critical-only delta review found no newly introduced Critical defect, both prior Gator findings remain resolved, and the protected workflows are now dispatched for this head.
Blocking findings:
- No blocking findings remain
Carried findings:
- None;
GATOR-025c11b8-01andGATOR-025c11b8-02remain resolved
Gator metadata
- Validation: Project-valid because PR #2643 implements the accepted chart ownership boundary in linked issue #2485.
- Docs: Fern Kubernetes setup documentation and chart/architecture guidance remain sufficient.
- Checks: Current-head Branch Checks are queued and Helm Lint is in progress.
- E2E:
test:e2eremains applied;/ok to test d0576b2edbce4061fd6eac029498fa0987e2338aupdated the mirror and Branch E2E Checks are in progress. - Head SHA:
d0576b2edbce4061fd6eac029498fa0987e2338a - Base SHA:
9b6d904e880a20178bb54cb5ede1a60689be71a1 - Merge base SHA:
9b6d904e880a20178bb54cb5ede1a60689be71a1 - Patch ID:
653d3aacdea19102ea805d281660fe601c518e76 - Gator payload:
7 - Review mode:
critical_only - Previous reviewed SHA:
ca67ab54eeadccaa307e6b1ab345231523bcbe16 - Review budget exhausted: yes
- Maintainer decision required: no
- Next state:
gator:watch-pipeline
|
Validation update for head d0576b2: Branch Checks and Helm Lint are passing, and the latest Gator review has no blocking findings. The only failures in https://github.com/NVIDIA/OpenShell/actions/runs/33426019324 are:
This PR does not modify either affected E2E test or its workflow, and sibling lifecycle/workspace coverage passed in the same run. I cannot rerun failed jobs from this fork account (GitHub returns: "Must have admin rights to Repository"). Could a maintainer please rerun the failed jobs on this head? If either reproduces, I will investigate it separately rather than broaden this Helm-chart split PR. |
d0576b2 to
083a056
Compare
|
/ok to test 083a056 |
BlockedGator is blocked because the current-head Branch E2E run failed in Next action: a maintainer should re-run the failed jobs in current-head workflow run 33447580533. Gator will re-check the required gate afterward. Gator metadata
|
Signed-off-by: Dhiraj Bokde <dbokde@nvidia.com>
Keep workspace manifests valid after value validation and default legacy reused values to the combined resource topology.
The Rust cache restores target/ after VM runtime artifacts are staged, overwriting target/vm-runtime-compressed before openshell-driver-vm is built. Stage the compressed runtime outside target and pass that location through OPENSHELL_VM_RUNTIME_COMPRESSED_DIR so build.rs can embed the supervisor. Also locate the Helm split-ownership test repository root from the script path rather than git rev-parse. The test runs in a container where the GitHub checkout can be owned by a different UID and rejected as dubious ownership. Signed-off-by: Dhiraj Bokde <dbokde@nvidia.com>
The split-chart ownership regression uses yq to inspect rendered YAML, but the Helm CI container installs only tools declared in mise. Declare and lock yq so mise install --locked provides the test dependency. Signed-off-by: Dhiraj Bokde <dbokde@nvidia.com>
083a056 to
a8c448b
Compare
|
/ok to test a8c448b |
Maintainer Approval NeededGator validation and PR monitoring are complete. The current head is patch-equivalent to the previously reviewed head, no blocking review feedback remains, and the required Branch Checks, Helm Lint, and E2E gates are green. Human maintainer approval is now required. Gator metadata
|
Monitoring CompleteMonitoring is complete because this PR has merged. Final status: the PR reached I removed the active Gator metadata
|
Summary
Split the OpenShell Helm deployment into independently installable gateway and
workspace charts. This lets platform operators manage a shared gateway
separately from namespace-local sandbox prerequisites while preserving the
existing combined installation by default.
Related Issue
Closes #2485
Changes
openshell-workspacechart containing only the sandbox ServiceAccount,namespaced Role and RoleBinding, and sandbox SSH NetworkPolicy.
workspaceResources.enabledswitch to thegateway chart.
ServiceAccount name and namespace values.
Kubernetes object ownership.
Testing
NVIDIA/OpenShell:main(65745a0).mise run pre-commit.mise run helm:test(111 gateway tests and 4 workspace tests).disjoint.
RoleBinding, and NetworkPolicy are omitted;
helm-chart-0.0.0.tgzandopenshell-workspace-0.0.0.tgz.ownership, covered by chart tests).
Checklist
updated.