diff --git a/humans.txt b/humans.txt index dcaa20fc94..12b1296c8d 100644 --- a/humans.txt +++ b/humans.txt @@ -38,6 +38,7 @@ Dann Church @D3nn Roli Schilter @rndstr Mitchel Humpherys @mgalgs Fred Cox @mcfedr +Adam Johnson @adamjohnson01 /* Thanks */ diff --git a/pkg/ami/api.go b/pkg/ami/api.go index 7f6ab4e4f9..620e52bcde 100644 --- a/pkg/ami/api.go +++ b/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") } diff --git a/pkg/apis/eksctl.io/v1alpha5/defaults.go b/pkg/apis/eksctl.io/v1alpha5/defaults.go index e74b5f69d3..76e8429efb 100644 --- a/pkg/apis/eksctl.io/v1alpha5/defaults.go +++ b/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 { diff --git a/pkg/apis/eksctl.io/v1alpha5/defaults_test.go b/pkg/apis/eksctl.io/v1alpha5/defaults_test.go index d9beafcb9f..dd8b1437cc 100644 --- a/pkg/apis/eksctl.io/v1alpha5/defaults_test.go +++ b/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(), }, diff --git a/pkg/apis/eksctl.io/v1alpha5/types.go b/pkg/apis/eksctl.io/v1alpha5/types.go index d12bc9e30e..69c48f0910 100644 --- a/pkg/apis/eksctl.io/v1alpha5/types.go +++ b/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"` diff --git a/pkg/apis/eksctl.io/v1alpha5/validation.go b/pkg/apis/eksctl.io/v1alpha5/validation.go index 06a0345860..c036eb0aeb 100644 --- a/pkg/apis/eksctl.io/v1alpha5/validation.go +++ b/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 } diff --git a/pkg/cfn/builder/api_test.go b/pkg/cfn/builder/api_test.go index dbffda6a19..c1d8ae5f48 100644 --- a/pkg/cfn/builder/api_test.go +++ b/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) diff --git a/pkg/cfn/builder/nodegroup.go b/pkg/cfn/builder/nodegroup.go index 0db0b9ed22..f522a3c12c 100644 --- a/pkg/cfn/builder/nodegroup.go +++ b/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), }, }} } diff --git a/pkg/ctl/cmdutils/nodegroup_filter_test.go b/pkg/ctl/cmdutils/nodegroup_filter_test.go index 03ed632d0d..46c017faac 100644 --- a/pkg/ctl/cmdutils/nodegroup_filter_test.go +++ b/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"} diff --git a/pkg/ctl/cmdutils/nodegroup_flags.go b/pkg/ctl/cmdutils/nodegroup_flags.go index 9c95259a37..382d40b735 100644 --- a/pkg/ctl/cmdutils/nodegroup_flags.go +++ b/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)") diff --git a/pkg/eks/api.go b/pkg/eks/api.go index 8d0f39764e..873f62f8f9 100644 --- a/pkg/eks/api.go +++ b/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