Skip to content

chore(deps): bump Grove to v0.2.0-beta1 - #17851

Merged
julienmancuso merged 4 commits into
mainfrom
codex/bump-grove-v0.2.0-beta1
Nov 3, 2026
Merged

julienmancuso merged 4 commits into
mainfrom
codex/bump-grove-v0.2.0-beta1

Conversation

@julienmancuso

@julienmancuso julienmancuso commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Bump Dynamo 1.7.0's bundled Grove chart and Go API dependency from v0.1.0-alpha.15 to v0.2.0-beta1.

Overview

Preserve Dynamo's alpha client and LPX request-owner contracts while supporting Grove beta's CRDs and child-owner references. Grove-enabled operators retain a conversion-webhook availability dependency, documented in the upgrade notes.

Key Changes

Approximately 30 added production/config lines, excluding tests, docs, comments, and whitespace; a separate 54 lines implement the controller-test CRD fixture generator.

  • Chart.yaml: pin Grove beta, enable crdInstaller.upgradeCRDs, accept beta release tags, and prepare schema-preserving alpha CRDs for the controller-only test harness.
  • dgd_grove_watch_setup.go: accept alpha/beta Grove-authored owner references for readiness and LPX cleanup. Keep Dynamo-authored LPUPipelineRequest owners alpha-only, with rejection tests and mixed-version test data.
  • README.md.gotmpl: explain shared-manager startup/cache risks and a bundled rollback through an alpha.16 platform chart under the same Helm release, using client-side apply.

Validation

Behavior Before Result
Beta child readiness Alpha-only owner filters dropped beta updates and stalled Planner startup TestGroveWatchMapsOnlyDGDControllerOwnedPodCliques covers alpha/beta and mixed hierarchies; DGDR lifecycle, profiling, and validation passed in the previous full-CI run.
LPX request indexing The widened index accepted beta owners that Dynamo does not author TestGetPipelineRequests reproduced that acceptance before the correction and now rejects beta, foreign-group, unsupported-version, wrong-kind, and non-controller owners.
Controller-only Grove API access Beta storage required an absent conversion service TestCRDsPreservePinnedAlphaSchema and TestPodCliqueSetRequestsWithoutGroveConversionService validate prepared CRDs and alpha CRUD without Grove. Deployment CI uses the real beta converter and fail-closed admission.
Chart/preflight compatibility Old installer key and alpha-only tag parsing 81 Helm tests and 9 Python tests passed with CI's Helm 3.17.3, including test_orders_alpha_beta_and_stable_versions and test_alpha_requests_are_covered_by_fail_closed_webhooks.

After rebasing onto c80a17b463, the full operator suite, LPX race tests, and generated-README consistency check passed. Fresh CI for signed head 27ad2cb368 is running.

Local validation used envtest; a running Grove controller and an in-place Grove upgrade were not exercised locally because the Docker daemon is unavailable.

Issues and Ongoing Work

Pull Request Checklist
  • I used Dynamo's /pr-checklist skill.

Summary by CodeRabbit

  • New Features
    • Platform 1.7.x supports Grove v0.2.0-beta1 and later, including beta release tags.
    • When no minimum replica count is specified for a clique or scaling group, it now defaults to minAvailable; explicitly configured minimums are unchanged.
  • Documentation
    • Upgrade notes cover Grove and CRD upgrades, conversion webhook connectivity, and rollback migration requirements.
    • Bundled Grove upgrades now use crdInstaller.upgradeCRDs; externally managed Grove installations should be upgraded with matching CRDs before Dynamo.

@julienmancuso
julienmancuso requested a review from a team as a code owner November 2, 2026 17:07
@github-actions github-actions Bot added chore documentation Improvements or additions to documentation deployment::k8s Relates to dynamo deployment in kubernetes labels Nov 2, 2026
@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Walkthrough

