diff --git a/pkg/tiller/hooks.go b/pkg/tiller/hooks.go index b26243479..996253384 100644 --- a/pkg/tiller/hooks.go +++ b/pkg/tiller/hooks.go @@ -51,96 +51,149 @@ type manifest struct { head *util.SimpleHead } -// sortManifests takes a map of filename/YAML contents and sorts them into hook types. +type result struct { + hooks []*release.Hook + generic []manifest +} + +type manifestFile struct { + entries map[string]string + path string + apis chartutil.VersionSet +} + +// 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. // -// To determine hook type, this looks for a YAML structure like this: -// -// kind: SomeKind -// apiVersion: v1 -// metadata: -// annotations: -// helm.sh/hook: pre-install -// -// Where HOOK_NAME is one of the known hooks. -// -// If a file declares more than one hook, it will be copied into all of the applicable -// hook buckets. (Note: label keys are not unique within the labels section). -// // 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 SortOrder) ([]*release.Hook, []manifest, error) { - hs := []*release.Hook{} - generic := []manifest{} + result := &result{} + + for filePath, c := range files { - for n, 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(n), "_") { + if strings.HasPrefix(path.Base(filePath), "_") { continue } - // Skip empty files, and log this. + // Skip empty files and log this. if len(strings.TrimSpace(c)) == 0 { - log.Printf("info: manifest %q is empty. Skipping.", n) + log.Printf("info: manifest %q is empty. Skipping.", filePath) continue } - var sh util.SimpleHead - err := yaml.Unmarshal([]byte(c), &sh) + manifestFile := &manifestFile{ + entries: util.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 +// +func (file *manifestFile) sort(result *result) error { + for _, m := range file.entries { + var entry util.SimpleHead + err := yaml.Unmarshal([]byte(m), &entry) if err != nil { - e := fmt.Errorf("YAML parse error on %s: %s", n, err) - return hs, generic, e + e := fmt.Errorf("YAML parse error on %s: %s", file.path, err) + return e } - if sh.Version != "" && !apis.Has(sh.Version) { - return hs, generic, fmt.Errorf("apiVersion %q in %s is not available", sh.Version, n) + if entry.Version != "" && !file.apis.Has(entry.Version) { + return fmt.Errorf("apiVersion %q in %s is not available", entry.Version, file.path) } - if sh.Metadata == nil || sh.Metadata.Annotations == nil || len(sh.Metadata.Annotations) == 0 { - generic = append(generic, manifest{name: n, content: c, head: &sh}) + if !hasAnyAnnotation(entry) { + result.generic = append(result.generic, manifest{ + name: file.path, + content: m, + head: &entry, + }) continue } - hookTypes, ok := sh.Metadata.Annotations[hooks.HookAnno] + hookTypes, ok := entry.Metadata.Annotations[hooks.HookAnno] if !ok { - generic = append(generic, manifest{name: n, content: c, head: &sh}) + result.generic = append(result.generic, manifest{ + name: file.path, + content: m, + head: &entry, + }) continue } - hws, _ := sh.Metadata.Annotations[hooks.HookWeightAnno] - hw, err := strconv.Atoi(hws) - if err != nil { - hw = 0 - } + hw := calculateHookWeight(entry) h := &release.Hook{ - Name: sh.Metadata.Name, - Kind: sh.Kind, - Path: n, - Manifest: c, + Name: entry.Metadata.Name, + Kind: entry.Kind, + Path: file.path, + Manifest: m, Events: []release.Hook_Event{}, - Weight: int32(hw), + Weight: hw, } - isHook := false + isKnownHook := false for _, hookType := range strings.Split(hookTypes, ",") { hookType = strings.ToLower(strings.TrimSpace(hookType)) e, ok := events[hookType] if ok { - isHook = true + isKnownHook = true h.Events = append(h.Events, e) } } - if !isHook { + if !isKnownHook { log.Printf("info: skipping unknown hook: %q", hookTypes) continue } - hs = append(hs, h) + + result.hooks = append(result.hooks, h) + } + + return nil +} + +func hasAnyAnnotation(entry util.SimpleHead) bool { + if entry.Metadata == nil || + entry.Metadata.Annotations == nil || + len(entry.Metadata.Annotations) == 0 { + return false } - return hs, sortByKind(generic, sort), nil + + return true +} + +func calculateHookWeight(entry util.SimpleHead) int32 { + hws, _ := entry.Metadata.Annotations[hooks.HookWeightAnno] + hw, err := strconv.Atoi(hws) + if err != nil { + hw = 0 + } + + return int32(hw) } diff --git a/pkg/tiller/hooks_test.go b/pkg/tiller/hooks_test.go index 823e7469c..eabdc7eac 100644 --- a/pkg/tiller/hooks_test.go +++ b/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 - hooks []release.Hook_Event + kind []string + hooks map[string][]release.Hook_Event manifest string }{ { - name: "first", + name: []string{"first"}, path: "one", - kind: "Job", - hooks: []release.Hook_Event{release.Hook_PRE_INSTALL}, + kind: []string{"Job"}, + 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", - hooks: []release.Hook_Event{release.Hook_POST_INSTALL}, + kind: []string{"ReplicaSet"}, + 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", - hooks: []release.Hook_Event{}, + kind: []string{"ReplicaSet"}, + 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", - hooks: []release.Hook_Event{}, + kind: []string{"Pod"}, + 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", - hooks: []release.Hook_Event{release.Hook_POST_DELETE, release.Hook_POST_INSTALL}, + kind: []string{"ReplicaSet"}, + 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", - hooks: []release.Hook_Event{}, + kind: []string{"ReplicaSet"}, + 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", - hooks: []release.Hook_Event{}, + kind: []string{"ReplicaSet"}, + 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 { - t.Errorf("Expected 1 generic manifest, got %d", len(generic)) + if len(generic) != 2 { + t.Errorf("Expected 2 generic manifests, got %d", len(generic)) } - if len(hs) != 3 { - t.Errorf("Expected 3 hooks, got %d", len(hs)) + if len(hs) != 4 { + t.Errorf("Expected 4 hooks, got %d", len(hs)) } for _, out := range hs { @@ -142,17 +162,30 @@ metadata: if out.Path != expect.path { t.Errorf("Expected path %s, got %s", expect.path, out.Path) } - if out.Name != expect.name { - t.Errorf("Expected name %s, got %s", expect.name, out.Name) + nameFound := false + 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) + if !nameFound { + t.Errorf("Got unexpected name %s", out.Name) } - for i := 0; i < len(out.Events); i++ { - if out.Events[i] != expect.hooks[i] { - 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 { @@ -161,27 +194,40 @@ metadata: } // Verify the sort order - sorted := make([]manifest, len(data)) - for i, s := range data { - var sh util.SimpleHead - err := yaml.Unmarshal([]byte(s.manifest), &sh) - if err != nil { - // This is expected for manifests that are corrupt or empty. - t.Log(err) - } - sorted[i] = manifest{ - content: s.manifest, - name: s.name, - head: &sh, + sorted := []manifest{} + 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(m), &sh) + if err != nil { + // This is expected for manifests that are corrupt or empty. + t.Log(err) + } + + //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) + } + + 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) { diff --git a/pkg/tiller/release_server_test.go b/pkg/tiller/release_server_test.go index 6fd2100b0..8425a58f5 100644 --- a/pkg/tiller/release_server_test.go +++ b/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