change name of singular block device in loop to be less confusing; fix snapshot tests

builder/amazon/ebsvolume/step_snapshot_ebs_volumes.go

@@ -29,19 +29,19 @@ func (s *stepSnapshotEBSVolumes) Run(ctx context.Context, state multistep.StateB
 
 	s.snapshotMap = make(map[string]*BlockDevice)
 
-	for _, instanceBlockDevices := range instance.BlockDeviceMappings {
+	for _, instanceBlockDevice := range instance.BlockDeviceMappings {
 		for _, configVolumeMapping := range s.VolumeMapping {
 			//Find the config entry for the instance blockDevice
-			if configVolumeMapping.DeviceName == *instanceBlockDevices.DeviceName {
+			if configVolumeMapping.DeviceName == *instanceBlockDevice.DeviceName {
 				//Skip Volumes that are not set to create snapshot
 				if configVolumeMapping.SnapshotVolume != true {
 					continue
 				}
 
-				ui.Message(fmt.Sprintf("Compiling list of tags to apply to snapshot from Volume %s...", *instanceBlockDevices.DeviceName))
+				ui.Message(fmt.Sprintf("Compiling list of tags to apply to snapshot from Volume %s...", *instanceBlockDevice.DeviceName))
 				tags, err := awscommon.TagMap(configVolumeMapping.SnapshotTags).EC2Tags(s.Ctx, s.AccessConfig.SessionRegion(), state)
 				if err != nil {
-					err := fmt.Errorf("Error generating tags for snapshot %s: %s", *instanceBlockDevices.DeviceName, err)
+					err := fmt.Errorf("Error generating tags for snapshot %s: %s", *instanceBlockDevice.DeviceName, err)
 					state.Put("error", err)
 					ui.Error(err.Error())
 					return multistep.ActionHalt
@@ -54,7 +54,7 @@ func (s *stepSnapshotEBSVolumes) Run(ctx context.Context, state multistep.StateB
 				}
 
 				input := &ec2.CreateSnapshotInput{
-					VolumeId:          aws.String(*instanceBlockDevices.Ebs.VolumeId),
+					VolumeId:          aws.String(*instanceBlockDevice.Ebs.VolumeId),
 					TagSpecifications: []*ec2.TagSpecification{tagSpec},
 				}
 
@@ -63,15 +63,15 @@ func (s *stepSnapshotEBSVolumes) Run(ctx context.Context, state multistep.StateB
 					input.TagSpecifications = nil
 				}
 
-				ui.Message(fmt.Sprintf("Requesting snapshot of volume: %s...", *instanceBlockDevices.Ebs.VolumeId))
+				ui.Message(fmt.Sprintf("Requesting snapshot of volume: %s...", *instanceBlockDevice.Ebs.VolumeId))
 				snapshot, err := ec2conn.CreateSnapshot(input)
 				if err != nil || snapshot == nil {
-					err := fmt.Errorf("Error generating snapsot for volume %s: %s", *instanceBlockDevices.Ebs.VolumeId, err)
+					err := fmt.Errorf("Error generating snapsot for volume %s: %s", *instanceBlockDevice.Ebs.VolumeId, err)
 					state.Put("error", err)
 					ui.Error(err.Error())
 					return multistep.ActionHalt
 				}
-				ui.Message(fmt.Sprintf("Requested Snapshot of Volume %s: %s", *instanceBlockDevices.Ebs.VolumeId, *snapshot.SnapshotId))
+				ui.Message(fmt.Sprintf("Requested Snapshot of Volume %s: %s", *instanceBlockDevice.Ebs.VolumeId, *snapshot.SnapshotId))
 				s.snapshotMap[*snapshot.SnapshotId] = &configVolumeMapping
 			}
 		}

builder/amazon/ebsvolume/step_snapshot_ebs_volumes_test.go

@@ -3,10 +3,12 @@ package ebsvolume
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"sync"
 	"testing"
 
 	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/aws/request"
 
 	//"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/aws/aws-sdk-go/service/ec2"
@@ -31,6 +33,21 @@ type mockEC2Conn struct {
 	lock sync.Mutex
 }
 
+func (m *mockEC2Conn) CreateSnapshot(input *ec2.CreateSnapshotInput) (*ec2.Snapshot, error) {
+	snap := &ec2.Snapshot{
+		// This isn't typical amazon format, but injecting the volume id into
+		// this field lets us verify that the right volume was snapshotted with
+		// a simple string comparison
+		SnapshotId: aws.String(fmt.Sprintf("snap-of-%s", *input.VolumeId)),
+	}
+
+	return snap, nil
+}
+
+func (m *mockEC2Conn) WaitUntilSnapshotCompletedWithContext(aws.Context, *ec2.DescribeSnapshotsInput, ...request.WaiterOption) error {
+	return nil
+}
+
 func getMockConn(config *common.AccessConfig, target string) (ec2iface.EC2API, error) {
 	mockConn := &mockEC2Conn{
 		Config: aws.NewConfig(),
@@ -50,6 +67,25 @@ func tState(t *testing.T) multistep.StateBag {
 	conn, _ := getMockConn(&common.AccessConfig{}, "us-east-2")
 
 	state.Put("ec2", conn)
+	// Store a fake instance that contains a block device that matches the
+	// volumes defined in the config above
+	state.Put("instance", &ec2.Instance{
+		InstanceId: aws.String("instance-id"),
+		BlockDeviceMappings: []*ec2.InstanceBlockDeviceMapping{
+			{
+				DeviceName: aws.String("/dev/xvda"),
+				Ebs: &ec2.EbsInstanceBlockDevice{
+					VolumeId: aws.String("vol-1234"),
+				},
+			},
+			{
+				DeviceName: aws.String("/dev/xvdb"),
+				Ebs: &ec2.EbsInstanceBlockDevice{
+					VolumeId: aws.String("vol-5678"),
+				},
+			},
+		},
+	})
 	return state
 }
 
@@ -63,6 +99,7 @@ func TestStepSnapshot_run_simple(t *testing.T) {
 			"device_name":           "/dev/xvdb",
 			"volume_size":           "32",
 			"delete_on_termination": true,
+			"snapshot_volume":       true,
 		},
 	}
 
@@ -78,35 +115,11 @@ func TestStepSnapshot_run_simple(t *testing.T) {
 	}
 
 	state := tState(t)
-	state.Put("instance", &ec2.Instance{
-		InstanceId: aws.String("instance-id"),
-	})
 
 	accessConfig := common.FakeAccessConfig()
 
-	volMap := BlockDevices{
-		{
-	awscommon.BlockDevice `mapstructure:",squash"`
-	// Key/value pair tags to apply to the volume. These are retained after the builder
-	// completes. This is a [template engine](/docs/templates/legacy_json_templates/engine), see
-	// [Build template data](#build-template-data) for more information.
-	Tags map[string]string `mapstructure:"tags" required:"false"`
-	// Same as [`tags`](#tags) but defined as a singular repeatable block
-	// containing a `key` and a `value` field. In HCL2 mode the
-	// [`dynamic_block`](/docs/templates/hcl_templates/expressions#dynamic-blocks)
-	// will allow you to create those programatically.
-	Tag config.KeyValues `mapstructure:"tag" required:"false"`
-
-	// Create a Snapshot of this Volume.
-	SnapshotVolume bool `mapstructure:"snapshot_volume" required:"false"`
-
-	awscommon.SnapshotConfig `mapstructure:",squash"`
-}
-}
-	//Todo add fake volumes, for the snap shot step to Snapshot
-
 	step := stepSnapshotEBSVolumes{
-		PollingConfig: new(common.AWSPollingConfig), //Dosnt look like builder sets this up
+		PollingConfig: new(common.AWSPollingConfig),
 		AccessConfig:  accessConfig,
 		VolumeMapping: b.config.VolumeMappings,
 		Ctx:           b.config.ctx,
@@ -117,4 +130,51 @@ func TestStepSnapshot_run_simple(t *testing.T) {
 	if len(step.snapshotMap) != 1 {
 		t.Fatalf("Missing Snapshot from step")
 	}
+
+	if volmapping := step.snapshotMap["snap-of-vol-5678"]; volmapping == nil {
+		t.Fatalf("Didn't snapshot correct volume: Map is %#v", step.snapshotMap)
+	}
+}
+
+func TestStepSnapshot_run_no_snaps(t *testing.T) {
+	var b Builder
+	config := testConfig() //from builder_test
+
+	//Set some snapshot settings
+	config["ebs_volumes"] = []map[string]interface{}{
+		{
+			"device_name":           "/dev/xvdb",
+			"volume_size":           "32",
+			"delete_on_termination": true,
+			"snapshot_volume":       false,
+		},
+	}
+
+	generatedData, warnings, err := b.Prepare(config)
+	if len(warnings) > 0 {
+		t.Fatalf("bad: %#v", warnings)
+	}
+	if err != nil {
+		t.Fatalf("should not have error: %s", err)
+	}
+	if len(generatedData) == 0 {
+		t.Fatalf("Generated data should not be empty")
+	}
+
+	state := tState(t)
+
+	accessConfig := common.FakeAccessConfig()
+
+	step := stepSnapshotEBSVolumes{
+		PollingConfig: new(common.AWSPollingConfig),
+		AccessConfig:  accessConfig,
+		VolumeMapping: b.config.VolumeMappings,
+		Ctx:           b.config.ctx,
+	}
+
+	step.Run(context.Background(), state)
+
+	if len(step.snapshotMap) != 0 {
+		t.Fatalf("Shouldn't have snapshotted any volumes")
+	}
 }