Merge pull request helm#1238 from technosophos/fix/1228-order-manifests

fix(tiller): Order the manifests before sending to k8s

cmd/tiller/hooks.go

@@ -71,6 +71,13 @@ func (v versionSet) Has(apiVersion string) bool {
 	return ok
 }
 
+// manifest represents a manifest file, which has a name and some content.
+type manifest struct {
+	name    string
+	content string
+	head    *simpleHead
+}
+
 // sortManifests takes a map of filename/YAML contents and sorts them into hook types.
 //
 // The resulting hooks struct will be populated with all of the generated hooks.
@@ -92,9 +99,9 @@ func (v versionSet) Has(apiVersion string) bool {
 //
 // Files that do not parse into the expected format are simply placed into a map and
 // returned.
-func sortManifests(files map[string]string, apis versionSet) ([]*release.Hook, map[string]string, error) {
+func sortManifests(files map[string]string, apis versionSet, sort SortOrder) ([]*release.Hook, []manifest, error) {
 	hs := []*release.Hook{}
-	generic := map[string]string{}
+	generic := []manifest{}
 
 	for n, c := range files {
 		// Skip partials. We could return these as a separate map, but there doesn't
@@ -121,13 +128,13 @@ func sortManifests(files map[string]string, apis versionSet) ([]*release.Hook, m
 		}
 
 		if sh.Metadata == nil || sh.Metadata.Annotations == nil || len(sh.Metadata.Annotations) == 0 {
-			generic[n] = c
+			generic = append(generic, manifest{name: n, content: c, head: &sh})
 			continue
 		}
 
 		hookTypes, ok := sh.Metadata.Annotations[hookAnno]
 		if !ok {
-			generic[n] = c
+			generic = append(generic, manifest{name: n, content: c, head: &sh})
 			continue
 		}
 		h := &release.Hook{

cmd/tiller/hooks_test.go

@@ -116,7 +116,7 @@ metadata:
 		manifests[o.path] = o.manifest
 	}
 
-	hs, generic, err := sortManifests(manifests, newVersionSet("v1", "v1beta1"))
+	hs, generic, err := sortManifests(manifests, newVersionSet("v1", "v1beta1"), InstallOrder)
 	if err != nil {
 		t.Fatalf("Unexpected error: %s", err)
 	}

cmd/tiller/kind_sorter.go

@@ -0,0 +1,75 @@
+/*
+Copyright 2016 The Kubernetes Authors All rights reserved.
+
+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 main
+
+import (
+	"sort"
+)
+
+// SortOrder is an ordering of Kinds.
+type SortOrder []string
+
+// InstallOrder is the order in which manifests should be installed (by Kind)
+var InstallOrder SortOrder = []string{"Namespace", "Secret", "ConfigMap", "PersistentVolume", "ServiceAccount", "Service", "Pod", "ReplicationController", "Deployment", "DaemonSet", "Ingress", "Job"}
+
+// UninstallOrder is the order in which manifests should be uninstalled (by Kind)
+var UninstallOrder SortOrder = []string{"Service", "Pod", "ReplicationController", "Deployment", "DaemonSet", "ConfigMap", "Secret", "PersistentVolume", "ServiceAccount", "Ingress", "Job", "Namespace"}
+
+// sortByKind does an in-place sort of manifests by Kind.
+//
+// Results are sorted by 'ordering'
+func sortByKind(manifests []manifest, ordering SortOrder) []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 SortOrder) *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, ok := k.ordering[a.head.Kind]
+	if !ok {
+		// Unknown is always last
+		return false
+	}
+	second, ok := k.ordering[b.head.Kind]
+	if !ok {
+		return true
+	}
+	return first < second
+}

cmd/tiller/kind_sorter_test.go

@@ -0,0 +1,72 @@
+/*
+Copyright 2016 The Kubernetes Authors All rights reserved.
+
+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 main
+
+import (
+	"testing"
+)
+
+func TestKindSorter(t *testing.T) {
+	manifests := []manifest{
+		{
+			name:    "m",
+			content: "",
+			head:    &simpleHead{Kind: "Deployment"},
+		},
+		{
+			name:    "l",
+			content: "",
+			head:    &simpleHead{Kind: "Service"},
+		},
+		{
+			name:    "!",
+			content: "",
+			head:    &simpleHead{Kind: "HonkyTonkSet"},
+		},
+		{
+			name:    "h",
+			content: "",
+			head:    &simpleHead{Kind: "Namespace"},
+		},
+		{
+			name:    "e",
+			content: "",
+			head:    &simpleHead{Kind: "ConfigMap"},
+		},
+	}
+
+	res := sortByKind(manifests, InstallOrder)
+	got := ""
+	expect := "helm!"
+	for _, r := range res {
+		got += r.name
+	}
+	if got != expect {
+		t.Errorf("Expected %q, got %q", expect, got)
+	}
+
+	expect = "lmeh!"
+	got = ""
+	res = sortByKind(manifests, UninstallOrder)
+	for _, r := range res {
+		got += r.name
+	}
+	if got != expect {
+		t.Errorf("Expected %q, got %q", expect, got)
+	}
+
+}

cmd/tiller/release_server.go

@@ -548,7 +548,7 @@ func (s *releaseServer) renderResources(ch *chart.Chart, values chartutil.Values
 	if err != nil {
 		return nil, nil, "", fmt.Errorf("Could not get apiVersions from Kubernetes: %s", err)
 	}
-	hooks, manifests, err := sortManifests(files, vs)
+	hooks, manifests, err := sortManifests(files, vs, InstallOrder)
 	if err != nil {
 		// By catching parse errors here, we can prevent bogus releases from going
 		// to Kubernetes.
@@ -557,9 +557,9 @@ func (s *releaseServer) renderResources(ch *chart.Chart, values chartutil.Values
 
 	// Aggregate all valid manifests into one big doc.
 	b := bytes.NewBuffer(nil)
-	for name, file := range manifests {
-		b.WriteString("\n---\n# Source: " + name + "\n")
-		b.WriteString(file)
+	for _, m := range manifests {
+		b.WriteString("\n---\n# Source: " + m.name + "\n")
+		b.WriteString(m.content)
 	}
 
 	return hooks, b, notes, nil
@@ -707,11 +707,27 @@ func (s *releaseServer) UninstallRelease(c ctx.Context, req *services.UninstallR
 		}
 	}
 
-	b := bytes.NewBuffer([]byte(rel.Manifest))
-	if err := s.env.KubeClient.Delete(rel.Namespace, b); err != nil {
-		log.Printf("uninstall: Failed deletion of %q: %s", req.Name, err)
+	vs, err := s.getVersionSet()
+	if err != nil {
+		return nil, fmt.Errorf("Could not get apiVersions from Kubernetes: %s", err)
+	}
+
+	manifests := splitManifests(rel.Manifest)
+	_, files, err := sortManifests(manifests, vs, UninstallOrder)
+	if err != nil {
+		// We could instead just delete everything in no particular order.
 		return nil, err
 	}
+	// Note: We could re-join these into one file and delete just that one. Or
+	// we could collect errors (instead of bailing on the first error) and try
+	// to delete as much as possible instead of failing at the first error.
+	for _, file := range files {
+		b := bytes.NewBufferString(file.content)
+		if err := s.env.KubeClient.Delete(rel.Namespace, b); err != nil {
+			log.Printf("uninstall: Failed deletion of %q: %s", req.Name, err)
+			return nil, err
+		}
+	}
 
 	if !req.DisableHooks {
 		if err := s.execHook(rel.Hooks, rel.Name, rel.Namespace, postDelete); err != nil {
@@ -754,3 +770,18 @@ func (r byDate) Swap(p, q int) {
 func (r byDate) Less(p, q int) bool {
 	return r[p].Info.LastDeployed.Seconds < r[q].Info.LastDeployed.Seconds
 }
+
+func splitManifests(bigfile string) map[string]string {
+	// This is not the best way of doing things, but it's how k8s itself does it.
+	// Basically, we're quickly splitting a stream of YAML documents into an
+	// array of YAML docs. In the current implementation, the file name is just
+	// a place holder, and doesn't have any further meaning.
+	sep := "\n---\n"
+	tpl := "manifest-%d"
+	res := map[string]string{}
+	tmp := strings.Split(bigfile, sep)
+	for i, d := range tmp {
+		res[fmt.Sprintf(tpl, i)] = d
+	}
+	return res
+}