-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Scale managed nodegroup with --name flag #4383
Scale managed nodegroup with --name flag #4383
Conversation
ef29f03
to
3615f75
Compare
@@ -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() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could Context
could be a When
and then just write using a config file
? But I don't have a strong opinion on that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Well done Niki! :) 🎉
@@ -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 returns true if this clusterConfig contains a managed or un-managed nodegroup with the given name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment needs to be updated to match the signature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, updated it!
3615f75
to
6659f2b
Compare
Description
Fixes bug in #2655
The issue was that when a user was doing
eksctl scale nodegroup --config-file x --name y
, eksctl was not looking in the config file for managed nodegroups. This only worked for unmanaged nodegroups.Iterating over
managedNodeGroups
as well and then returning the nodegroup base (which is common to both types of ng) fixes this issue.A separate PR will address scaling all nodegroups in the config file if the name flag is not provided.
Checklist
README.md
, or theuserdocs
directory)area/nodegroup
) and kind (e.g.kind/improvement
)BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