Merge branch 'master' into circle_ci_windows

CHANGELOG.md

@@ -23,6 +23,7 @@
 * core: Fix bug where sensitive variables contianing commas were not being
     properly sanitized in UI calls. [GH-7997]
 * provisioner/ansible: Fix provisioner dropped errors [GH-8045]
+* builder/proxmox: Fix panic caused by cancelling build [GH-8067] [GH-8072]
 
 ## 1.4.3 (August 14, 2019)
 

builder/amazon/ebsvolume/builder.go

@@ -23,9 +23,10 @@ type Config struct {
 	awscommon.AccessConfig `mapstructure:",squash"`
 	awscommon.RunConfig    `mapstructure:",squash"`
 
-	VolumeMappings     []BlockDevice `mapstructure:"ebs_volumes"`
-	AMIENASupport      *bool         `mapstructure:"ena_support"`
-	AMISriovNetSupport bool          `mapstructure:"sriov_support"`
+	AMIENASupport      *bool            `mapstructure:"ena_support"`
+	AMISriovNetSupport bool             `mapstructure:"sriov_support"`
+	VolumeMappings     []BlockDevice    `mapstructure:"ebs_volumes"`
+	VolumeRunTags      awscommon.TagMap `mapstructure:"run_volume_tags"`
 
 	launchBlockDevices awscommon.BlockDevices
 	ctx                interpolate.Context
@@ -120,21 +121,22 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
 			AssociatePublicIpAddress:          b.config.AssociatePublicIpAddress,
 			BlockDevices:                      b.config.launchBlockDevices,
 			BlockDurationMinutes:              b.config.BlockDurationMinutes,
-			Ctx:                               b.config.ctx,
 			Comm:                              &b.config.RunConfig.Comm,
+			Ctx:                               b.config.ctx,
 			Debug:                             b.config.PackerDebug,
 			EbsOptimized:                      b.config.EbsOptimized,
 			ExpectedRootDevice:                "ebs",
 			IamInstanceProfile:                b.config.IamInstanceProfile,
 			InstanceInitiatedShutdownBehavior: b.config.InstanceInitiatedShutdownBehavior,
 			InstanceType:                      b.config.InstanceType,
 			SourceAMI:                         b.config.SourceAmi,
-			SpotPrice:                         b.config.SpotPrice,
 			SpotInstanceTypes:                 b.config.SpotInstanceTypes,
+			SpotPrice:                         b.config.SpotPrice,
 			SpotTags:                          b.config.SpotTags,
 			Tags:                              b.config.RunTags,
 			UserData:                          b.config.UserData,
 			UserDataFile:                      b.config.UserDataFile,
+			VolumeTags:                        b.config.VolumeRunTags,
 		}
 	} else {
 		instanceStep = &awscommon.StepRunSourceInstance{
@@ -154,6 +156,7 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
 			Tags:                              b.config.RunTags,
 			UserData:                          b.config.UserData,
 			UserDataFile:                      b.config.UserDataFile,
+			VolumeTags:                        b.config.VolumeRunTags,
 		}
 	}
 

builder/amazon/ebsvolume/step_tag_ebs_volumes.go

