From 66c4f7453e0975b986bfbfd9d1690fe5ebcc4a61 Mon Sep 17 00:00:00 2001 From: Justin Scott Date: Mon, 14 Aug 2017 23:43:14 -0700 Subject: [PATCH 1/3] feat(tiller): sort manifests alphabetically if they are same kind Updates Tiller manifest sorting so that manifests of the same kind are then sorted alphabetically. Closes #1696 --- pkg/tiller/kind_sorter.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/tiller/kind_sorter.go b/pkg/tiller/kind_sorter.go index fbeb75c35..9b4780771 100644 --- a/pkg/tiller/kind_sorter.go +++ b/pkg/tiller/kind_sorter.go @@ -123,5 +123,9 @@ func (k *kindSorter) Less(i, j int) bool { if !ok { return true } + if first == second { + // same kind so sub sort alphanumeric + return a.name < b.name + } return first < second } From 87d2b7ab678fcff141e19d3f8af8adfecbaa2716 Mon Sep 17 00:00:00 2001 From: Justin Scott Date: Tue, 15 Aug 2017 01:53:18 -0700 Subject: [PATCH 2/3] Add test for kind sorter sub sort --- pkg/tiller/kind_sorter_test.go | 51 ++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/pkg/tiller/kind_sorter_test.go b/pkg/tiller/kind_sorter_test.go index ca622f9ae..b7f3bb23f 100644 --- a/pkg/tiller/kind_sorter_test.go +++ b/pkg/tiller/kind_sorter_test.go @@ -147,3 +147,54 @@ func TestKindSorter(t *testing.T) { }) } } + +// TestKindSorterSubSort verifies manifests of same kind are also sorted alphanumeric +func TestKindSorterSubSort(t *testing.T) { + manifests := []manifest{ + { + name: "a", + head: &util.SimpleHead{Kind: "ClusterRole"}, + }, + { + name: "A", + head: &util.SimpleHead{Kind: "ClusterRole"}, + }, + { + name: "0", + head: &util.SimpleHead{Kind: "ConfigMap"}, + }, + { + name: "1", + head: &util.SimpleHead{Kind: "ConfigMap"}, + }, + { + name: "z", + head: &util.SimpleHead{Kind: "ClusterRoleBinding"}, + }, + { + name: "!", + head: &util.SimpleHead{Kind: "ClusterRoleBinding"}, + }, + } + for _, test := range []struct { + description string + order SortOrder + expected string + }{ + {"cm,clusterRole,clusterRoleBinding", InstallOrder, "01Aa!z"}, + } { + 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() + for _, r := range sortByKind(manifests, test.order) { + buf.WriteString(r.name) + } + if got := buf.String(); got != test.expected { + t.Errorf("Expected %q, got %q", test.expected, got) + } + }) + } +} From f239050996f30901770314f68b425fcef29b19be Mon Sep 17 00:00:00 2001 From: Justin Scott Date: Wed, 16 Aug 2017 00:32:08 -0700 Subject: [PATCH 3/3] Update kind subsort to sort unknown kinds alphabetically. Remove length check from subsort test. Add check for unknowns. --- pkg/tiller/kind_sorter.go | 19 ++++++++++--------- pkg/tiller/kind_sorter_test.go | 18 ++++++++++++++---- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/pkg/tiller/kind_sorter.go b/pkg/tiller/kind_sorter.go index 9b4780771..e0549e2f4 100644 --- a/pkg/tiller/kind_sorter.go +++ b/pkg/tiller/kind_sorter.go @@ -114,18 +114,19 @@ func (k *kindSorter) Swap(i, j int) { k.manifests[i], k.manifests[j] = k.manifes func (k *kindSorter) Less(i, j int) bool { a := k.manifests[i] b := k.manifests[j] - first, ok := k.ordering[a.head.Kind] - if !ok { - // Unknown is always last + first, aok := k.ordering[a.head.Kind] + second, bok := k.ordering[b.head.Kind] + if first == second { + // same kind (including unknown) so sub sort alphanumeric + return a.name < b.name + } + // unknown kind is last + if !aok { return false } - second, ok := k.ordering[b.head.Kind] - if !ok { + if !bok { return true } - if first == second { - // same kind so sub sort alphanumeric - return a.name < b.name - } + // sort different kinds return first < second } diff --git a/pkg/tiller/kind_sorter_test.go b/pkg/tiller/kind_sorter_test.go index b7f3bb23f..a707176b3 100644 --- a/pkg/tiller/kind_sorter_test.go +++ b/pkg/tiller/kind_sorter_test.go @@ -175,19 +175,29 @@ func TestKindSorterSubSort(t *testing.T) { name: "!", head: &util.SimpleHead{Kind: "ClusterRoleBinding"}, }, + { + name: "u3", + head: &util.SimpleHead{Kind: "Unknown"}, + }, + { + name: "u1", + head: &util.SimpleHead{Kind: "Unknown"}, + }, + { + name: "u2", + head: &util.SimpleHead{Kind: "Unknown"}, + }, } for _, test := range []struct { description string order SortOrder expected string }{ - {"cm,clusterRole,clusterRoleBinding", InstallOrder, "01Aa!z"}, + // expectation is sorted by kind (unknown is last) and then sub sorted alphabetically within each group + {"cm,clusterRole,clusterRoleBinding,Unknown", InstallOrder, "01Aa!zu1u2u3"}, } { 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() for _, r := range sortByKind(manifests, test.order) { buf.WriteString(r.name)