-
Notifications
You must be signed in to change notification settings - Fork 564
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: prepare
and cleanup
release event hooks
#349
Merged
mumoshu
merged 1 commit into
roboll:master
from
mumoshu:prepare-cleanup-release-evt-hooks
Sep 21, 2018
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
package event | ||
|
||
import ( | ||
"fmt" | ||
"github.com/roboll/helmfile/environment" | ||
"github.com/roboll/helmfile/helmexec" | ||
"github.com/roboll/helmfile/tmpl" | ||
"go.uber.org/zap" | ||
"os" | ||
) | ||
|
||
type Hook struct { | ||
Name string `yaml:"name"` | ||
Events []string `yaml:"events"` | ||
Command string `yaml:"command"` | ||
Args []string `yaml:"args"` | ||
} | ||
|
||
type event struct { | ||
Name string | ||
} | ||
|
||
type Bus struct { | ||
Runner helmexec.Runner | ||
Hooks []Hook | ||
|
||
BasePath string | ||
StateFilePath string | ||
Namespace string | ||
|
||
Env environment.Environment | ||
|
||
ReadFile func(string) ([]byte, error) | ||
Logger *zap.SugaredLogger | ||
} | ||
|
||
func (bus *Bus) Trigger(evt string, context map[string]interface{}) (bool, error) { | ||
if bus.Runner == nil { | ||
bus.Runner = helmexec.ShellRunner{ | ||
Dir: bus.BasePath, | ||
} | ||
} | ||
|
||
executed := false | ||
|
||
for _, hook := range bus.Hooks { | ||
contained := false | ||
for _, e := range hook.Events { | ||
contained = contained || e == evt | ||
} | ||
if !contained { | ||
continue | ||
} | ||
|
||
var err error | ||
|
||
name := hook.Name | ||
if name == "" { | ||
name = hook.Command | ||
} | ||
|
||
fmt.Fprintf(os.Stderr, "%s: basePath=%s\n", bus.StateFilePath, bus.BasePath) | ||
|
||
data := map[string]interface{}{ | ||
"Environment": bus.Env, | ||
"Namespace": bus.Namespace, | ||
"Event": event{ | ||
Name: evt, | ||
}, | ||
} | ||
for k, v := range context { | ||
data[k] = v | ||
} | ||
render := tmpl.NewTextRenderer(bus.ReadFile, bus.BasePath, data) | ||
|
||
bus.Logger.Debugf("hook[%s]: triggered by event \"%s\"\n", name, evt) | ||
|
||
command, err := render.RenderTemplateText(hook.Command) | ||
if err != nil { | ||
return false, fmt.Errorf("hook[%s]: %v", name, err) | ||
} | ||
|
||
args := make([]string, len(hook.Args)) | ||
for i, raw := range hook.Args { | ||
args[i], err = render.RenderTemplateText(raw) | ||
if err != nil { | ||
return false, fmt.Errorf("hook[%s]: %v", name, err) | ||
} | ||
} | ||
|
||
bytes, err := bus.Runner.Execute(command, args) | ||
bus.Logger.Debugf("hook[%s]: %s\n", name, string(bytes)) | ||
if err != nil { | ||
return false, fmt.Errorf("hook[%s]: command `%s` failed: %v", name, command, err) | ||
} | ||
|
||
executed = true | ||
} | ||
|
||
return executed, nil | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,113 @@ | ||
package event | ||
|
||
import ( | ||
"fmt" | ||
"github.com/roboll/helmfile/environment" | ||
"github.com/roboll/helmfile/helmexec" | ||
"os" | ||
"testing" | ||
) | ||
|
||
var logger = helmexec.NewLogger(os.Stdout, "warn") | ||
|
||
type runner struct { | ||
} | ||
|
||
func (r *runner) Execute(cmd string, args []string) ([]byte, error) { | ||
if cmd == "ng" { | ||
return nil, fmt.Errorf("cmd failed due to invalid cmd: %s", cmd) | ||
} | ||
for _, a := range args { | ||
if a == "ng" { | ||
return nil, fmt.Errorf("cmd failed due to invalid arg: %s", a) | ||
} | ||
} | ||
return []byte(""), nil | ||
} | ||
|
||
func TestTrigger(t *testing.T) { | ||
cases := []struct { | ||
name string | ||
hook *Hook | ||
triggeredEvt string | ||
expectedResult bool | ||
expectedErr string | ||
}{ | ||
{ | ||
"okhook1", | ||
&Hook{"okhook1", []string{"foo"}, "ok", []string{}}, | ||
"foo", | ||
true, | ||
"", | ||
}, | ||
{ | ||
"missinghook1", | ||
&Hook{"okhook1", []string{"foo"}, "ok", []string{}}, | ||
"bar", | ||
false, | ||
"", | ||
}, | ||
{ | ||
"nohook1", | ||
nil, | ||
"bar", | ||
false, | ||
"", | ||
}, | ||
{ | ||
"nghook1", | ||
&Hook{"nghook1", []string{"foo"}, "ng", []string{}}, | ||
"foo", | ||
false, | ||
"hook[nghook1]: command `ng` failed: cmd failed due to invalid cmd: ng", | ||
}, | ||
{ | ||
"nghook2", | ||
&Hook{"nghook2", []string{"foo"}, "ok", []string{"ng"}}, | ||
"foo", | ||
false, | ||
"hook[nghook2]: command `ok` failed: cmd failed due to invalid arg: ng", | ||
}, | ||
} | ||
readFile := func(filename string) ([]byte, error) { | ||
return nil, fmt.Errorf("unexpected call to readFile: %s", filename) | ||
} | ||
for _, c := range cases { | ||
hooks := []Hook{} | ||
if c.hook != nil { | ||
hooks = append(hooks, *c.hook) | ||
} | ||
bus := &Bus{ | ||
Hooks: hooks, | ||
StateFilePath: "path/to/helmfile.yaml", | ||
BasePath: "path/to", | ||
Namespace: "myns", | ||
Env: environment.Environment{Name: "prod"}, | ||
Logger: logger, | ||
ReadFile: readFile, | ||
} | ||
|
||
bus.Runner = &runner{} | ||
data := map[string]interface{}{ | ||
"Release": "myrel", | ||
"HelmfileCommand": "mycmd", | ||
} | ||
ok, err := bus.Trigger(c.triggeredEvt, data) | ||
|
||
if ok != c.expectedResult { | ||
t.Errorf("unexpected result for case \"%s\": expected=%v, actual=%v", c.name, c.expectedResult, ok) | ||
} | ||
|
||
if c.expectedErr != "" { | ||
if err == nil { | ||
t.Error("error expected, but not occurred") | ||
} else if err.Error() != c.expectedErr { | ||
t.Errorf("unexpected error for case \"%s\": expected=%s, actual=%v", c.name, c.expectedErr, err) | ||
} | ||
} else { | ||
if err != nil { | ||
t.Errorf("unexpected error for case \"%s\": %v", c.name, err) | ||
} | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a technical reason you didn't go with this the structure proposed in #330?:
I see that you would want to register the same command for a specific hook, but that seems to be over generic (I believe it is more common to have differing calls on each event, than the same call on multiple events).
YAML anchors could come handy if you have the same command on multiple events.
The
preChartLoad
schema is verifiable at load time.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! Indeed my intention was to make it even more generic with (hopefully) comparable complexity to the third option in my proposal.
I think so too. My intention here was to provide the most DRY way to support this use-case:
See #295 (comment) for more context.
Good point! I was thinking to add validation for unsupported events. That is,
events: ["foo"]
results in an error likeunsupported event to hook: foo
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And I wasn't sure if "events
are the only possible hook target in helmfile. Googling around, the term
hook` seems to be used for NOT only for expressing extension points associated to events. For example, something like "action hook" can be considered:An event hook reacts on specific event and doesn't have ability to return data from the hook to helmfile.
An action hook, on the other hand, could be used for passing data from the hook to helmfile, in an output format helmfile expects for the specific action. In the above example, the
lint
action hook can return a specifically formatted json to add lint results that are integrated into the original output ofhelmfile lint
.Explicitly wording hook targets as
events
prevents ambiguity and creates future possibility like this.