Skip to content

Commit

Permalink
Merge pull request #4383 from nikimanoledaki/scale-ng-name-bug
Browse files Browse the repository at this point in the history
Scale managed nodegroup with --name flag
  • Loading branch information
nikimanoledaki authored Nov 2, 2021
2 parents 013cd78 + 6659f2b commit 18e4c0d
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 31 deletions.
6 changes: 3 additions & 3 deletions pkg/actions/nodegroup/scale.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5"
)

func (m *Manager) Scale(ng *api.NodeGroup) error {
func (m *Manager) Scale(ng *api.NodeGroupBase) error {
logger.Info("scaling nodegroup %q in cluster %s", ng.Name, m.cfg.Metadata.Name)

nodegroupStackInfos, err := m.stackManager.DescribeNodeGroupStacksAndResources()
Expand Down Expand Up @@ -48,7 +48,7 @@ func (m *Manager) Scale(ng *api.NodeGroup) error {
return nil
}

func (m *Manager) scaleUnmanagedNodeGroup(ng *api.NodeGroup, stackInfo manager.StackInfo) error {
func (m *Manager) scaleUnmanagedNodeGroup(ng *api.NodeGroupBase, stackInfo manager.StackInfo) error {
asgName := ""
for _, resource := range stackInfo.Resources {
if *resource.LogicalResourceId == "NodeGroup" {
Expand Down Expand Up @@ -86,7 +86,7 @@ func (m *Manager) scaleUnmanagedNodeGroup(ng *api.NodeGroup, stackInfo manager.S
return nil
}

func (m *Manager) scaleManagedNodeGroup(ng *api.NodeGroup) error {
func (m *Manager) scaleManagedNodeGroup(ng *api.NodeGroupBase) error {
scalingConfig := &eks.NodegroupScalingConfig{}

if ng.MaxSize != nil {
Expand Down
14 changes: 6 additions & 8 deletions pkg/actions/nodegroup/scale_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ var _ = Describe("Scale", func() {
clusterName, ngName string
p *mockprovider.MockProvider
cfg *api.ClusterConfig
ng *api.NodeGroup
ng *api.NodeGroupBase
m *nodegroup.Manager
fakeStackManager *fakes.FakeStackManager
)
Expand All @@ -37,13 +37,11 @@ var _ = Describe("Scale", func() {
cfg = api.NewClusterConfig()
cfg.Metadata.Name = clusterName

ng = &api.NodeGroup{
NodeGroupBase: &api.NodeGroupBase{
Name: ngName,
ScalingConfig: &api.ScalingConfig{
MinSize: aws.Int(1),
DesiredCapacity: aws.Int(3),
},
ng = &api.NodeGroupBase{
Name: ngName,
ScalingConfig: &api.ScalingConfig{
MinSize: aws.Int(1),
DesiredCapacity: aws.Int(3),
},
}
m = nodegroup.New(cfg, &eks.ClusterProvider{Provider: p}, nil)
Expand Down
24 changes: 20 additions & 4 deletions pkg/apis/eksctl.io/v1alpha5/nodegroups.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package v1alpha5

import "fmt"

// HasInstanceType returns whether some node in the group fulfils the type check
func HasInstanceType(nodeGroup *NodeGroup, hasType func(string) bool) bool {
if hasType(nodeGroup.InstanceType) {
Expand Down Expand Up @@ -44,14 +46,28 @@ func ClusterHasInstanceType(cfg *ClusterConfig, hasType func(string) bool) bool
return false
}

// HasNodegroup returns true if this clusterConfig contains a managed or un-managed nodegroup with the given name
func (c *ClusterConfig) FindNodegroup(name string) *NodeGroup {
// FindNodegroup checks if the clusterConfig contains a nodegroup with the given name
func (c *ClusterConfig) FindNodegroup(name string) (*NodeGroupBase, error) {
var foundNg []*NodeGroupBase
for _, ng := range c.NodeGroups {
if name == ng.NameString() {
return ng
foundNg = append(foundNg, ng.NodeGroupBase)
}
}
return nil

for _, ng := range c.ManagedNodeGroups {
if name == ng.NameString() {
foundNg = append(foundNg, ng.NodeGroupBase)
}
}

if len(foundNg) == 0 {
return nil, fmt.Errorf("nodegroup %s not found in config file", name)
} else if len(foundNg) > 1 {
return nil, fmt.Errorf("found more than 1 nodegroup with name %s", name)
}

return foundNg[0], nil
}

// GetAllNodeGroupNames collects and returns names for both managed and unmanaged nodegroups
Expand Down
14 changes: 7 additions & 7 deletions pkg/ctl/cmdutils/scale.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
)

// NewScaleNodeGroupLoader will load config or use flags for 'eksctl scale nodegroup'
func NewScaleNodeGroupLoader(cmd *Cmd, ng *api.NodeGroup) ClusterConfigLoader {
func NewScaleNodeGroupLoader(cmd *Cmd, ng *api.NodeGroupBase) ClusterConfigLoader {
l := newCommonClusterConfigLoader(cmd)

l.flagsIncompatibleWithConfigFile.Insert(
Expand All @@ -22,9 +22,9 @@ func NewScaleNodeGroupLoader(cmd *Cmd, ng *api.NodeGroup) ClusterConfigLoader {
return err
}

loadedNG := l.ClusterConfig.FindNodegroup(ng.Name)
if loadedNG == nil {
return fmt.Errorf("node group %s not found", ng.Name)
loadedNG, err := l.ClusterConfig.FindNodegroup(ng.Name)
if err != nil {
return err
}

if err := validateNumberOfNodes(loadedNG); err != nil {
Expand Down Expand Up @@ -54,7 +54,7 @@ func NewScaleNodeGroupLoader(cmd *Cmd, ng *api.NodeGroup) ClusterConfigLoader {
return l
}

func validateNameArgument(cmd *Cmd, ng *api.NodeGroup) error {
func validateNameArgument(cmd *Cmd, ng *api.NodeGroupBase) error {
if ng.Name != "" && cmd.NameArg != "" {
return ErrFlagAndArg("--name", ng.Name, cmd.NameArg)
}
Expand All @@ -70,7 +70,7 @@ func validateNameArgument(cmd *Cmd, ng *api.NodeGroup) error {
return nil
}

func validateNumberOfNodes(ng *api.NodeGroup) error {
func validateNumberOfNodes(ng *api.NodeGroupBase) error {
if ng.ScalingConfig == nil {
ng.ScalingConfig = &api.ScalingConfig{}
}
Expand Down Expand Up @@ -103,7 +103,7 @@ func validateNumberOfNodes(ng *api.NodeGroup) error {
}

//only 1 of desired/min/max has to be set on the cli
func validateNumberOfNodesCLI(ng *api.NodeGroup) error {
func validateNumberOfNodesCLI(ng *api.NodeGroupBase) error {
if ng.ScalingConfig == nil {
ng.ScalingConfig = &api.ScalingConfig{}
}
Expand Down
41 changes: 38 additions & 3 deletions pkg/ctl/cmdutils/scale_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ var _ = Describe("scale node group config file loader", func() {
NameArg: params.name,
}

ng := api.NewNodeGroup()
ng := api.NewNodeGroup().BaseNodeGroup()
err := NewScaleNodeGroupLoader(cmd, ng).Load()
if params.err != nil {
Expect(err).To(HaveOccurred())
Expand All @@ -60,7 +60,7 @@ var _ = Describe("scale node group config file loader", func() {
}),
Entry("no node group matched", scaleNodeGroupCase{
name: "123123",
err: fmt.Errorf("node group 123123 not found"),
err: fmt.Errorf("nodegroup 123123 not found in config file"),
}),
Entry("with no desired capacity", scaleNodeGroupCase{
name: "ng-no-desired-capacity",
Expand Down Expand Up @@ -101,7 +101,7 @@ var _ = Describe("scale node group config file loader", func() {
NameArg: params.name,
}

ng := api.NewNodeGroup()
ng := api.NewNodeGroup().BaseNodeGroup()
ng.MinSize = params.minSize
ng.MaxSize = params.maxSize
ng.DesiredCapacity = params.desiredSize
Expand Down Expand Up @@ -166,4 +166,39 @@ var _ = Describe("scale node group config file loader", func() {
err: fmt.Errorf("at least one of minimum, maximum and desired nodes must be set"),
}),
)

Describe("for managed nodegroups", func() {
Context("when using a config file", func() {
It("setting --name finds that individual nodegroup", func() {
ngName := "mng-ng"
ng := &api.NodeGroupBase{
Name: ngName,
ScalingConfig: &api.ScalingConfig{
DesiredCapacity: aws.Int(2),
},
}

config := api.NewClusterConfig()
config.Metadata.Name = "test-cluster"
config.ManagedNodeGroups = []*api.ManagedNodeGroup{
{
NodeGroupBase: &api.NodeGroupBase{
Name: ngName,
ScalingConfig: &api.ScalingConfig{
DesiredCapacity: aws.Int(2),
},
},
},
}
cmd := &Cmd{
CobraCommand: newCmd(),
ClusterConfig: config,
ProviderConfig: api.ProviderConfig{},
}

err := NewScaleNodeGroupLoader(cmd, ng).Load()
Expect(err).NotTo(HaveOccurred())
})
})
})
})
8 changes: 4 additions & 4 deletions pkg/ctl/scale/nodegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ import (
)

func scaleNodeGroupCmd(cmd *cmdutils.Cmd) {
scaleNodeGroupWithRunFunc(cmd, func(cmd *cmdutils.Cmd, ng *api.NodeGroup) error {
scaleNodeGroupWithRunFunc(cmd, func(cmd *cmdutils.Cmd, ng *api.NodeGroupBase) error {
return doScaleNodeGroup(cmd, ng)
})
}

func scaleNodeGroupWithRunFunc(cmd *cmdutils.Cmd, runFunc func(cmd *cmdutils.Cmd, ng *api.NodeGroup) error) {
func scaleNodeGroupWithRunFunc(cmd *cmdutils.Cmd, runFunc func(cmd *cmdutils.Cmd, ng *api.NodeGroupBase) error) {
cfg := api.NewClusterConfig()
ng := cfg.NewNodeGroup()
ng := cfg.NewNodeGroup().BaseNodeGroup()
cmd.ClusterConfig = cfg

cmd.SetDescription("nodegroup", "Scale a nodegroup", "", "ng")
Expand Down Expand Up @@ -56,7 +56,7 @@ func scaleNodeGroupWithRunFunc(cmd *cmdutils.Cmd, runFunc func(cmd *cmdutils.Cmd
cmdutils.AddCommonFlagsForAWS(cmd.FlagSetGroup, &cmd.ProviderConfig, true)
}

func doScaleNodeGroup(cmd *cmdutils.Cmd, ng *api.NodeGroup) error {
func doScaleNodeGroup(cmd *cmdutils.Cmd, ng *api.NodeGroupBase) error {
if err := cmdutils.NewScaleNodeGroupLoader(cmd, ng).Load(); err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/ctl/scale/nodegroup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ import (

var _ = Describe("scale", func() {
Describe("nodegroup", func() {
DescribeTable("create cluster successfully",
DescribeTable("scales a nodegroup successfully",
func(args ...string) {
cmd := newMockEmptyCmd(args...)
count := 0
cmdutils.AddResourceCmd(cmdutils.NewGrouping(), cmd.parentCmd, func(cmd *cmdutils.Cmd) {
scaleNodeGroupWithRunFunc(cmd, func(cmd *cmdutils.Cmd, ng *v1alpha5.NodeGroup) error {
scaleNodeGroupWithRunFunc(cmd, func(cmd *cmdutils.Cmd, ng *v1alpha5.NodeGroupBase) error {
if len(ng.Name) != 0 {
Expect(ng.Name).To(Or(Equal("nodeGroup"), Equal("")))
} else {
Expand Down

0 comments on commit 18e4c0d

Please sign in to comment.