From 306b51b40bb1331e062ccabdea4c857d23e4b375 Mon Sep 17 00:00:00 2001 From: Niklas Wagner Date: Wed, 24 Mar 2021 16:05:54 +0100 Subject: [PATCH] Sort uninstall kinds correctly Signed-off-by: Niklas Wagner --- pkg/action/action.go | 2 +- pkg/action/uninstall.go | 2 +- pkg/releaseutil/kind_sorter.go | 49 +++++++++++++++++++++--------- pkg/releaseutil/manifest_sorter.go | 4 +-- 4 files changed, 39 insertions(+), 18 deletions(-) diff --git a/pkg/action/action.go b/pkg/action/action.go index 79bb4f638..5d63aeffb 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -162,7 +162,7 @@ func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values // Sort hooks, manifests, and partials. Only hooks and manifests are returned, // as partials are not used after renderer.Render. Empty manifests are also // removed here. - hs, manifests, err := releaseutil.SortManifests(files, caps.APIVersions, releaseutil.InstallOrder) + hs, manifests, err := releaseutil.SortManifests(files, caps.APIVersions, false) if err != nil { // By catching parse errors here, we can prevent bogus releases from going // to Kubernetes. diff --git a/pkg/action/uninstall.go b/pkg/action/uninstall.go index c762159cb..3ccfcae32 100644 --- a/pkg/action/uninstall.go +++ b/pkg/action/uninstall.go @@ -181,7 +181,7 @@ func (u *Uninstall) deleteRelease(rel *release.Release) (string, []error) { } manifests := releaseutil.SplitManifests(rel.Manifest) - _, files, err := releaseutil.SortManifests(manifests, caps.APIVersions, releaseutil.UninstallOrder) + _, files, err := releaseutil.SortManifests(manifests, caps.APIVersions, true) if err != nil { // We could instead just delete everything in no particular order. // FIXME: One way to delete at this point would be to try a label-based diff --git a/pkg/releaseutil/kind_sorter.go b/pkg/releaseutil/kind_sorter.go index 89289c5de..3b063659d 100644 --- a/pkg/releaseutil/kind_sorter.go +++ b/pkg/releaseutil/kind_sorter.go @@ -17,6 +17,7 @@ limitations under the License. package releaseutil import ( + "log" "sort" "helm.sh/helm/v3/pkg/release" @@ -108,9 +109,10 @@ var UninstallOrder KindSortOrder = []string{ // sort manifests by kind. // // Results are sorted by 'ordering', keeping order of items with equal kind/priority -func sortManifestsByKind(m []Manifest, ordering KindSortOrder) []Manifest { +func sortManifestsByKind(manifests []Manifest, uninstall bool) []Manifest { + m := manifests sort.SliceStable(m, func(i, j int) bool { - return lessByKind(m[i], m[j], m[i].Head.Kind, m[j].Head.Kind, m[i].InstallBefore, m[j].InstallBefore, ordering) + return lessByKind(m[i], m[j], m[i].Head.Kind, m[j].Head.Kind, m[i].InstallBefore, m[j].InstallBefore, uninstall) }) return m @@ -119,18 +121,18 @@ func sortManifestsByKind(m []Manifest, ordering KindSortOrder) []Manifest { // sort hooks by kind, using an out-of-place sort to preserve the input parameters. // // Results are sorted by 'ordering', keeping order of items with equal kind/priority -func sortHooksByKind(hooks []*release.Hook, ordering KindSortOrder) []*release.Hook { +func sortHooksByKind(hooks []*release.Hook, uninstall bool) []*release.Hook { h := hooks sort.SliceStable(h, func(i, j int) bool { - return lessByKind(h[i], h[j], h[i].Kind, h[j].Kind, nil, nil, ordering) + return lessByKind(h[i], h[j], h[i].Kind, h[j].Kind, []string{}, []string{}, uninstall) }) return h } -func lessByKind(a interface{}, b interface{}, kindA string, kindB string, beforeA []string, beforeB []string, o KindSortOrder) bool { - first, aok := installOrderIndex(kindA, beforeA, o) - second, bok := installOrderIndex(kindB, beforeB, o) +func lessByKind(a interface{}, b interface{}, kindA string, kindB string, beforeA []string, beforeB []string, uninstall bool) bool { + first, aok := installOrderIndex(kindA, beforeA, uninstall) + second, bok := installOrderIndex(kindB, beforeB, uninstall) if !aok && !bok { // if both are unknown then sort alphabetically by kind, keep original order if same kind @@ -151,26 +153,45 @@ func lessByKind(a interface{}, b interface{}, kindA string, kindB string, before } // installOrderIndex returns the lowest index number of all beforeKinds -func installOrderIndex(kind string, beforeKinds []string, o KindSortOrder) (int, bool) { - ordering := make(map[string]int, len(o)) - for v, k := range o { +func installOrderIndex(kind string, beforeKinds []string, uninstall bool) (int, bool) { + order := InstallOrder + if uninstall { + order = UninstallOrder + } + + ordering := make(map[string]int, len(order)) + for v, k := range order { ordering[k] = v } orderIndex, foundIndex := ordering[kind] // reset orderIndex for unknown resources - if !foundIndex { - orderIndex = len(o) + // when we're uninstalling we're actually searching for the HIGHEST index, so 0 is fine as initial value + if !foundIndex && !uninstall { + orderIndex = len(order) } for _, kind := range beforeKinds { i, ok := ordering[kind] - if ok && i < orderIndex { + if !ok { + continue + } + // we're searching for the lowest index when installing + if i < orderIndex && !uninstall { foundIndex = true - // set orderIndex 1 BEFORE the actual index, so it get executed BEFORE it + // set orderIndex 1 BEFORE the actual index, so it get installed BEFORE it orderIndex = i - 1 } + // we're searching for the highest index when uninstalling + if i > orderIndex && uninstall { + foundIndex = true + // set orderIndex 1 AFTER the actual index, so it get uninstalled AFTER it + orderIndex = i + 1 + } } + + log.Printf("kind: %v / beforeKinds: %v = %v (foundIndex: %v / uninstall: %v)", kind, beforeKinds, orderIndex, foundIndex, uninstall) + return orderIndex, foundIndex } diff --git a/pkg/releaseutil/manifest_sorter.go b/pkg/releaseutil/manifest_sorter.go index 8f5f61263..1a1522898 100644 --- a/pkg/releaseutil/manifest_sorter.go +++ b/pkg/releaseutil/manifest_sorter.go @@ -76,7 +76,7 @@ var events = map[string]release.HookEvent{ // // Files that do not parse into the expected format are simply placed into a map and // returned. -func SortManifests(files map[string]string, apis chartutil.VersionSet, ordering KindSortOrder) ([]*release.Hook, []Manifest, error) { +func SortManifests(files map[string]string, apis chartutil.VersionSet, uninstall bool) ([]*release.Hook, []Manifest, error) { result := &result{} var sortedFilePaths []string @@ -109,7 +109,7 @@ func SortManifests(files map[string]string, apis chartutil.VersionSet, ordering } } - return sortHooksByKind(result.hooks, ordering), sortManifestsByKind(result.generic, ordering), nil + return sortHooksByKind(result.hooks, uninstall), sortManifestsByKind(result.generic, uninstall), nil } // sort takes a manifestFile object which may contain multiple resource definition