From 05d0fcb774d8134b49e60587f4dd7db98c8ef7b7 Mon Sep 17 00:00:00 2001 From: Jonathan Chauncey Date: Tue, 21 Mar 2017 15:21:24 -0400 Subject: [PATCH 1/4] feat(hooks): Adds weighted hooks closes #2136 * Adds new annotation `helm.sh/hookWeight` * Sorts executing hooks of similar kind in ascending order * There is no upper or lower bounds on the weights --- pkg/hooks/hooks.go | 3 ++ pkg/proto/hapi/release/hook.pb.go | 2 + pkg/tiller/hook_sorter.go | 53 ++++++++++++++++++++++ pkg/tiller/hook_sorter_test.go | 73 +++++++++++++++++++++++++++++++ pkg/tiller/hooks.go | 10 +++++ pkg/tiller/release_server.go | 14 +++--- 6 files changed, 149 insertions(+), 6 deletions(-) create mode 100644 pkg/tiller/hook_sorter.go create mode 100644 pkg/tiller/hook_sorter_test.go diff --git a/pkg/hooks/hooks.go b/pkg/hooks/hooks.go index ee467ea27..d6966a87a 100644 --- a/pkg/hooks/hooks.go +++ b/pkg/hooks/hooks.go @@ -23,6 +23,9 @@ import ( // HookAnno is the label name for a hook const HookAnno = "helm.sh/hook" +// HookWeightAnno is the label name for a hook weight +const HookWeightAnno = "helm.sh/hookWeight" + // Types of hooks const ( PreInstall = "pre-install" diff --git a/pkg/proto/hapi/release/hook.pb.go b/pkg/proto/hapi/release/hook.pb.go index 956ca15f3..435d2c80e 100644 --- a/pkg/proto/hapi/release/hook.pb.go +++ b/pkg/proto/hapi/release/hook.pb.go @@ -100,6 +100,8 @@ type Hook struct { Events []Hook_Event `protobuf:"varint,5,rep,packed,name=events,enum=hapi.release.Hook_Event" json:"events,omitempty"` // LastRun indicates the date/time this was last run. LastRun *google_protobuf.Timestamp `protobuf:"bytes,6,opt,name=last_run,json=lastRun" json:"last_run,omitempty"` + // Weight indicates the sort order for execution among similar Hook types + Weight int `protobuf:"bytes,7,opt,name=weight" json:"weight,omitempty"` } func (m *Hook) Reset() { *m = Hook{} } diff --git a/pkg/tiller/hook_sorter.go b/pkg/tiller/hook_sorter.go new file mode 100644 index 000000000..42d546620 --- /dev/null +++ b/pkg/tiller/hook_sorter.go @@ -0,0 +1,53 @@ +/* +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 tiller + +import ( + "sort" + + "k8s.io/helm/pkg/proto/hapi/release" +) + +// sortByHookWeight does an in-place sort of hooks by their supplied weight. +func sortByHookWeight(hooks []*release.Hook) []*release.Hook { + hs := newHookWeightSorter(hooks) + sort.Sort(hs) + return hs.hooks +} + +type hookWeightSorter struct { + hooks []*release.Hook +} + +func newHookWeightSorter(h []*release.Hook) *hookWeightSorter { + return &hookWeightSorter{ + hooks: h, + } +} + +func (hs *hookWeightSorter) Len() int { return len(hs.hooks) } + +func (hs *hookWeightSorter) Swap(i, j int) { + hs.hooks[i], hs.hooks[j] = hs.hooks[j], hs.hooks[i] +} + +func (hs *hookWeightSorter) Less(i, j int) bool { + if hs.hooks[i].Weight == hs.hooks[j].Weight { + return hs.hooks[i].Name < hs.hooks[j].Name + } + return hs.hooks[i].Weight < hs.hooks[j].Weight +} diff --git a/pkg/tiller/hook_sorter_test.go b/pkg/tiller/hook_sorter_test.go new file mode 100644 index 000000000..ac5b9bf8d --- /dev/null +++ b/pkg/tiller/hook_sorter_test.go @@ -0,0 +1,73 @@ +/* +Copyright 2017 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 tiller + +import ( + "testing" + + "k8s.io/helm/pkg/proto/hapi/release" +) + +func TestHookSorter(t *testing.T) { + hooks := []*release.Hook{ + { + Name: "g", + Kind: "pre-install", + Weight: 99, + }, + { + Name: "f", + Kind: "pre-install", + Weight: 3, + }, + { + Name: "b", + Kind: "pre-install", + Weight: -3, + }, + { + Name: "e", + Kind: "pre-install", + Weight: 3, + }, + { + Name: "a", + Kind: "pre-install", + Weight: -10, + }, + { + Name: "c", + Kind: "pre-install", + Weight: 0, + }, + { + Name: "d", + Kind: "pre-install", + Weight: 3, + }, + } + + res := sortByHookWeight(hooks) + got := "" + expect := "abcdefg" + for _, r := range res { + got += r.Name + } + if got != expect { + t.Errorf("Expected %q, got %q", expect, got) + } +} diff --git a/pkg/tiller/hooks.go b/pkg/tiller/hooks.go index 0b5510b8c..b6dfeb9cd 100644 --- a/pkg/tiller/hooks.go +++ b/pkg/tiller/hooks.go @@ -20,6 +20,7 @@ import ( "fmt" "log" "path" + "strconv" "strings" "github.com/ghodss/yaml" @@ -109,12 +110,21 @@ func sortManifests(files map[string]string, apis chartutil.VersionSet, sort Sort generic = append(generic, manifest{name: n, content: c, head: &sh}) continue } + + hw := 0 + hws, _ := sh.Metadata.Annotations[hooks.HookWeightAnno] + hw, err = strconv.Atoi(hws) + if err != nil { + hw = 0 + } + h := &release.Hook{ Name: sh.Metadata.Name, Kind: sh.Kind, Path: n, Manifest: c, Events: []release.Hook_Event{}, + Weight: hw, } isHook := false diff --git a/pkg/tiller/release_server.go b/pkg/tiller/release_server.go index 873d2335a..a73dd9a73 100644 --- a/pkg/tiller/release_server.go +++ b/pkg/tiller/release_server.go @@ -911,17 +911,18 @@ func (s *ReleaseServer) execHook(hs []*release.Hook, name, namespace, hook strin } log.Printf("Executing %s hooks for %s", hook, name) + executingHooks := []*release.Hook{} for _, h := range hs { - found := false for _, e := range h.Events { if e == code { - found = true + executingHooks = append(executingHooks, h) } } - // If this doesn't implement the hook, skip it. - if !found { - continue - } + } + + executingHooks = sortByHookWeight(executingHooks) + + for _, h := range executingHooks { b := bytes.NewBufferString(h.Manifest) if err := kubeCli.Create(namespace, b, timeout, false); err != nil { @@ -937,6 +938,7 @@ func (s *ReleaseServer) execHook(hs []*release.Hook, name, namespace, hook strin } h.LastRun = timeconv.Now() } + log.Printf("Hooks complete for %s %s", hook, name) return nil } From 91e7f6bfd69c4fde8cd382051c17486da304fc19 Mon Sep 17 00:00:00 2001 From: Jonathan Chauncey Date: Wed, 22 Mar 2017 14:57:09 -0400 Subject: [PATCH 2/4] docs(docs/charts_hooks.md): Add information about hook weights --- docs/charts_hooks.md | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/docs/charts_hooks.md b/docs/charts_hooks.md index 56cecd788..9b64199e2 100644 --- a/docs/charts_hooks.md +++ b/docs/charts_hooks.md @@ -58,16 +58,18 @@ hooks, the lifecycle is altered like this: 1. User runs `helm install foo` 2. Chart is loaded into Tiller 3. After some verification, Tiller renders the `foo` templates -4. Tiller executes the `pre-install` hook (loading hook resources into +4. Tiller prepares to execute the `pre-install` hooks (loading hook resources into Kubernetes) -5. Tiller waits until the hook is "Ready" -6. Tiller loads the resulting resources into Kubernetes. Note that if the `--wait` +5. Tiller sorts hooks by weight (assigning a weight of 0 by default) and by name for those hooks with the same weight in ascending order. +6. Tiller then loads the hook with the lowest weight first (negative to positive) +7. Tiller waits until the hook is "Ready" +8. Tiller loads the resulting resources into Kubernetes. Note that if the `--wait` flag is set, Tiller will wait until all resources are in a ready state and will not run the `post-install` hook until they are ready. -7. Tiller executes the `post-install` hook (loading hook resources) -8. Tiller waits until the hook is "Ready" -9. Tiller returns the release name (and other data) to the client -10. The client exits +9. Tiller executes the `post-install` hook (loading hook resources) +10. Tiller waits until the hook is "Ready" +11. Tiller returns the release name (and other data) to the client +12. The client exits What does it mean to wait until a hook is ready? This depends on the resource declared in the hook. If the resources is a `Job` kind, Tiller @@ -114,6 +116,7 @@ metadata: # This is what defines this resource as a hook. Without this line, the # job is considered part of the release. "helm.sh/hook": post-install + "helm.sh/hookWeight": "-5" spec: template: metadata: @@ -154,3 +157,12 @@ When subcharts declare hooks, those are also evaluated. There is no way for a top-level chart to disable the hooks declared by subcharts. And again, there is no guaranteed ordering. +It is also possible to define a weight for a hook which will help build a deterministic executing order. Weights are defined using the following annotation: + +``` + annotations: + "helm.sh/hookWeight": "5" +``` + +Hook weights can be positive or negative numbers but must be represented as strings. When Tiller starts the execution cycle of hooks of a particular Kind it will sort those hooks in ascending order. + From b9ef8dbe563e6626c047d1cf91b1367ac7f087c2 Mon Sep 17 00:00:00 2001 From: Jonathan Chauncey Date: Mon, 3 Apr 2017 11:25:39 -0400 Subject: [PATCH 3/4] fix(hooks): Change annotation from hookWeight to hook-weight --- pkg/hooks/hooks.go | 2 +- pkg/tiller/hooks.go | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/hooks/hooks.go b/pkg/hooks/hooks.go index d6966a87a..e1fccb416 100644 --- a/pkg/hooks/hooks.go +++ b/pkg/hooks/hooks.go @@ -24,7 +24,7 @@ import ( const HookAnno = "helm.sh/hook" // HookWeightAnno is the label name for a hook weight -const HookWeightAnno = "helm.sh/hookWeight" +const HookWeightAnno = "helm.sh/hook-weight" // Types of hooks const ( diff --git a/pkg/tiller/hooks.go b/pkg/tiller/hooks.go index b6dfeb9cd..3833ba9ea 100644 --- a/pkg/tiller/hooks.go +++ b/pkg/tiller/hooks.go @@ -111,9 +111,8 @@ func sortManifests(files map[string]string, apis chartutil.VersionSet, sort Sort continue } - hw := 0 hws, _ := sh.Metadata.Annotations[hooks.HookWeightAnno] - hw, err = strconv.Atoi(hws) + hw, err := strconv.Atoi(hws) if err != nil { hw = 0 } From fe57500930e8c797fe12dffa5364afab534ac959 Mon Sep 17 00:00:00 2001 From: Jonathan Chauncey Date: Mon, 3 Apr 2017 16:18:54 -0400 Subject: [PATCH 4/4] docs(chart_hooks.md): Rename annotation from hookWeight to hook-weight --- docs/charts_hooks.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/charts_hooks.md b/docs/charts_hooks.md index 9b64199e2..a5babc481 100644 --- a/docs/charts_hooks.md +++ b/docs/charts_hooks.md @@ -116,7 +116,7 @@ metadata: # This is what defines this resource as a hook. Without this line, the # job is considered part of the release. "helm.sh/hook": post-install - "helm.sh/hookWeight": "-5" + "helm.sh/hook-weight": "-5" spec: template: metadata: @@ -161,7 +161,7 @@ It is also possible to define a weight for a hook which will help build a determ ``` annotations: - "helm.sh/hookWeight": "5" + "helm.sh/hook-weight": "5" ``` Hook weights can be positive or negative numbers but must be represented as strings. When Tiller starts the execution cycle of hooks of a particular Kind it will sort those hooks in ascending order.