From 3f444997b29a6186df81af32ed76e84d22076fb0 Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Fri, 7 Jul 2017 10:54:58 +0200 Subject: [PATCH] Make expunge optional and improve logging output (#5099) --- builder/cloudstack/config.go | 3 +- .../cloudstack/step_configure_networking.go | 2 - builder/cloudstack/step_create_instance.go | 58 +++++++++++++------ builder/cloudstack/step_prepare_config.go | 1 - builder/cloudstack/step_shutdown_instance.go | 1 - .../source/docs/builders/cloudstack.html.md | 3 + 6 files changed, 45 insertions(+), 23 deletions(-) diff --git a/builder/cloudstack/config.go b/builder/cloudstack/config.go index 4f87c06eb..a7193adf2 100644 --- a/builder/cloudstack/config.go +++ b/builder/cloudstack/config.go @@ -27,9 +27,10 @@ type Config struct { HTTPGetOnly bool `mapstructure:"http_get_only"` SSLNoVerify bool `mapstructure:"ssl_no_verify"` + CIDRList []string `mapstructure:"cidr_list"` DiskOffering string `mapstructure:"disk_offering"` DiskSize int64 `mapstructure:"disk_size"` - CIDRList []string `mapstructure:"cidr_list"` + Expunge bool `mapstructure:"expunge"` Hypervisor string `mapstructure:"hypervisor"` InstanceName string `mapstructure:"instance_name"` Keypair string `mapstructure:"keypair"` diff --git a/builder/cloudstack/step_configure_networking.go b/builder/cloudstack/step_configure_networking.go index df1054270..b7161f281 100644 --- a/builder/cloudstack/step_configure_networking.go +++ b/builder/cloudstack/step_configure_networking.go @@ -163,7 +163,6 @@ func (s *stepSetupNetworking) Run(state multistep.StateBag) multistep.StepAction } ui.Message("Networking has been setup!") - return multistep.ActionContinue } @@ -235,6 +234,5 @@ func (s *stepSetupNetworking) Cleanup(state multistep.StateBag) { } ui.Message("Networking has been cleaned!") - return } diff --git a/builder/cloudstack/step_create_instance.go b/builder/cloudstack/step_create_instance.go index 62c0ca163..1693b654f 100644 --- a/builder/cloudstack/step_create_instance.go +++ b/builder/cloudstack/step_create_instance.go @@ -90,13 +90,7 @@ func (s *stepCreateInstance) Run(state multistep.StateBag) multistep.StepAction httpPort, } - renderedUserData, err := interpolate.Render(config.UserData, &s.Ctx) - if err != nil { - state.Put("error", fmt.Errorf("Error rendering user_data: %s", err)) - return multistep.ActionHalt - } - - ud, err := getUserData(renderedUserData, config.HTTPGetOnly) + ud, err := s.generateUserData(config.UserData, config.HTTPGetOnly) if err != nil { state.Put("error", err) return multistep.ActionHalt @@ -140,6 +134,7 @@ func (s *stepCreateInstance) Run(state multistep.StateBag) multistep.StepAction // Cleanup any resources that may have been created during the Run phase. func (s *stepCreateInstance) Cleanup(state multistep.StateBag) { client := state.Get("client").(*cloudstack.CloudStackClient) + config := state.Get("config").(*Config) ui := state.Get("ui").(packer.Ui) instanceID, ok := state.Get("instance_id").(string) @@ -150,9 +145,6 @@ func (s *stepCreateInstance) Cleanup(state multistep.StateBag) { // Create a new parameter struct. p := client.VirtualMachine.NewDestroyVirtualMachineParams(instanceID) - // Set expunge so the instance is completely removed - p.SetExpunge(true) - ui.Say("Deleting instance...") if _, err := client.VirtualMachine.DestroyVirtualMachine(p); err != nil { // This is a very poor way to be told the ID does no longer exist :( @@ -162,23 +154,53 @@ func (s *stepCreateInstance) Cleanup(state multistep.StateBag) { return } - ui.Error(fmt.Sprintf("Error destroying instance: %s", err)) + ui.Error(fmt.Sprintf("Error destroying instance. Please destroy it manually.\n\n"+ + "\tName: %s\n"+ + "\tError: %s", config.InstanceName, err)) + return + } + + // We could expunge the VM while destroying it, but if the user doesn't have + // rights that single call could error out leaving the VM running. So but + // splitting these calls we make sure the VM is always deleted, even when the + // expunge fails. + if config.Expunge { + // Create a new parameter struct. + p := client.VirtualMachine.NewExpungeVirtualMachineParams(instanceID) + + ui.Say("Expunging instance...") + if _, err := client.VirtualMachine.ExpungeVirtualMachine(p); err != nil { + // This is a very poor way to be told the ID does no longer exist :( + if strings.Contains(err.Error(), fmt.Sprintf( + "Invalid parameter id value=%s due to incorrect long value format, "+ + "or entity does not exist", instanceID)) { + return + } + + ui.Error(fmt.Sprintf("Error expunging instance. Please expunge it manually.\n\n"+ + "\tName: %s\n"+ + "\tError: %s", config.InstanceName, err)) + return + } } ui.Message("Instance has been deleted!") - return } -// getUserData returns the user data as a base64 encoded string. -func getUserData(userData string, httpGETOnly bool) (string, error) { - ud := base64.StdEncoding.EncodeToString([]byte(userData)) +// generateUserData returns the user data as a base64 encoded string. +func (s *stepCreateInstance) generateUserData(userData string, httpGETOnly bool) (string, error) { + renderedUserData, err := interpolate.Render(userData, &s.Ctx) + if err != nil { + return "", fmt.Errorf("Error rendering user_data: %s", err) + } - // deployVirtualMachine uses POST by default, so max userdata is 32K + ud := base64.StdEncoding.EncodeToString([]byte(renderedUserData)) + + // DeployVirtualMachine uses POST by default which allows 32K of + // userdata. If using GET instead the userdata is limited to 2K. maxUD := 32768 - if httpGETOnly { - // deployVirtualMachine using GET instead, so max userdata is 2K maxUD = 2048 } diff --git a/builder/cloudstack/step_prepare_config.go b/builder/cloudstack/step_prepare_config.go index 2021f8214..b1390b9da 100644 --- a/builder/cloudstack/step_prepare_config.go +++ b/builder/cloudstack/step_prepare_config.go @@ -138,7 +138,6 @@ func (s *stepPrepareConfig) Run(state multistep.StateBag) multistep.StepAction { } ui.Message("Config has been prepared!") - return multistep.ActionContinue } diff --git a/builder/cloudstack/step_shutdown_instance.go b/builder/cloudstack/step_shutdown_instance.go index 5333c0755..638b0e58b 100644 --- a/builder/cloudstack/step_shutdown_instance.go +++ b/builder/cloudstack/step_shutdown_instance.go @@ -35,7 +35,6 @@ func (s *stepShutdownInstance) Run(state multistep.StateBag) multistep.StepActio } ui.Message("Instance has been shutdown!") - return multistep.ActionContinue } diff --git a/website/source/docs/builders/cloudstack.html.md b/website/source/docs/builders/cloudstack.html.md index 22665cb2f..f6a5fa9dd 100644 --- a/website/source/docs/builders/cloudstack.html.md +++ b/website/source/docs/builders/cloudstack.html.md @@ -83,6 +83,9 @@ builder. - `disk_size` (int) - The size (in GB) of the root disk of the new instance. This option is only available when using `source_template`. +- `expunge` (boolean) - Set to `true` to expunge the instance when it is + destroyed. Defaults to `false`. + - `http_directory` (string) - Path to a directory to serve using an HTTP server. The files in this directory will be available over HTTP that will be requestable from the virtual machine. This is useful for hosting