From 5027e6aa5e0ece36231ca44db88f6c1a7d8c7d1b Mon Sep 17 00:00:00 2001 From: Niklas Date: Fri, 8 May 2026 00:59:26 +0200 Subject: [PATCH] chore: Deduplicate preparation code of sync and apply (#2523) This commit deduplicates the preparation logic for sync and apply by moving it to a common function. Signed-off-by: Niklas Ott Co-authored-by: Raphael Luba --- pkg/app/app.go | 64 ++++++++++++++++++++++++++------------------------ 1 file changed, 33 insertions(+), 31 deletions(-) diff --git a/pkg/app/app.go b/pkg/app/app.go index 5042f26f..097bfed3 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -1708,39 +1708,60 @@ func (a *App) getSelectedReleases(r *Run, includeTransitiveNeeds bool) ([]state. return selected, deduplicated, nil } -func (a *App) apply(r *Run, c ApplyConfigProvider) (bool, bool, []error) { +// GetPlannedAndSelectedReleasesWithNeeds returns the planned releases and the selected releases used for planning. +// The planned releases include dependency releases only when includeNeeds is true and skipNeeds is false. +func (a *App) GetPlannedAndSelectedReleasesWithNeeds(r *Run, skipNeeds bool, includeNeeds bool, includeTransitiveNeeds bool) ([]state.ReleaseSpec, []state.ReleaseSpec, error) { st := r.state - helm := r.helm - helm.SetExtraArgs(GetArgs(c.Args(), r.state)...) - - selectedReleases, selectedAndNeededReleases, err := a.getSelectedReleases(r, c.IncludeTransitiveNeeds()) + selectedReleases, selectedAndNeededReleases, err := a.getSelectedReleases(r, includeTransitiveNeeds) if err != nil { - return false, false, []error{err} + return nil, nil, err } if len(selectedReleases) == 0 { - return false, false, nil + return nil, nil, nil } // This is required when you're trying to deduplicate releases by the selector. // Without this, `PlanReleases` conflates duplicates and return both in `batches`, // even if we provided `SelectedReleases: selectedReleases`. // See https://github.com/roboll/helmfile/issues/1818 for more context. + originalReleases := st.Releases st.Releases = selectedAndNeededReleases + defer func() { + st.Releases = originalReleases + }() - plan, err := st.PlanReleases(state.PlanOptions{Reverse: false, SelectedReleases: selectedReleases, SkipNeeds: c.SkipNeeds(), IncludeNeeds: c.IncludeNeeds(), IncludeTransitiveNeeds: c.IncludeTransitiveNeeds()}) + batches, err := st.PlanReleases(state.PlanOptions{Reverse: false, SelectedReleases: selectedReleases, SkipNeeds: skipNeeds, IncludeNeeds: includeNeeds, IncludeTransitiveNeeds: includeTransitiveNeeds}) if err != nil { - return false, false, []error{err} + return nil, nil, err } var releasesWithNeeds []state.ReleaseSpec - for _, rs := range plan { + for _, rs := range batches { for _, r := range rs { releasesWithNeeds = append(releasesWithNeeds, r.ReleaseSpec) } } + return releasesWithNeeds, selectedAndNeededReleases, nil +} + +func (a *App) apply(r *Run, c ApplyConfigProvider) (bool, bool, []error) { + st := r.state + helm := r.helm + + helm.SetExtraArgs(GetArgs(c.Args(), r.state)...) + + releasesWithNeeds, selectedAndNeededReleases, err := a.GetPlannedAndSelectedReleasesWithNeeds(r, c.SkipNeeds(), c.IncludeNeeds(), c.IncludeTransitiveNeeds()) + if err != nil { + return false, false, []error{err} + } + + if len(releasesWithNeeds) == 0 { + return false, false, nil + } + // Do build deps and prepare only on selected releases so that we won't waste time // on running various helm commands on unnecessary releases st.Releases = releasesWithNeeds @@ -2193,33 +2214,14 @@ func (a *App) SyncState(r *Run, c SyncConfigProvider) (bool, []error) { st := r.state helm := r.helm - selectedReleases, selectedAndNeededReleases, err := a.getSelectedReleases(r, c.IncludeTransitiveNeeds()) + releasesWithNeeds, selectedAndNeededReleases, err := a.GetPlannedAndSelectedReleasesWithNeeds(r, c.SkipNeeds(), c.IncludeNeeds(), c.IncludeTransitiveNeeds()) if err != nil { return false, []error{err} } - if len(selectedReleases) == 0 { + if len(releasesWithNeeds) == 0 { return false, nil } - // This is required when you're trying to deduplicate releases by the selector. - // Without this, `PlanReleases` conflates duplicates and return both in `batches`, - // even if we provided `SelectedReleases: selectedReleases`. - // See https://github.com/roboll/helmfile/issues/1818 for more context. - st.Releases = selectedAndNeededReleases - - batches, err := st.PlanReleases(state.PlanOptions{Reverse: false, SelectedReleases: selectedReleases, SkipNeeds: c.SkipNeeds(), IncludeNeeds: c.IncludeNeeds(), IncludeTransitiveNeeds: c.IncludeTransitiveNeeds()}) - if err != nil { - return false, []error{err} - } - - var releasesWithNeeds []state.ReleaseSpec - - for _, rs := range batches { - for _, r := range rs { - releasesWithNeeds = append(releasesWithNeeds, r.ReleaseSpec) - } - } - // Do build deps and prepare only on selected releases so that we won't waste time // on running various helm commands on unnecessary releases st.Releases = releasesWithNeeds