feat: implement HIP-0025 resource creation sequencing#32314
Conversation
Generic DAG with deterministic topological batching and cycle detection, the subchart dependency DAG builder (reading depends-on field and the helm.sh/depends-on/subcharts annotation against post-processed c.Dependencies() state), and the new depends-on field on Chart.yaml dependency entries. Refs: HIP-0025 Signed-off-by: Rohit Gudi <50377477+caretak3r@users.noreply.github.com>
…adata Resource-group annotation parsing into a DAG, the SequencingInfo field recorded on releases for sequenced uninstall/rollback, and the helm-internal annotation stripping helper. Backward-compatible JSON round-trip for releases stored without SequencingInfo. Refs: HIP-0025 Signed-off-by: Rohit Gudi <50377477+caretak3r@users.noreply.github.com>
OrderedWaitStrategy, custom readiness evaluation via .status-scoped JSONPath expressions (helm.sh/readiness-success / -failure), and the status reader that layers custom readiness over kstatus. Refs: HIP-0025 Signed-off-by: Rohit Gudi <50377477+caretak3r@users.noreply.github.com>
Lint validation for subchart and resource-group dependency cycles, orphan group references, and the both-or-neither readiness annotation rule. Refs: HIP-0025 Signed-off-by: Rohit Gudi <50377477+caretak3r@users.noreply.github.com>
The sequenced deployment engine driving per-batch create-and-wait across the resource-group and subchart DAGs, wired into install, upgrade, rollback (respecting stored SequencingInfo), and reverse-order uninstall. Default (non-ordered) paths are unchanged. Refs: HIP-0025 Signed-off-by: Rohit Gudi <50377477+caretak3r@users.noreply.github.com>
…command The --wait=ordered and --readiness-timeout flags across install/upgrade/uninstall/rollback, ordered helm template output with resource-group delimiters, and the helm dag debugging command. Refs: HIP-0025 Signed-off-by: Rohit Gudi <50377477+caretak3r@users.noreply.github.com>
…ion keys Address Copilot review on PR helm#32038: - dag.go: printChild returned early for subcharts whose chart metadata is unavailable (storage-decoded charts at rollback/uninstall), hiding the structural resource-group batches buildStructuralLevel generates. Print an accurate note and still recurse into the structural child level. The old 'not found in chart dependencies' message was also misleading. - sequencing.go: hoist HelmInternalSequencingAnnotations() out of the per-resource Visit closure so the slice is cloned once per batch. Signed-off-by: Rohit Gudi <50377477+caretak3r@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Implements HIP-0025 resource creation sequencing for Helm v4 by introducing a pure, DAG-driven sequencing planner (subchart + resource-group DAGs) and wiring it into CLI/template output, lint, and action flows (install/upgrade/rollback/uninstall) with optional per-resource custom readiness.
Changes:
- Added generic DAG utilities plus sequencing plan builder to compute ordered apply/delete batches across subcharts and resource groups.
- Integrated
--wait=orderedand--readiness-timeoutinto commands/actions, including reverse-order rollback/uninstall behavior and release schema gating. - Added custom JSONPath-based readiness evaluation and comprehensive unit/golden coverage, plus a
helm dagdiagnostics command and sequencing lint rules.
Reviewed changes
Copilot reviewed 77 out of 77 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/release/v1/util/resource_group.go | Resource-group annotations parsing + DAG builder |
| pkg/release/v1/util/resource_group_test.go | Resource-group parsing/DAG tests |
| pkg/release/v1/util/manifest.go | Strip invalid sequencing annotation keys from template/apply output |
| pkg/release/v1/util/manifest_test.go | Stripping behavior tests |
| pkg/release/v1/sequence/plan.go | Sequencing plan types + reverse/display helpers |
| pkg/release/v1/sequence/plan_test.go | Plan helper tests + import-purity check |
| pkg/release/v1/sequence/manifest_parse.go | Parse stored/template manifests into typed manifests |
| pkg/release/v1/sequence/builder.go | Plan builder: subchart batching + resource-group batching |
| pkg/release/v1/release.go | Release schema: sequenced marker + legacy decoding |
| pkg/release/v1/release_test.go | Release schema compatibility tests |
| pkg/kube/statuswait.go | Custom readiness integration into status waiter |
| pkg/kube/readiness.go | JSONPath readiness expression parsing/validation/eval |
| pkg/kube/options.go | Wait options: enable custom readiness |
| pkg/kube/custom_readiness_status_reader.go | StatusReader wrapper implementing custom readiness |
| pkg/kube/client.go | Ordered wait strategy + status watcher wiring |
| pkg/kube/client_wait_strategy_test.go | Ordered wait/custom readiness option tests |
| pkg/cmd/upgrade.go | Propagate readiness-timeout; enable ordered wait flag |
| pkg/cmd/uninstall.go | Enable ordered wait flag |
| pkg/cmd/uninstall_test.go | CLI test: uninstall --wait=ordered |
| pkg/cmd/testdata/testcharts/sequenced-isolated-chart/templates/dd-plain.yaml | Test chart manifest (unsequenced resource) |
| pkg/cmd/testdata/testcharts/sequenced-isolated-chart/templates/cc-gamma.yaml | Test chart manifest (isolated group) |
| pkg/cmd/testdata/testcharts/sequenced-isolated-chart/templates/bb-beta.yaml | Test chart manifest (depends-on group) |
| pkg/cmd/testdata/testcharts/sequenced-isolated-chart/templates/aa-alpha.yaml | Test chart manifest (root group) |
| pkg/cmd/testdata/testcharts/sequenced-isolated-chart/Chart.yaml | Test chart metadata |
| pkg/cmd/testdata/testcharts/sequenced-chart/templates/cc-unsequenced-configmap.yaml | Test chart manifest (unsequenced) |
| pkg/cmd/testdata/testcharts/sequenced-chart/templates/bb-app-configmap.yaml | Test chart manifest (group + depends-on) |
| pkg/cmd/testdata/testcharts/sequenced-chart/templates/aa-databases-configmap.yaml | Test chart manifest (group) |
| pkg/cmd/testdata/testcharts/sequenced-chart/charts/worker/templates/aa-worker-configmap.yaml | Subchart test manifest (group) |
| pkg/cmd/testdata/testcharts/sequenced-chart/charts/worker/Chart.yaml | Subchart metadata |
| pkg/cmd/testdata/testcharts/sequenced-chart/Chart.yaml | Test chart metadata + dependency |
| pkg/cmd/testdata/output/template-ordered-isolated.txt | Golden: ordered template output (isolated demotion) |
| pkg/cmd/testdata/output/template-ordered-delimiters.txt | Golden: ordered template output (delimiters) |
| pkg/cmd/testdata/output/dag-sequenced-isolated.txt | Golden: helm dag output (isolated) |
| pkg/cmd/testdata/output/dag-sequenced-chart.txt | Golden: helm dag output (sequenced chart) |
| pkg/cmd/template.go | Ordered template rendering + stripping + warnings |
| pkg/cmd/template_test.go | Ordered template tests + compat/strip invariants |
| pkg/cmd/root.go | Register dag command |
| pkg/cmd/rollback.go | Enable ordered wait + readiness-timeout |
| pkg/cmd/install.go | Enable ordered wait + readiness-timeout |
| pkg/cmd/get_manifest.go | Clarify verbatim stored manifest output behavior |
| pkg/cmd/get_manifest_test.go | Test: get manifest prints stored annotations verbatim |
| pkg/cmd/flags.go | --wait=ordered parsing + readiness-timeout flag |
| pkg/cmd/flags_test.go | Flag parsing tests (wait/readiness-timeout) |
| pkg/cmd/dag.go | New helm dag command (diagnostics) |
| pkg/cmd/dag_test.go | helm dag golden + error-path tests |
| pkg/chart/v2/util/subchart_dag.go | Build subchart DAG from depends-on + annotation |
| pkg/chart/v2/util/dependencies.go | Rewrite depends-on refs before alias rewrite |
| pkg/chart/v2/util/dag.go | Generic DAG implementation |
| pkg/chart/v2/util/dag_test.go | Generic DAG tests |
| pkg/chart/v2/lint/rules/sequencing.go | Sequencing lint rule: plan build + readiness validation |
| pkg/chart/v2/lint/lint.go | Register sequencing lint rules |
| pkg/chart/v2/lint/lint_test.go | Lint integration test for sequencing rules |
| pkg/chart/v2/dependency.go | Add depends-on to chart dependencies schema |
| pkg/chart/v2/dependency_test.go | DependsOn sanitization test |
| pkg/chart/v2/dependency_json_test.go | JSON/YAML schema compatibility tests |
| pkg/action/warning_system_test.go | Action-layer warning surface tests |
| pkg/action/validate.go | Fail-fast on nil REST client in conflict checks |
| pkg/action/validate_test.go | Nil-client regression test |
| pkg/action/sequencing.go | Sequenced apply/delete engine + stripping + batch waits |
| pkg/action/rollback.go | Sequenced rollback path + sequencing-gated behavior |
| pkg/action/rollback_test.go | Rollback sequencing-annotation strip regression test |
| pkg/action/install.go | Sequenced install path + plan build preflight |
| pkg/action/hooks.go | Strip sequencing annotations from hook resources |
| pkg/action/hooks_test.go | Hook strip regression test |
| pkg/action/backward_compat_test.go | Backward-compat scenarios for ordered wait + rollback |
| pkg/action/action.go | Render resources returns sorted manifests for sequencing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func isExactYAMLKeyLine(line string, indent int, key string) bool { | ||
| body := lineBody(line) | ||
| if yamlLineIndent(line) != indent { | ||
| return false | ||
| } | ||
| return strings.TrimRight(body[indent:], " \t") == key+":" | ||
| } |
| switch entry[0] { | ||
| case '\'', '"': | ||
| quote := entry[0] | ||
| end := strings.IndexByte(entry[1:], quote) | ||
| if end == -1 { | ||
| return "", false | ||
| } | ||
| key := entry[1 : end+1] | ||
| rest := entry[end+2:] | ||
| if !strings.HasPrefix(rest, ":") { | ||
| return "", false | ||
| } | ||
| return key, true | ||
| default: | ||
| for _, key := range helmInternalSequencingAnnotations { | ||
| if strings.HasPrefix(entry, key+":") { | ||
| return key, true | ||
| } | ||
| } | ||
| return "", false | ||
| } |
|
Promptless prepared a documentation update related to this change. Triggered by helm/helm#32314 (HIP-0025 resource creation sequencing). The docs for HIP-0025 have been updated to match this implementation: documenting Review: Document HIP-0025 resource sequencing (docs PR #2068) |
kunalworldwide
left a comment
There was a problem hiding this comment.
This is a large but well-structured implementation of HIP-0025. The two-level DAG approach — subchart dependencies plus resource-group annotations — gives chart authors flexibility without forcing everyone to adopt sequenced installs.
A few things that stand out:
- The backward-compat tests are important:
--wait=orderedis new, but existing--waitbehavior must remain unchanged. The test file covers that. - The lint rule for sequencing is a good safety net; invalid
dependsOnreferences will be caught athelm linttime rather than at install time. - The DAG utility in
pkg/chart/v2/util/dag.gois reusable and well-scoped.
One question: the dependsOn field in Chart.yaml uses chart/subchart names. Is there validation to prevent circular subchart dependencies, and if so, where does the cycle error surface? I see DAG construction in subchart_dag.go but didn't spot an explicit cycle check in the diff.
LGTM otherwise.
|
A couple things to note:
helm/pkg/release/v1/sequence/builder.go Lines 166 to 170 in 9470c5a helm/pkg/release/v1/sequence/builder.go Lines 293 to 297 in 9470c5a since the plan is built up-front this should fail fast before any resource is applied ( |
|
I have this in the PR, but also want to call out the issue around the double
I would be able to remove the “workarounds” in the code, and address the two copilot suggestions, if the
|
What this PR does / why we need it:
Implements HIP-0025: Better Support for Resource Creation Sequencing, adding native DAG-based ordering for resource groups and subcharts to Helm v4.
Today Helm applies all rendered manifests at once. Chart authors who need ordered creation must lean on hooks (tedious to maintain) or bake ordering into the application itself. HIP-0025 gives authors a first-class way to declare deployment order via annotations and a
Chart.yamlfield, and gives operators a--wait=orderedflag to opt in. Charts and releases that don't use it are unaffected.How it works
When
--wait=orderedis set, Helm builds two levels of dependency graph and applies resources in topological order (Kahn's algorithm), waiting for each batch to become ready before starting the next:flowchart TD subgraph L1["Level 1 — subchart ordering"] A["Chart.yaml <code>depends-on</code><br/>+ helm.sh/depends-on/subcharts"] --> B["Subchart DAG"] B --> C["Topological batches"] end C -->|"for each batch, recursing<br/>into nested subcharts"| D subgraph L2["Level 2 — resource-group ordering"] D["helm.sh/resource-group<br/>+ helm.sh/depends-on/resource-groups"] --> E["Resource-group DAG"] E --> F["Topological batches"] end F --> G["Apply batch"] G --> H["Wait for readiness<br/>kstatus or custom JSONPath"] H -->|"next batch"| GReadiness defaults to kstatus; authors can override it per-resource with JSONPath expressions via
helm.sh/readiness-success/helm.sh/readiness-failure(operators==,!=,<,<=,>,>=).uninstallandrollbackof a sequenced release walk the DAG in reverse.What's added
Annotations
helm.sh/resource-grouphelm.sh/depends-on/resource-groupshelm.sh/depends-on/subchartshelm.sh/readiness-successhelm.sh/readiness-failureChart.yaml
depends-onondependencies[]entries — subchart ordering by name or alias.CLI flags
--wait=orderedinstall,upgrade,rollback,uninstall--readiness-timeoutinstall,upgrade,rollback1m; must not exceed--timeout)Also adds a
helm dagdiagnostic command that prints the computed batches for a chart without touching a cluster.Schema (all backward-compatible)
DependsOn []stringonchart.Dependency— optional field.SequencingInfo{Enabled, Strategy}persisted onrelease.Releaseso rollback/uninstall know to use the sequenced path.OrderedWaitStrategyconstant inpkg/kube.Design notes
pkg/chart/v2/util/dag.go) — topological sort decoupled from Helm types and independently tested; cycle detection surfaces at plan-build time with a chart-path-qualified error before anything is applied.chartPathis threaded through recursion so nested subchart manifests are routed correctly at arbitrary depth.--wait=ordered, behave exactly as before; the sequenced path is gated onSequencingInfo.Enabled.helm.sh/hook-weight.Limitations / open trade-offs
Sequential within a batch. Resources in the same batch are applied sequentially today; intra-batch parallelism is deferred (needs error aggregation and log-interleaving handling) and can be added later without an interface change.
Multi-slash annotation keys.
helm.sh/depends-on/resource-groupsandhelm.sh/depends-on/subchartsare not valid Kubernetes qualified names (multiple/). Helm consumes them at render/sequencing time and strips the rendered-manifest one before server-side apply, so the API server never sees it;helm template --wait=orderedemits## START/ENDbatch delimiters and is not intended to be piped straight tokubectl apply. Whether these keys should instead be single-slash forms — which would remove the stripping machinery entirely — is an open question I'd welcome maintainer guidance on.Commits
Staged into 7 reviewable commits:
cf0b6995bChart.yamldepends-onschemaeabd8147b371ca4b66ba2dd29ad555553ecd50d3fd188helm dagcommand9470c5a6fhelm dag; hoist annotation keysHow to test
make test-unit,make lint, andmake vetare clean; DCO signed off on all commits.Special notes for your reviewer:
pkg/chart/v2/util,pkg/release/v1/sequence,pkg/kube, plus tests and golden fixtures underpkg/cmd/testdata.If applicable:
docs neededlabel should be applied if so)refs https://github.com/helm/community/blob/main/hips/hip-0025.md
Signed-off-by: Rohit Gudi 50377477+caretak3r@users.noreply.github.com