diff --git a/pkg/app/app.go b/pkg/app/app.go index 38c084a5..b8e1b073 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -779,7 +779,14 @@ func (a *App) dag(r *Run) error { } func (a *App) ListReleases(c ListConfigProvider) error { - releasesChan := make(chan []*HelmRelease, 100) + // Collect the per-state results under a mutex. ForEachState may visit + // states concurrently, and a bounded channel that is drained only after the + // visit finishes blocks forever once more states carry releases than the + // buffer can hold (the previous 100-slot channel deadlocked at 101 states). + var ( + releasesMu sync.Mutex + releases []*HelmRelease + ) err := a.ForEachState(func(run *Run) (_ bool, errs []error) { var stateReleases []*HelmRelease @@ -812,20 +819,14 @@ func (a *App) ListReleases(c ListConfigProvider) error { } if len(stateReleases) > 0 { - releasesChan <- stateReleases + releasesMu.Lock() + releases = append(releases, stateReleases...) + releasesMu.Unlock() } return }, false, SetFilter(true)) - close(releasesChan) - - // Collect all releases from channel - var releases []*HelmRelease - for rels := range releasesChan { - releases = append(releases, rels...) - } - if err != nil { return err } diff --git a/pkg/app/app_list_test.go b/pkg/app/app_list_test.go index 9fbb4644..b0a7fa7a 100644 --- a/pkg/app/app_list_test.go +++ b/pkg/app/app_list_test.go @@ -3,8 +3,10 @@ package app import ( "bytes" "encoding/json" + "fmt" "os" "testing" + "time" "github.com/google/go-cmp/cmp" "github.com/helmfile/vals" @@ -303,6 +305,80 @@ func TestListWithJSONOutput(t *testing.T) { }) } +// TestListWithManyStates guards against a deadlock in ListReleases: per-state +// results used to be collected through a channel with a fixed buffer of 100 +// that was drained only after every state had been visited, so the 101st state +// carrying releases blocked forever and `helmfile list` hung without output. +func TestListWithManyStates(t *testing.T) { + const stateCount = 101 + + files := map[string]string{} + for i := 1; i <= stateCount; i++ { + files[fmt.Sprintf("/path/to/helmfile.d/helmfile_%03d.yaml", i)] = fmt.Sprintf(` +releases: +- name: release-%03d + namespace: default + chart: incubator/raw +`, i) + } + + stdout := os.Stdout + defer func() { os.Stdout = stdout }() + + var buffer bytes.Buffer + syncWriter := testhelper.NewSyncWriter(&buffer) + logger := helmexec.NewLogger(syncWriter, "debug") + + valsRuntime, err := vals.New(vals.Options{CacheSize: 32}) + if err != nil { + t.Fatalf("unexpected error creating vals runtime: %v", err) + } + + app := appWithFs(&App{ + OverrideHelmBinary: DefaultHelmBinary, + fs: ffs.DefaultFileSystem(), + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, + valsRuntime: valsRuntime, + }, files) + + expectNoCallsToHelm(app) + + // A regression shows up as a hang, not as an error, so run the listing in + // a goroutine and fail fast instead of waiting for the go test timeout. + type result struct { + out string + err error + } + done := make(chan result, 1) + go func() { + var listErr error + out, err := testutil.CaptureStdout(func() { + listErr = app.ListReleases(configImpl{skipCharts: true, output: "json"}) + }) + if err == nil { + err = listErr + } + done <- result{out: out, err: err} + }() + + var res result + select { + case res = <-done: + case <-time.After(60 * time.Second): + t.Fatalf("ListReleases did not return within 60s for %d states: deadlock while collecting releases", stateCount) + } + assert.NoError(t, res.err) + + var releases []HelmRelease + if err := json.Unmarshal([]byte(res.out), &releases); err != nil { + t.Fatalf("failed to parse JSON output: %v", err) + } + assert.Len(t, releases, stateCount, "expected one release per state") +} + func TestListWithLockFileVersion(t *testing.T) { files := map[string]string{ "/path/to/helmfile.yaml": `