Skip to content

Commit

Permalink
Add support for multiple block device mappings
Browse files Browse the repository at this point in the history
  • Loading branch information
Alexander Demichev committed Jun 17, 2020
1 parent cca9ed8 commit 6f41370
Show file tree
Hide file tree
Showing 3 changed files with 161 additions and 55 deletions.
74 changes: 48 additions & 26 deletions pkg/actuators/machine/instances.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package machine

import (
"encoding/base64"
"errors"
"fmt"
"sort"
"strconv"
Expand Down Expand Up @@ -181,9 +182,11 @@ func getAMI(AMI awsproviderv1.AWSResourceReference, client awsclient.Client) (*s
return nil, fmt.Errorf("AMI ID or AMI filters need to be specified")
}

func getBlockDeviceMappings(blockDeviceMappings []awsproviderv1.BlockDeviceMappingSpec, AMI string, client awsclient.Client) ([]*ec2.BlockDeviceMapping, error) {
if len(blockDeviceMappings) == 0 || blockDeviceMappings[0].EBS == nil {
return []*ec2.BlockDeviceMapping{}, nil
func getBlockDeviceMappings(blockDeviceMappingSpecs []awsproviderv1.BlockDeviceMappingSpec, AMI string, client awsclient.Client) ([]*ec2.BlockDeviceMapping, error) {
blockDeviceMappings := make([]*ec2.BlockDeviceMapping, 0)

if len(blockDeviceMappingSpecs) == 0 {
return blockDeviceMappings, nil
}

// Get image object to get the RootDeviceName
Expand All @@ -199,33 +202,52 @@ func getBlockDeviceMappings(blockDeviceMappings []awsproviderv1.BlockDeviceMappi
klog.Errorf("No image for given AMI was found")
return nil, fmt.Errorf("no image for given AMI not found")
}
deviceName := describeAMIResult.Images[0].RootDeviceName

// Only support one blockDeviceMapping
volumeSize := blockDeviceMappings[0].EBS.VolumeSize
volumeType := blockDeviceMappings[0].EBS.VolumeType
blockDeviceMapping := ec2.BlockDeviceMapping{
DeviceName: deviceName,
Ebs: &ec2.EbsBlockDevice{
VolumeSize: volumeSize,
VolumeType: volumeType,
Encrypted: blockDeviceMappings[0].EBS.Encrypted,
},
}

if aws.StringValue(blockDeviceMappings[0].EBS.KMSKey.ID) != "" {
klog.V(3).Infof("Using KMS key ID %q for encrypting EBS volume", *blockDeviceMappings[0].EBS.KMSKey.ID)
blockDeviceMapping.Ebs.KmsKeyId = blockDeviceMappings[0].EBS.KMSKey.ID
} else if aws.StringValue(blockDeviceMappings[0].EBS.KMSKey.ARN) != "" {
klog.V(3).Info("Using KMS key ARN for encrypting EBS volume") // ARN usually have account ids, therefore are sensitive data so shouldn't log the value
blockDeviceMapping.Ebs.KmsKeyId = blockDeviceMappings[0].EBS.KMSKey.ARN
}
rootDeviceFound := false
for _, blockDeviceMappingSpec := range blockDeviceMappingSpecs {
if blockDeviceMappingSpec.EBS == nil {
continue
}

deviceName := blockDeviceMappingSpec.DeviceName
volumeSize := blockDeviceMappingSpec.EBS.VolumeSize
volumeType := blockDeviceMappingSpec.EBS.VolumeType
deleteOnTermination := true

if blockDeviceMappingSpec.DeviceName == nil {
if rootDeviceFound {
return nil, errors.New("non root device must have name")
}
rootDeviceFound = true
deviceName = describeAMIResult.Images[0].RootDeviceName
}

blockDeviceMapping := ec2.BlockDeviceMapping{
DeviceName: deviceName,
Ebs: &ec2.EbsBlockDevice{
VolumeSize: volumeSize,
VolumeType: volumeType,
Encrypted: blockDeviceMappingSpec.EBS.Encrypted,
DeleteOnTermination: &deleteOnTermination,
},
}

if *volumeType == ec2.VolumeTypeIo1 {
blockDeviceMapping.Ebs.Iops = blockDeviceMappingSpec.EBS.Iops
}

if aws.StringValue(blockDeviceMappingSpec.EBS.KMSKey.ID) != "" {
klog.V(3).Infof("Using KMS key ID %q for encrypting EBS volume", *blockDeviceMappingSpec.EBS.KMSKey.ID)
blockDeviceMapping.Ebs.KmsKeyId = blockDeviceMappingSpec.EBS.KMSKey.ID
} else if aws.StringValue(blockDeviceMappingSpec.EBS.KMSKey.ARN) != "" {
klog.V(3).Info("Using KMS key ARN for encrypting EBS volume") // ARN usually have account ids, therefore are sensitive data so shouldn't log the value
blockDeviceMapping.Ebs.KmsKeyId = blockDeviceMappingSpec.EBS.KMSKey.ARN
}

if *volumeType == "io1" {
blockDeviceMapping.Ebs.Iops = blockDeviceMappings[0].EBS.Iops
blockDeviceMappings = append(blockDeviceMappings, &blockDeviceMapping)
}

return []*ec2.BlockDeviceMapping{&blockDeviceMapping}, nil
return blockDeviceMappings, nil
}

func launchInstance(machine *machinev1.Machine, machineProviderConfig *awsproviderv1.AWSMachineProviderConfig, userData []byte, client awsclient.Client) (*ec2.Instance, error) {
Expand Down
139 changes: 111 additions & 28 deletions pkg/actuators/machine/instances_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,11 @@ func TestBuildEC2Filters(t *testing.T) {
func TestGetBlockDeviceMappings(t *testing.T) {
rootDeviceName := "/dev/sda1"
volumeSize := int64(16384)
deviceName2 := "/dev/sda2"
volumeSize2 := int64(16385)
deleteOnTermination := true
volumeType := "ssd"

mockCtrl := gomock.NewController(t)
mockAWSClient := mockaws.NewMockClient(mockCtrl)
mockAWSClient.EXPECT().DescribeImages(gomock.Any()).Return(&ec2.DescribeImagesOutput{
Expand All @@ -106,50 +110,129 @@ func TestGetBlockDeviceMappings(t *testing.T) {
},
}, nil).AnyTimes()

oneBlockDevice := []awsproviderv1.BlockDeviceMappingSpec{
{
DeviceName: &rootDeviceName,
EBS: &awsproviderv1.EBSBlockDeviceSpec{
VolumeSize: &volumeSize,
VolumeType: &volumeType,
},
NoDevice: nil,
VirtualName: nil,
},
}

oneExpectedBlockDevice := []*ec2.BlockDeviceMapping{
{
DeviceName: &rootDeviceName,
Ebs: &ec2.EbsBlockDevice{
VolumeSize: &volumeSize,
VolumeType: &volumeType,
DeleteOnTermination: &deleteOnTermination,
},
NoDevice: nil,
VirtualName: nil,
},
}

blockDevices := []awsproviderv1.BlockDeviceMappingSpec{
{
DeviceName: &rootDeviceName,
EBS: &awsproviderv1.EBSBlockDeviceSpec{
VolumeSize: &volumeSize,
VolumeType: &volumeType,
},
NoDevice: nil,
VirtualName: nil,
},
{
DeviceName: &deviceName2,
EBS: &awsproviderv1.EBSBlockDeviceSpec{
VolumeSize: &volumeSize2,
VolumeType: &volumeType,
},
NoDevice: nil,
VirtualName: nil,
},
}

twoExpectedDevices := []*ec2.BlockDeviceMapping{
{
DeviceName: &rootDeviceName,
Ebs: &ec2.EbsBlockDevice{
VolumeSize: &volumeSize,
VolumeType: &volumeType,
DeleteOnTermination: &deleteOnTermination,
},
NoDevice: nil,
VirtualName: nil,
},
{
DeviceName: &deviceName2,
Ebs: &ec2.EbsBlockDevice{
VolumeSize: &volumeSize2,
VolumeType: &volumeType,
DeleteOnTermination: &deleteOnTermination,
},
NoDevice: nil,
VirtualName: nil,
},
}

blockDevicesOneEmptyName := make([]awsproviderv1.BlockDeviceMappingSpec, len(blockDevices))
copy(blockDevicesOneEmptyName, blockDevices)
blockDevicesOneEmptyName[0].DeviceName = nil

blockDevicesTwoEmptyNames := make([]awsproviderv1.BlockDeviceMappingSpec, len(blockDevicesOneEmptyName))
copy(blockDevicesTwoEmptyNames, blockDevicesOneEmptyName)
blockDevicesTwoEmptyNames[1].DeviceName = nil

testCases := []struct {
description string
blockDevices []awsproviderv1.BlockDeviceMappingSpec
expected []*ec2.BlockDeviceMapping
expectedErr bool
}{
{
description: "When it gets an empty blockDevices list",
blockDevices: []awsproviderv1.BlockDeviceMappingSpec{},
expected: []*ec2.BlockDeviceMapping{},
},
{
description: "When it gets one blockDevice",
blockDevices: []awsproviderv1.BlockDeviceMappingSpec{
{
DeviceName: &rootDeviceName,
EBS: &awsproviderv1.EBSBlockDeviceSpec{
VolumeSize: &volumeSize,
VolumeType: &volumeType,
},
NoDevice: nil,
VirtualName: nil,
},
},
expected: []*ec2.BlockDeviceMapping{
{
DeviceName: &rootDeviceName,
Ebs: &ec2.EbsBlockDevice{
VolumeSize: &volumeSize,
VolumeType: &volumeType,
},
NoDevice: nil,
VirtualName: nil,
},
},
description: "When it gets one blockDevice",
blockDevices: oneBlockDevice,
expected: oneExpectedBlockDevice,
},
{
description: "When it gets two blockDevices",
blockDevices: blockDevices,
expected: twoExpectedDevices,
},
{
description: "When it gets two blockDevices and one with empty device name",
blockDevices: blockDevicesOneEmptyName,
expected: twoExpectedDevices,
},
{
description: "Fail when it gets two blockDevices and two with empty device name",
blockDevices: blockDevicesTwoEmptyNames,
expectedErr: true,
},
}

for _, tc := range testCases {
got, err := getBlockDeviceMappings(tc.blockDevices, "existing-AMI", mockAWSClient)
if err != nil {
t.Errorf("error when calling getBlockDeviceMappings: %v", err)
}
if !reflect.DeepEqual(got, tc.expected) {
t.Errorf("Case: %s. Got: %v, expected: %v", tc.description, got, tc.expected)
if tc.expectedErr {
if err == nil {
t.Error("Expected error")
}
} else {
if err != nil {
t.Errorf("error when calling getBlockDeviceMappings: %v", err)
}
if !reflect.DeepEqual(got, tc.expected) {
t.Errorf("Got: %v, expected: %v", got, tc.expected)
}
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/apis/awsprovider/v1beta1/awsproviderconfig_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ type AWSMachineProviderConfig struct {
// should be added once it is created.
LoadBalancers []LoadBalancerReference `json:"loadBalancers,omitempty"`

// BlockDevices is the set of block device mapping associated to this instance
// BlockDevices is the set of block device mapping associated to this instance,
// block device without a name will be used as a root device and only one device without a name is allowed
// https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/block-device-mapping-concepts.html
BlockDevices []BlockDeviceMappingSpec `json:"blockDevices,omitempty"`

Expand Down

0 comments on commit 6f41370

Please sign in to comment.