From e919131f1770b675c87e2d99ecdb21665494228e Mon Sep 17 00:00:00 2001 From: yxxhero Date: Sat, 9 Sep 2023 10:44:23 +0800 Subject: [PATCH] fix: includes-transitive issue Signed-off-by: yxxhero --- pkg/app/app.go | 35 +++++++++++++++-------------------- pkg/app/app_test.go | 4 ++-- pkg/app/diff_test.go | 2 +- pkg/config/apply.go | 2 +- pkg/config/diff.go | 2 +- pkg/config/lint.go | 2 +- pkg/config/sync.go | 2 +- pkg/config/template.go | 2 +- pkg/state/state.go | 42 +++++++++++++++++++++++------------------- pkg/state/state_run.go | 16 ++++++++-------- 10 files changed, 54 insertions(+), 55 deletions(-) diff --git a/pkg/app/app.go b/pkg/app/app.go index 6a7ffd36..844b6aeb 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -189,7 +189,8 @@ func (a *App) Diff(c DiffConfigProvider) error { IncludeCRDs: &includeCRDs, Validate: c.Validate(), Concurrency: c.Concurrency(), - IncludeTransitiveNeeds: c.IncludeNeeds(), + IncludeTransitiveNeeds: c.IncludeTransitiveNeeds(), + IncludeNeeds: c.IncludeNeeds(), }, func() { msg, matched, affected, errs = a.diff(run, c) }) @@ -255,8 +256,9 @@ func (a *App) Template(c TemplateConfigProvider) error { SkipCleanup: c.SkipCleanup(), Validate: c.Validate(), Concurrency: c.Concurrency(), - IncludeTransitiveNeeds: c.IncludeNeeds(), Set: c.Set(), + IncludeTransitiveNeeds: c.IncludeTransitiveNeeds(), + IncludeNeeds: c.IncludeNeeds(), }, func() { ok, errs = a.template(run, c) }) @@ -327,7 +329,8 @@ func (a *App) Lint(c LintConfigProvider) error { SkipDeps: c.SkipDeps(), SkipCleanup: c.SkipCleanup(), Concurrency: c.Concurrency(), - IncludeTransitiveNeeds: c.IncludeNeeds(), + IncludeTransitiveNeeds: c.IncludeTransitiveNeeds(), + IncludeNeeds: c.IncludeNeeds(), }, func() { ok, lintErrs, errs = a.lint(run, c) }) @@ -420,7 +423,8 @@ func (a *App) Apply(c ApplyConfigProvider) error { SkipCleanup: c.RetainValuesFiles() || c.SkipCleanup(), Validate: c.Validate(), Concurrency: c.Concurrency(), - IncludeTransitiveNeeds: c.IncludeNeeds(), + IncludeTransitiveNeeds: c.IncludeTransitiveNeeds(), + IncludeNeeds: c.IncludeNeeds(), }, func() { matched, updated, es := a.apply(run, c) @@ -973,7 +977,7 @@ func (a *App) ForEachState(do func(*Run) (bool, []error), includeTransitiveNeeds return err } -func printBatches(batches [][]state.Release) string { +func printBatches(batches [][]state.ReleaseSpec) string { buf := &bytes.Buffer{} w := new(tabwriter.Writer) @@ -985,7 +989,7 @@ func printBatches(batches [][]state.Release) string { for i, batch := range batches { ids := []string{} for _, r := range batch { - ids = append(ids, state.ReleaseToID(&r.ReleaseSpec)) + ids = append(ids, state.ReleaseToID(&r)) } fmt.Fprintf(w, "%d\t%s\n", i+1, strings.Join(ids, ", ")) } @@ -1005,7 +1009,7 @@ func withDAG(templated *state.HelmState, helm helmexec.Interface, logger *zap.Su return withBatches(opts.Purpose, templated, batches, helm, logger, converge) } -func withBatches(purpose string, templated *state.HelmState, batches [][]state.Release, helm helmexec.Interface, logger *zap.SugaredLogger, converge func(*state.HelmState, helmexec.Interface) (bool, []error)) (bool, []error) { +func withBatches(purpose string, templated *state.HelmState, batches [][]state.ReleaseSpec, helm helmexec.Interface, logger *zap.SugaredLogger, converge func(*state.HelmState, helmexec.Interface) (bool, []error)) (bool, []error) { numBatches := len(batches) if purpose == "" { @@ -1018,10 +1022,7 @@ func withBatches(purpose string, templated *state.HelmState, batches [][]state.R for i, batch := range batches { var targets []state.ReleaseSpec - - for _, marked := range batch { - targets = append(targets, marked.ReleaseSpec) - } + targets = append(targets, batch...) var releaseIds []string for _, r := range targets { @@ -1347,9 +1348,7 @@ func (a *App) apply(r *Run, c ApplyConfigProvider) (bool, bool, []error) { var toApplyWithNeeds []state.ReleaseSpec for _, rs := range plan { - for _, r := range rs { - toApplyWithNeeds = append(toApplyWithNeeds, r.ReleaseSpec) - } + toApplyWithNeeds = append(toApplyWithNeeds, rs...) } // Do build deps and prepare only on selected releases so that we won't waste time @@ -1743,9 +1742,7 @@ func (a *App) sync(r *Run, c SyncConfigProvider) (bool, []error) { var toSyncWithNeeds []state.ReleaseSpec for _, rs := range batches { - for _, r := range rs { - toSyncWithNeeds = append(toSyncWithNeeds, r.ReleaseSpec) - } + toSyncWithNeeds = append(toSyncWithNeeds, rs...) } // Do build deps and prepare only on selected releases so that we won't waste time @@ -1948,9 +1945,7 @@ func (a *App) withNeeds(r *Run, c DAGConfig, includeDisabled bool, f func(*state var selectedReleasesWithNeeds []state.ReleaseSpec for _, rs := range batches { - for _, r := range rs { - selectedReleasesWithNeeds = append(selectedReleasesWithNeeds, r.ReleaseSpec) - } + selectedReleasesWithNeeds = append(selectedReleasesWithNeeds, rs...) } var toRender []state.ReleaseSpec diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index d0e5746c..09f56826 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -2141,7 +2141,7 @@ func (c configImpl) SkipTests() bool { } func (c configImpl) IncludeNeeds() bool { - return c.includeNeeds || c.IncludeTransitiveNeeds() + return c.includeNeeds } func (c configImpl) IncludeTransitiveNeeds() bool { @@ -2273,7 +2273,7 @@ func (a applyConfig) SkipNeeds() bool { } func (a applyConfig) IncludeNeeds() bool { - return a.includeNeeds || a.IncludeTransitiveNeeds() + return a.includeNeeds } func (a applyConfig) IncludeTransitiveNeeds() bool { diff --git a/pkg/app/diff_test.go b/pkg/app/diff_test.go index 11d7a766..6d57a73d 100644 --- a/pkg/app/diff_test.go +++ b/pkg/app/diff_test.go @@ -81,7 +81,7 @@ func (a diffConfig) SkipNeeds() bool { } func (a diffConfig) IncludeNeeds() bool { - return a.includeNeeds || a.IncludeTransitiveNeeds() + return a.includeNeeds } func (a diffConfig) IncludeTransitiveNeeds() bool { diff --git a/pkg/config/apply.go b/pkg/config/apply.go index f4dcabf3..af56582d 100644 --- a/pkg/config/apply.go +++ b/pkg/config/apply.go @@ -113,7 +113,7 @@ func (a *ApplyImpl) DiffOutput() string { // IncludeNeeds returns the include needs. func (a *ApplyImpl) IncludeNeeds() bool { - return a.ApplyOptions.IncludeNeeds || a.IncludeTransitiveNeeds() + return a.ApplyOptions.IncludeNeeds } // IncludeTests returns the include tests. diff --git a/pkg/config/diff.go b/pkg/config/diff.go index 5212c8a9..55f52ca2 100644 --- a/pkg/config/diff.go +++ b/pkg/config/diff.go @@ -72,7 +72,7 @@ func (t *DiffImpl) Concurrency() int { // IncludeNeeds returns the include needs func (t *DiffImpl) IncludeNeeds() bool { - return t.DiffOptions.IncludeNeeds || t.IncludeTransitiveNeeds() + return t.DiffOptions.IncludeNeeds } // IncludeTransitiveNeeds returns the include transitive needs diff --git a/pkg/config/lint.go b/pkg/config/lint.go index 46cf02f8..282894d9 100644 --- a/pkg/config/lint.go +++ b/pkg/config/lint.go @@ -58,7 +58,7 @@ func (l *LintImpl) SkipCleanup() bool { // IncludeNeeds returns the include needs func (l *LintImpl) IncludeNeeds() bool { - return l.LintOptions.IncludeNeeds || l.IncludeTransitiveNeeds() + return l.LintOptions.IncludeNeeds } // IncludeTransitiveNeeds returns the include transitive needs diff --git a/pkg/config/sync.go b/pkg/config/sync.go index d8858fab..e0028c3a 100644 --- a/pkg/config/sync.go +++ b/pkg/config/sync.go @@ -58,7 +58,7 @@ func (t *SyncImpl) Concurrency() int { // IncludeNeeds returns the include needs func (t *SyncImpl) IncludeNeeds() bool { - return t.SyncOptions.IncludeNeeds || t.IncludeTransitiveNeeds() + return t.SyncOptions.IncludeNeeds } // IncludeTransitiveNeeds returns the include transitive needs diff --git a/pkg/config/template.go b/pkg/config/template.go index 00b6e455..1ea327df 100644 --- a/pkg/config/template.go +++ b/pkg/config/template.go @@ -69,7 +69,7 @@ func (t *TemplateImpl) IncludeCRDs() bool { // IncludeNeeds returns the include needs func (t *TemplateImpl) IncludeNeeds() bool { - return t.TemplateOptions.IncludeNeeds || t.IncludeTransitiveNeeds() + return t.TemplateOptions.IncludeNeeds } // IncludeTransitiveNeeds returns the include transitive needs diff --git a/pkg/state/state.go b/pkg/state/state.go index 3c284851..f65d29db 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -1045,6 +1045,7 @@ type ChartPrepareOptions struct { OutputDir string OutputDirTemplate string IncludeTransitiveNeeds bool + IncludeNeeds bool Concurrency int KubeVersion string Set []string @@ -2208,9 +2209,7 @@ func markExcludedReleases(releases []ReleaseSpec, selectors []string, commonLabe } filteredReleases = append(filteredReleases, res) } - if includeTransitiveNeeds { - unmarkNeedsAndTransitives(filteredReleases, releases) - } + unmarkNeeds(filteredReleases, releases, includeTransitiveNeeds) return filteredReleases, nil } @@ -2237,38 +2236,43 @@ func ConditionEnabled(r ReleaseSpec, values map[string]any) (bool, error) { return conditionMatch, nil } -func unmarkNeedsAndTransitives(filteredReleases []Release, allReleases []ReleaseSpec) { - needsWithTranstives := collectAllNeedsWithTransitives(filteredReleases, allReleases) - unmarkReleases(needsWithTranstives, filteredReleases) +func unmarkNeeds(filteredReleases []Release, allReleases []ReleaseSpec, includeTransitiveNeeds bool) { + needsWithTranstives := collectAllNeeds(filteredReleases, allReleases, includeTransitiveNeeds) + unmarkReleases(needsWithTranstives, filteredReleases, includeTransitiveNeeds) } -func collectAllNeedsWithTransitives(filteredReleases []Release, allReleases []ReleaseSpec) map[string]struct{} { - needsWithTranstives := map[string]struct{}{} +func collectAllNeeds(filteredReleases []Release, allReleases []ReleaseSpec, includeTransitiveNeeds bool) map[string]struct{} { + needs := map[string]struct{}{} for _, r := range filteredReleases { if !r.Filtered { - collectNeedsWithTransitives(r.ReleaseSpec, allReleases, needsWithTranstives) + collectNeeds(r.ReleaseSpec, allReleases, needs, includeTransitiveNeeds) } } - return needsWithTranstives + return needs } -func unmarkReleases(toUnmark map[string]struct{}, releases []Release) { +func unmarkReleases(toUnmark map[string]struct{}, releases []Release, includeTransitiveNeeds bool) { for i, r := range releases { if _, ok := toUnmark[ReleaseToID(&r.ReleaseSpec)]; ok { + if releases[i].Filtered && !includeTransitiveNeeds { + releases[i].Needs = nil + } releases[i].Filtered = false } } } -func collectNeedsWithTransitives(release ReleaseSpec, allReleases []ReleaseSpec, needsWithTranstives map[string]struct{}) { +func collectNeeds(release ReleaseSpec, allReleases []ReleaseSpec, needs map[string]struct{}, includeTransitiveNeeds bool) { for _, id := range release.Needs { - if _, exists := needsWithTranstives[id]; !exists { - needsWithTranstives[id] = struct{}{} - releaseParts := strings.Split(id, "/") - releaseName := releaseParts[len(releaseParts)-1] - for _, r := range allReleases { - if r.Name == releaseName { - collectNeedsWithTransitives(r, allReleases, needsWithTranstives) + if _, exists := needs[id]; !exists { + needs[id] = struct{}{} + if includeTransitiveNeeds { + releaseParts := strings.Split(id, "/") + releaseName := releaseParts[len(releaseParts)-1] + for _, r := range allReleases { + if r.Name == releaseName { + collectNeeds(r, allReleases, needs, includeTransitiveNeeds) + } } } } diff --git a/pkg/state/state_run.go b/pkg/state/state_run.go index 53646bc9..369d502d 100644 --- a/pkg/state/state_run.go +++ b/pkg/state/state_run.go @@ -98,8 +98,8 @@ type PlanOptions struct { SelectedReleases []ReleaseSpec } -func (st *HelmState) PlanReleases(opts PlanOptions) ([][]Release, error) { - marked, err := st.SelectReleases(opts.IncludeTransitiveNeeds) +func (st *HelmState) PlanReleases(opts PlanOptions) ([][]ReleaseSpec, error) { + marked, err := st.GetSelectedReleases(opts.IncludeTransitiveNeeds) if err != nil { return nil, err } @@ -112,7 +112,7 @@ func (st *HelmState) PlanReleases(opts PlanOptions) ([][]Release, error) { return groups, nil } -func SortedReleaseGroups(releases []Release, opts PlanOptions) ([][]Release, error) { +func SortedReleaseGroups(releases []ReleaseSpec, opts PlanOptions) ([][]ReleaseSpec, error) { reverse := opts.Reverse groups, err := GroupReleasesByDependency(releases, opts) @@ -129,13 +129,13 @@ func SortedReleaseGroups(releases []Release, opts PlanOptions) ([][]Release, err return groups, nil } -func GroupReleasesByDependency(releases []Release, opts PlanOptions) ([][]Release, error) { - idToReleases := map[string][]Release{} +func GroupReleasesByDependency(releases []ReleaseSpec, opts PlanOptions) ([][]ReleaseSpec, error) { + idToReleases := map[string][]ReleaseSpec{} idToIndex := map[string]int{} d := dag.New() for i, r := range releases { - id := ReleaseToID(&r.ReleaseSpec) + id := ReleaseToID(&r) idToReleases[id] = append(idToReleases[id], r) idToIndex[id] = i @@ -226,13 +226,13 @@ func GroupReleasesByDependency(releases []Release, opts PlanOptions) ([][]Releas return nil, err } - var result [][]Release + var result [][]ReleaseSpec for groupIndex := 0; groupIndex < len(plan); groupIndex++ { dagNodesInGroup := plan[groupIndex] var idsInGroup []string - var releasesInGroup []Release + var releasesInGroup []ReleaseSpec for _, node := range dagNodesInGroup { idsInGroup = append(idsInGroup, node.Id)