Merge pull request #779 from wintoncode/auto_device_lookup

Dynamically lookup the RootDeviceName of the AMI being used

humans.txt

@@ -38,6 +38,7 @@ Dann Church             @D3nn
 Roli Schilter           @rndstr
 Mitchel Humpherys       @mgalgs
 Fred Cox                @mcfedr
+Adam Johnson            @adamjohnson01
 
 /* Thanks */
 

pkg/ami/api.go

@@ -10,6 +10,8 @@ import (
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/aws/aws-sdk-go/service/ec2/ec2iface"
 
+	"fmt"
+
 	api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5"
 )
 
@@ -39,28 +41,38 @@ var ImageClasses = []string{
 	"ImageClassGPU",
 }
 
-// IsAvailable checks if a given AMI ID is available in AWS EC2
-func IsAvailable(api ec2iface.EC2API, id string) (bool, error) {
+// Use checks if a given AMI ID is available in AWS EC2 as well as checking and populating RootDevice information
+func Use(ec2api ec2iface.EC2API, ng *api.NodeGroup) error {
 	input := &ec2.DescribeImagesInput{
-		ImageIds: []*string{&id},
+		ImageIds: []*string{&ng.AMI},
 	}
 
-	output, err := api.DescribeImages(input)
+	output, err := ec2api.DescribeImages(input)
 	if err != nil {
-		return false, errors.Wrapf(err, "unable to find %q", id)
+		return errors.Wrapf(err, "unable to find image %q", ng.AMI)
 	}
 
+	// This will never return more than one as we are looking up a single ami id
 	if len(output.Images) < 1 {
-		return false, nil
+		return NewErrNotFound(ng.AMI)
+	}
+
+	// Instance-store AMIs cannot have their root volume size managed
+	if *output.Images[0].RootDeviceType == "instance-store" {
+		return fmt.Errorf("%q is an instance-store AMI and EBS block device mappings not supported for instance-store AMIs", ng.AMI)
+	}
+
+	if *output.Images[0].RootDeviceType == "ebs" && !api.IsSetAndNonEmptyString(ng.VolumeName) {
+		ng.VolumeName = output.Images[0].RootDeviceName
 	}
 
-	return *output.Images[0].State == "available", nil
+	return nil
 }
 
 // FindImage will get the AMI to use for the EKS nodes by querying AWS EC2 API.
 // It will only look for images with a status of available and it will pick the
 // image with the newest creation date.
-func FindImage(api ec2iface.EC2API, ownerAccount, namePattern string) (string, error) {
+func FindImage(ec2api ec2iface.EC2API, ownerAccount, namePattern string) (string, error) {
 	input := &ec2.DescribeImagesInput{
 		Owners: []*string{&ownerAccount},
 		Filters: []*ec2.Filter{
@@ -87,7 +99,7 @@ func FindImage(api ec2iface.EC2API, ownerAccount, namePattern string) (string, e
 		},
 	}
 
-	output, err := api.DescribeImages(input)
+	output, err := ec2api.DescribeImages(input)
 	if err != nil {
 		return "", errors.Wrapf(err, "error querying AWS for images")
 	}

pkg/apis/eksctl.io/v1alpha5/defaults.go

@@ -45,10 +45,8 @@ func SetNodeGroupDefaults(_ int, ng *NodeGroup) error {
 		}
 	}
 
-	if ng.VolumeSize > 0 {
-		if ng.VolumeType == "" {
-			ng.VolumeType = DefaultNodeVolumeType
-		}
+	if !IsSetAndNonEmptyString(ng.VolumeType) {
+		ng.VolumeType = &DefaultNodeVolumeType
 	}
 
 	if ng.IAM == nil {

pkg/apis/eksctl.io/v1alpha5/defaults_test.go

@@ -14,6 +14,7 @@ var _ = Describe("Default settings", func() {
 
 		It("Providing an SSH key enables SSH", func() {
 			testNodeGroup := NodeGroup{
+				VolumeSize: &DefaultNodeVolumeSize,
 				SSH: &NodeGroupSSH{
 					Allow:         Disabled(),
 					PublicKeyPath: &testKeyPath,
@@ -27,6 +28,7 @@ var _ = Describe("Default settings", func() {
 
 		It("Enabling SSH without a key uses default key", func() {
 			testNodeGroup := NodeGroup{
+				VolumeSize: &DefaultNodeVolumeSize,
 				SSH: &NodeGroupSSH{
 					Allow: Enabled(),
 				},

pkg/apis/eksctl.io/v1alpha5/types.go

@@ -78,8 +78,6 @@ const (
 	// DefaultNodeCount defines the default number of nodes to be created
 	DefaultNodeCount = 2
 
-	// DefaultNodeVolumeType defines the default root volume type to use
-	DefaultNodeVolumeType = NodeVolumeTypeGP2
 	// NodeVolumeTypeGP2 is General Purpose SSD
 	NodeVolumeTypeGP2 = "gp2"
 	// NodeVolumeTypeIO1 is Provisioned IOPS SSD
@@ -128,6 +126,12 @@ var (
 
 	// DefaultNodeSSHPublicKeyPath is the default path to SSH public key
 	DefaultNodeSSHPublicKeyPath = "~/.ssh/id_rsa.pub"
+
+	// DefaultNodeVolumeType defines the default root volume type to use
+	DefaultNodeVolumeType = NodeVolumeTypeGP2
+
+	// DefaultNodeVolumeSize defines the default root volume size
+	DefaultNodeVolumeSize = 0
 )
 
 // Enabled return pointer to true value
@@ -338,8 +342,8 @@ func (c *ClusterConfig) NewNodeGroup() *NodeGroup {
 		},
 		DesiredCapacity: nil,
 		InstanceType:    DefaultNodeType,
-		VolumeSize:      0,
-		VolumeType:      DefaultNodeVolumeType,
+		VolumeSize:      &DefaultNodeVolumeSize,
+		VolumeType:      &DefaultNodeVolumeType,
 		IAM: &NodeGroupIAM{
 			WithAddonPolicies: NodeGroupIAMAddonPolicies{
 				ImageBuilder: Disabled(),
@@ -391,9 +395,11 @@ type NodeGroup struct {
 	MaxSize *int `json:"maxSize,omitempty"`
 
 	// +optional
-	VolumeSize int `json:"volumeSize"`
+	VolumeSize *int `json:"volumeSize"`
+	// +optional
+	VolumeType *string `json:"volumeType"`
 	// +optional
-	VolumeType string `json:"volumeType"`
+	VolumeName *string `json:"volumeName,omitempty"`
 	// +optional
 	MaxPodsPerNode int `json:"maxPodsPerNode,omitempty"`
 

pkg/apis/eksctl.io/v1alpha5/validation.go

@@ -120,24 +120,32 @@ func ValidateNodeGroup(i int, ng *NodeGroup) error {
 		return fmt.Errorf("%s.name must be set", path)
 	}
 
-	if ng.IAM == nil {
-		return nil
+	if ng.VolumeSize == nil {
+		if IsSetAndNonEmptyString(ng.VolumeType) {
+			return fmt.Errorf("%s.volumeType can not be set without %s.volumeSize", path, path)
+		}
+		if IsSetAndNonEmptyString(ng.VolumeName) {
+			return fmt.Errorf("%s.volumeName can not be set without %s.volumeSize", path, path)
+		}
 	}
 
-	if err := validateNodeGroupIAM(i, ng, ng.IAM.InstanceProfileARN, "instanceProfileARN", path); err != nil {
-		return err
-	}
-	if err := validateNodeGroupIAM(i, ng, ng.IAM.InstanceRoleARN, "instanceRoleARN", path); err != nil {
-		return err
-	}
+	if ng.IAM != nil {
+		if err := validateNodeGroupIAM(i, ng, ng.IAM.InstanceProfileARN, "instanceProfileARN", path); err != nil {
+			return err
+		}
+		if err := validateNodeGroupIAM(i, ng, ng.IAM.InstanceRoleARN, "instanceRoleARN", path); err != nil {
+			return err
+		}
 
-	if err := ValidateNodeGroupLabels(ng); err != nil {
-		return err
-	}
+		if err := ValidateNodeGroupLabels(ng); err != nil {
+			return err
+		}
 
-	if err := validateNodeGroupSSH(ng.SSH); err != nil {
-		return fmt.Errorf("only one ssh public key can be specified per node-group")
+		if err := validateNodeGroupSSH(ng.SSH); err != nil {
+			return fmt.Errorf("only one ssh public key can be specified per node-group")
+		}
 	}
+
 	return nil
 }
 

pkg/cfn/builder/api_test.go

@@ -246,8 +246,12 @@ var _ = Describe("CloudFormation template builder API", func() {
 		ng.Name = "ng-abcd1234"
 		ng.InstanceType = "t2.medium"
 		ng.AMIFamily = "AmazonLinux2"
-		ng.VolumeSize = 2
-		ng.VolumeType = api.NodeVolumeTypeIO1
+		ng.VolumeSize = new(int)
+		*ng.VolumeSize = 2
+		ng.VolumeType = new(string)
+		*ng.VolumeType = api.NodeVolumeTypeIO1
+		ng.VolumeName = new(string)
+		*ng.VolumeName = "/dev/xvda"
 
 		if withFullVPC {
 			cfg.VPC = testVPC()
@@ -321,7 +325,6 @@ var _ = Describe("CloudFormation template builder API", func() {
 	}
 
 	Describe("GetAllOutputsFromClusterStack", func() {
-		defaultSSHKeyPath := "~/.ssh/id_rsa.pub"
 		expected := &api.ClusterConfig{
 			TypeMeta: api.ClusterConfigTypeMeta(),
 			Metadata: &api.ClusterMeta{
@@ -352,8 +355,9 @@ var _ = Describe("CloudFormation template builder API", func() {
 						AttachIDs:  []string{},
 					},
 					DesiredCapacity: nil,
-					VolumeSize:      2,
-					VolumeType:      api.NodeVolumeTypeIO1,
+					VolumeSize:      aws.Int(2),
+					VolumeType:      aws.String(api.NodeVolumeTypeIO1),
+					VolumeName:      aws.String("/dev/xvda"),
 					IAM: &api.NodeGroupIAM{
 						WithAddonPolicies: api.NodeGroupIAMAddonPolicies{
 							ImageBuilder: api.Disabled(),
@@ -368,7 +372,7 @@ var _ = Describe("CloudFormation template builder API", func() {
 					},
 					SSH: &api.NodeGroupSSH{
 						Allow:         api.Disabled(),
-						PublicKeyPath: &defaultSSHKeyPath,
+						PublicKeyPath: &api.DefaultNodeSSHPublicKeyPath,
 					},
 				},
 			},
@@ -949,7 +953,7 @@ var _ = Describe("CloudFormation template builder API", func() {
 	Context("NodeGroupEBS", func() {
 		cfg, ng := newClusterConfigAndNodegroup(true)
 
-		ng.VolumeSize = 0
+		ng.VolumeSize = nil
 		ng.IAM.WithAddonPolicies.EBS = api.Enabled()
 
 		build(cfg, "eksctl-test-ebs-cluster", ng)
@@ -994,7 +998,7 @@ var _ = Describe("CloudFormation template builder API", func() {
 	Context("NodeGroupFSX", func() {
 		cfg, ng := newClusterConfigAndNodegroup(true)
 
-		ng.VolumeSize = 0
+		ng.VolumeSize = nil
 		ng.IAM.WithAddonPolicies.FSX = api.Enabled()
 
 		build(cfg, "eksctl-test-fsx-cluster", ng)
@@ -1028,7 +1032,7 @@ var _ = Describe("CloudFormation template builder API", func() {
 	Context("NodeGroupEFS", func() {
 		cfg, ng := newClusterConfigAndNodegroup(true)
 
-		ng.VolumeSize = 0
+		ng.VolumeSize = nil
 		ng.IAM.WithAddonPolicies.EFS = api.Enabled()
 
 		build(cfg, "eksctl-test-efs-cluster", ng)

pkg/cfn/builder/nodegroup.go

@@ -126,12 +126,12 @@ func (n *NodeGroupResourceSet) addResourcesForNodeGroup() error {
 		launchTemplateData.KeyName = gfn.NewString(*n.spec.SSH.PublicKeyName)
 	}
 
-	if n.spec.VolumeSize > 0 {
+	if volumeSize := n.spec.VolumeSize; volumeSize != nil && *volumeSize > 0 {
 		launchTemplateData.BlockDeviceMappings = []gfn.AWSEC2LaunchTemplate_BlockDeviceMapping{{
-			DeviceName: gfn.NewString("/dev/xvda"),
+			DeviceName: gfn.NewString(*n.spec.VolumeName),
 			Ebs: &gfn.AWSEC2LaunchTemplate_Ebs{
-				VolumeSize: gfn.NewInteger(n.spec.VolumeSize),
-				VolumeType: gfn.NewString(n.spec.VolumeType),
+				VolumeSize: gfn.NewInteger(*volumeSize),
+				VolumeType: gfn.NewString(*n.spec.VolumeType),
 			},
 		}}
 	}

pkg/ctl/cmdutils/nodegroup_filter_test.go

@@ -243,10 +243,13 @@ func newClusterConfig() *api.ClusterConfig {
 func addGroupA(cfg *api.ClusterConfig) {
 	var ng *api.NodeGroup
 
+	ng1aVolSize := 768
+	ng1aVolType := api.NodeVolumeTypeIO1
+
 	ng = cfg.NewNodeGroup()
 	ng.Name = "test-ng1a"
-	ng.VolumeSize = 768
-	ng.VolumeType = "io1"
+	ng.VolumeSize = &ng1aVolSize
+	ng.VolumeType = &ng1aVolType
 	ng.IAM.AttachPolicyARNs = []string{"foo"}
 	ng.Labels = map[string]string{"group": "a", "seq": "1"}
 	ng.SSH = nil
@@ -284,9 +287,11 @@ func addGroupB(cfg *api.ClusterConfig) {
 	ng.Labels = map[string]string{"group": "b", "seq": "1"}
 	ng.SSH = nil
 
+	ng3bVolSize := 192
+
 	ng = cfg.NewNodeGroup()
 	ng.Name = "test-ng3b"
-	ng.VolumeSize = 192
+	ng.VolumeSize = &ng3bVolSize
 	ng.SecurityGroups.AttachIDs = []string{"sg-1", "sg-2"}
 	ng.SecurityGroups.WithLocal = api.Disabled()
 	ng.Labels = map[string]string{"group": "b", "seq": "1"}

pkg/ctl/cmdutils/nodegroup_flags.go

@@ -31,8 +31,8 @@ func AddCommonCreateNodeGroupFlags(cmd *cobra.Command, fs *pflag.FlagSet, p *api
 		}
 	})
 
-	fs.IntVar(&ng.VolumeSize, "node-volume-size", ng.VolumeSize, "node volume size in GB")
-	fs.StringVar(&ng.VolumeType, "node-volume-type", ng.VolumeType, fmt.Sprintf("node volume type (valid options: %s)", strings.Join(api.SupportedNodeVolumeTypes(), ", ")))
+	fs.IntVar(ng.VolumeSize, "node-volume-size", *ng.VolumeSize, "node volume size in GB")
+	fs.StringVar(ng.VolumeType, "node-volume-type", *ng.VolumeType, fmt.Sprintf("node volume type (valid options: %s)", strings.Join(api.SupportedNodeVolumeTypes(), ", ")))
 
 	fs.IntVar(&ng.MaxPodsPerNode, "max-pods-per-node", 0, "maximum number of pods per node (set automatically if unspecified)")
 

pkg/eks/api.go

@@ -247,17 +247,9 @@ func (c *ClusterProvider) EnsureAMI(version string, ng *api.NodeGroup) error {
 		ng.AMI = id
 	}
 
-	// Check the AMI is available
-	available, err := ami.IsAvailable(c.Provider.EC2(), ng.AMI)
-	if err != nil {
-		return errors.Wrapf(err, "%s is not available", ng.AMI)
-	}
+	// Check the AMI is available and populate RootDevice information
+	return ami.Use(c.Provider.EC2(), ng)
 
-	if !available {
-		return ami.NewErrNotFound(ng.AMI)
-	}
-
-	return nil
 }
 
 // SetNodeLabels initialises and validate node labels based on cluster and nodegroup names