Skip to content

fix: prevent concurrent Helm configuration races - #688

Merged
alexey-igrychev merged 1 commit into
2from
fix/helm/hook-output-race
Aug 7, 2026
Merged

fix: prevent concurrent Helm configuration races#688
alexey-igrychev merged 1 commit into
2from
fix/helm/hook-output-race

Conversation

@alexey-igrychev

@alexey-igrychev alexey-igrychev commented Aug 7, 2026

Copy link
Copy Markdown
Member

Summary

Concurrent Helm commands sharing one action configuration no longer race while initializing its hook output. Race-instrumented callers no longer exit with Go race-detector status 66 from this conflict.

What

  • Concurrent Configuration.Init and Configuration.SetHookOutputFunc calls serialize writes to HookOutputFunc with the configuration mutex.
  • No Helm CLI surface or normal command behavior changes.
  • VERIFIED: remote Linux CGO_ENABLED=1 task --yes test:ginkgo paths='./pkg/helm/pkg/action' tags='ai_tests' parallel=false -- --race passes.

Why

#686 locked Configuration.Init, but SetHookOutputFunc still wrote HookOutputFunc without that lock. Parallel command initialization therefore retained one unsynchronized write pair; using the existing configuration mutex closes that pair.

Review in cubic

@alexey-igrychev

Copy link
Copy Markdown
Member Author

Verification

  • Remote Linux: CGO_ENABLED=1 task --yes test:ginkgo paths='./pkg/helm/pkg/action' tags='ai_tests' parallel=false -- --race passed.
  • Mutation: removed SetHookOutputFunc mutex → TestAIConfigurationInitAndSetHookOutputFuncConcurrent reported Init/SetHookOutputFunc data race.

Follow-up

@alexey-igrychev

Copy link
Copy Markdown
Member Author

Verification update

  • Full Nelm build is blocked before this diff: clean branch 2 base 9d7f0cb fails task --yes build at pkg/plan/plan_execute.go:281 because dyntracker.DynamicReadinessTrackerOptions lacks CaseInsensitiveConditionTracking.

Follow-up

  • BLOCKER: restore branch 2 build compatibility before merging this PR and updating werf/werf.

Signed-off-by: Aleksei Igrychev <aleksei.igrychev@palark.com>
@alexey-igrychev
alexey-igrychev force-pushed the fix/helm/hook-output-race branch from 5a3553b to 8b420d3 Compare August 7, 2026 10:50
@alexey-igrychev

Copy link
Copy Markdown
Member Author

Verification update

  • After rebase onto Nelm 2 commit f5a2103, remote Linux task --yes build passed.
  • Remote Linux CGO_ENABLED=1 task --yes test:ginkgo paths='./pkg/helm/pkg/action' tags='ai_tests' parallel=false -- --race passed.

Follow-up

  • Update werf to the rebased Nelm commit before merging.

@alexey-igrychev
alexey-igrychev marked this pull request as ready for review August 7, 2026 11:58
@alexey-igrychev
alexey-igrychev merged commit 6f46733 into 2 Aug 7, 2026
9 checks passed
@alexey-igrychev
alexey-igrychev deleted the fix/helm/hook-output-race branch August 7, 2026 11:58
alexey-igrychev added a commit to werf/werf that referenced this pull request Aug 7, 2026
## Summary

werf `3` now uses Nelm `1.28` revision `8b420d326cbc`, which serializes
concurrent action configuration initialization and hook-output setup.
The authorized update also brings the Nelm `1.27` and `1.28` change
range plus its required kubedog revision.

## What

- Parallel Helm commands sharing one configuration no longer race
between `Configuration.Init` and hook-output setup, preventing
race-instrumented werf from exiting with code 66 for that pair.
- Update `github.com/werf/nelm` from
`v1.26.2-0.20260807085132-ec98995b35c2` to
`v1.28.1-0.20260807104951-8b420d326cbc` and its transitive
`github.com/werf/kubedog` requirement.
- Nelm `1.27` adds `--lookup-resources` for offline chart render/lint
lookups and legacy release-lock control for install, uninstall, and
rollback actions.
- Nelm `1.28` adds disabled-by-default
`NELM_FEAT_ADOPT_DECKHOUSE_CONTROLLER_FIELDS` and
`NELM_FEAT_CASE_INSENSITIVE_CONDITION_TRACKING` gates.
- Deploys create the release namespace under strict RBAC unless
`--no-create-namespace` is set; managed-field reconstruction no longer
blocks deployment on incompatible historical manifests; and `release
get` is optimized.
- The Nelm changelog covers the released `v1.26.2...v1.28.0` range;
post-release branch commits supply the configuration race fix and
case-insensitive-condition gate.
- VERIFIED: `task format &amp;&amp; task build &amp;&amp; task lint
&amp;&amp; task test:unit` passed locally.
- VERIFIED: focused remote Linux compose race E2E reports the
independent Docker Buildx `logutil.Pause`/global logrus-output race and
no longer reports the Nelm configuration race.

## Why

Nelm `#686` protected `Configuration.Init` but left `SetHookOutputFunc`
unsynchronized, so parallel werf build workers could still write one
shared Helm configuration concurrently. Nelm `#688` uses the existing
configuration mutex for both writes; the broader 1.28 bump is explicitly
authorized and provides the kubedog API required by its branch.

Tracked in #7775.

Depends on [werf/nelm#688](werf/nelm#688).

Signed-off-by: Aleksei Igrychev <aleksei.igrychev@palark.com>
alexey-igrychev added a commit to werf/werf that referenced this pull request Aug 10, 2026
…7805)

## Summary

Bump `werf/nelm` to `06096861c106` so concurrent release tracking no
longer data-races on the progress table printer. The race was reproduced
under the race detector in `e2e_complex` during release uninstall, where
`ProgressTablesPrinter.Stop` read state that the `Start` worker was
still writing.

## What

- Deploy no longer triggers a data race in
`ProgressTablesPrinter.Start`/`Stop` during concurrent release tracking
(werf/nelm#690).
- Deploy now matches Kubernetes resource condition types
case-insensitively during tracking (werf/nelm feat + werf/kubedog bump
to `e8f61a4bc90a`).
- The nelm range `8b420d326cbc..06096861c106` also carries werf/nelm#688
(concurrent Helm configuration race); nelm branch `2` was rebased
between the two pins, so the range is `diverged` rather than strictly
linear.
- VERIFIED: `task build` (macOS, non-CGO) succeeds against the new pins.

## Why

werf/nelm#690 moves printer state initialization before the tracking
goroutine starts, so `Stop` no longer reads a field the worker is
concurrently assigning. It is the last dependency race left open in
`e2e_complex`. nelm is an ordinary versioned dependency, so the fix
lands as a nelm PR plus this bump; the intermediate commits on branch
`2` come along with the tip.

Signed-off-by: Aleksei Igrychev <aleksei.igrychev@palark.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant