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

Use pointers for ng.(DesiredCapacity|MinSize|MaxSize) #426

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions pkg/apis/eksctl.io/v1alpha4/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ func (c *ClusterConfig) NewNodeGroup() *NodeGroup {
WithLocal: NewBoolTrue(),
WithShared: NewBoolTrue(),
},
DesiredCapacity: DefaultNodeCount,
DesiredCapacity: nil,
InstanceType: DefaultNodeType,
VolumeSize: 0,
VolumeType: DefaultNodeVolumeType,
Expand Down Expand Up @@ -327,11 +327,11 @@ type NodeGroup struct {
SecurityGroups *NodeGroupSGs `json:"securityGroups,omitempty"`

// +optional
DesiredCapacity int `json:"desiredCapacity"`
DesiredCapacity *int `json:"desiredCapacity"`
// +optional
MinSize int `json:"minSize,omitempty"`
MinSize *int `json:"minSize,omitempty"`
// +optional
MaxSize int `json:"maxSize,omitempty"`
MaxSize *int `json:"maxSize,omitempty"`

// +optional
VolumeSize int `json:"volumeSize"`
Expand Down
2 changes: 1 addition & 1 deletion pkg/cfn/builder/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ var _ = Describe("CloudFormation template builder API", func() {
WithShared: api.NewBoolTrue(),
AttachIDs: []string{},
},
DesiredCapacity: 2,
DesiredCapacity: nil,
VolumeSize: 2,
VolumeType: api.NodeVolumeTypeIO1,
IAM: &api.NodeGroupIAM{
Expand Down
62 changes: 36 additions & 26 deletions pkg/cfn/builder/nodegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,25 +59,28 @@ func (n *NodeGroupResourceSet) AddAllResources() error {
}
n.userData = gfn.NewString(userData)

switch {
case n.spec.MinSize == 0 && n.spec.MaxSize == 0:
n.spec.MinSize = n.spec.DesiredCapacity
n.spec.MaxSize = n.spec.DesiredCapacity
case n.spec.MinSize > 0 && n.spec.MaxSize > 0:
if n.spec.DesiredCapacity == api.DefaultNodeCount {
msgPrefix := fmt.Sprintf("as --nodes-min=%d and --nodes-max=%d were given", n.spec.MinSize, n.spec.MaxSize)
if n.spec.DesiredCapacity < n.spec.MinSize {
n.spec.DesiredCapacity = n.spec.MaxSize
logger.Info("%s, --nodes=%d was set automatically as default value (--node=%d) was outside the set renge",
msgPrefix, n.spec.DesiredCapacity, api.DefaultNodeCount)
} else {
logger.Info("%s, default value of --nodes=%d was kept as it is within the set range",
msgPrefix, n.spec.DesiredCapacity)
}
// Ensure MinSize is set, as it is required by the ASG cfn resource
if n.spec.MinSize == nil {
if n.spec.DesiredCapacity == nil {
count := api.DefaultNodeCount
n.spec.MinSize = &count
logger.Info("--nodes-min=%d was set automatically", count)
} else {
n.spec.MinSize = n.spec.DesiredCapacity
}
if n.spec.DesiredCapacity > n.spec.MaxSize {
return fmt.Errorf("cannot use --nodes-max=%d and --nodes=%d at the same time", n.spec.MaxSize, n.spec.DesiredCapacity)
} else if n.spec.DesiredCapacity != nil && *n.spec.DesiredCapacity < *n.spec.MinSize {
return fmt.Errorf("cannot use --nodes-min=%d and --nodes=%d at the same time", *n.spec.MinSize, *n.spec.DesiredCapacity)
}

// Ensure MaxSize is set, as it is required by the ASG cfn resource
if n.spec.MaxSize == nil {
if n.spec.DesiredCapacity == nil {
n.spec.MaxSize = n.spec.MinSize
}
} else if n.spec.DesiredCapacity != nil && *n.spec.DesiredCapacity > *n.spec.MaxSize {
return fmt.Errorf("cannot use --nodes-max=%d and --nodes=%d at the same time", *n.spec.MaxSize, *n.spec.DesiredCapacity)
} else if *n.spec.MaxSize < *n.spec.MinSize {
return fmt.Errorf("cannot use --nodes-min=%d and --nodes-max=%d at the same time", *n.spec.MinSize, *n.spec.MaxSize)
}

n.addResourcesForIAM()
Expand Down Expand Up @@ -179,16 +182,23 @@ func (n *NodeGroupResourceSet) addResourcesForNodeGroup() error {
},
)
}
ngProps := map[string]interface{}{
"LaunchConfigurationName": refLC,
"VPCZoneIdentifier": vpcZoneIdentifier,
"Tags": tags,
}
if n.spec.DesiredCapacity != nil {
ngProps["DesiredCapacity"] = fmt.Sprintf("%d", *n.spec.DesiredCapacity)
}
if n.spec.MinSize != nil {
ngProps["MinSize"] = fmt.Sprintf("%d", *n.spec.MinSize)
}
if n.spec.MaxSize != nil {
ngProps["MaxSize"] = fmt.Sprintf("%d", *n.spec.MaxSize)
}
n.newResource("NodeGroup", &awsCloudFormationResource{
Type: "AWS::AutoScaling::AutoScalingGroup",
Properties: map[string]interface{}{
"LaunchConfigurationName": refLC,
"DesiredCapacity": fmt.Sprintf("%d", n.spec.DesiredCapacity),
"MinSize": fmt.Sprintf("%d", n.spec.MinSize),
"MaxSize": fmt.Sprintf("%d", n.spec.MaxSize),
"VPCZoneIdentifier": vpcZoneIdentifier,
"Tags": tags,
},
Type: "AWS::AutoScaling::AutoScalingGroup",
Properties: ngProps,
UpdatePolicy: map[string]map[string]string{
"AutoScalingRollingUpdate": {
"MinInstancesInService": "1",
Expand Down
6 changes: 3 additions & 3 deletions pkg/cfn/manager/nodegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func (c *StackCollection) ScaleNodeGroup(ng *api.NodeGroup) error {
currentMaxSize := gjson.Get(template, maxSizePath)
currentMinSize := gjson.Get(template, minSizePath)

if int64(ng.DesiredCapacity) == currentCapacity.Int() {
if ng.DesiredCapacity != nil && int64(*ng.DesiredCapacity) == currentCapacity.Int() {
logger.Info("desired capacity of nodegroup %q in cluster %q is already %d", ng.Name, clusterName, ng.DesiredCapacity)
return nil
}
Expand All @@ -171,7 +171,7 @@ func (c *StackCollection) ScaleNodeGroup(ng *api.NodeGroup) error {
descriptionBuffer.WriteString(fmt.Sprintf("desired capacity from %s to %d", currentCapacity.Str, ng.DesiredCapacity))

// If the desired number of nodes is less than the min then update the min
if int64(ng.DesiredCapacity) < currentMinSize.Int() {
if int64(*ng.DesiredCapacity) < currentMinSize.Int() {
newMinSize := fmt.Sprintf("%d", ng.DesiredCapacity)
template, err = sjson.Set(template, minSizePath, newMinSize)
if err != nil {
Expand All @@ -180,7 +180,7 @@ func (c *StackCollection) ScaleNodeGroup(ng *api.NodeGroup) error {
descriptionBuffer.WriteString(fmt.Sprintf(", min size from %s to %d", currentMinSize.Str, ng.DesiredCapacity))
}
// If the desired number of nodes is greater than the max then update the max
if int64(ng.DesiredCapacity) > currentMaxSize.Int() {
if int64(*ng.DesiredCapacity) > currentMaxSize.Int() {
newMaxSize := fmt.Sprintf("%d", ng.DesiredCapacity)
template, err = sjson.Set(template, maxSizePath, newMaxSize)
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion pkg/cfn/manager/nodegroup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ var _ = Describe("StackCollection NodeGroup", func() {

It("should be a no-op if attempting to scale to the existing desired capacity", func() {
ng.Name = "12345"
ng.DesiredCapacity = 2
cap := 2
ng.DesiredCapacity = &cap

err := sc.ScaleNodeGroup(ng)

Expand Down
25 changes: 21 additions & 4 deletions pkg/ctl/cmdutils/nodegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"strings"

"github.com/spf13/cobra"
"github.com/spf13/pflag"

"github.com/weaveworks/eksctl/pkg/ami"
Expand All @@ -16,13 +17,29 @@ const (
)

// AddCommonCreateNodeGroupFlags adds common flags for creating a node group
func AddCommonCreateNodeGroupFlags(fs *pflag.FlagSet, p *api.ProviderConfig, cfg *api.ClusterConfig, ng *api.NodeGroup) {
func AddCommonCreateNodeGroupFlags(cmd *cobra.Command, fs *pflag.FlagSet, p *api.ProviderConfig, cfg *api.ClusterConfig, ng *api.NodeGroup) {
var desiredCapacity int
var minSize int
var maxSize int

fs.StringVarP(&ng.InstanceType, "node-type", "t", defaultNodeType, "node instance type")
fs.IntVarP(&ng.DesiredCapacity, "nodes", "N", api.DefaultNodeCount, "total number of nodes (for a static ASG)")
fs.IntVarP(&desiredCapacity, "nodes", "N", api.DefaultNodeCount, "total number of nodes (for a static ASG)")

// TODO: https://github.com/weaveworks/eksctl/issues/28
fs.IntVarP(&ng.MinSize, "nodes-min", "m", 0, "minimum nodes in ASG")
fs.IntVarP(&ng.MaxSize, "nodes-max", "M", 0, "maximum nodes in ASG")
fs.IntVarP(&minSize, "nodes-min", "m", api.DefaultNodeCount, "minimum nodes in ASG")
fs.IntVarP(&maxSize, "nodes-max", "M", api.DefaultNodeCount, "maximum nodes in ASG")

cmd.PreRun = func(cmd *cobra.Command, args[] string) {
if f := cmd.Flag("nodes"); f.Changed {
ng.DesiredCapacity = &desiredCapacity
}
if f := cmd.Flag("nodes-min"); f.Changed {
ng.MinSize= &minSize
}
if f := cmd.Flag("nodes-max"); f.Changed {
ng.MaxSize= &maxSize
}
}

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(), ", ")))
Expand Down
2 changes: 1 addition & 1 deletion pkg/ctl/create/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func createClusterCmd(g *cmdutils.Grouping) *cobra.Command {
group.InFlagSet("Initial nodegroup", func(fs *pflag.FlagSet) {
fs.StringVar(&ng.Name, "nodegroup-name", "", fmt.Sprintf("name of the nodegroup (generated if unspecified, e.g. %q)", exampleNodeGroupName))
fs.BoolVar(&withoutNodeGroup, "without-nodegroup", false, "if set, initial nodegroup will not be created")
cmdutils.AddCommonCreateNodeGroupFlags(fs, p, cfg, ng)
cmdutils.AddCommonCreateNodeGroupFlags(cmd, fs, p, cfg, ng)
})

group.InFlagSet("Cluster and nodegroup add-ons", func(fs *pflag.FlagSet) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/ctl/create/nodegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func createNodeGroupCmd(g *cmdutils.Grouping) *cobra.Command {

group.InFlagSet("New nodegroup", func(fs *pflag.FlagSet) {
fs.StringVarP(&ng.Name, "name", "n", "", fmt.Sprintf("name of the new nodegroup (generated if unspecified, e.g. %q)", exampleNodeGroupName))
cmdutils.AddCommonCreateNodeGroupFlags(fs, p, cfg, ng)
cmdutils.AddCommonCreateNodeGroupFlags(cmd, fs, p, cfg, ng)
})

group.InFlagSet("IAM addons", func(fs *pflag.FlagSet) {
Expand Down
10 changes: 8 additions & 2 deletions pkg/ctl/scale/nodegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,13 @@ func scaleNodeGroupCmd(g *cmdutils.Grouping) *cobra.Command {
fs.StringVar(&cfg.Metadata.Name, "cluster", "", "EKS cluster name")
fs.StringVarP(&ng.Name, "name", "n", "", "Name of the nodegroup to scale")

fs.IntVarP(&ng.DesiredCapacity, "nodes", "N", -1, "total number of nodes (scale to this number)")
var desiredCapacity int
fs.IntVarP(&desiredCapacity, "nodes", "N", -1, "total number of nodes (scale to this number)")
cmd.PreRun = func(cmd *cobra.Command, args[] string) {
if f := cmd.Flag("nodes"); f.Changed {
ng.DesiredCapacity = &desiredCapacity
}
}

cmdutils.AddRegionFlag(fs, p)
})
Expand Down Expand Up @@ -70,7 +76,7 @@ func doScaleNodeGroup(p *api.ProviderConfig, cfg *api.ClusterConfig, ng *api.Nod
return fmt.Errorf("--name must be set")
}

if ng.DesiredCapacity < 0 {
if ng.DesiredCapacity == nil || *ng.DesiredCapacity < 0 {
return fmt.Errorf("number of nodes must be 0 or greater. Use the --nodes/-N flag")
}

Expand Down
8 changes: 7 additions & 1 deletion pkg/ctl/utils/wait_nodes.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,13 @@ func waitNodesCmd(g *cmdutils.Grouping) *cobra.Command {

group.InFlagSet("General", func(fs *pflag.FlagSet) {
fs.StringVar(&waitNodesKubeconfigPath, "kubeconfig", "kubeconfig", "path to read kubeconfig")
fs.IntVarP(&ng.MinSize, "nodes-min", "m", api.DefaultNodeCount, "minimum number of nodes to wait for")
var minSize int
fs.IntVarP(&minSize, "nodes-min", "m", api.DefaultNodeCount, "minimum number of nodes to wait for")
cmd.PreRun = func(cmd *cobra.Command, args[] string) {
if f := cmd.Flag("nodes-min"); f.Changed {
ng.MinSize = &minSize
}
}
fs.DurationVar(&p.WaitTimeout, "timeout", api.DefaultWaitTimeout, "how long to wait")
})

Expand Down
4 changes: 2 additions & 2 deletions pkg/eks/nodegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func getNodes(clientSet *clientset.Clientset, ng *api.NodeGroup) (int, error) {

// WaitForNodes waits till the nodes are ready
func (c *ClusterProvider) WaitForNodes(clientSet *clientset.Clientset, ng *api.NodeGroup) error {
if ng.MinSize == 0 {
if *ng.MinSize == 0 {
return nil
}
timer := time.After(c.Provider.WaitTimeout())
Expand All @@ -97,7 +97,7 @@ func (c *ClusterProvider) WaitForNodes(clientSet *clientset.Clientset, ng *api.N
}

logger.Info("waiting for at least %d node(s) to become ready in %q", ng.MinSize, ng.Name)
for !timeout && counter <= ng.MinSize {
for !timeout && counter <= *ng.MinSize {
select {
case event := <-watcher.ResultChan():
logger.Debug("event = %#v", event)
Expand Down
6 changes: 3 additions & 3 deletions pkg/utils/kubeconfig/kubeconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,9 @@ var _ = Describe("Kubeconfig", func() {
SSHPublicKeyPath: "~/.ssh/id_rsa.pub",
SSHPublicKey: []uint8(nil),
SSHPublicKeyName: "",
DesiredCapacity: 2,
MinSize: 0,
MaxSize: 0,
DesiredCapacity: nil,
MinSize: nil,
MaxSize: nil,
MaxPodsPerNode: 0,
IAM: &eksctlapi.NodeGroupIAM{
AttachPolicyARNs: []string(nil),
Expand Down