Skip to content

Commit 62cee77

Browse files
committed
fix: bound Helm release operations by timeout
1 parent 57ae948 commit 62cee77

2 files changed

Lines changed: 67 additions & 8 deletions

File tree

pkg/component/helm/client.go

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"fmt"
77
"io"
88
"os"
9+
"time"
910

1011
"helm.sh/helm/v4/pkg/action"
1112
"helm.sh/helm/v4/pkg/cli"
@@ -84,7 +85,9 @@ func applyRelease(ctx context.Context, spec *chartSpec, dryRun bool) (releaseAct
8485
return releaseActionResult{Operation: releaseOperationInstall}, resolveErr
8586
}
8687
spec.Lifecycle = lifecycle
87-
manifest, installErr := installRelease(ctx, actx, spec, dryRun)
88+
operationCtx, cancel := releaseOperationContext(ctx, lifecycle.Policy.Timeout)
89+
defer cancel()
90+
manifest, installErr := installRelease(operationCtx, actx, spec, dryRun)
8891
return releaseActionResult{Manifest: manifest, Operation: releaseOperationInstall, Lifecycle: lifecycle}, installErr
8992
} else if historyErr != nil {
9093
return releaseActionResult{}, fmt.Errorf("%w %q: %w", errUtils.ErrHelmReleaseHistory, spec.ReleaseName, historyErr)
@@ -94,10 +97,29 @@ func applyRelease(ctx context.Context, spec *chartSpec, dryRun bool) (releaseAct
9497
return releaseActionResult{Operation: releaseOperationUpgrade}, resolveErr
9598
}
9699
spec.Lifecycle = lifecycle
97-
manifest, upgradeErr := upgradeRelease(ctx, actx, spec, dryRun)
100+
operationCtx, cancel := releaseOperationContext(ctx, lifecycle.Policy.Timeout)
101+
defer cancel()
102+
manifest, upgradeErr := upgradeRelease(operationCtx, actx, spec, dryRun)
98103
return releaseActionResult{Manifest: manifest, Operation: releaseOperationUpgrade, Lifecycle: lifecycle}, upgradeErr
99104
}
100105

