fix: Move a bunch of sorters to the releaseutils package

Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>

pkg/action/install.go

@@ -32,7 +32,6 @@ import (
 	"k8s.io/helm/pkg/hapi/release"
 	"k8s.io/helm/pkg/hooks"
 	"k8s.io/helm/pkg/releaseutil"
-	"k8s.io/helm/pkg/tiller"
 	"k8s.io/helm/pkg/version"
 )
 
@@ -303,7 +302,7 @@ func (i *Install) renderResources(ch *chart.Chart, values chartutil.Values, vs c
 	// as partials are not used after renderer.Render. Empty manifests are also
 	// removed here.
 	// TODO: Can we migrate SortManifests out of pkg/tiller?
-	hooks, manifests, err := tiller.SortManifests(files, vs, tiller.InstallOrder)
+	hooks, manifests, err := releaseutil.SortManifests(files, vs, releaseutil.InstallOrder)
 	if err != nil {
 		// By catching parse errors here, we can prevent bogus releases from going
 		// to Kubernetes.

pkg/releaseutil/kind_sorter.go

@@ -0,0 +1,146 @@
+/*
+Copyright The Helm Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package releaseutil
+
+import "sort"
+
+// KindSortOrder is an ordering of Kinds.
+type KindSortOrder []string
+
+// InstallOrder is the order in which manifests should be installed (by Kind).
+//
+// Those occurring earlier in the list get installed before those occurring later in the list.
+var InstallOrder KindSortOrder = []string{
+	"Namespace",
+	"ResourceQuota",
+	"LimitRange",
+	"Secret",
+	"ConfigMap",
+	"StorageClass",
+	"PersistentVolume",
+	"PersistentVolumeClaim",
+	"ServiceAccount",
+	"CustomResourceDefinition",
+	"ClusterRole",
+	"ClusterRoleBinding",
+	"Role",
+	"RoleBinding",
+	"Service",
+	"DaemonSet",
+	"Pod",
+	"ReplicationController",
+	"ReplicaSet",
+	"Deployment",
+	"StatefulSet",
+	"Job",
+	"CronJob",
+	"Ingress",
+	"APIService",
+}
+
+// UninstallOrder is the order in which manifests should be uninstalled (by Kind).
+//
+// Those occurring earlier in the list get uninstalled before those occurring later in the list.
+var UninstallOrder KindSortOrder = []string{
+	"APIService",
+	"Ingress",
+	"Service",
+	"CronJob",
+	"Job",
+	"StatefulSet",
+	"Deployment",
+	"ReplicaSet",
+	"ReplicationController",
+	"Pod",
+	"DaemonSet",
+	"RoleBinding",
+	"Role",
+	"ClusterRoleBinding",
+	"ClusterRole",
+	"CustomResourceDefinition",
+	"ServiceAccount",
+	"PersistentVolumeClaim",
+	"PersistentVolume",
+	"StorageClass",
+	"ConfigMap",
+	"Secret",
+	"LimitRange",
+	"ResourceQuota",
+	"Namespace",
+}
+
+// sortByKind does an in-place sort of manifests by Kind.
+//
+// Results are sorted by 'ordering'
+func sortByKind(manifests []Manifest, ordering KindSortOrder) []Manifest {
+	ks := newKindSorter(manifests, ordering)
+	sort.Sort(ks)
+	return ks.manifests
+}
+
+type kindSorter struct {
+	ordering  map[string]int
+	manifests []Manifest
+}
+
+func newKindSorter(m []Manifest, s KindSortOrder) *kindSorter {
+	o := make(map[string]int, len(s))
+	for v, k := range s {
+		o[k] = v
+	}
+
+	return &kindSorter{
+		manifests: m,
+		ordering:  o,
+	}
+}
+
+func (k *kindSorter) Len() int { return len(k.manifests) }
+
+func (k *kindSorter) Swap(i, j int) { k.manifests[i], k.manifests[j] = k.manifests[j], k.manifests[i] }
+
+func (k *kindSorter) Less(i, j int) bool {
+	a := k.manifests[i]
+	b := k.manifests[j]
+	first, aok := k.ordering[a.Head.Kind]
+	second, bok := k.ordering[b.Head.Kind]
+	// if same kind (including unknown) sub sort alphanumeric
+	if first == second {
+		// if both are unknown and of different kind sort by kind alphabetically
+		if !aok && !bok && a.Head.Kind != b.Head.Kind {
+			return a.Head.Kind < b.Head.Kind
+		}
+		return a.Name < b.Name
+	}
+	// unknown kind is last
+	if !aok {
+		return false
+	}
+	if !bok {
+		return true
+	}
+	// sort different kinds
+	return first < second
+}
+
+// SortByKind sorts manifests in InstallOrder
+func SortByKind(manifests []Manifest) []Manifest {
+	ordering := InstallOrder
+	ks := newKindSorter(manifests, ordering)
+	sort.Sort(ks)
+	return ks.manifests
+}

pkg/releaseutil/kind_sorter_test.go

@@ -0,0 +1,215 @@
+/*
+Copyright The Helm Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package releaseutil
+
+import (
+	"bytes"
+	"testing"
+)
+
+func TestKindSorter(t *testing.T) {
+	manifests := []Manifest{
+		{
+			Name: "i",
+			Head: &SimpleHead{Kind: "ClusterRole"},
+		},
+		{
+			Name: "j",
+			Head: &SimpleHead{Kind: "ClusterRoleBinding"},
+		},
+		{
+			Name: "e",
+			Head: &SimpleHead{Kind: "ConfigMap"},
+		},
+		{
+			Name: "u",
+			Head: &SimpleHead{Kind: "CronJob"},
+		},
+		{
+			Name: "2",
+			Head: &SimpleHead{Kind: "CustomResourceDefinition"},
+		},
+		{
+			Name: "n",
+			Head: &SimpleHead{Kind: "DaemonSet"},
+		},
+		{
+			Name: "r",
+			Head: &SimpleHead{Kind: "Deployment"},
+		},
+		{
+			Name: "!",
+			Head: &SimpleHead{Kind: "HonkyTonkSet"},
+		},
+		{
+			Name: "v",
+			Head: &SimpleHead{Kind: "Ingress"},
+		},
+		{
+			Name: "t",
+			Head: &SimpleHead{Kind: "Job"},
+		},
+		{
+			Name: "c",
+			Head: &SimpleHead{Kind: "LimitRange"},
+		},
+		{
+			Name: "a",
+			Head: &SimpleHead{Kind: "Namespace"},
+		},
+		{
+			Name: "f",
+			Head: &SimpleHead{Kind: "PersistentVolume"},
+		},
+		{
+			Name: "g",
+			Head: &SimpleHead{Kind: "PersistentVolumeClaim"},
+		},
+		{
+			Name: "o",
+			Head: &SimpleHead{Kind: "Pod"},
+		},
+		{
+			Name: "q",
+			Head: &SimpleHead{Kind: "ReplicaSet"},
+		},
+		{
+			Name: "p",
+			Head: &SimpleHead{Kind: "ReplicationController"},
+		},
+		{
+			Name: "b",
+			Head: &SimpleHead{Kind: "ResourceQuota"},
+		},
+		{
+			Name: "k",
+			Head: &SimpleHead{Kind: "Role"},
+		},
+		{
+			Name: "l",
+			Head: &SimpleHead{Kind: "RoleBinding"},
+		},
+		{
+			Name: "d",
+			Head: &SimpleHead{Kind: "Secret"},
+		},
+		{
+			Name: "m",
+			Head: &SimpleHead{Kind: "Service"},
+		},
+		{
+			Name: "h",
+			Head: &SimpleHead{Kind: "ServiceAccount"},
+		},
+		{
+			Name: "s",
+			Head: &SimpleHead{Kind: "StatefulSet"},
+		},
+		{
+			Name: "1",
+			Head: &SimpleHead{Kind: "StorageClass"},
+		},
+		{
+			Name: "w",
+			Head: &SimpleHead{Kind: "APIService"},
+		},
+	}
+
+	for _, test := range []struct {
+		description string
+		order       KindSortOrder
+		expected    string
+	}{
+		{"install", InstallOrder, "abcde1fgh2ijklmnopqrstuvw!"},
+		{"uninstall", UninstallOrder, "wvmutsrqponlkji2hgf1edcba!"},
+	} {
+		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)
+			}
+		})
+	}
+}
+
+// TestKindSorterSubSort verifies manifests of same kind are also sorted alphanumeric
+func TestKindSorterSubSort(t *testing.T) {
+	manifests := []Manifest{
+		{
+			Name: "a",
+			Head: &SimpleHead{Kind: "ClusterRole"},
+		},
+		{
+			Name: "A",
+			Head: &SimpleHead{Kind: "ClusterRole"},
+		},
+		{
+			Name: "0",
+			Head: &SimpleHead{Kind: "ConfigMap"},
+		},
+		{
+			Name: "1",
+			Head: &SimpleHead{Kind: "ConfigMap"},
+		},
+		{
+			Name: "z",
+			Head: &SimpleHead{Kind: "ClusterRoleBinding"},
+		},
+		{
+			Name: "!",
+			Head: &SimpleHead{Kind: "ClusterRoleBinding"},
+		},
+		{
+			Name: "u2",
+			Head: &SimpleHead{Kind: "Unknown"},
+		},
+		{
+			Name: "u1",
+			Head: &SimpleHead{Kind: "Unknown"},
+		},
+		{
+			Name: "t3",
+			Head: &SimpleHead{Kind: "Unknown2"},
+		},
+	}
+	for _, test := range []struct {
+		description string
+		order       KindSortOrder
+		expected    string
+	}{
+		// expectation is sorted by kind (unknown is last) and then sub sorted alphabetically within each group
+		{"cm,clusterRole,clusterRoleBinding,Unknown,Unknown2", InstallOrder, "01Aa!zu1u2t3"},
+	} {
+		var buf bytes.Buffer
+		t.Run(test.description, func(t *testing.T) {
+			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)
+			}
+		})
+	}
+}

pkg/releaseutil/manifest_sorter.go

@@ -0,0 +1,220 @@
+/*
+Copyright The Helm Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package releaseutil
+
+import (
+	"log"
+	"path"
+	"strconv"
+	"strings"
+
+	"github.com/pkg/errors"
+	yaml "gopkg.in/yaml.v2"
+	"k8s.io/helm/pkg/chartutil"
+	"k8s.io/helm/pkg/hapi/release"
+	"k8s.io/helm/pkg/hooks"
+)
+
+// Manifest represents a manifest file, which has a name and some content.
+type Manifest struct {
+	Name    string
+	Content string
+	Head    *SimpleHead
+}
+
+// manifestFile represents a file that contains a manifest.
+type manifestFile struct {
+	entries map[string]string
+	path    string
+	apis    chartutil.VersionSet
+}
+
+// result is an intermediate structure used during sorting.
+type result struct {
+	hooks   []*release.Hook
+	generic []Manifest
+}
+
+// TODO: Refactor this out. It's here because naming conventions were not followed through.
+// So fix the Test hook names and then remove this.
+var events = map[string]release.HookEvent{
+	hooks.PreInstall:         release.HookPreInstall,
+	hooks.PostInstall:        release.HookPostInstall,
+	hooks.PreDelete:          release.HookPreDelete,
+	hooks.PostDelete:         release.HookPostDelete,
+	hooks.PreUpgrade:         release.HookPreUpgrade,
+	hooks.PostUpgrade:        release.HookPostUpgrade,
+	hooks.PreRollback:        release.HookPreRollback,
+	hooks.PostRollback:       release.HookPostRollback,
+	hooks.ReleaseTestSuccess: release.HookReleaseTestSuccess,
+	hooks.ReleaseTestFailure: release.HookReleaseTestFailure,
+}
+
+// SortManifests takes a map of filename/YAML contents, splits the file
+// by manifest entries, and sorts the entries into hook types.
+//
+// The resulting hooks struct will be populated with all of the generated hooks.
+// Any file that does not declare one of the hook types will be placed in the
+// 'generic' bucket.
+//
+// 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, sort KindSortOrder) ([]*release.Hook, []Manifest, error) {
+	result := &result{}
+
+	for filePath, c := range files {
+
+		// Skip partials. We could return these as a separate map, but there doesn't
+		// seem to be any need for that at this time.
+		if strings.HasPrefix(path.Base(filePath), "_") {
+			continue
+		}
+		// Skip empty files and log this.
+		if len(strings.TrimSpace(c)) == 0 {
+			log.Printf("info: manifest %q is empty. Skipping.", filePath)
+			continue
+		}
+
+		manifestFile := &manifestFile{
+			entries: SplitManifests(c),
+			path:    filePath,
+			apis:    apis,
+		}
+
+		if err := manifestFile.sort(result); err != nil {
+			return result.hooks, result.generic, err
+		}
+	}
+
+	return result.hooks, sortByKind(result.generic, sort), nil
+}
+
+// sort takes a manifestFile object which may contain multiple resource definition
+// entries and sorts each entry by hook types, and saves the resulting hooks and
+// generic manifests (or non-hooks) to the result struct.
+//
+// To determine hook type, it looks for a YAML structure like this:
+//
+//  kind: SomeKind
+//  apiVersion: v1
+// 	metadata:
+//		annotations:
+//			helm.sh/hook: pre-install
+//
+// To determine the policy to delete the hook, it looks for a YAML structure like this:
+//
+//  kind: SomeKind
+//  apiVersion: v1
+//  metadata:
+// 		annotations:
+// 			helm.sh/hook-delete-policy: hook-succeeded
+func (file *manifestFile) sort(result *result) error {
+	for _, m := range file.entries {
+		var entry SimpleHead
+		if err := yaml.Unmarshal([]byte(m), &entry); err != nil {
+			return errors.Wrapf(err, "YAML parse error on %s", file.path)
+		}
+
+		if entry.Version != "" && !file.apis.Has(entry.Version) {
+			return errors.Errorf("apiVersion %q in %s is not available", entry.Version, file.path)
+		}
+
+		if !hasAnyAnnotation(entry) {
+			result.generic = append(result.generic, Manifest{
+				Name:    file.path,
+				Content: m,
+				Head:    &entry,
+			})
+			continue
+		}
+
+		hookTypes, ok := entry.Metadata.Annotations[hooks.HookAnno]
+		if !ok {
+			result.generic = append(result.generic, Manifest{
+				Name:    file.path,
+				Content: m,
+				Head:    &entry,
+			})
+			continue
+		}
+
+		hw := calculateHookWeight(entry)
+
+		h := &release.Hook{
+			Name:           entry.Metadata.Name,
+			Kind:           entry.Kind,
+			Path:           file.path,
+			Manifest:       m,
+			Events:         []release.HookEvent{},
+			Weight:         hw,
+			DeletePolicies: []release.HookDeletePolicy{},
+		}
+
+		isUnknownHook := false
+		for _, hookType := range strings.Split(hookTypes, ",") {
+			hookType = strings.ToLower(strings.TrimSpace(hookType))
+			e, ok := events[hookType]
+			if !ok {
+				isUnknownHook = true
+				break
+			}
+			h.Events = append(h.Events, e)
+		}
+
+		if isUnknownHook {
+			log.Printf("info: skipping unknown hook: %q", hookTypes)
+			continue
+		}
+
+		result.hooks = append(result.hooks, h)
+
+		operateAnnotationValues(entry, hooks.HookDeleteAnno, func(value string) {
+			h.DeletePolicies = append(h.DeletePolicies, release.HookDeletePolicy(value))
+		})
+	}
+
+	return nil
+}
+
+// hasAnyAnnotation returns true if the given entry has any annotations at all.
+func hasAnyAnnotation(entry SimpleHead) bool {
+	return entry.Metadata != nil &&
+		entry.Metadata.Annotations != nil &&
+		len(entry.Metadata.Annotations) != 0
+}
+
+// calculateHookWeight finds the weight in the hook weight annotation.
+//
+// If no weight is found, the assigned weight is 0
+func calculateHookWeight(entry SimpleHead) int {
+	hws := entry.Metadata.Annotations[hooks.HookWeightAnno]
+	hw, err := strconv.Atoi(hws)
+	if err != nil {
+		hw = 0
+	}
+	return hw
+}
+
+// operateAnnotationValues finds the given annotation and runs the operate function with the value of that annotation
+func operateAnnotationValues(entry SimpleHead, annotation string, operate func(p string)) {
+	if dps, ok := entry.Metadata.Annotations[annotation]; ok {
+		for _, dp := range strings.Split(dps, ",") {
+			dp = strings.ToLower(strings.TrimSpace(dp))
+			operate(dp)
+		}
+	}
+}

pkg/releaseutil/manifest_sorter_test.go

@@ -0,0 +1,229 @@
+/*
+Copyright The Helm Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package releaseutil
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/ghodss/yaml"
+
+	"k8s.io/helm/pkg/chartutil"
+	"k8s.io/helm/pkg/hapi/release"
+)
+
+func TestSortManifests(t *testing.T) {
+
+	data := []struct {
+		name     []string
+		path     string
+		kind     []string
+		hooks    map[string][]release.HookEvent
+		manifest string
+	}{
+		{
+			name:  []string{"first"},
+			path:  "one",
+			kind:  []string{"Job"},
+			hooks: map[string][]release.HookEvent{"first": {release.HookPreInstall}},
+			manifest: `apiVersion: v1
+kind: Job
+metadata:
+  name: first
+  labels:
+    doesnot: matter
+  annotations:
+    "helm.sh/hook": pre-install
+`,
+		},
+		{
+			name:  []string{"second"},
+			path:  "two",
+			kind:  []string{"ReplicaSet"},
+			hooks: map[string][]release.HookEvent{"second": {release.HookPostInstall}},
+			manifest: `kind: ReplicaSet
+apiVersion: v1beta1
+metadata:
+  name: second
+  annotations:
+    "helm.sh/hook": post-install
+`,
+		}, {
+			name:  []string{"third"},
+			path:  "three",
+			kind:  []string{"ReplicaSet"},
+			hooks: map[string][]release.HookEvent{"third": nil},
+			manifest: `kind: ReplicaSet
+apiVersion: v1beta1
+metadata:
+  name: third
+  annotations:
+    "helm.sh/hook": no-such-hook
+`,
+		}, {
+			name:  []string{"fourth"},
+			path:  "four",
+			kind:  []string{"Pod"},
+			hooks: map[string][]release.HookEvent{"fourth": nil},
+			manifest: `kind: Pod
+apiVersion: v1
+metadata:
+  name: fourth
+  annotations:
+    nothing: here`,
+		}, {
+			name:  []string{"fifth"},
+			path:  "five",
+			kind:  []string{"ReplicaSet"},
+			hooks: map[string][]release.HookEvent{"fifth": {release.HookPostDelete, release.HookPostInstall}},
+			manifest: `kind: ReplicaSet
+apiVersion: v1beta1
+metadata:
+  name: fifth
+  annotations:
+    "helm.sh/hook": post-delete, post-install
+`,
+		}, {
+			// Regression test: files with an underscore in the base name should be skipped.
+			name:     []string{"sixth"},
+			path:     "six/_six",
+			kind:     []string{"ReplicaSet"},
+			hooks:    map[string][]release.HookEvent{"sixth": nil},
+			manifest: `invalid manifest`, // This will fail if partial is not skipped.
+		}, {
+			// Regression test: files with no content should be skipped.
+			name:     []string{"seventh"},
+			path:     "seven",
+			kind:     []string{"ReplicaSet"},
+			hooks:    map[string][]release.HookEvent{"seventh": nil},
+			manifest: "",
+		},
+		{
+			name:  []string{"eighth", "example-test"},
+			path:  "eight",
+			kind:  []string{"ConfigMap", "Pod"},
+			hooks: map[string][]release.HookEvent{"eighth": nil, "example-test": {release.HookReleaseTestSuccess}},
+			manifest: `kind: ConfigMap
+apiVersion: v1
+metadata:
+  name: eighth
+data:
+  name: value
+---
+apiVersion: v1
+kind: Pod
+metadata:
+  name: example-test
+  annotations:
+    "helm.sh/hook": test-success
+`,
+		},
+	}
+
+	manifests := make(map[string]string, len(data))
+	for _, o := range data {
+		manifests[o.path] = o.manifest
+	}
+
+	hs, generic, err := SortManifests(manifests, chartutil.NewVersionSet("v1", "v1beta1"), InstallOrder)
+	if err != nil {
+		t.Fatalf("Unexpected error: %s", err)
+	}
+
+	// This test will fail if 'six' or 'seven' was added.
+	if len(generic) != 2 {
+		t.Errorf("Expected 2 generic manifests, got %d", len(generic))
+	}
+
+	if len(hs) != 4 {
+		t.Errorf("Expected 4 hooks, got %d", len(hs))
+	}
+
+	for _, out := range hs {
+		found := false
+		for _, expect := range data {
+			if out.Path == expect.path {
+				found = true
+				if out.Path != expect.path {
+					t.Errorf("Expected path %s, got %s", expect.path, out.Path)
+				}
+				nameFound := false
+				for _, expectedName := range expect.name {
+					if out.Name == expectedName {
+						nameFound = true
+					}
+				}
+				if !nameFound {
+					t.Errorf("Got unexpected name %s", out.Name)
+				}
+				kindFound := false
+				for _, expectedKind := range expect.kind {
+					if out.Kind == expectedKind {
+						kindFound = true
+					}
+				}
+				if !kindFound {
+					t.Errorf("Got unexpected kind %s", out.Kind)
+				}
+
+				expectedHooks := expect.hooks[out.Name]
+				if !reflect.DeepEqual(expectedHooks, out.Events) {
+					t.Errorf("expected events: %v but got: %v", expectedHooks, out.Events)
+				}
+
+			}
+		}
+		if !found {
+			t.Errorf("Result not found: %v", out)
+		}
+	}
+
+	// Verify the sort order
+	sorted := []Manifest{}
+	for _, s := range data {
+		manifests := SplitManifests(s.manifest)
+
+		for _, m := range manifests {
+			var sh SimpleHead
+			err := yaml.Unmarshal([]byte(m), &sh)
+			if err != nil {
+				// This is expected for manifests that are corrupt or empty.
+				t.Log(err)
+				continue
+			}
+
+			name := sh.Metadata.Name
+
+			//only keep track of non-hook manifests
+			if err == nil && s.hooks[name] == nil {
+				another := Manifest{
+					Content: m,
+					Name:    name,
+					Head:    &sh,
+				}
+				sorted = append(sorted, another)
+			}
+		}
+	}
+
+	sorted = sortByKind(sorted, InstallOrder)
+	for i, m := range generic {
+		if m.Content != sorted[i].Content {
+			t.Errorf("Expected %q, got %q", m.Content, sorted[i].Content)
+		}
+	}
+}

pkg/releaseutil/manifest_test.go

@@ -21,7 +21,7 @@ import (
 	"testing"
 )
 
-const manifestFile = `
+const mockManifestFile = `
 
 ---
 apiVersion: v1
@@ -50,7 +50,7 @@ spec:
     cmd: fake-command`
 
 func TestSplitManifest(t *testing.T) {
-	manifests := SplitManifests(manifestFile)
+	manifests := SplitManifests(mockManifestFile)
 	if len(manifests) != 1 {
 		t.Errorf("Expected 1 manifest, got %v", len(manifests))
 	}

pkg/releaseutil/sorter.go

@@ -27,20 +27,26 @@ type list []*rspb.Release
 func (s list) Len() int      { return len(s) }
 func (s list) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
 
+// ByName sorts releases by name
 type ByName struct{ list }
 
+// Less compares to releases
 func (s ByName) Less(i, j int) bool { return s.list[i].Name < s.list[j].Name }
 
+// ByDate sorts releases by date
 type ByDate struct{ list }
 
+// Less compares to releases
 func (s ByDate) Less(i, j int) bool {
 	ti := s.list[i].Info.LastDeployed.Second()
 	tj := s.list[j].Info.LastDeployed.Second()
 	return ti < tj
 }
 
+// ByRevision sorts releases by revision number
 type ByRevision struct{ list }
 
+// Less compares to releases
 func (s ByRevision) Less(i, j int) bool {
 	return s.list[i].Version < s.list[j].Version
 }

pkg/tiller/hooks.go

@@ -62,7 +62,7 @@ type manifestFile struct {
 	apis    chartutil.VersionSet
 }
 
-// sortManifests takes a map of filename/YAML contents, splits the file
+// SortManifests takes a map of filename/YAML contents, splits the file
 // by manifest entries, and sorts the entries into hook types.
 //
 // The resulting hooks struct will be populated with all of the generated hooks.