Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamically lookup the RootDeviceName of the AMI being used #779

Merged
merged 10 commits into from
May 10, 2019
1 change: 1 addition & 0 deletions humans.txt
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ Dann Church @D3nn
Roli Schilter @rndstr
Mitchel Humpherys @mgalgs
Fred Cox @mcfedr
Adam Johnson @adamjohnson01

/* Thanks */

Expand Down
23 changes: 17 additions & 6 deletions pkg/ami/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/aws/aws-sdk-go/service/ec2/ec2iface"

api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5"
"fmt"
)

const (
Expand Down Expand Up @@ -39,22 +40,32 @@ 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(api ec2iface.EC2API, ng *api.NodeGroup) error {
input := &ec2.DescribeImagesInput{
ImageIds: []*string{&id},
ImageIds: []*string{&ng.AMI},
}

output, err := api.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)
}

return *output.Images[0].State == "available", nil
// 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" {
ng.VolumeName = *output.Images[0].RootDeviceName
}

return nil
}

// FindImage will get the AMI to use for the EKS nodes by querying AWS EC2 API.
Expand Down
6 changes: 3 additions & 3 deletions pkg/apis/eksctl.io/v1alpha5/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ func SetNodeGroupDefaults(_ int, ng *NodeGroup) error {
}
}

if ng.VolumeSize > 0 {
if ng.VolumeType == "" {
ng.VolumeType = DefaultNodeVolumeType
if *ng.VolumeSize > 0 {
if *ng.VolumeType == "" {
*ng.VolumeType = DefaultNodeVolumeType
}
}

Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/eksctl.io/v1alpha5/defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(),
},
Expand Down
19 changes: 13 additions & 6 deletions pkg/apis/eksctl.io/v1alpha5/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -128,6 +126,13 @@ 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
Expand Down Expand Up @@ -338,8 +343,8 @@ func (c *ClusterConfig) NewNodeGroup() *NodeGroup {
},
DesiredCapacity: nil,
InstanceType: DefaultNodeType,
VolumeSize: 0,
VolumeType: DefaultNodeVolumeType,
VolumeSize: &DefaultNodeVolumeSize,
VolumeType: &DefaultNodeVolumeType,
IAM: &NodeGroupIAM{
WithAddonPolicies: NodeGroupIAMAddonPolicies{
ImageBuilder: Disabled(),
Expand Down Expand Up @@ -391,9 +396,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"`

Expand Down
4 changes: 4 additions & 0 deletions pkg/apis/eksctl.io/v1alpha5/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ func ValidateNodeGroup(i int, ng *NodeGroup) error {
return nil
}

if ng.VolumeSize == nil && IsSetAndNonEmptyString(ng.VolumeType) {
return fmt.Errorf("VolumeType can not be set without VolumeSize")
}

if err := validateNodeGroupIAM(i, ng, ng.IAM.InstanceProfileARN, "instanceProfileARN", path); err != nil {
return err
}
Expand Down
24 changes: 15 additions & 9 deletions pkg/cfn/builder/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,12 +242,16 @@ var _ = Describe("CloudFormation template builder API", func() {
Endpoint: endpoint,
}

volType := api.NodeVolumeTypeIO1
volSize := 2

cfg.AvailabilityZones = testAZs
ng.Name = "ng-abcd1234"
ng.InstanceType = "t2.medium"
ng.AMIFamily = "AmazonLinux2"
ng.VolumeSize = 2
ng.VolumeType = api.NodeVolumeTypeIO1
ng.VolumeSize = &volSize
ng.VolumeType = &volType
ng.VolumeName = "/dev/xvda"

if withFullVPC {
cfg.VPC = testVPC()
Expand Down Expand Up @@ -321,7 +325,8 @@ var _ = Describe("CloudFormation template builder API", func() {
}

Describe("GetAllOutputsFromClusterStack", func() {
defaultSSHKeyPath := "~/.ssh/id_rsa.pub"
volSize := 2
volType := api.NodeVolumeTypeIO1
expected := &api.ClusterConfig{
TypeMeta: api.ClusterConfigTypeMeta(),
Metadata: &api.ClusterMeta{
Expand Down Expand Up @@ -352,8 +357,9 @@ var _ = Describe("CloudFormation template builder API", func() {
AttachIDs: []string{},
},
DesiredCapacity: nil,
VolumeSize: 2,
VolumeType: api.NodeVolumeTypeIO1,
VolumeSize: &volSize,
VolumeType: &volType,
VolumeName: "/dev/xvda",
IAM: &api.NodeGroupIAM{
WithAddonPolicies: api.NodeGroupIAMAddonPolicies{
ImageBuilder: api.Disabled(),
Expand All @@ -368,7 +374,7 @@ var _ = Describe("CloudFormation template builder API", func() {
},
SSH: &api.NodeGroupSSH{
Allow: api.Disabled(),
PublicKeyPath: &defaultSSHKeyPath,
PublicKeyPath: &api.DefaultNodeSSHPublicKeyPath,
},
},
},
Expand Down Expand Up @@ -949,7 +955,7 @@ var _ = Describe("CloudFormation template builder API", func() {
Context("NodeGroupEBS", func() {
cfg, ng := newClusterConfigAndNodegroup(true)

ng.VolumeSize = 0
*ng.VolumeSize = 0
ng.IAM.WithAddonPolicies.EBS = api.Enabled()

build(cfg, "eksctl-test-ebs-cluster", ng)
Expand Down Expand Up @@ -994,7 +1000,7 @@ var _ = Describe("CloudFormation template builder API", func() {
Context("NodeGroupFSX", func() {
cfg, ng := newClusterConfigAndNodegroup(true)

ng.VolumeSize = 0
*ng.VolumeSize = 0
ng.IAM.WithAddonPolicies.FSX = api.Enabled()

build(cfg, "eksctl-test-fsx-cluster", ng)
Expand Down Expand Up @@ -1028,7 +1034,7 @@ var _ = Describe("CloudFormation template builder API", func() {
Context("NodeGroupEFS", func() {
cfg, ng := newClusterConfigAndNodegroup(true)

ng.VolumeSize = 0
*ng.VolumeSize = 0
ng.IAM.WithAddonPolicies.EFS = api.Enabled()

build(cfg, "eksctl-test-efs-cluster", ng)
Expand Down
8 changes: 4 additions & 4 deletions pkg/cfn/builder/nodegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,12 @@ func (n *NodeGroupResourceSet) addResourcesForNodeGroup() error {
launchTemplateData.KeyName = gfn.NewString(*n.spec.SSH.PublicKeyName)
}

if n.spec.VolumeSize > 0 {
if *n.spec.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(*n.spec.VolumeSize),
VolumeType: gfn.NewString(*n.spec.VolumeType),
},
}}
}
Expand Down
11 changes: 8 additions & 3 deletions pkg/ctl/cmdutils/nodegroup_filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"}
Expand Down
4 changes: 2 additions & 2 deletions pkg/ctl/cmdutils/nodegroup_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)")

Expand Down
12 changes: 2 additions & 10 deletions pkg/eks/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down