The platform chart and operator update their Grove version references to v0.2.0-beta1. Preflight checks recognize beta release tags. The chart configuration and upgrade documentation describe CRD upgrades and Grove compatibility.

Changes

Grove beta integration

Layer / File(s) Summary
Align Grove versions and CRD paths
deploy/helm/charts/platform/Chart.yaml, deploy/operator/go.mod, deploy/operator/Makefile, deploy/operator/internal/dynamo/grove_schema_test.go
The chart annotation and operator dependency now reference Grove v0.2.0-beta1. The operator CRD path and test import use core/crds.
Accept beta releases in preflight
deploy/helm/charts/platform/files/grove-preflight/check.py, deploy/helm/charts/platform/tests/grove_preflight_test.yaml, deploy/helm/charts/platform/tests/test_grove_preflight.py
Preflight parsing accepts alpha, beta, and stable tags, and orders releases by prerelease type and number. Tests cover this ordering and invalid image tags.
Update chart configuration and upgrade guidance
deploy/helm/charts/platform/values.yaml, deploy/helm/charts/platform/README.md, deploy/helm/charts/platform/README.md.gotmpl
The CRD installer setting changes to upgradeCRDs, retaining a default of true. The platform documentation adds Grove compatibility, upgrade, conversion, rollback, and replica-default details.

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

Merge Risk: Moderate · up to 1f545

Rollback documentation could lead operators to roll back to a Grove version that drops fields from stored resources. The DGD cluster tests may fail against the beta Grove CRD because the conversion service is not provided in the test environment. Both should be fixed before merge.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely identifies the main change: upgrading Grove to v0.2.0-beta1.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The pull request description includes all required sections: Overview, Key Changes, Validation, Issues and Ongoing Work, an issue reference, and the checklist. It provides detailed scope, testing resu…
Full details: Description check

Explanation

