fix(tiller): track hooks in multi-def manifests

resolves #2290

pkg/tiller/hooks.go

@@ -88,8 +88,10 @@ func sortManifests(files map[string]string, apis chartutil.VersionSet, sort Sort
 			continue
 		}
 
+		entries := util.SplitManifests(c)
+		for _, m := range entries {
 			var sh util.SimpleHead
-		err := yaml.Unmarshal([]byte(c), &sh)
+			err := yaml.Unmarshal([]byte(m), &sh)
 
 			if err != nil {
 				e := fmt.Errorf("YAML parse error on %s: %s", n, err)
@@ -101,13 +103,13 @@ func sortManifests(files map[string]string, apis chartutil.VersionSet, sort Sort
 			}
 
 			if sh.Metadata == nil || sh.Metadata.Annotations == nil || len(sh.Metadata.Annotations) == 0 {
-			generic = append(generic, manifest{name: n, content: c, head: &sh})
+				generic = append(generic, manifest{name: n, content: m, head: &sh})
 				continue
 			}
 
 			hookTypes, ok := sh.Metadata.Annotations[hooks.HookAnno]
 			if !ok {
-			generic = append(generic, manifest{name: n, content: c, head: &sh})
+				generic = append(generic, manifest{name: n, content: m, head: &sh})
 				continue
 			}
 
@@ -116,12 +118,13 @@ func sortManifests(files map[string]string, apis chartutil.VersionSet, sort Sort
 			if err != nil {
 				hw = 0
 			}
+			fmt.Println("NAME: " + sh.Metadata.Name)
 
 			h := &release.Hook{
 				Name:     sh.Metadata.Name,
 				Kind:     sh.Kind,
-			Path:     n,
+				Path:     n, //TODO: fix by putting back into big loop
-			Manifest: c,
+				Manifest: m,
 				Events:   []release.Hook_Event{},
 				Weight:   int32(hw),
 			}
@@ -142,5 +145,8 @@ func sortManifests(files map[string]string, apis chartutil.VersionSet, sort Sort
 			}
 			hs = append(hs, h)
 		}
+
+	}
+
 	return hs, sortByKind(generic, sort), nil
 }

