Skip to content

Commit fb46e32

Browse files
committed
fix: report Helm lifecycle and reverse delete order
1 parent 2c91cdb commit fb46e32

13 files changed

Lines changed: 133 additions & 10 deletions

docs/fixes/2026-07-31-native-helm-release-lifecycle.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@ state. Caller cancellation propagates through direct and dependency-ordered
99
execution into install and upgrade actions and into delete wait and hook phases;
1010
Helm 4 does not expose a context-aware uninstall request.
1111

12+
Atmos reports the selected action and effective release policy before the Helm
13+
action begins, including any `hookOnly` to `watcher` promotion required by
14+
failure recovery. Bulk delete uses reverse dependency order so dependents are
15+
removed before the releases they consume.
16+
1217
## Migration notes
1318

1419
- An omitted `release.timeout` remains `0s` (unbounded) for one minor release and emits a

docs/prd/native-helm-release-lifecycle.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,7 @@ The following rules apply:
369369
- A release that fails and is successfully rolled back still returns failure to the scheduler.
370370
- A rollback or uninstall failure preserves the original release failure and adds the recovery failure.
371371
- Dependents never run after timeout, failed readiness, failed hooks, failed rollback, or cancellation.
372+
- Bulk delete traverses the selected graph in reverse topological order so dependents are removed before their dependencies.
372373
- A future mixed-kind scheduler consumes the same provider result; it must not reinterpret Helm readiness.
373374

374375
## Timeout Semantics

pkg/component/graph.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ type GraphExecutionOptions struct {
3737
SubCommand string
3838
Flags map[string]any
3939
Selection *GraphSelection
40+
ReverseOrder bool
4041
}
4142

4243
// GraphNodeSkipObserver is implemented by providers that need to record graph
@@ -65,7 +66,11 @@ func ExecuteGraph(ctx context.Context, opts *GraphExecutionOptions) error {
6566
return nil
6667
}
6768