The pull request description includes all required sections: Overview, Key Changes, Validation, Issues and Ongoing Work, an issue reference, and the checklist. It provides detailed scope, testing results, limitations, and follow-up work.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 3 files. (7 skipped: 7 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between e421c62 and 1f545ff.

Files ignored due to path filters: 1
  • deploy/operator/go.sum is excluded by !**/*.sum
Files selected for processing: 10
  • deploy/helm/charts/platform/Chart.yaml
  • deploy/helm/charts/platform/README.md
  • deploy/helm/charts/platform/README.md.gotmpl
  • deploy/helm/charts/platform/files/grove-preflight/check.py
  • deploy/helm/charts/platform/tests/grove_preflight_test.yaml
  • deploy/helm/charts/platform/tests/test_grove_preflight.py
  • deploy/helm/charts/platform/values.yaml
  • deploy/operator/Makefile
  • deploy/operator/go.mod
  • deploy/operator/internal/dynamo/grove_schema_test.go

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

@coderabbitai coderabbitai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Fix comments: Fix CodeRabbit comments on this PR

Comment thread deploy/operator/Makefile Resolved
@julienmancuso
julienmancuso requested a review from a team as a code owner November 2, 2026 17:46
@pull-request-size pull-request-size Bot added size/L and removed size/M labels Nov 2, 2026
@julienmancuso

Copy link
Copy Markdown
Contributor Author

/ok to test 1a56838

@devin-ai-integration

devin-ai-integration Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

⏹️ Dynamo PR CI cancelled — run 41666824425 (attempt 4) on 27ad2cb368

Gate checks: ✅ backend-status-check · ❌ deploy-status-check · ✅ dynamo-status-check

Framework Snapshot
vLLM ⏹️ 1 ✅ 2

Failure details

No real jobs failed. ⏹️ vLLM Snapshot Deploy Test was cancelled during Run Snapshot Deploy Test; with no job failures reported, this looks like a manual/concurrency cancellation rather than fail-fast, but the cancellation source is unconfirmed.

For agents
{"pr": 17851, "run_id": 41666824425, "run_attempt": 4, "head_sha": "27ad2cb36894638467713743c39bcc0472c9d879", "failures": []}

Posted automatically by Devin for run 41666824425. Updated on every full-CI run of this PR.

@julienmancuso
julienmancuso requested a review from a team as a code owner November 2, 2026 19:36
@julienmancuso

Copy link
Copy Markdown
Contributor Author

/ok to test 96ade7d

@andrewpaprotsky andrewpaprotsky left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed e421c62..96ade7d (22 files), focusing on the LPX changes in deploy/operator/internal/controller/lpx/. Codex, Gemini and Claude Code's /code-review each reviewed the range; then all three cross-checked every finding.

The LPX owner checks in this PR are consistent on their own: they keep the kind, controller and UID checks, and downstream identity checks compare UIDs.

All three reviewers dropped one finding in the cross-check: that setup-clustertest-crds-in-cluster breaks clusters already running Grove beta. That target runs only on a dedicated Kind cluster without Grove.

Not checked: no reviewer built or ran anything. The Go unit and envtest suites, the Helm and Python chart tests, the clustertests and a live Grove beta + LPX deployment were not run. Nobody traced which status the scheduler gives the blocker (attempt timeout or Unsupported).

Outside the diff

[P3] The legacy topology repair never runs for PodCliqueSets stored as v1beta1 (flagged by: claude) at deploy/operator/internal/controller/dgd_grove_workload_renderer.go:304

Grove's beta-to-alpha conversion never returns packDomain. For a PodCliqueSet stored as v1beta1, existing.PackDomain is therefore always empty, and prepareLegacyGroveTopologyConstraintRepair returns at this line.

Nothing fails: Grove v0.2.0-beta1 admission accepts the one-step topologyName repair that results (topologyconstraints.go). But TestLegacyGroveTopologyConstraintUpgradeValidatesAgainstPinnedCRD covers only objects still stored as v1alpha1.

Cross-check: Gemini kept this at P3. Codex and Claude dropped it.


claude-opus-5-5 (xhigh), gpt-6-astra (ultra), gemini-4-argon (HIGH)

Comment thread deploy/operator/go.mod Resolved
julienmancuso and others added 4 commits November 3, 2026 08:35
Signed-off-by: Julien Mancuso <jmancuso@nvidia.com>
Signed-off-by: Julien Mancuso <jmancuso@nvidia.com>
Grove v0.2.0-beta1 creates children with grove.io/v1beta1 controller references. Conversion to Dynamo's alpha client types preserves those references, so alpha-only owner filters drop child readiness events. Planner startup can then wait indefinitely on stale DGD worker readiness.

Accept both served Grove versions in DGD event routing, LPX request indexing, and released Agent Pod ownership verification. Preserve existing kind, controller, namespace, and UID checks. Cover beta and mixed-version ownership in the existing regression tables.

Validation: regression tests fail before the fix and pass afterward; full operator Go suite including envtest passed; CI-pinned golangci-lint reports 0 issues; pre-commit passed.

Signed-off-by: Julien Mancuso <jmancuso@nvidia.com>
Signed-off-by: Julien Mancuso <jmancuso@nvidia.com>
@julienmancuso
julienmancuso force-pushed the codex/bump-grove-v0.2.0-beta1 branch from 96ade7d to 27ad2cb Compare November 3, 2026 15:43
@julienmancuso

Copy link
Copy Markdown
Contributor Author

/ok to test 27ad2cb

@julienmancuso
julienmancuso enabled auto-merge (squash) November 3, 2026 18:57
@julienmancuso
julienmancuso merged commit 87f3aa0 into main Nov 3, 2026
463 of 477 checks passed
@julienmancuso
julienmancuso deleted the codex/bump-grove-v0.2.0-beta1 branch November 3, 2026 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

actions chore deployment::k8s Relates to dynamo deployment in kubernetes documentation Improvements or additions to documentation size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Upgrade Dynamo main to Grove v0.2.0-beta1

4 participants