pkg/tiller/hooks_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package tiller
 
 import (
+	"reflect"
 	"testing"
 
 	"github.com/ghodss/yaml"
@@ -29,17 +30,17 @@ import (
 func TestSortManifests(t *testing.T) {
 
 	data := []struct {
-		name     string
+		name     []string
 		path     string
-		kind     string
+		kind     []string
-		hooks    []release.Hook_Event
+		hooks    map[string][]release.Hook_Event
 		manifest string
 	}{
 		{
-			name:  "first",
+			name:  []string{"first"},
 			path:  "one",
-			kind:  "Job",
+			kind:  []string{"Job"},
-			hooks: []release.Hook_Event{release.Hook_PRE_INSTALL},
+			hooks: map[string][]release.Hook_Event{"first": {release.Hook_PRE_INSTALL}},
 			manifest: `apiVersion: v1
 kind: Job
 metadata:
@@ -51,10 +52,10 @@ metadata:
 `,
 		},
 		{
-			name:  "second",
+			name:  []string{"second"},
 			path:  "two",
-			kind:  "ReplicaSet",
+			kind:  []string{"ReplicaSet"},
-			hooks: []release.Hook_Event{release.Hook_POST_INSTALL},
+			hooks: map[string][]release.Hook_Event{"second": {release.Hook_POST_INSTALL}},
 			manifest: `kind: ReplicaSet
 apiVersion: v1beta1
 metadata:
@@ -63,10 +64,10 @@ metadata:
     "helm.sh/hook": post-install
 `,
 		}, {
-			name:  "third",
+			name:  []string{"third"},
 			path:  "three",
-			kind:  "ReplicaSet",
+			kind:  []string{"ReplicaSet"},
-			hooks: []release.Hook_Event{},
+			hooks: map[string][]release.Hook_Event{"third": nil},
 			manifest: `kind: ReplicaSet
 apiVersion: v1beta1
 metadata:
@@ -75,22 +76,21 @@ metadata:
     "helm.sh/hook": no-such-hook
 `,
 		}, {
-			name:  "fourth",
+			name:  []string{"fourth"},
 			path:  "four",
-			kind:  "Pod",
+			kind:  []string{"Pod"},
-			hooks: []release.Hook_Event{},
+			hooks: map[string][]release.Hook_Event{"fourth": nil},
 			manifest: `kind: Pod
 apiVersion: v1
 metadata:
   name: fourth
   annotations:
-    nothing: here
+    nothing: here`,
-`,
 		}, {
-			name:  "fifth",
+			name:  []string{"fifth"},
 			path:  "five",
-			kind:  "ReplicaSet",
+			kind:  []string{"ReplicaSet"},
-			hooks: []release.Hook_Event{release.Hook_POST_DELETE, release.Hook_POST_INSTALL},
+			hooks: map[string][]release.Hook_Event{"fifth": {release.Hook_POST_DELETE, release.Hook_POST_INSTALL}},
 			manifest: `kind: ReplicaSet
 apiVersion: v1beta1
 metadata:
@@ -100,19 +100,39 @@ metadata:
 `,
 		}, {
 			// Regression test: files with an underscore in the base name should be skipped.
-			name:     "sixth",
+			name:     []string{"sixth"},
 			path:     "six/_six",
-			kind:     "ReplicaSet",
+			kind:     []string{"ReplicaSet"},
-			hooks:    []release.Hook_Event{},
+			hooks:    map[string][]release.Hook_Event{"sixth": nil},
 			manifest: `invalid manifest`, // This will fail if partial is not skipped.
 		}, {
 			// Regression test: files with no content should be skipped.
-			name:     "seventh",
+			name:     []string{"seventh"},
 			path:     "seven",
-			kind:     "ReplicaSet",
+			kind:     []string{"ReplicaSet"},
-			hooks:    []release.Hook_Event{},
+			hooks:    map[string][]release.Hook_Event{"seventh": nil},
 			manifest: "",
 		},
+		{
+			name:  []string{"eighth", "example-test"},
+			path:  "eight",
+			kind:  []string{"ConfigMap", "Pod"},
+			hooks: map[string][]release.Hook_Event{"eighth": nil, "example-test": {release.Hook_RELEASE_TEST_SUCCESS}},
+			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))
@@ -126,12 +146,12 @@ metadata:
 	}
 
 	// This test will fail if 'six' or 'seven' was added.
-	if len(generic) != 1 {
+	if len(generic) != 2 {
-		t.Errorf("Expected 1 generic manifest, got %d", len(generic))
+		t.Errorf("Expected 2 generic manifests, got %d", len(generic))
 	}
 
-	if len(hs) != 3 {
+	if len(hs) != 4 {
-		t.Errorf("Expected 3 hooks, got %d", len(hs))
+		t.Errorf("Expected 4 hooks, got %d", len(hs))
 	}
 
 	for _, out := range hs {
@@ -142,18 +162,31 @@ metadata:
 				if out.Path != expect.path {
 					t.Errorf("Expected path %s, got %s", expect.path, out.Path)
 				}
-				if out.Name != expect.name {
+				nameFound := false
-					t.Errorf("Expected name %s, got %s", expect.name, out.Name)
+				for _, expectedName := range expect.name {
+					if out.Name == expectedName {
+						nameFound = true
 					}
-				if out.Kind != expect.kind {
-					t.Errorf("Expected kind %s, got %s", expect.kind, out.Kind)
 				}
-				for i := 0; i < len(out.Events); i++ {
+				if !nameFound {
-					if out.Events[i] != expect.hooks[i] {
+					t.Errorf("Got unexpected name %s", out.Name)
-						t.Errorf("Expected event %d, got %d", expect.hooks[i], out.Events[i])
 				}
+				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)
@@ -161,27 +194,40 @@ metadata:
 	}
 
 	// Verify the sort order
-	sorted := make([]manifest, len(data))
+	sorted := []manifest{}
-	for i, s := range data {
+	for _, s := range data {
+		manifests := util.SplitManifests(s.manifest)
+		mCount := 0
+		for _, m := range manifests {
+			name := s.name[mCount]
+
 			var sh util.SimpleHead
-		err := yaml.Unmarshal([]byte(s.manifest), &sh)
+			err := yaml.Unmarshal([]byte(m), &sh)
 			if err != nil {
 				// This is expected for manifests that are corrupt or empty.
 				t.Log(err)
 			}
-		sorted[i] = manifest{
+
-			content: s.manifest,
+			//only keep track of non-hook manifests
-			name:    s.name,
+			if err == nil && s.hooks[name] == nil {
+				another := manifest{
+					content: m,
+					name:    name,
 					head:    &sh,
 				}
+				sorted = append(sorted, another)
 			}
+
+			mCount++
+		}
+	}
+
 	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)
 		}
 	}
-
 }
 
 func TestVersionSet(t *testing.T) {

pkg/tiller/release_server_test.go

@@ -47,8 +47,7 @@ metadata:
   annotations:
     "helm.sh/hook": post-install,pre-delete
 data:
-  name: value
+  name: value`
-`
 
 var manifestWithTestHook = `
 apiVersion: v1
@@ -81,8 +80,7 @@ metadata:
   annotations:
     "helm.sh/hook": post-upgrade,pre-upgrade
 data:
-  name: value
+  name: value`
-`
 
 var manifestWithRollbackHooks = `apiVersion: v1
 kind: ConfigMap