106+
// releaseOperationContext applies the effective lifecycle timeout to every
107+
// cluster-side Helm action. A zero timeout intentionally remains unbounded
108+
// during the timeout-default migration.
109+
func releaseOperationContext(parent context.Context, timeout time.Duration) (context.Context, context.CancelFunc) {
110+
if timeout <= 0 {
111+
return parent, func() {}
112+
}
113+
return context.WithTimeout(parent, timeout)
114+
}
115+
116+
func releaseWaitOptions(ctx context.Context) []kube.WaitOption {
117+
return []kube.WaitOption{
118+
kube.WithWaitContext(ctx),
119+
kube.WithWaitForDeleteMethodContext(ctx),
120+
}
121+
}
122+
101123
func installRelease(ctx context.Context, actx *actionContext, spec *chartSpec, dryRun bool) (string, error) {
102124
client := action.NewInstall(actx.cfg)
103125
client.SetRegistryClient(actx.cfg.RegistryClient)
@@ -106,6 +128,7 @@ func installRelease(ctx context.Context, actx *actionContext, spec *chartSpec, d
106128
client.CreateNamespace = true
107129
client.Version = spec.Version
108130
configureInstallLifecycle(client, spec.Lifecycle.Policy)
131+
client.WaitOptions = releaseWaitOptions(ctx)
109132
if dryRun {
110133
client.DryRunStrategy = action.DryRunServer
111134
}
@@ -121,6 +144,7 @@ func upgradeRelease(ctx context.Context, actx *actionContext, spec *chartSpec, d
121144
client.Namespace = spec.Namespace
122145
client.Version = spec.Version
123146
configureUpgradeLifecycle(client, spec.Lifecycle.Policy)
147+
client.WaitOptions = releaseWaitOptions(ctx)
124148
if dryRun {
125149
client.DryRunStrategy = action.DryRunServer
126150
}
@@ -203,6 +227,8 @@ func deleteRelease(ctx context.Context, spec *chartSpec, dryRun bool) error {
203227
if err := ctx.Err(); err != nil {
204228
return err
205229
}
230+
operationCtx, cancel := releaseOperationContext(ctx, spec.Lifecycle.Policy.Timeout)
231+
defer cancel()
206232

207233
actx, err := newActionContext(spec.Namespace)
208234
if err != nil {
@@ -211,21 +237,18 @@ func deleteRelease(ctx context.Context, spec *chartSpec, dryRun bool) error {
211237

212238
client := action.NewUninstall(actx.cfg)
213239
configureUninstallLifecycle(client, spec.Lifecycle.Policy, dryRun)
214-
client.WaitOptions = []kube.WaitOption{
215-
kube.WithWaitContext(ctx),
216-
kube.WithWaitForDeleteMethodContext(ctx),
217-
}
240+
client.WaitOptions = releaseWaitOptions(operationCtx)
218241
if _, err := client.Run(spec.ReleaseName); err != nil {
219242
if errors.Is(err, driver.ErrReleaseNotFound) {
220243
return nil
221244
}
222245
uninstallErr := fmt.Errorf("%w %q: %w", errUtils.ErrHelmReleaseUninstall, spec.ReleaseName, err)
223-
if ctxErr := ctx.Err(); ctxErr != nil {
246+
if ctxErr := operationCtx.Err(); ctxErr != nil {
224247
return errors.Join(ctxErr, uninstallErr)
225248
}
226249
return uninstallErr
227250
}
228-
if err := ctx.Err(); err != nil {
251+
if err := operationCtx.Err(); err != nil {
229252
return err
230253
}
231254
return nil

pkg/component/helm/client_lifecycle_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"io"
66
"path/filepath"
77
"testing"
8+
"time"
89

910
"github.com/stretchr/testify/assert"
1011
"github.com/stretchr/testify/require"
@@ -177,3 +178,38 @@ func TestDeleteReleaseHonorsCanceledContext(t *testing.T) {
177178
require.NoError(t, getErr)
178179
assert.NotEmpty(t, deployed)
179180
}
181+
182+
func TestApplyReleaseUsesLifecycleTimeoutAndWaitContext(t *testing.T) {
183+
actx := memoryActionContext(t)
184+
stubActionContext(t, actx)
185+
spec := testdataChartSpec(t, "bounded-upgrade")
186+
187+
_, err := applyRelease(context.Background(), spec, false)
188+
require.NoError(t, err)
189+
190+
kubeClient, ok := actx.cfg.KubeClient.(*kubefake.FailingKubeClient)
191+
require.True(t, ok)
192+
kubeClient.RecordedWaitOptions = nil
193+
kubeClient.WaitDuration = 2 * time.Second
194+
timeout := "25ms"
195+
spec.Release.Upgrade.Timeout = &timeout
196+
197+
started := time.Now()
198+
result, err := applyRelease(context.Background(), spec, false)
199+
elapsed := time.Since(started)
200+
201+
require.ErrorIs(t, err, context.DeadlineExceeded)
202+
assert.Equal(t, releaseOperationUpgrade, result.Operation)
203+
assert.Less(t, elapsed, time.Second, "upgrade must return at the lifecycle deadline")
204+
assert.NotEmpty(t, kubeClient.RecordedWaitOptions, "Helm waiters must receive the operation context")
205+
}
206+
207+
func TestReleaseOperationContextPreservesZeroTimeout(t *testing.T) {
208+
parent := context.Background()
209+
ctx, cancel := releaseOperationContext(parent, 0)
210+
defer cancel()
211+
212+
assert.Same(t, parent, ctx)
213+
_, hasDeadline := ctx.Deadline()
214+
assert.False(t, hasDeadline)
215+
}

0 commit comments

Comments
 (0)