From a9fb6532ae3d19f5a452b93cf9e105fa06520389 Mon Sep 17 00:00:00 2001 From: Cheng Sun Date: Wed, 10 Dec 2014 10:41:59 +0000 Subject: [PATCH 01/10] Improve HIMN IP-finding error handling Previous code could dereference nil if there was no error, but an empty IP was found --- builder/xenserver/step_start_on_himn.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/builder/xenserver/step_start_on_himn.go b/builder/xenserver/step_start_on_himn.go index 6151f9e..4e17807 100644 --- a/builder/xenserver/step_start_on_himn.go +++ b/builder/xenserver/step_start_on_himn.go @@ -63,7 +63,7 @@ func (self *stepStartOnHIMN) Run(state multistep.StateBag) multistep.StepAction log.Printf("Ref: %s", instance.Ref) //Check for instance.Ref in map - if vm_ip, ok := ips[himn_vif.Ref]; ok { + if vm_ip, ok := ips[himn_vif.Ref]; ok && vm_ip != "" { ui.Say("Found the VM's IP: " + vm_ip) himn_iface_ip = vm_ip return true, nil @@ -76,15 +76,13 @@ func (self *stepStartOnHIMN) Run(state multistep.StateBag) multistep.StepAction Timeout: 100 * time.Second, }.Wait(state) - if err != nil || himn_iface_ip == "" { + if err != nil { ui.Error(fmt.Sprintf("Unable to find an IP on the Host-internal management interface: %s", err.Error())) return multistep.ActionHalt } - if himn_iface_ip != "" { - state.Put("himn_ssh_address", himn_iface_ip) - ui.Say("Stored VM's IP " + himn_iface_ip) - } + state.Put("himn_ssh_address", himn_iface_ip) + ui.Say("Stored VM's IP " + himn_iface_ip) // Wait for the VM to boot, and check we can ping this interface From 649798b4ac8b3a17256c8208bc844790a4d3dea3 Mon Sep 17 00:00:00 2001 From: Cheng Sun Date: Wed, 10 Dec 2014 11:22:31 +0000 Subject: [PATCH 02/10] Separate config defaults from validation --- builder/xenserver/builder.go | 44 ++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/builder/xenserver/builder.go b/builder/xenserver/builder.go index 523e5ec..e4a4ece 100644 --- a/builder/xenserver/builder.go +++ b/builder/xenserver/builder.go @@ -119,10 +119,31 @@ func (self *Builder) Prepare(raws ...interface{}) (params []string, retErr error self.config.RawSSHWaitTimeout = "200m" } + if self.config.InstanceMemory == "" { + self.config.InstanceMemory = "1024000000" + } + + if self.config.CloneTemplate == "" { + self.config.CloneTemplate = "Other install media" + } + if self.config.OutputDir == "" { self.config.OutputDir = fmt.Sprintf("output-%s", self.config.PackerBuildName) } + if len(self.config.PlatformArgs) == 0 { + pargs := make(map[string]string) + pargs["viridian"] = "false" + pargs["nx"] = "true" + pargs["pae"] = "true" + pargs["apic"] = "true" + pargs["timeoffset"] = "0" + pargs["acpi"] = "1" + self.config.PlatformArgs = pargs + } + + // Template substitution + templates := map[string]*string{ "username": &self.config.Username, "password": &self.config.Password, @@ -156,6 +177,8 @@ func (self *Builder) Prepare(raws ...interface{}) (params []string, retErr error } } + // Validation + /* if self.config.IsoUrl == "" { errs = packer.MultiErrorAppend( @@ -166,7 +189,7 @@ func (self *Builder) Prepare(raws ...interface{}) (params []string, retErr error self.config.BootWait, err = time.ParseDuration(self.config.RawBootWait) if err != nil { errs = packer.MultiErrorAppend( - errs, errors.New("Failed to parse boot_wait.")) + errs, fmt.Errorf("Failed to parse boot_wait: %s", err)) } self.config.SSHWaitTimeout, err = time.ParseDuration(self.config.RawSSHWaitTimeout) @@ -223,19 +246,11 @@ func (self *Builder) Prepare(raws ...interface{}) (params []string, retErr error errs, errors.New("An instance name must be specified.")) } - if self.config.InstanceMemory == "" { - self.config.InstanceMemory = "1024000000" - } - if self.config.RootDiskSize == "" { errs = packer.MultiErrorAppend( errs, errors.New("A root disk size must be specified.")) } - if self.config.CloneTemplate == "" { - self.config.CloneTemplate = "Other install media" - } - /* if self.config.LocalIp == "" { errs = packer.MultiErrorAppend( @@ -243,17 +258,6 @@ func (self *Builder) Prepare(raws ...interface{}) (params []string, retErr error } */ - if len(self.config.PlatformArgs) == 0 { - pargs := make(map[string]string) - pargs["viridian"] = "false" - pargs["nx"] = "true" - pargs["pae"] = "true" - pargs["apic"] = "true" - pargs["timeoffset"] = "0" - pargs["acpi"] = "1" - self.config.PlatformArgs = pargs - } - if self.config.HTTPPortMin > self.config.HTTPPortMax { errs = packer.MultiErrorAppend( errs, errors.New("the HTTP min port must be less than the max")) From 97ca6d5cabcd3293801ae09b60087b42b3b74643 Mon Sep 17 00:00:00 2001 From: Cheng Sun Date: Wed, 10 Dec 2014 12:31:02 +0000 Subject: [PATCH 03/10] Also close listener connection when port forwarding fails Follow-up to 97df6fd283f7f3626e6ad80830dbeb0815428d08 that actually fixes the "Waiting for SSH" hang --- builder/xenserver/ssh.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/builder/xenserver/ssh.go b/builder/xenserver/ssh.go index 935221d..66bf6af 100644 --- a/builder/xenserver/ssh.go +++ b/builder/xenserver/ssh.go @@ -84,19 +84,22 @@ func execute_ssh_cmd(cmd, host, port, username, password string) (stdout string, } func forward(local_conn net.Conn, config *gossh.ClientConfig, server, remote_dest string, remote_port uint) error { + defer local_conn.Close() + ssh_client_conn, err := gossh.Dial("tcp", server+":22", config) if err != nil { log.Printf("local ssh.Dial error: %s", err) return err } + defer ssh_client_conn.Close() remote_loc := fmt.Sprintf("%s:%d", remote_dest, remote_port) ssh_conn, err := ssh_client_conn.Dial("tcp", remote_loc) if err != nil { log.Printf("ssh.Dial error: %s", err) - ssh_client_conn.Close() return err } + defer ssh_conn.Close() txDone := make(chan struct{}) rxDone := make(chan struct{}) @@ -117,12 +120,8 @@ func forward(local_conn net.Conn, config *gossh.ClientConfig, server, remote_des close(rxDone) }() - go func() { - <-txDone - <-rxDone - ssh_client_conn.Close() - ssh_conn.Close() - }() + <-txDone + <-rxDone return nil } From 602255e9c1c8ef434949436bb832a1f151ae9794 Mon Sep 17 00:00:00 2001 From: Cheng Sun Date: Wed, 10 Dec 2014 11:04:09 +0000 Subject: [PATCH 04/10] Add keep_instance parameter Determines when to keep the VM instance and when to clean it up Can be one of "always", "never" (default), "on_success" --- builder/xenserver/builder.go | 31 +++++++++++++++++++++++ builder/xenserver/step_create_instance.go | 5 ++++ 2 files changed, 36 insertions(+) diff --git a/builder/xenserver/builder.go b/builder/xenserver/builder.go index e4a4ece..768b7f1 100644 --- a/builder/xenserver/builder.go +++ b/builder/xenserver/builder.go @@ -63,6 +63,8 @@ type config struct { OutputDir string `mapstructure:"output_directory"` + KeepInstance string `mapstructure:"keep_instance"` + tpl *packer.ConfigTemplate } @@ -131,6 +133,10 @@ func (self *Builder) Prepare(raws ...interface{}) (params []string, retErr error self.config.OutputDir = fmt.Sprintf("output-%s", self.config.PackerBuildName) } + if self.config.KeepInstance == "" { + self.config.KeepInstance = "never" + } + if len(self.config.PlatformArgs) == 0 { pargs := make(map[string]string) pargs["viridian"] = "false" @@ -167,6 +173,7 @@ func (self *Builder) Prepare(raws ...interface{}) (params []string, retErr error "ssh_password": &self.config.SSHPassword, "ssh_key_path": &self.config.SSHKeyPath, "output_directory": &self.config.OutputDir, + "keep_instance": &self.config.KeepInstance, } for n, ptr := range templates { @@ -251,6 +258,13 @@ func (self *Builder) Prepare(raws ...interface{}) (params []string, retErr error errs, errors.New("A root disk size must be specified.")) } + switch self.config.KeepInstance { + case "always", "never", "on_success": + default: + errs = packer.MultiErrorAppend( + errs, errors.New("keep_instance must be one of 'always', 'never', 'on_success'")) + } + /* if self.config.LocalIp == "" { errs = packer.MultiErrorAppend( @@ -396,6 +410,23 @@ func (self *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (pa return artifact, nil } +// all steps should check config.ShouldKeepInstance first before cleaning up +func (cfg config) ShouldKeepInstance(state multistep.StateBag) bool { + switch cfg.KeepInstance { + case "always": + return true + case "never": + return false + case "on_success": + // only keep instance if build was successful + _, cancelled := state.GetOk(multistep.StateCancelled) + _, halted := state.GetOk(multistep.StateHalted) + return !(cancelled || halted) + default: + panic(fmt.Sprintf("Unknown keep_instance value '%s'", cfg.KeepInstance)) + } +} + func (self *Builder) Cancel() { if self.runner != nil { log.Println("Cancelling the step runner...") diff --git a/builder/xenserver/step_create_instance.go b/builder/xenserver/step_create_instance.go index 6ebf8cd..85645ab 100644 --- a/builder/xenserver/step_create_instance.go +++ b/builder/xenserver/step_create_instance.go @@ -214,6 +214,11 @@ func (self *stepCreateInstance) Run(state multistep.StateBag) multistep.StepActi } func (self *stepCreateInstance) Cleanup(state multistep.StateBag) { + config := state.Get("config").(config) + if config.ShouldKeepInstance(state) { + return + } + ui := state.Get("ui").(packer.Ui) if self.instance != nil { From b433754c8360ff73201c128a1608b907f4a69471 Mon Sep 17 00:00:00 2001 From: Cheng Sun Date: Wed, 10 Dec 2014 14:18:18 +0000 Subject: [PATCH 05/10] Factor out common port-finding code Fixes bug where we would infinite-loop trying to find an HTTP server port when all ports were unavailable (e.g. range [80,80] and no root permission) Also fixes bug where SSH port forwarding would try ports up to but not including HostPortMax. Also add clarification in error messages for which port range to expand --- builder/xenserver/find_port.go | 25 ++++++++++++++++ .../xenserver/step_forward_port_over_ssh.go | 29 ++++--------------- builder/xenserver/step_http_server.go | 26 ++++------------- 3 files changed, 35 insertions(+), 45 deletions(-) create mode 100644 builder/xenserver/find_port.go diff --git a/builder/xenserver/find_port.go b/builder/xenserver/find_port.go new file mode 100644 index 0000000..a6a8492 --- /dev/null +++ b/builder/xenserver/find_port.go @@ -0,0 +1,25 @@ +package xenserver + +import ( + "fmt" + "log" + "net" +) + +// FindPort finds and starts listening on a port in the range [portMin, portMax] +// returns the listener and the port number on success, or nil, 0 on failure +func FindPort(portMin uint, portMax uint) (net.Listener, uint) { + log.Printf("Looking for an available port between %d and %d", portMin, portMax) + + for port := portMin; port <= portMax; port++ { + log.Printf("Trying port: %d", port) + l, err := net.Listen("tcp", fmt.Sprintf(":%d", port)) + if err == nil { + return l, port + } else { + log.Printf("Port %d unavailable: %s", port, err.Error()) + } + } + + return nil, 0 +} diff --git a/builder/xenserver/step_forward_port_over_ssh.go b/builder/xenserver/step_forward_port_over_ssh.go index a41c670..4c407c8 100644 --- a/builder/xenserver/step_forward_port_over_ssh.go +++ b/builder/xenserver/step_forward_port_over_ssh.go @@ -4,8 +4,6 @@ import ( "fmt" "github.com/mitchellh/multistep" "github.com/mitchellh/packer/packer" - "log" - "net" ) type stepForwardPortOverSSH struct { @@ -25,32 +23,15 @@ func (self *stepForwardPortOverSSH) Run(state multistep.StateBag) multistep.Step // Find a free local port: - log.Printf("Looking for an available port between %d and %d", - self.HostPortMin, - self.HostPortMax) + l, sshHostPort := FindPort(self.HostPortMin, self.HostPortMax) - var sshHostPort uint - var foundPort bool - - foundPort = false - - for i := self.HostPortMin; i < self.HostPortMax; i++ { - sshHostPort = i - log.Printf("Trying port: %d", sshHostPort) - l, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", sshHostPort)) - if err == nil { - l.Close() - foundPort = true - break - } - - } - - if !foundPort { - ui.Error("Error: unable to find free host port. Try providing a larger range") + if l == nil || sshHostPort == 0 { + ui.Error("Error: unable to find free host port. Try providing a larger range [host_port_min, host_port_max]") return multistep.ActionHalt } + l.Close() + ui.Say(fmt.Sprintf("Creating a local port forward over SSH on local port %d", sshHostPort)) remotePort, _ := self.RemotePort(state) diff --git a/builder/xenserver/step_http_server.go b/builder/xenserver/step_http_server.go index 538a729..f233f6e 100644 --- a/builder/xenserver/step_http_server.go +++ b/builder/xenserver/step_http_server.go @@ -6,8 +6,6 @@ import ( "fmt" "github.com/mitchellh/multistep" "github.com/mitchellh/packer/packer" - "log" - "math/rand" "net" "net/http" ) @@ -36,32 +34,18 @@ func (s *stepHTTPServer) Run(state multistep.StateBag) multistep.StepAction { return multistep.ActionContinue } - // Find an available TCP port for our HTTP server - var httpAddr string - portRange := int(config.HTTPPortMax - config.HTTPPortMin) - for { - var err error - var offset uint = 0 + s.l, httpPort = FindPort(config.HTTPPortMin, config.HTTPPortMax) - if portRange > 0 { - // Intn will panic if portRange == 0, so we do a check. - offset = uint(rand.Intn(portRange)) - } - - httpPort = offset + config.HTTPPortMin - httpAddr = fmt.Sprintf(":%d", httpPort) - log.Printf("Trying port: %d", httpPort) - s.l, err = net.Listen("tcp", httpAddr) - if err == nil { - break - } + if s.l == nil || httpPort == 0 { + ui.Error("Error: unable to find free HTTP server port. Try providing a larger range [http_port_min, http_port_max]") + return multistep.ActionHalt } ui.Say(fmt.Sprintf("Starting HTTP server on port %d", httpPort)) // Start the HTTP server and run it in the background fileServer := http.FileServer(http.Dir(config.HTTPDir)) - server := &http.Server{Addr: httpAddr, Handler: fileServer} + server := &http.Server{Addr: fmt.Sprintf(":%d", httpPort), Handler: fileServer} go server.Serve(s.l) // Save the address into the state so it can be accessed in the future From 6180bd1ca5b371025cac6aace61c7d07366c40e8 Mon Sep 17 00:00:00 2001 From: Cheng Sun Date: Wed, 10 Dec 2014 18:32:27 +0000 Subject: [PATCH 06/10] Add keep_instance to example config --- examples/centos-6.4.conf | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/examples/centos-6.4.conf b/examples/centos-6.4.conf index 9e4ee99..18af4cb 100644 --- a/examples/centos-6.4.conf +++ b/examples/centos-6.4.conf @@ -17,6 +17,7 @@ [ "", " ks=http://{{ .HTTPIP }}:{{ .HTTPPort }}/centos6-ks.cfg" - ] + ], + "keep_instance": "always" }] } From 783eee0a39fc2c40bc118b9e52e1e6034eeb8a98 Mon Sep 17 00:00:00 2001 From: Cheng Sun Date: Thu, 11 Dec 2014 12:26:45 +0000 Subject: [PATCH 07/10] Add user variables to template processor --- builder/xenserver/builder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builder/xenserver/builder.go b/builder/xenserver/builder.go index 768b7f1..c6de7c8 100644 --- a/builder/xenserver/builder.go +++ b/builder/xenserver/builder.go @@ -86,10 +86,10 @@ func (self *Builder) Prepare(raws ...interface{}) (params []string, retErr error } self.config.tpl, err = packer.NewConfigTemplate() - if err != nil { return nil, err } + self.config.tpl.UserVars = self.config.PackerUserVars // Set default vaules From 322817e827660ff39f10a36dbf7a2e0aeaea641c Mon Sep 17 00:00:00 2001 From: Cheng Sun Date: Thu, 11 Dec 2014 14:19:28 +0000 Subject: [PATCH 08/10] Fix export UUID --- builder/xenserver/step_shutdown_and_export.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builder/xenserver/step_shutdown_and_export.go b/builder/xenserver/step_shutdown_and_export.go index ed25fd4..a966c25 100644 --- a/builder/xenserver/step_shutdown_and_export.go +++ b/builder/xenserver/step_shutdown_and_export.go @@ -69,7 +69,7 @@ func (stepShutdownAndExport) Run(state multistep.StateBag) multistep.StepAction export_url := fmt.Sprintf("https://%s/export?vm=%s&session_id=%s", client.Host, - instance.Ref, + instance_uuid, client.Session.(string), ) From 13ca6ef95b3f33d618ecc33e2dba6e40fd1027ce Mon Sep 17 00:00:00 2001 From: Cheng Sun Date: Thu, 11 Dec 2014 14:30:39 +0000 Subject: [PATCH 09/10] export_format: allow export format to be configured "export_format" can be one of "xva" (default), "vdi_raw" --- builder/xenserver/builder.go | 15 +++- builder/xenserver/step_shutdown_and_export.go | 76 ++++++++++--------- 2 files changed, 56 insertions(+), 35 deletions(-) diff --git a/builder/xenserver/builder.go b/builder/xenserver/builder.go index c6de7c8..50107bf 100644 --- a/builder/xenserver/builder.go +++ b/builder/xenserver/builder.go @@ -61,7 +61,8 @@ type config struct { SSHUser string `mapstructure:"ssh_username"` SSHKeyPath string `mapstructure:"ssh_key_path"` - OutputDir string `mapstructure:"output_directory"` + OutputDir string `mapstructure:"output_directory"` + ExportFormat string `mapstructure:"export_format"` KeepInstance string `mapstructure:"keep_instance"` @@ -133,6 +134,10 @@ func (self *Builder) Prepare(raws ...interface{}) (params []string, retErr error self.config.OutputDir = fmt.Sprintf("output-%s", self.config.PackerBuildName) } + if self.config.ExportFormat == "" { + self.config.ExportFormat = "xva" + } + if self.config.KeepInstance == "" { self.config.KeepInstance = "never" } @@ -173,6 +178,7 @@ func (self *Builder) Prepare(raws ...interface{}) (params []string, retErr error "ssh_password": &self.config.SSHPassword, "ssh_key_path": &self.config.SSHKeyPath, "output_directory": &self.config.OutputDir, + "export_format": &self.config.ExportFormat, "keep_instance": &self.config.KeepInstance, } @@ -258,6 +264,13 @@ func (self *Builder) Prepare(raws ...interface{}) (params []string, retErr error errs, errors.New("A root disk size must be specified.")) } + switch self.config.ExportFormat { + case "xva", "vdi_raw": + default: + errs = packer.MultiErrorAppend( + errs, errors.New("export_format must be one of 'xva', 'vdi_raw'")) + } + switch self.config.KeepInstance { case "always", "never", "on_success": default: diff --git a/builder/xenserver/step_shutdown_and_export.go b/builder/xenserver/step_shutdown_and_export.go index a966c25..96d53d2 100644 --- a/builder/xenserver/step_shutdown_and_export.go +++ b/builder/xenserver/step_shutdown_and_export.go @@ -65,44 +65,52 @@ func (stepShutdownAndExport) Run(state multistep.StateBag) multistep.StepAction return multistep.ActionHalt } - //Export the VM + switch config.ExportFormat { + case "xva": + // export the VM - export_url := fmt.Sprintf("https://%s/export?vm=%s&session_id=%s", - client.Host, - instance_uuid, - client.Session.(string), - ) - - export_filename := fmt.Sprintf("%s/%s.xva", config.OutputDir, config.InstanceName) - ui.Say("Getting metadata " + export_url) - downloadFile(export_url, export_filename) - - disks, err := instance.GetDisks() - if err != nil { - ui.Error(fmt.Sprintf("Could not get VM disks: %s", err.Error())) - return multistep.ActionHalt - } - for _, disk := range disks { - disk_uuid, err := disk.GetUuid() - if err != nil { - ui.Error(fmt.Sprintf("Could not get disk with UUID '%s': %s", disk_uuid, err.Error())) - return multistep.ActionHalt - } - - // Basic auth in URL request is required as session token is not - // accepted for some reason. - // @todo: raise with XAPI team. - disk_export_url := fmt.Sprintf("https://%s:%s@%s/export_raw_vdi?vdi=%s", - client.Username, - client.Password, + export_url := fmt.Sprintf("https://%s/export?vm=%s&session_id=%s", client.Host, - disk_uuid, + instance_uuid, + client.Session.(string), ) - ui.Say("Getting " + disk_export_url) - disk_export_filename := fmt.Sprintf("%s/%s.raw", config.OutputDir, disk_uuid) - ui.Say("Downloading " + disk_uuid) - downloadFile(disk_export_url, disk_export_filename) + export_filename := fmt.Sprintf("%s/%s.xva", config.OutputDir, config.InstanceName) + ui.Say("Getting XVA " + export_url) + downloadFile(export_url, export_filename) + + case "vdi_raw": + // export the disks + + disks, err := instance.GetDisks() + if err != nil { + ui.Error(fmt.Sprintf("Could not get VM disks: %s", err.Error())) + return multistep.ActionHalt + } + for _, disk := range disks { + disk_uuid, err := disk.GetUuid() + if err != nil { + ui.Error(fmt.Sprintf("Could not get disk with UUID '%s': %s", disk_uuid, err.Error())) + return multistep.ActionHalt + } + + // Basic auth in URL request is required as session token is not + // accepted for some reason. + // @todo: raise with XAPI team. + disk_export_url := fmt.Sprintf("https://%s:%s@%s/export_raw_vdi?vdi=%s", + client.Username, + client.Password, + client.Host, + disk_uuid, + ) + + disk_export_filename := fmt.Sprintf("%s/%s.raw", config.OutputDir, disk_uuid) + ui.Say("Getting VDI " + disk_export_url) + downloadFile(disk_export_url, disk_export_filename) + } + + default: + panic(fmt.Sprintf("Unknown export_format '%s'", config.ExportFormat)) } ui.Say("Download completed: " + config.OutputDir) From 971ab833761a03bf2ad5790cf66e3e6fe8a76f35 Mon Sep 17 00:00:00 2001 From: Cheng Sun Date: Thu, 11 Dec 2014 14:31:22 +0000 Subject: [PATCH 10/10] Fix comment typo --- builder/xenserver/builder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builder/xenserver/builder.go b/builder/xenserver/builder.go index 50107bf..4a9e8fa 100644 --- a/builder/xenserver/builder.go +++ b/builder/xenserver/builder.go @@ -92,7 +92,7 @@ func (self *Builder) Prepare(raws ...interface{}) (params []string, retErr error } self.config.tpl.UserVars = self.config.PackerUserVars - // Set default vaules + // Set default values if self.config.HostPortMin == 0 { self.config.HostPortMin = 5900