Merge pull request #2672 from lightpriest/b-aws-ebs-block-device

Fix and refactor block device mapping builder

builder/amazon/common/block_device.go

@@ -30,35 +30,40 @@ func buildBlockDevices(b []BlockDevice) []*ec2.BlockDeviceMapping {
 	var blockDevices []*ec2.BlockDeviceMapping
 
 	for _, blockDevice := range b {
-		ebsBlockDevice := &ec2.EbsBlockDevice{
-			VolumeType:          aws.String(blockDevice.VolumeType),
-			VolumeSize:          aws.Int64(blockDevice.VolumeSize),
-			DeleteOnTermination: aws.Bool(blockDevice.DeleteOnTermination),
+		mapping := &ec2.BlockDeviceMapping {
+			DeviceName: aws.String(blockDevice.DeviceName),
 		}
 
-		// IOPS is only valid for SSD Volumes
-		if blockDevice.VolumeType != "" && blockDevice.VolumeType != "standard" && blockDevice.VolumeType != "gp2" {
-			ebsBlockDevice.Iops = aws.Int64(blockDevice.IOPS)
-		}
+		if blockDevice.NoDevice {
+			mapping.NoDevice = aws.String("")
+		} else if strings.HasPrefix(blockDevice.VirtualName, "ephemeral") {
+			mapping.VirtualName = aws.String(blockDevice.VirtualName)
+		} else {
+			ebsBlockDevice := &ec2.EbsBlockDevice{
+				DeleteOnTermination: aws.Bool(blockDevice.DeleteOnTermination),
+			}
 
-		// You cannot specify Encrypted if you specify a Snapshot ID
-		if blockDevice.SnapshotId != "" {
-			ebsBlockDevice.SnapshotId = aws.String(blockDevice.SnapshotId)
-		} else if blockDevice.Encrypted {
-			ebsBlockDevice.Encrypted = aws.Bool(blockDevice.Encrypted)
-		}
+			if blockDevice.VolumeType != "" {
+				ebsBlockDevice.VolumeType = aws.String(blockDevice.VolumeType)
+			}
 
-		mapping := &ec2.BlockDeviceMapping{
-			DeviceName:  aws.String(blockDevice.DeviceName),
-			VirtualName: aws.String(blockDevice.VirtualName),
-		}
+			if blockDevice.VolumeSize > 0 {
+				ebsBlockDevice.VolumeSize = aws.Int64(blockDevice.VolumeSize)
+			}
 
-		if !strings.HasPrefix(blockDevice.VirtualName, "ephemeral") {
-			mapping.Ebs = ebsBlockDevice
-		}
+			// IOPS is only valid for io1 type
+			if blockDevice.VolumeType == "io1" {
+				ebsBlockDevice.Iops = aws.Int64(blockDevice.IOPS)
+			}
 
-		if blockDevice.NoDevice {
-			mapping.NoDevice = aws.String("")
+			// You cannot specify Encrypted if you specify a Snapshot ID
+			if blockDevice.SnapshotId != "" {
+				ebsBlockDevice.SnapshotId = aws.String(blockDevice.SnapshotId)
+			} else if blockDevice.Encrypted {
+				ebsBlockDevice.Encrypted = aws.Bool(blockDevice.Encrypted)
+			}
+
+			mapping.Ebs = ebsBlockDevice
 		}
 
 		blockDevices = append(blockDevices, mapping)

builder/amazon/common/block_device_test.go

@@ -24,7 +24,6 @@ func TestBlockDevice(t *testing.T) {
 
 			Result: &ec2.BlockDeviceMapping{
 				DeviceName:  aws.String("/dev/sdb"),
-				VirtualName: aws.String(""),
 				Ebs: &ec2.EbsBlockDevice{
 					SnapshotId:          aws.String("snap-1234"),
 					VolumeType:          aws.String("standard"),
@@ -41,9 +40,7 @@ func TestBlockDevice(t *testing.T) {
 
 			Result: &ec2.BlockDeviceMapping{
 				DeviceName:  aws.String("/dev/sdb"),
-				VirtualName: aws.String(""),
 				Ebs: &ec2.EbsBlockDevice{
-					VolumeType:          aws.String(""),
 					VolumeSize:          aws.Int64(8),
 					DeleteOnTermination: aws.Bool(false),
 				},
@@ -60,7 +57,6 @@ func TestBlockDevice(t *testing.T) {
 
 			Result: &ec2.BlockDeviceMapping{
 				DeviceName:  aws.String("/dev/sdb"),
-				VirtualName: aws.String(""),
 				Ebs: &ec2.EbsBlockDevice{
 					VolumeType:          aws.String("io1"),
 					VolumeSize:          aws.Int64(8),
@@ -69,6 +65,25 @@ func TestBlockDevice(t *testing.T) {
 				},
 			},
 		},
+		{
+			Config: &BlockDevice{
+				DeviceName:          "/dev/sdb",
+				VolumeType:          "gp2",
+				VolumeSize:          8,
+				DeleteOnTermination: true,
+				Encrypted:           true,
+			},
+
+			Result: &ec2.BlockDeviceMapping{
+				DeviceName:  aws.String("/dev/sdb"),
+				Ebs: &ec2.EbsBlockDevice{
+					VolumeType:          aws.String("gp2"),
+					VolumeSize:          aws.Int64(8),
+					DeleteOnTermination: aws.Bool(true),
+					Encrypted:           aws.Bool(true),
+				},
+			},
+		},
 		{
 			Config: &BlockDevice{
 				DeviceName:  "/dev/sdb",
@@ -80,6 +95,17 @@ func TestBlockDevice(t *testing.T) {
 				VirtualName: aws.String("ephemeral0"),
 			},
 		},
+		{
+			Config: &BlockDevice{
+				DeviceName: "/dev/sdb",
+				NoDevice:   true,
+			},
+
+			Result: &ec2.BlockDeviceMapping{
+				DeviceName:  aws.String("/dev/sdb"),
+				NoDevice:    aws.String(""),
+			},
+		},
 	}
 
 	for _, tc := range cases {