From 5bd8fee7083aa3cf5dedbbe69396e6cfbe837d08 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Tue, 24 Sep 2019 09:44:19 -0700 Subject: [PATCH 1/7] Creates a final "cleanup" provisioner to run if an error occurs during a provisioning step, allowing users to perform any custom cleanup tasks that must happen on the VM before the VM is shut down and destroyed. --- common/step_provision.go | 35 +++++++++++-- packer/build.go | 41 +++++++++++---- packer/core.go | 107 +++++++++++++++++++++++---------------- packer/hook.go | 1 + template/parse.go | 34 +++++++++++++ template/template.go | 1 + 6 files changed, 162 insertions(+), 57 deletions(-) diff --git a/common/step_provision.go b/common/step_provision.go index dfa756b72..2c13b37d5 100644 --- a/common/step_provision.go +++ b/common/step_provision.go @@ -2,6 +2,7 @@ package common import ( "context" + "fmt" "log" "time" @@ -22,7 +23,8 @@ type StepProvision struct { Comm packer.Communicator } -func (s *StepProvision) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { +func (s *StepProvision) runWithHook(ctx context.Context, state multistep.StateBag, hooktype string) multistep.StepAction { + // hooktype will be either packer.HookProvision or packer.HookCleanupProvision comm := s.Comm if comm == nil { raw, ok := state.Get("communicator").(packer.Communicator) @@ -35,17 +37,29 @@ func (s *StepProvision) Run(ctx context.Context, state multistep.StateBag) multi // Run the provisioner in a goroutine so we can continually check // for cancellations... - log.Println("Running the provision hook") + if hooktype == packer.HookProvision { + log.Println("Running the provision hook") + } else if hooktype == packer.HookCleanupProvision { + ui.Say("Provisioning step had errors: Running the cleanup provisioner, if present...") + } errCh := make(chan error, 1) go func() { - errCh <- hook.Run(ctx, packer.HookProvision, ui, comm, nil) + errCh <- hook.Run(ctx, hooktype, ui, comm, nil) }() for { select { case err := <-errCh: if err != nil { - state.Put("error", err) + if hooktype == packer.HookProvision { + // We don't overwrite the error if it's a cleanup + // provisioner being run. + state.Put("error", err) + } else if hooktype == packer.HookCleanupProvision { + origErr := state.Get("error").(error) + state.Put("error", fmt.Errorf("Cleanup failed: %s. "+ + "Original Provisioning error: %s", err, origErr)) + } return multistep.ActionHalt } @@ -62,4 +76,15 @@ func (s *StepProvision) Run(ctx context.Context, state multistep.StateBag) multi } } -func (*StepProvision) Cleanup(multistep.StateBag) {} +func (s *StepProvision) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { + return s.runWithHook(ctx, state, packer.HookProvision) +} + +func (s *StepProvision) Cleanup(state multistep.StateBag) { + // We have a "final" provisioner that gets defined by "on-error-script" + // which we only call if there's an error during the provision run and + // the "on-error-script" is defined. + if _, ok := state.GetOk("error"); ok { + s.runWithHook(context.Background(), state, packer.HookCleanupProvision) + } +} diff --git a/packer/build.go b/packer/build.go index 649ef42a5..badd67c1f 100644 --- a/packer/build.go +++ b/packer/build.go @@ -84,15 +84,16 @@ type Build interface { // multiple files, of course, but it should be for only a single provider // (such as VirtualBox, EC2, etc.). type coreBuild struct { - name string - builder Builder - builderConfig interface{} - builderType string - hooks map[string][]Hook - postProcessors [][]coreBuildPostProcessor - provisioners []coreBuildProvisioner - templatePath string - variables map[string]string + name string + builder Builder + builderConfig interface{} + builderType string + hooks map[string][]Hook + postProcessors [][]coreBuildPostProcessor + provisioners []coreBuildProvisioner + cleanupProvisioner coreBuildProvisioner + templatePath string + variables map[string]string debug bool force bool @@ -164,6 +165,17 @@ func (b *coreBuild) Prepare() (warn []string, err error) { } } + // Prepare the on-error-cleanup provisioner + if b.cleanupProvisioner.pType != "" { + configs := make([]interface{}, len(b.cleanupProvisioner.config), len(b.cleanupProvisioner.config)+1) + copy(configs, b.cleanupProvisioner.config) + configs = append(configs, packerConfig) + err = b.cleanupProvisioner.provisioner.Prepare(configs...) + if err != nil { + return + } + } + // Prepare the post-processors for _, ppSeq := range b.postProcessors { for _, corePP := range ppSeq { @@ -222,6 +234,17 @@ func (b *coreBuild) Run(ctx context.Context, originalUi Ui) ([]Artifact, error) }) } + if b.cleanupProvisioner.pType != "" { + hookedCleanupProvisioner := &HookedProvisioner{ + b.cleanupProvisioner.provisioner, + b.cleanupProvisioner.config, + b.cleanupProvisioner.pType, + } + hooks[HookCleanupProvision] = []Hook{&ProvisionHook{ + Provisioners: []*HookedProvisioner{hookedCleanupProvisioner}, + }} + } + hook := &DispatchHook{Mapping: hooks} artifacts := make([]Artifact, 0, 1) diff --git a/packer/core.go b/packer/core.go index 3e9895dcb..bcdd91b30 100644 --- a/packer/core.go +++ b/packer/core.go @@ -112,6 +112,49 @@ func (c *Core) BuildNames() []string { return r } +func (c *Core) generateCoreBuildProvisioner(rawP *template.Provisioner, rawName string) (coreBuildProvisioner, error) { + // Get the provisioner + cbp := coreBuildProvisioner{} + provisioner, err := c.components.Provisioner(rawP.Type) + if err != nil { + return cbp, fmt.Errorf( + "error initializing provisioner '%s': %s", + rawP.Type, err) + } + if provisioner == nil { + return cbp, fmt.Errorf( + "provisioner type not found: %s", rawP.Type) + } + + // Get the configuration + config := make([]interface{}, 1, 2) + config[0] = rawP.Config + if rawP.Override != nil { + if override, ok := rawP.Override[rawName]; ok { + config = append(config, override) + } + } + // If we're pausing, we wrap the provisioner in a special pauser. + if rawP.PauseBefore > 0 { + provisioner = &PausedProvisioner{ + PauseBefore: rawP.PauseBefore, + Provisioner: provisioner, + } + } else if rawP.Timeout > 0 { + provisioner = &TimeoutProvisioner{ + Timeout: rawP.Timeout, + Provisioner: provisioner, + } + } + cbp = coreBuildProvisioner{ + pType: rawP.Type, + provisioner: provisioner, + config: config, + } + + return cbp, nil +} + // Build returns the Build object for the given name. func (c *Core) Build(n string) (Build, error) { // Setup the builder @@ -140,46 +183,23 @@ func (c *Core) Build(n string) (Build, error) { if rawP.OnlyExcept.Skip(rawName) { continue } - - // Get the provisioner - provisioner, err := c.components.Provisioner(rawP.Type) + cbp, err := c.generateCoreBuildProvisioner(rawP, rawName) if err != nil { - return nil, fmt.Errorf( - "error initializing provisioner '%s': %s", - rawP.Type, err) - } - if provisioner == nil { - return nil, fmt.Errorf( - "provisioner type not found: %s", rawP.Type) + return nil, err } - // Get the configuration - config := make([]interface{}, 1, 2) - config[0] = rawP.Config - if rawP.Override != nil { - if override, ok := rawP.Override[rawName]; ok { - config = append(config, override) - } - } + provisioners = append(provisioners, cbp) + } - // If we're pausing, we wrap the provisioner in a special pauser. - if rawP.PauseBefore > 0 { - provisioner = &PausedProvisioner{ - PauseBefore: rawP.PauseBefore, - Provisioner: provisioner, - } - } else if rawP.Timeout > 0 { - provisioner = &TimeoutProvisioner{ - Timeout: rawP.Timeout, - Provisioner: provisioner, - } + var cleanupProvisioner coreBuildProvisioner + if c.Template.CleanupProvisioner != nil { + // This is a special instantiation of the shell-local provisioner that + // is only run on error at end of provisioning step before other step + // cleanup occurs. + cleanupProvisioner, err = c.generateCoreBuildProvisioner(c.Template.CleanupProvisioner, rawName) + if err != nil { + return nil, err } - - provisioners = append(provisioners, coreBuildProvisioner{ - pType: rawP.Type, - provisioner: provisioner, - config: config, - }) } // Setup the post-processors @@ -232,14 +252,15 @@ func (c *Core) Build(n string) (Build, error) { // TODO hooks one day return &coreBuild{ - name: n, - builder: builder, - builderConfig: configBuilder.Config, - builderType: configBuilder.Type, - postProcessors: postProcessors, - provisioners: provisioners, - templatePath: c.Template.Path, - variables: c.variables, + name: n, + builder: builder, + builderConfig: configBuilder.Config, + builderType: configBuilder.Type, + postProcessors: postProcessors, + provisioners: provisioners, + cleanupProvisioner: cleanupProvisioner, + templatePath: c.Template.Path, + variables: c.variables, }, nil } diff --git a/packer/hook.go b/packer/hook.go index 67074aa7d..a0b146023 100644 --- a/packer/hook.go +++ b/packer/hook.go @@ -6,6 +6,7 @@ import ( // This is the hook that should be fired for provisioners to run. const HookProvision = "packer_provision" +const HookCleanupProvision = "packer_cleanup_provision" // A Hook is used to hook into an arbitrarily named location in a build, // allowing custom behavior to run at certain points along a build. diff --git a/template/parse.go b/template/parse.go index cd4467b8f..dbc812198 100644 --- a/template/parse.go +++ b/template/parse.go @@ -6,6 +6,7 @@ import ( "encoding/json" "fmt" "io" + "log" "os" "path/filepath" "sort" @@ -28,6 +29,7 @@ type rawTemplate struct { Push map[string]interface{} `json:"push,omitempty"` PostProcessors []interface{} `mapstructure:"post-processors" json:"post-processors,omitempty"` Provisioners []interface{} `json:"provisioners,omitempty"` + CleanupProvisioner interface{} `mapstructure:"on-error-script" json:"on-error-script,omitempty"` Variables map[string]interface{} `json:"variables,omitempty"` SensitiveVariables []string `mapstructure:"sensitive-variables" json:"sensitive-variables,omitempty"` @@ -242,6 +244,38 @@ func (r *rawTemplate) Template() (*Template, error) { result.Provisioners = append(result.Provisioners, &p) } + // Gather the on-error-script + log.Printf("r.CleanupProvisioner is %#v", r.CleanupProvisioner) + if r.CleanupProvisioner != nil { + var p Provisioner + if err := r.decoder(&p, nil).Decode(r.CleanupProvisioner); err != nil { + errs = multierror.Append(errs, fmt.Errorf( + "On Error Cleanup provisioner: %s", err)) + } + + // Type is required before any richer validation + log.Printf("p is %#v", p) + if p.Type == "" { + errs = multierror.Append(errs, fmt.Errorf( + "on error cleanup provisioner missing 'type'")) + } + + // Set the raw configuration and delete any special keys + p.Config = r.CleanupProvisioner.(map[string]interface{}) + + delete(p.Config, "except") + delete(p.Config, "only") + delete(p.Config, "override") + delete(p.Config, "pause_before") + delete(p.Config, "type") + delete(p.Config, "timeout") + + if len(p.Config) == 0 { + p.Config = nil + } + result.CleanupProvisioner = &p + } + // If we have errors, return those with a nil result if errs != nil { return nil, errs diff --git a/template/template.go b/template/template.go index 6013313de..aba88c7d0 100644 --- a/template/template.go +++ b/template/template.go @@ -24,6 +24,7 @@ type Template struct { SensitiveVariables []*Variable Builders map[string]*Builder Provisioners []*Provisioner + CleanupProvisioner *Provisioner PostProcessors [][]*PostProcessor // RawContents is just the raw data for this template From 0683bc409b0bc92b9571586113bcd59f038c947b Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 25 Sep 2019 13:38:12 -0700 Subject: [PATCH 2/7] add test for error-cleanup provisioner, and fix tests by fixing null builder to use an actual none communicator instead of skipping communicator generation altogether --- builder/null/builder.go | 16 +++++----- command/build_cleanup_script_test.go | 29 +++++++++++++++++++ command/build_test.go | 17 ++++++++++- .../cleanup-script/template.json | 18 ++++++++++++ go.mod | 1 + go.sum | 3 ++ packer/hook.go | 1 - packer/provisioner.go | 1 - template/parse.go | 1 - 9 files changed, 74 insertions(+), 13 deletions(-) create mode 100644 command/build_cleanup_script_test.go create mode 100644 command/test-fixtures/cleanup-script/template.json diff --git a/builder/null/builder.go b/builder/null/builder.go index ea7ac2343..5939cd024 100644 --- a/builder/null/builder.go +++ b/builder/null/builder.go @@ -29,15 +29,13 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (packer.Artifact, error) { steps := []multistep.Step{} - if b.config.CommConfig.Type != "none" { - steps = append(steps, - &communicator.StepConnect{ - Config: &b.config.CommConfig, - Host: CommHost(b.config.CommConfig.Host()), - SSHConfig: b.config.CommConfig.SSHConfigFunc(), - }, - ) - } + steps = append(steps, + &communicator.StepConnect{ + Config: &b.config.CommConfig, + Host: CommHost(b.config.CommConfig.Host()), + SSHConfig: b.config.CommConfig.SSHConfigFunc(), + }, + ) steps = append(steps, new(common.StepProvision), diff --git a/command/build_cleanup_script_test.go b/command/build_cleanup_script_test.go new file mode 100644 index 000000000..de309d437 --- /dev/null +++ b/command/build_cleanup_script_test.go @@ -0,0 +1,29 @@ +package command + +import ( + "path/filepath" + "testing" +) + +func TestBuildWithCleanupScript(t *testing.T) { + c := &BuildCommand{ + Meta: testMetaFile(t), + } + + args := []string{ + "-parallel=false", + filepath.Join(testFixture("cleanup-script"), "template.json"), + } + + defer cleanup() + + // build should exit with error code! + if code := c.Run(args); code == 0 { + fatalCommand(t, c.Meta) + } + + if !fileExists("ducky.txt") { + t.Errorf("Expected to find ducky.txt") + } + +} diff --git a/command/build_test.go b/command/build_test.go index ac29bda99..d96ce6998 100644 --- a/command/build_test.go +++ b/command/build_test.go @@ -10,8 +10,11 @@ import ( "github.com/google/go-cmp/cmp" "github.com/hashicorp/packer/builder/file" + "github.com/hashicorp/packer/builder/null" "github.com/hashicorp/packer/packer" shell_local "github.com/hashicorp/packer/post-processor/shell-local" + "github.com/hashicorp/packer/provisioner/shell" + sl "github.com/hashicorp/packer/provisioner/shell-local" ) func TestBuildOnlyFileCommaFlags(t *testing.T) { @@ -176,7 +179,18 @@ func fileExists(filename string) bool { func testCoreConfigBuilder(t *testing.T) *packer.CoreConfig { components := packer.ComponentFinder{ Builder: func(n string) (packer.Builder, error) { - return &file.Builder{}, nil + if n == "file" { + return &file.Builder{}, nil + } + return &null.Builder{}, nil + }, + Provisioner: func(n string) (packer.Provisioner, error) { + if n == "shell" { + return &shell.Provisioner{}, nil + } else if n == "shell-local" { + return &sl.Provisioner{}, nil + } + return nil, fmt.Errorf("requested provisioner not implemented in this test") }, PostProcessor: func(n string) (packer.PostProcessor, error) { return &shell_local.PostProcessor{}, nil @@ -212,6 +226,7 @@ func cleanup() { os.RemoveAll("fuchsias.txt") os.RemoveAll("lilas.txt") os.RemoveAll("campanules.txt") + os.RemoveAll("ducky.txt") } func TestBuildCommand_ParseArgs(t *testing.T) { diff --git a/command/test-fixtures/cleanup-script/template.json b/command/test-fixtures/cleanup-script/template.json new file mode 100644 index 000000000..afc7c5aeb --- /dev/null +++ b/command/test-fixtures/cleanup-script/template.json @@ -0,0 +1,18 @@ +{ + "builders": [ + { + "type": "null", + "communicator": "none" + } + ], + "provisioners": [ + { + "type": "shell-local", + "inline": ["exit 2"] + } + ], + "on-error-script": { + "type": "shell-local", + "inline": ["echo 'rubber ducky'> ducky.txt"] + } +} \ No newline at end of file diff --git a/go.mod b/go.mod index 85273036c..7f8f8157e 100644 --- a/go.mod +++ b/go.mod @@ -97,6 +97,7 @@ require ( github.com/mitchellh/go-fs v0.0.0-20180402234041-7b48fa161ea7 github.com/mitchellh/go-homedir v1.0.0 github.com/mitchellh/go-vnc v0.0.0-20150629162542-723ed9867aed + github.com/mitchellh/gox v1.0.1 // indirect github.com/mitchellh/iochan v1.0.0 github.com/mitchellh/mapstructure v0.0.0-20180111000720-b4575eea38cc github.com/mitchellh/panicwrap v0.0.0-20170106182340-fce601fe5557 diff --git a/go.sum b/go.sum index 11a593796..66c8020da 100644 --- a/go.sum +++ b/go.sum @@ -206,6 +206,7 @@ github.com/hashicorp/go-syslog v1.0.0/go.mod h1:qPfqrKkXGihmCqbJM2mZgkZGvKG1dFdv github.com/hashicorp/go-uuid v1.0.0/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= github.com/hashicorp/go-uuid v1.0.1 h1:fv1ep09latC32wFoVwnqcnKJGnMSdBanPczbHAYm1BE= github.com/hashicorp/go-uuid v1.0.1/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= +github.com/hashicorp/go-version v1.0.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= github.com/hashicorp/go-version v1.1.0 h1:bPIoEKD27tNdebFGGxxYwcL4nepeY4j1QP23PFRGzg0= github.com/hashicorp/go-version v1.1.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= github.com/hashicorp/go-version v1.2.0 h1:3vNe/fWF5CBgRIguda1meWhsZHy3m8gCJ5wx+dIzX/E= @@ -301,6 +302,8 @@ github.com/mitchellh/go-testing-interface v1.0.0/go.mod h1:kRemZodwjscx+RGhAo8eI github.com/mitchellh/go-vnc v0.0.0-20150629162542-723ed9867aed h1:FI2NIv6fpef6BQl2u3IZX/Cj20tfypRF4yd+uaHOMtI= github.com/mitchellh/go-vnc v0.0.0-20150629162542-723ed9867aed/go.mod h1:3rdaFaCv4AyBgu5ALFM0+tSuHrBh6v692nyQe3ikrq0= github.com/mitchellh/gox v0.4.0/go.mod h1:Sd9lOJ0+aimLBi73mGofS1ycjY8lL3uZM3JPS42BGNg= +github.com/mitchellh/gox v1.0.1 h1:x0jD3dcHk9a9xPSDN6YEL4xL6Qz0dvNYm8yZqui5chI= +github.com/mitchellh/gox v1.0.1/go.mod h1:ED6BioOGXMswlXa2zxfh/xdd5QhwYliBFn9V18Ap4z4= github.com/mitchellh/iochan v1.0.0 h1:C+X3KsSTLFVBr/tK1eYN/vs4rJcvsiLU338UhYPJWeY= github.com/mitchellh/iochan v1.0.0/go.mod h1:JwYml1nuB7xOzsp52dPpHFffvOCDupsG0QubkSMEySY= github.com/mitchellh/mapstructure v0.0.0-20160808181253-ca63d7c062ee/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y= diff --git a/packer/hook.go b/packer/hook.go index a0b146023..25df5c855 100644 --- a/packer/hook.go +++ b/packer/hook.go @@ -45,7 +45,6 @@ func (h *DispatchHook) Run(ctx context.Context, name string, ui Ui, comm Communi if err := ctx.Err(); err != nil { return err } - if err := hook.Run(ctx, name, ui, comm, data); err != nil { return err } diff --git a/packer/provisioner.go b/packer/provisioner.go index ec350fd50..ff89c0c42 100644 --- a/packer/provisioner.go +++ b/packer/provisioner.go @@ -50,7 +50,6 @@ func (h *ProvisionHook) Run(ctx context.Context, name string, ui Ui, comm Commun "`communicator` config was set to \"none\". If you have any provisioners\n" + "then a communicator is required. Please fix this to continue.") } - for _, p := range h.Provisioners { ts := CheckpointReporter.AddSpan(p.TypeName, "provisioner", p.Config) diff --git a/template/parse.go b/template/parse.go index dbc812198..5f8117aa0 100644 --- a/template/parse.go +++ b/template/parse.go @@ -245,7 +245,6 @@ func (r *rawTemplate) Template() (*Template, error) { } // Gather the on-error-script - log.Printf("r.CleanupProvisioner is %#v", r.CleanupProvisioner) if r.CleanupProvisioner != nil { var p Provisioner if err := r.decoder(&p, nil).Decode(r.CleanupProvisioner); err != nil { From 59efa0faee6b09b4a57334eb4babe4c6c3acb644 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 25 Sep 2019 13:43:29 -0700 Subject: [PATCH 3/7] rename option to error-cleanup-privisoner, which I think is clearer --- command/test-fixtures/cleanup-script/template.json | 2 +- common/step_provision.go | 4 ++-- template/parse.go | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/command/test-fixtures/cleanup-script/template.json b/command/test-fixtures/cleanup-script/template.json index afc7c5aeb..eea3e7ef0 100644 --- a/command/test-fixtures/cleanup-script/template.json +++ b/command/test-fixtures/cleanup-script/template.json @@ -11,7 +11,7 @@ "inline": ["exit 2"] } ], - "on-error-script": { + "error-cleanup-provisioner": { "type": "shell-local", "inline": ["echo 'rubber ducky'> ducky.txt"] } diff --git a/common/step_provision.go b/common/step_provision.go index 2c13b37d5..07ba39f81 100644 --- a/common/step_provision.go +++ b/common/step_provision.go @@ -81,9 +81,9 @@ func (s *StepProvision) Run(ctx context.Context, state multistep.StateBag) multi } func (s *StepProvision) Cleanup(state multistep.StateBag) { - // We have a "final" provisioner that gets defined by "on-error-script" + // We have a "final" provisioner that gets defined by "error-cleanup-provisioner" // which we only call if there's an error during the provision run and - // the "on-error-script" is defined. + // the "error-cleanup-provisioner" is defined. if _, ok := state.GetOk("error"); ok { s.runWithHook(context.Background(), state, packer.HookCleanupProvision) } diff --git a/template/parse.go b/template/parse.go index 5f8117aa0..02748239c 100644 --- a/template/parse.go +++ b/template/parse.go @@ -29,7 +29,7 @@ type rawTemplate struct { Push map[string]interface{} `json:"push,omitempty"` PostProcessors []interface{} `mapstructure:"post-processors" json:"post-processors,omitempty"` Provisioners []interface{} `json:"provisioners,omitempty"` - CleanupProvisioner interface{} `mapstructure:"on-error-script" json:"on-error-script,omitempty"` + CleanupProvisioner interface{} `mapstructure:"error-cleanup-provisioner" json:"error-cleanup-provisioner,omitempty"` Variables map[string]interface{} `json:"variables,omitempty"` SensitiveVariables []string `mapstructure:"sensitive-variables" json:"sensitive-variables,omitempty"` @@ -244,7 +244,7 @@ func (r *rawTemplate) Template() (*Template, error) { result.Provisioners = append(result.Provisioners, &p) } - // Gather the on-error-script + // Gather the error-cleanup-provisioner if r.CleanupProvisioner != nil { var p Provisioner if err := r.decoder(&p, nil).Decode(r.CleanupProvisioner); err != nil { From c57e0a98372b6556609c43f3c50884aedcea056c Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 25 Sep 2019 13:55:16 -0700 Subject: [PATCH 4/7] remove logline --- template/parse.go | 1 - 1 file changed, 1 deletion(-) diff --git a/template/parse.go b/template/parse.go index 02748239c..eb608d54a 100644 --- a/template/parse.go +++ b/template/parse.go @@ -253,7 +253,6 @@ func (r *rawTemplate) Template() (*Template, error) { } // Type is required before any richer validation - log.Printf("p is %#v", p) if p.Type == "" { errs = multierror.Append(errs, fmt.Errorf( "on error cleanup provisioner missing 'type'")) From a2824a942d43f42c243f0c7b82500a92eea38332 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 25 Sep 2019 14:43:32 -0700 Subject: [PATCH 5/7] remove extraneous import --- template/parse.go | 1 - 1 file changed, 1 deletion(-) diff --git a/template/parse.go b/template/parse.go index eb608d54a..4ba824bd3 100644 --- a/template/parse.go +++ b/template/parse.go @@ -6,7 +6,6 @@ import ( "encoding/json" "fmt" "io" - "log" "os" "path/filepath" "sort" From 1293812b11869944567a6d35e0ae5a2b33bdfd2c Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Fri, 27 Sep 2019 13:25:10 -0700 Subject: [PATCH 6/7] clean up modules --- go.mod | 1 - go.sum | 7 ------- 2 files changed, 8 deletions(-) diff --git a/go.mod b/go.mod index 7f8f8157e..85273036c 100644 --- a/go.mod +++ b/go.mod @@ -97,7 +97,6 @@ require ( github.com/mitchellh/go-fs v0.0.0-20180402234041-7b48fa161ea7 github.com/mitchellh/go-homedir v1.0.0 github.com/mitchellh/go-vnc v0.0.0-20150629162542-723ed9867aed - github.com/mitchellh/gox v1.0.1 // indirect github.com/mitchellh/iochan v1.0.0 github.com/mitchellh/mapstructure v0.0.0-20180111000720-b4575eea38cc github.com/mitchellh/panicwrap v0.0.0-20170106182340-fce601fe5557 diff --git a/go.sum b/go.sum index 66c8020da..53868beac 100644 --- a/go.sum +++ b/go.sum @@ -206,7 +206,6 @@ github.com/hashicorp/go-syslog v1.0.0/go.mod h1:qPfqrKkXGihmCqbJM2mZgkZGvKG1dFdv github.com/hashicorp/go-uuid v1.0.0/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= github.com/hashicorp/go-uuid v1.0.1 h1:fv1ep09latC32wFoVwnqcnKJGnMSdBanPczbHAYm1BE= github.com/hashicorp/go-uuid v1.0.1/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= -github.com/hashicorp/go-version v1.0.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= github.com/hashicorp/go-version v1.1.0 h1:bPIoEKD27tNdebFGGxxYwcL4nepeY4j1QP23PFRGzg0= github.com/hashicorp/go-version v1.1.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= github.com/hashicorp/go-version v1.2.0 h1:3vNe/fWF5CBgRIguda1meWhsZHy3m8gCJ5wx+dIzX/E= @@ -302,8 +301,6 @@ github.com/mitchellh/go-testing-interface v1.0.0/go.mod h1:kRemZodwjscx+RGhAo8eI github.com/mitchellh/go-vnc v0.0.0-20150629162542-723ed9867aed h1:FI2NIv6fpef6BQl2u3IZX/Cj20tfypRF4yd+uaHOMtI= github.com/mitchellh/go-vnc v0.0.0-20150629162542-723ed9867aed/go.mod h1:3rdaFaCv4AyBgu5ALFM0+tSuHrBh6v692nyQe3ikrq0= github.com/mitchellh/gox v0.4.0/go.mod h1:Sd9lOJ0+aimLBi73mGofS1ycjY8lL3uZM3JPS42BGNg= -github.com/mitchellh/gox v1.0.1 h1:x0jD3dcHk9a9xPSDN6YEL4xL6Qz0dvNYm8yZqui5chI= -github.com/mitchellh/gox v1.0.1/go.mod h1:ED6BioOGXMswlXa2zxfh/xdd5QhwYliBFn9V18Ap4z4= github.com/mitchellh/iochan v1.0.0 h1:C+X3KsSTLFVBr/tK1eYN/vs4rJcvsiLU338UhYPJWeY= github.com/mitchellh/iochan v1.0.0/go.mod h1:JwYml1nuB7xOzsp52dPpHFffvOCDupsG0QubkSMEySY= github.com/mitchellh/mapstructure v0.0.0-20160808181253-ca63d7c062ee/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y= @@ -406,12 +403,8 @@ github.com/vmware/govmomi v0.0.0-20170707011325-c2105a174311 h1:s5pyxd5S6wRs2WpE github.com/vmware/govmomi v0.0.0-20170707011325-c2105a174311/go.mod h1:URlwyTFZX72RmxtxuaFL2Uj3fD1JTvZdx59bHWk6aFU= github.com/xanzy/go-cloudstack v0.0.0-20190526095453-42f262b63ed0 h1:NJrcIkdzq0C3I8ypAZwFE9RHtGbfp+mJvqIcoFATZuk= github.com/xanzy/go-cloudstack v0.0.0-20190526095453-42f262b63ed0/go.mod h1:sBh287mCRwCz6zyXHMmw7sSZGPohVpnx+o+OY4M+i3A= -github.com/yandex-cloud/go-genproto v0.0.0-20190802103534-6089d9ff8d82 h1:HLQoCLW2021qJKLrGQbcdEikBJ3gfYF94JYqZuw6Qxg= -github.com/yandex-cloud/go-genproto v0.0.0-20190802103534-6089d9ff8d82/go.mod h1:HEUYX/p8966tMUHHT+TsS0hF/Ca/NYwqprC5WXSDMfE= github.com/yandex-cloud/go-genproto v0.0.0-20190916101622-7617782d381e h1:hzwq5GUKP0aQzDja1XP4sBYyOmnezs/RVtzP+xiLbfI= github.com/yandex-cloud/go-genproto v0.0.0-20190916101622-7617782d381e/go.mod h1:HEUYX/p8966tMUHHT+TsS0hF/Ca/NYwqprC5WXSDMfE= -github.com/yandex-cloud/go-sdk v0.0.0-20190802103531-4ab1dac90bf7 h1:rWXARBMLHylvASK6spamDC8zSL5v2voZop3M6SBul9Y= -github.com/yandex-cloud/go-sdk v0.0.0-20190802103531-4ab1dac90bf7/go.mod h1:Eml0jFLU4VVHgIN8zPHMuNwZXVzUMILyO6lQZSfz854= github.com/yandex-cloud/go-sdk v0.0.0-20190916101744-c781afa45829 h1:2FGwbx03GpP1Ulzg/L46tSoKh9t4yg8BhMKQl/Ff1x8= github.com/yandex-cloud/go-sdk v0.0.0-20190916101744-c781afa45829/go.mod h1:Eml0jFLU4VVHgIN8zPHMuNwZXVzUMILyO6lQZSfz854= go.opencensus.io v0.21.0 h1:mU6zScU4U1YAFPHEHYk+3JC4SY7JxgkqS10ZOSyksNg= From 4c29f88a0a69309915049cca577e9a920490f5c2 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Fri, 27 Sep 2019 13:39:12 -0700 Subject: [PATCH 7/7] reuse code --- template/parse.go | 80 ++++++++++++++++++++--------------------------- 1 file changed, 34 insertions(+), 46 deletions(-) diff --git a/template/parse.go b/template/parse.go index 4ba824bd3..903c98138 100644 --- a/template/parse.go +++ b/template/parse.go @@ -57,6 +57,34 @@ func (r *rawTemplate) MarshalJSON() ([]byte, error) { return json.Marshal(m) } +func (r *rawTemplate) decodeProvisioner(raw interface{}) (Provisioner, error) { + var p Provisioner + if err := r.decoder(&p, nil).Decode(raw); err != nil { + return p, fmt.Errorf("Error decoding provisioner: %s", err) + + } + + // Type is required before any richer validation + if p.Type == "" { + return p, fmt.Errorf("Provisioner missing 'type'") + } + + // Set the raw configuration and delete any special keys + p.Config = raw.(map[string]interface{}) + + delete(p.Config, "except") + delete(p.Config, "only") + delete(p.Config, "override") + delete(p.Config, "pause_before") + delete(p.Config, "type") + delete(p.Config, "timeout") + + if len(p.Config) == 0 { + p.Config = nil + } + return p, nil +} + // Template returns the actual Template object built from this raw // structure. func (r *rawTemplate) Template() (*Template, error) { @@ -212,64 +240,24 @@ func (r *rawTemplate) Template() (*Template, error) { result.Provisioners = make([]*Provisioner, 0, len(r.Provisioners)) } for i, v := range r.Provisioners { - var p Provisioner - if err := r.decoder(&p, nil).Decode(v); err != nil { + p, err := r.decodeProvisioner(v) + if err != nil { errs = multierror.Append(errs, fmt.Errorf( "provisioner %d: %s", i+1, err)) continue } - // Type is required before any richer validation - if p.Type == "" { - errs = multierror.Append(errs, fmt.Errorf( - "provisioner %d: missing 'type'", i+1)) - continue - } - - // Set the raw configuration and delete any special keys - p.Config = v.(map[string]interface{}) - - delete(p.Config, "except") - delete(p.Config, "only") - delete(p.Config, "override") - delete(p.Config, "pause_before") - delete(p.Config, "type") - delete(p.Config, "timeout") - - if len(p.Config) == 0 { - p.Config = nil - } - result.Provisioners = append(result.Provisioners, &p) } // Gather the error-cleanup-provisioner if r.CleanupProvisioner != nil { - var p Provisioner - if err := r.decoder(&p, nil).Decode(r.CleanupProvisioner); err != nil { - errs = multierror.Append(errs, fmt.Errorf( - "On Error Cleanup provisioner: %s", err)) - } - - // Type is required before any richer validation - if p.Type == "" { - errs = multierror.Append(errs, fmt.Errorf( - "on error cleanup provisioner missing 'type'")) + p, err := r.decodeProvisioner(r.CleanupProvisioner) + if err != nil { + errs = multierror.Append(errs, + fmt.Errorf("On Error Cleanup Provisioner error: %s", err)) } - // Set the raw configuration and delete any special keys - p.Config = r.CleanupProvisioner.(map[string]interface{}) - - delete(p.Config, "except") - delete(p.Config, "only") - delete(p.Config, "override") - delete(p.Config, "pause_before") - delete(p.Config, "type") - delete(p.Config, "timeout") - - if len(p.Config) == 0 { - p.Config = nil - } result.CleanupProvisioner = &p }