mirror of
https://github.com/helmfile/helmfile.git
synced 2026-10-18 15:14:04 +02:00
fix(list): avoid deadlock in helmfile list with more than 100 states (#2773)
fix(list): collect releases under a mutex to avoid deadlock with more than 100 states ListReleases gathered per-state results through a channel with a fixed buffer of 100 that was drained only after ForEachState had visited every state. The 101st state carrying releases therefore blocked on the send forever: `helmfile list` hung with no output and ignored SIGTERM. Append to the result slice under a mutex instead. ForEachState may run the callback concurrently, and the existing sort keeps the output order deterministic, so nothing else changes. Adds TestListWithManyStates (101 helmfile.d states, fails fast on a hang). Resolves #2772 Signed-off-by: Florian Müller <7556827+max06@users.noreply.github.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
+11
-10
@@ -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
|
||||
}
|
||||
|
||||
@@ -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": `
|
||||
|
||||
Reference in New Issue
Block a user