diff --git a/pkg/releaseutil/kind_sorter.go b/pkg/releaseutil/kind_sorter.go index a340dfc29..2034ba89d 100644 --- a/pkg/releaseutil/kind_sorter.go +++ b/pkg/releaseutil/kind_sorter.go @@ -17,6 +17,7 @@ limitations under the License. package releaseutil import ( + "reflect" "sort" "helm.sh/helm/v3/pkg/release" @@ -137,12 +138,11 @@ func lessByKind(a interface{}, b interface{}, kindA string, kindB string, o Kind first, aok := ordering[kindA] second, bok := ordering[kindB] + // both kinds are unknown + // the installing order of the unknown kinds is as the order that they are in original manifests/hooks, + // and the uninstalling order is the reverse order that they are in original manifests/hooks. if !aok && !bok { - // if both are unknown then sort alphabetically by kind, keep original order if same kind - if kindA != kindB { - return kindA < kindB - } - return first < second + return reflect.DeepEqual(o, UninstallOrder) } // unknown kind is last if !aok { diff --git a/pkg/releaseutil/kind_sorter_test.go b/pkg/releaseutil/kind_sorter_test.go index 71d355210..8d037a51b 100644 --- a/pkg/releaseutil/kind_sorter_test.go +++ b/pkg/releaseutil/kind_sorter_test.go @@ -18,13 +18,14 @@ package releaseutil import ( "bytes" + "sort" "testing" "helm.sh/helm/v3/pkg/release" ) -func TestKindSorter(t *testing.T) { - manifests := []Manifest{ +func buildManifestsForTestKindSorter() []Manifest { + return []Manifest{ { Name: "E", Head: &SimpleHead{Kind: "SecretList"}, @@ -165,15 +166,31 @@ func TestKindSorter(t *testing.T) { Name: "x", Head: &SimpleHead{Kind: "HorizontalPodAutoscaler"}, }, + { + Name: "N", + Head: &SimpleHead{Kind: "NginxVhost"}, + }, + { + Name: "U", + Head: &SimpleHead{Kind: "Unknown"}, + }, + { + Name: "R", + Head: &SimpleHead{Kind: "Registration"}, + }, } +} + +func TestKindSorter(t *testing.T) { + manifests := buildManifestsForTestKindSorter() for _, test := range []struct { description string order KindSortOrder expected string }{ - {"install", InstallOrder, "aAbcC3deEf1gh2iIjJkKlLmnopqrxstuvw!"}, - {"uninstall", UninstallOrder, "wvmutsxrqponLlKkJjIi2hg1fEed3CcbAa!"}, + {"install", InstallOrder, "aAbcC3deEf1gh2iIjJkKlLmnopqrxstuvw!NUR"}, + {"uninstall", UninstallOrder, "wvmutsxrqponLlKkJjIi2hg1fEed3CcbAaRUN!"}, } { var buf bytes.Buffer t.Run(test.description, func(t *testing.T) { @@ -197,6 +214,40 @@ func TestKindSorter(t *testing.T) { } } +func TestKindSorterFailedUsingAlphabeticalOrder(t *testing.T) { + manifests := buildManifestsForTestKindSorter() + + for _, test := range []struct { + description string + order KindSortOrder + expected string + actual string + }{ + {"install", InstallOrder, "aAbcC3deEf1gh2iIjJkKlLmnopqrxstuvw!NUR", "aAbcC3deEf1gh2iIjJkKlLmnopqrxstuvw!NRU"}, + {"uninstall", UninstallOrder, "wvmutsxrqponLlKkJjIi2hg1fEed3CcbAaRUN!", "wvmutsxrqponLlKkJjIi2hg1fEed3CcbAa!NRU"}, + } { + var buf bytes.Buffer + t.Run(test.description, func(t *testing.T) { + if got, want := len(test.expected), len(manifests); got != want { + t.Fatalf("Expected %d names in order, got %d", want, got) + } + defer buf.Reset() + orig := manifests + for _, r := range sortManifestsByKindForCRsUsingAlphabeticalOrder(manifests, test.order) { + buf.WriteString(r.Name) + } + if got := buf.String(); got == test.actual && got != test.expected { + t.Logf("Expected %q, got %q", test.expected, got) + } + for i, manifest := range orig { + if manifest != manifests[i] { + t.Fatal("Expected input to sortManifestsByKind to stay the same") + } + } + }) + } +} + // TestKindSorterKeepOriginalOrder verifies manifests of same kind are kept in original order func TestKindSorterKeepOriginalOrder(t *testing.T) { manifests := []Manifest{ @@ -329,3 +380,38 @@ func TestKindSorterForHooks(t *testing.T) { }) } } + +func sortManifestsByKindForCRsUsingAlphabeticalOrder(manifests []Manifest, ordering KindSortOrder) []Manifest { + sort.SliceStable(manifests, func(i, j int) bool { + return lessByKindForCRsUsingAlphabeticalOrder(manifests[i], manifests[j], manifests[i].Head.Kind, manifests[j].Head.Kind, ordering) + }) + + return manifests +} + +func lessByKindForCRsUsingAlphabeticalOrder(a interface{}, b interface{}, kindA string, kindB string, o KindSortOrder) bool { + ordering := make(map[string]int, len(o)) + for v, k := range o { + ordering[k] = v + } + + first, aok := ordering[kindA] + second, bok := ordering[kindB] + + if !aok && !bok { + // if both are unknown then sort alphabetically by kind, keep original order if same kind + if kindA != kindB { + return kindA < kindB + } + return first < second + } + // unknown kind is last + if !aok { + return false + } + if !bok { + return true + } + // sort different kinds, keep original order if same priority + return first < second +}