@@ -3,7 +3,9 @@ package ebsvolume
 import (
 	"context"
 	"fmt"
+	"strings"
 
+	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/hashicorp/packer/helper/multistep"
 	"github.com/hashicorp/packer/packer"
@@ -19,6 +21,7 @@ func (s *stepTagEBSVolumes) Run(ctx context.Context, state multistep.StateBag) m
 	ec2conn := state.Get("ec2").(*ec2.EC2)
 	instance := state.Get("instance").(*ec2.Instance)
 	ui := state.Get("ui").(packer.Ui)
+	config := state.Get("config").(*Config)
 
 	volumes := make(EbsVolumes)
 	for _, instanceBlockDevices := range instance.BlockDeviceMappings {
@@ -36,34 +39,82 @@ func (s *stepTagEBSVolumes) Run(ctx context.Context, state multistep.StateBag) m
 	state.Put("ebsvolumes", volumes)
 
 	if len(s.VolumeMapping) > 0 {
-		ui.Say("Tagging EBS volumes...")
+		// If run_volume_tags were set in the template any attached EBS
+		// volume will have had these tags applied when the instance was
+		// created. We now need to remove these tags to ensure only the EBS
+		// volume tags are applied (if any)
+		if config.VolumeRunTags.IsSet() {
+			ui.Say("Removing any tags applied to EBS volumes when the source instance was created...")
+
+			ui.Message("Compiling list of existing tags to remove...")
+			existingTags, err := config.VolumeRunTags.EC2Tags(s.Ctx, *ec2conn.Config.Region, state)
+			if err != nil {
+				err := fmt.Errorf("Error generating list of tags to remove: %s", err)
+				state.Put("error", err)
+				ui.Error(err.Error())
+				return multistep.ActionHalt
+			}
+			existingTags.Report(ui)
+
+			// Generate the list of volumes with tags to delete.
+			// Looping over the instance block device mappings allows us to
+			// obtain the volumeId
+			volumeIds := []string{}
+			for _, mapping := range s.VolumeMapping {
+				for _, v := range instance.BlockDeviceMappings {
+					if *v.DeviceName == mapping.DeviceName {
+						volumeIds = append(volumeIds, *v.Ebs.VolumeId)
+					}
+				}
+			}
 
+			// Delete the tags
+			ui.Message(fmt.Sprintf("Deleting 'run_volume_tags' on EBS Volumes: %s", strings.Join(volumeIds, ", ")))
+			_, err = ec2conn.DeleteTags(&ec2.DeleteTagsInput{
+				Resources: aws.StringSlice(volumeIds),
+				Tags:      existingTags,
+			})
+			if err != nil {
+				err := fmt.Errorf("Error deleting tags on EBS Volumes %s: %s", strings.Join(volumeIds, ", "), err)
+				state.Put("error", err)
+				ui.Error(err.Error())
+				return multistep.ActionHalt
+			}
+		}
+
+		ui.Say("Tagging EBS volumes...")
 		toTag := map[string][]*ec2.Tag{}
 		for _, mapping := range s.VolumeMapping {
 			if len(mapping.Tags) == 0 {
 				ui.Say(fmt.Sprintf("No tags specified for volume on %s...", mapping.DeviceName))
 				continue
 			}
 
+			ui.Message(fmt.Sprintf("Compiling list of tags to apply to volume on %s...", mapping.DeviceName))
 			tags, err := mapping.Tags.EC2Tags(s.Ctx, *ec2conn.Config.Region, state)
 			if err != nil {
-				err := fmt.Errorf("Error tagging device %s with %s", mapping.DeviceName, err)
+				err := fmt.Errorf("Error generating tags for device %s: %s", mapping.DeviceName, err)
 				state.Put("error", err)
 				ui.Error(err.Error())
 				return multistep.ActionHalt
 			}
 			tags.Report(ui)
 
+			// Generate the map of volumes and associated tags to apply.
+			// Looping over the instance block device mappings allows us to
+			// obtain the volumeId
 			for _, v := range instance.BlockDeviceMappings {
 				if *v.DeviceName == mapping.DeviceName {
 					toTag[*v.Ebs.VolumeId] = tags
 				}
 			}
 		}
 
+		// Apply the tags
 		for volumeId, tags := range toTag {
+			ui.Message(fmt.Sprintf("Applying tags to EBS Volume: %s", volumeId))
 			_, err := ec2conn.CreateTags(&ec2.CreateTagsInput{
-				Resources: []*string{&volumeId},
+				Resources: aws.StringSlice([]string{volumeId}),
 				Tags:      tags,
 			})
 			if err != nil {
@@ -72,7 +123,6 @@ func (s *stepTagEBSVolumes) Run(ctx context.Context, state multistep.StateBag) m
 				ui.Error(err.Error())
 				return multistep.ActionHalt
 			}
-
 		}
 	}
 

builder/proxmox/bootcommand_driver.go

@@ -102,13 +102,10 @@ func (p *proxmoxDriver) SendSpecial(special string, action bootcommand.KeyAction
 }
 
 func (p *proxmoxDriver) send(keys string) error {
-	res, err := p.client.MonitorCmd(p.vmRef, "sendkey "+keys)
+	err := p.client.Sendkey(p.vmRef, keys)
 	if err != nil {
 		return err
 	}
-	if data, ok := res["data"].(string); ok && len(data) > 0 {
-		return fmt.Errorf("failed to send keys: %s", data)
-	}
 
 	time.Sleep(p.interval)
 	return nil

builder/proxmox/builder.go

@@ -46,7 +46,7 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
 		return nil, err
 	}
 
-	err = b.proxmoxClient.Login(b.config.Username, b.config.Password)
+	err = b.proxmoxClient.Login(b.config.Username, b.config.Password, "")
 	if err != nil {
 		return nil, err
 	}

builder/proxmox/config.go

@@ -103,7 +103,7 @@ func NewConfig(raws ...interface{}) (*Config, []string, error) {
 		c.RawBootKeyInterval = os.Getenv(common.PackerKeyEnv)
 	}
 	if c.RawBootKeyInterval == "" {
-		c.BootKeyInterval = common.PackerKeyDefault
+		c.BootKeyInterval = 5 * time.Millisecond
 	} else {
 		if interval, err := time.ParseDuration(c.RawBootKeyInterval); err == nil {
 			c.BootKeyInterval = interval
@@ -151,6 +151,12 @@ func NewConfig(raws ...interface{}) (*Config, []string, error) {
 			log.Printf("Disk %d cache mode not set, using default 'none'", idx)
 			c.Disks[idx].CacheMode = "none"
 		}
+		// For any storage pool types which aren't in rxStorageTypes in proxmox-api/proxmox/config_qemu.go:651
+		// (currently zfspool and lvm), the format parameter is mandatory. Make sure this is still up to date
+		// when updating the vendored code!
+		if !contains([]string{"zfspool", "lvm"}, c.Disks[idx].StoragePoolType) && c.Disks[idx].DiskFormat == "" {
+			errs = packer.MultiErrorAppend(errs, errors.New(fmt.Sprintf("disk format must be specified for pool type %q", c.Disks[idx].StoragePoolType)))
+		}
 	}
 
 	errs = packer.MultiErrorAppend(errs, c.Comm.Prepare(&c.ctx)...)
@@ -197,3 +203,12 @@ func NewConfig(raws ...interface{}) (*Config, []string, error) {
 	packer.LogSecretFilter.Set(c.Password)
 	return c, nil, nil
 }
+
+func contains(haystack []string, needle string) bool {
+	for _, candidate := range haystack {
+		if candidate == needle {
+			return true
+		}
+	}
+	return false
+}

builder/proxmox/step_start_vm.go

@@ -30,6 +30,8 @@ func (s *stepStartVM) Run(ctx context.Context, state multistep.StateBag) multist
 	config := proxmox.ConfigQemu{
 		Name:         c.VMName,
 		Agent:        agent,
+		Boot:         "cdn", // Boot priority, c:CDROM -> d:Disk -> n:Network
+		QemuCpu:      "host",
 		Description:  "Packer ephemeral build VM",
 		Memory:       c.Memory,
 		QemuCores:    c.Cores,
@@ -142,7 +144,7 @@ func (s *stepStartVM) Cleanup(state multistep.StateBag) {
 	ui.Say("Stopping VM")
 	_, err := client.StopVm(vmRef)
 	if err != nil {
-		ui.Error(fmt.Sprintf("Error stop VM. Please stop and delete it manually: %s", err))
+		ui.Error(fmt.Sprintf("Error stopping VM. Please stop and delete it manually: %s", err))
 		return
 	}
 

builder/proxmox/step_type_boot_command.go

@@ -30,7 +30,7 @@ type bootCommandTemplateData struct {
 }
 
 type commandTyper interface {
-	MonitorCmd(*proxmox.VmRef, string) (map[string]interface{}, error)
+	Sendkey(*proxmox.VmRef, string) error
 }
 
 var _ commandTyper = &proxmox.Client{}

builder/proxmox/step_type_boot_command_test.go

@@ -13,93 +13,89 @@ import (
 )
 
 type commandTyperMock struct {
-	monitorCmd func(*proxmox.VmRef, string) (map[string]interface{}, error)
+	sendkey func(*proxmox.VmRef, string) error
 }
 
-func (m commandTyperMock) MonitorCmd(ref *proxmox.VmRef, cmd string) (map[string]interface{}, error) {
-	return m.monitorCmd(ref, cmd)
+func (m commandTyperMock) Sendkey(ref *proxmox.VmRef, cmd string) error {
+	return m.sendkey(ref, cmd)
 }
 
 var _ commandTyper = commandTyperMock{}
 
 func TestTypeBootCommand(t *testing.T) {
 	cs := []struct {
-		name                 string
-		builderConfig        *Config
-		expectCallMonitorCmd bool
-		monitorCmdErr        error
-		monitorCmdRet        map[string]interface{}
-		expectedKeysSent     string
-		expectedAction       multistep.StepAction
+		name              string
+		builderConfig     *Config
+		expectCallSendkey bool
+		sendkeyErr        error
+		expectedKeysSent  string
+		expectedAction    multistep.StepAction
 	}{
 		{
-			name:                 "simple boot command is typed",
-			builderConfig:        &Config{BootConfig: bootcommand.BootConfig{BootCommand: []string{"hello"}}},
-			expectCallMonitorCmd: true,
-			expectedKeysSent:     "hello",
-			expectedAction:       multistep.ActionContinue,
+			name:              "simple boot command is typed",
+			builderConfig:     &Config{BootConfig: bootcommand.BootConfig{BootCommand: []string{"hello"}}},
+			expectCallSendkey: true,
+			expectedKeysSent:  "hello",
+			expectedAction:    multistep.ActionContinue,
 		},
 		{
-			name:                 "interpolated boot command",
-			builderConfig:        &Config{BootConfig: bootcommand.BootConfig{BootCommand: []string{"hello<enter>world"}}},
-			expectCallMonitorCmd: true,
-			expectedKeysSent:     "helloretworld",
-			expectedAction:       multistep.ActionContinue,
+			name:              "interpolated boot command",
+			builderConfig:     &Config{BootConfig: bootcommand.BootConfig{BootCommand: []string{"hello<enter>world"}}},
+			expectCallSendkey: true,
+			expectedKeysSent:  "helloretworld",
+			expectedAction:    multistep.ActionContinue,
 		},
 		{
-			name:                 "merge multiple interpolated boot command",
-			builderConfig:        &Config{BootConfig: bootcommand.BootConfig{BootCommand: []string{"Hello World 2.0", "foo!bar@baz"}}},
-			expectCallMonitorCmd: true,
-			expectedKeysSent:     "shift-hellospcshift-worldspc2dot0fooshift-1barshift-2baz",
-			expectedAction:       multistep.ActionContinue,
+			name:              "merge multiple interpolated boot command",
+			builderConfig:     &Config{BootConfig: bootcommand.BootConfig{BootCommand: []string{"Hello World 2.0", "foo!bar@baz"}}},
+			expectCallSendkey: true,
+			expectedKeysSent:  "shift-hellospcshift-worldspc2dot0fooshift-1barshift-2baz",
+			expectedAction:    multistep.ActionContinue,
 		},
 		{
-			name:                 "without boot command monitorcmd should not be called",
-			builderConfig:        &Config{BootConfig: bootcommand.BootConfig{BootCommand: []string{}}},
-			expectCallMonitorCmd: false,
-			expectedAction:       multistep.ActionContinue,
+			name:              "without boot command sendkey should not be called",
+			builderConfig:     &Config{BootConfig: bootcommand.BootConfig{BootCommand: []string{}}},
+			expectCallSendkey: false,
+			expectedAction:    multistep.ActionContinue,
 		},
 		{
-			name:                 "invalid boot command template function",
-			builderConfig:        &Config{BootConfig: bootcommand.BootConfig{BootCommand: []string{"{{ foo }}"}}},
-			expectCallMonitorCmd: false,
-			expectedAction:       multistep.ActionHalt,
+			name:              "invalid boot command template function",
+			builderConfig:     &Config{BootConfig: bootcommand.BootConfig{BootCommand: []string{"{{ foo }}"}}},
+			expectCallSendkey: false,
+			expectedAction:    multistep.ActionHalt,
 		},
 		{
 			// When proxmox (or Qemu, really) doesn't recognize the keycode we send, we get no error back, but
 			// a map {"data": "invalid parameter: X"}, where X is the keycode.
-			name:                 "invalid keys sent to proxmox",
-			builderConfig:        &Config{BootConfig: bootcommand.BootConfig{BootCommand: []string{"x"}}},
-			expectCallMonitorCmd: true,
-			monitorCmdRet:        map[string]interface{}{"data": "invalid parameter: x"},
-			expectedKeysSent:     "x",
-			expectedAction:       multistep.ActionHalt,
+			name:              "invalid keys sent to proxmox",
+			builderConfig:     &Config{BootConfig: bootcommand.BootConfig{BootCommand: []string{"x"}}},
+			expectCallSendkey: true,
+			sendkeyErr:        fmt.Errorf("invalid parameter: x"),
+			expectedKeysSent:  "x",
+			expectedAction:    multistep.ActionHalt,
 		},
 		{
-			name:                 "error in typing should return halt",
-			builderConfig:        &Config{BootConfig: bootcommand.BootConfig{BootCommand: []string{"hello"}}},
-			expectCallMonitorCmd: true,
-			monitorCmdErr:        fmt.Errorf("some error"),
-			expectedKeysSent:     "h",
-			expectedAction:       multistep.ActionHalt,
+			name:              "error in typing should return halt",
+			builderConfig:     &Config{BootConfig: bootcommand.BootConfig{BootCommand: []string{"hello"}}},
+			expectCallSendkey: true,
+			sendkeyErr:        fmt.Errorf("some error"),
+			expectedKeysSent:  "h",
+			expectedAction:    multistep.ActionHalt,
 		},
 	}
 
 	for _, c := range cs {
 		t.Run(c.name, func(t *testing.T) {
 			accumulator := strings.Builder{}
 			typer := commandTyperMock{
-				monitorCmd: func(ref *proxmox.VmRef, cmd string) (map[string]interface{}, error) {
-					if !c.expectCallMonitorCmd {
-						t.Error("Did not expect MonitorCmd to be called")
-					}
-					if !strings.HasPrefix(cmd, "sendkey ") {
-						t.Errorf("Expected all commands to be sendkey, got %s", cmd)
+				sendkey: func(ref *proxmox.VmRef, cmd string) error {
+					if !c.expectCallSendkey {
+						t.Error("Did not expect sendkey to be called")
 					}
 
-					accumulator.WriteString(strings.TrimPrefix(cmd, "sendkey "))
+					accumulator.WriteString(cmd)
 
-					return c.monitorCmdRet, c.monitorCmdErr
+					return c.sendkeyErr
 				},
 			}