68-
log.Info("Processing components in dependency order", "component_type", opts.ComponentType, "count", len(order))
69+
orderName := "dependency"
70+
if opts.ReverseOrder {
71+
orderName = "reverse_dependency"
72+
}
73+
log.Info("Processing components", "component_type", opts.ComponentType, "order", orderName, "count", len(order))
6974
for i := range order {
7075
select {
7176
case <-ctx.Done():
@@ -127,9 +132,18 @@ func prepareExecutionOrder(opts *GraphExecutionOptions) (dependency.ExecutionOrd
127132
if err != nil {
128133
return nil, fmt.Errorf("%w: %w", errUtils.ErrTopologicalOrder, err)
129134
}
135+
if opts.ReverseOrder {
136+
reverseExecutionOrder(order)
137+
}
130138
return order, nil
131139
}
132140

141+
func reverseExecutionOrder(order dependency.ExecutionOrder) {
142+
for left, right := 0, len(order)-1; left < right; left, right = left+1, right-1 {
143+
order[left], order[right] = order[right], order[left]
144+
}
145+
}
146+
133147
// executeGraphNode executes a single graph node through the component provider.
134148
func executeGraphNode(ctx context.Context, opts *GraphExecutionOptions, node *dependency.Node) error {
135149
nodeInfo := *opts.Info

pkg/component/graph_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,23 @@ func TestExecuteGraphRunsComponentsInDependencyOrder(t *testing.T) {
164164
}
165165
}
166166

167+
func TestExecuteGraphRunsDeleteInReverseDependencyOrder(t *testing.T) {
168+
provider := &graphTestProvider{}
169+
err := ExecuteGraph(context.Background(), &GraphExecutionOptions{
170+
Provider: provider,
171+
Info: &schema.ConfigAndStacksInfo{},
172+
Stacks: graphTestStacks(),
173+
ComponentType: cfg.KubernetesComponentType,
174+
SubCommand: "delete",
175+
ReverseOrder: true,
176+
})
177+
178+
require.NoError(t, err)
179+
require.Len(t, provider.calls, 4)
180+
assertLessCallIndex(t, provider.calls, "api", "dev", "base", "dev")
181+
assertLessCallIndex(t, provider.calls, "worker", "dev", "base", "prod")
182+
}
183+
167184
func TestExecuteGraphNodeDoesNotRedispatchBulkSelection(t *testing.T) {
168185
provider := &graphTestProvider{}
169186
info := &schema.ConfigAndStacksInfo{

pkg/component/helm/client.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ func applyRelease(ctx context.Context, spec *chartSpec, dryRun bool) (releaseAct
8585
return releaseActionResult{Operation: releaseOperationInstall}, resolveErr
8686
}
8787
spec.Lifecycle = lifecycle
88+
reportResolvedLifecycle(lifecycle)
8889
operationCtx, cancel := releaseOperationContext(ctx, lifecycle.Policy.Timeout)
8990
defer cancel()
9091
manifest, installErr := installRelease(operationCtx, actx, spec, dryRun)
@@ -97,6 +98,7 @@ func applyRelease(ctx context.Context, spec *chartSpec, dryRun bool) (releaseAct
9798
return releaseActionResult{Operation: releaseOperationUpgrade}, resolveErr
9899
}
99100
spec.Lifecycle = lifecycle
101+
reportResolvedLifecycle(lifecycle)
100102
operationCtx, cancel := releaseOperationContext(ctx, lifecycle.Policy.Timeout)
101103
defer cancel()
102104
manifest, upgradeErr := upgradeRelease(operationCtx, actx, spec, dryRun)

pkg/component/helm/client_lifecycle_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ func TestReleaseOperationContextPreservesZeroTimeout(t *testing.T) {
212212
ctx, cancel := releaseOperationContext(parent, 0)
213213
defer cancel()
214214

215-
assert.Same(t, parent, ctx)
215+
assert.Equal(t, parent, ctx)
216216
_, hasDeadline := ctx.Deadline()
217217
assert.False(t, hasDeadline)
218218
}

pkg/component/helm/executor.go

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ func runWithHooks(
179179
if err != nil {
180180
return err
181181
}
182-
emitLifecycleWarnings(spec.Lifecycle.Warnings)
182+
reportResolvedLifecycle(spec.Lifecycle)
183183
}
184184
if err := ctx.GoContext().Err(); err != nil {
185185
return err
@@ -251,6 +251,27 @@ func emitLifecycleWarnings(warnings []lifecycleWarning) {
251251
}
252252
}
253253

254+
func reportResolvedLifecycle(resolution releaseLifecycleResolution) {
255+
emitLifecycleWarnings(resolution.Warnings)
256+
reason := "configured"
257+
for _, warning := range resolution.Warnings {
258+
if warning.Code == warningWaitDerived {
259+
reason = warning.Message
260+
break
261+
}
262+
}
263+
policy := resolution.Policy
264+
log.Debug("Resolved Helm release lifecycle",
265+
"operation", policy.Operation,
266+
"wait_strategy", policy.WaitStrategy,
267+
"wait_strategy_reason", reason,
268+
"wait_jobs", policy.WaitForJobs,
269+
"on_failure", policy.OnFailure,
270+
"timeout", policy.Timeout,
271+
"timeout_field", resolution.TimeoutField,
272+
)
273+
}
274+
254275
// runTemplate renders the chart and writes the manifests per the render options.
255276
func runTemplate(ctx *component.ExecutionContext, atmosConfig *schema.AtmosConfiguration, info *schema.ConfigAndStacksInfo, spec *chartSpec) ([]*unstructured.Unstructured, error) {
256277
objects, err := renderObjects(ctx.GoContext(), spec)

pkg/component/helm/executor_bulk.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ func executeBulk(
5858
SubCommand: command,
5959
Flags: ctx.Flags,
6060
Selection: selection,
61+
ReverseOrder: operation == OperationDelete,
6162
})
6263
if collector != nil {
6364
runHelmAggregateCIHook(ctx, atmosConfig, info, collector.resultSet(), graphErr)

pkg/component/helm/executor_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,16 @@ func TestExecuteBulkInitializesConfigAndGraph(t *testing.T) {
190190
assert.Equal(t, cfg.HelmComponentType, graphOpts.ComponentType)
191191
assert.Equal(t, "template", graphOpts.SubCommand)
192192
assert.Equal(t, ctx.Flags, graphOpts.Flags)
193+
assert.False(t, graphOpts.ReverseOrder)
194+
195+
graphOpts = nil
196+
require.NoError(t, executeBulk(ctx, &schema.AtmosConfiguration{}, &schema.ConfigAndStacksInfo{
197+
All: true,
198+
Stack: "dev",
199+
SubCommand: "delete",
200+
}, OperationDelete))
201+
require.NotNil(t, graphOpts)
202+
assert.True(t, graphOpts.ReverseOrder)
193203
}
194204

195205
func TestExecuteSingleSkipsDisabledComponent(t *testing.T) {

pkg/component/helm/lifecycle.go

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,21 @@ func decodeDeletePolicy(releaseMap map[string]any) (deletePolicyInput, error) {
325325
}
326326

327327
func resolveReleaseLifecycle(input releasePolicyInput, operation string, emitMigrationWarning bool) (releaseLifecycleResolution, error) {
328+
resolution, err := resolveReleaseLifecycleBase(input, operation, emitMigrationWarning)
329+
if err != nil {
330+
return releaseLifecycleResolution{}, err
331+
}
332+
if err := validateAndDeriveLifecycle(&resolution); err != nil {
333+
return releaseLifecycleResolution{}, err
334+
}
335+
return resolution, nil
336+
}
337+
338+
// resolveReleaseLifecycleBase applies configuration precedence without deriving
339+
// cross-field values. Callers that overlay CLI flags must do so before the
340+
// single validateAndDeriveLifecycle pass, otherwise a derived watcher strategy
341+
// loses both its source hookOnly value and the explanation for the promotion.
342+
func resolveReleaseLifecycleBase(input releasePolicyInput, operation string, emitMigrationWarning bool) (releaseLifecycleResolution, error) {
328343
resolution := releaseLifecycleResolution{
329344
Policy: defaultReleasePolicy(operation),
330345
TimeoutField: "built-in default",
@@ -359,9 +374,6 @@ func resolveReleaseLifecycle(input releasePolicyInput, operation string, emitMig
359374
Message: "helm release timeout is omitted; this release preserves 0s, but the default will become 5m in the next minor release",
360375
})
361376
}
362-
if err := validateAndDeriveLifecycle(&resolution); err != nil {
363-
return releaseLifecycleResolution{}, err
364-
}
365377
return resolution, nil
366378
}
367379

@@ -448,7 +460,7 @@ func validateAndDeriveLifecycle(resolution *releaseLifecycleResolution) error {
448460
// resolveReleaseLifecycleWithFlags resolves configuration for the selected
449461
// action, then overlays only explicitly supplied CLI values at highest priority.
450462
func resolveReleaseLifecycleWithFlags(input releasePolicyInput, operation string, flags map[string]any) (releaseLifecycleResolution, error) {
451-
resolution, err := resolveReleaseLifecycle(input, operation, true)
463+
resolution, err := resolveReleaseLifecycleBase(input, operation, true)
452464
if err != nil {
453465
return releaseLifecycleResolution{}, err
454466
}
@@ -502,7 +514,6 @@ func resolveReleaseLifecycleWithFlags(input releasePolicyInput, operation string
502514
resolution.Policy.CleanupOnFailure = value
503515
}
504516

505-
resolution.Warnings = removeLifecycleWarning(resolution.Warnings, warningWaitDerived)
506517
if err := validateAndDeriveLifecycle(&resolution); err != nil {
507518
return releaseLifecycleResolution{}, err
508519
}

0 commit comments

Comments
 (0)