Skip to content
Open
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
25 changes: 24 additions & 1 deletion pkg/cfn/manager/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -544,11 +544,34 @@ func nonTransitionalReadyStackStatuses() []types.StackStatus {
return []types.StackStatus{
types.StackStatusCreateComplete,
types.StackStatusUpdateComplete,
types.StackStatusRollbackComplete,
// Note: ROLLBACK_COMPLETE is deliberately excluded. It means the stack's
// CREATE failed and was rolled back, so no resources exist and CloudFormation
// refuses to update it; grouping it with healthy terminal states caused
// broken stacks to be treated as healthy existing nodegroups (see
// https://github.com/eksctl-io/eksctl/issues/8712). UPDATE_ROLLBACK_COMPLETE
// is genuinely healthy (an update rolled back to a known-good state) and
// remains included.
types.StackStatusUpdateRollbackComplete,
}
}

// StackStatusIsNotOperational reports whether the stack is in a terminal failed or
// rolled-back state (e.g. ROLLBACK_COMPLETE) in which it cannot serve the resources
// it represents. Such stacks must not be treated as healthy existing nodegroups: a
// nodegroup whose stack is in ROLLBACK_COMPLETE was never created successfully, and
// CloudFormation refuses to update it, so it has to be deleted and recreated.
func StackStatusIsNotOperational(s *Stack) bool {
switch s.StackStatus {
case types.StackStatusCreateFailed,
types.StackStatusRollbackComplete,
types.StackStatusRollbackFailed,
types.StackStatusUpdateRollbackFailed,
types.StackStatusDeleteFailed:
return true
}
return false
}

func allNonDeletedStackStatuses() []types.StackStatus {
return []types.StackStatus{
types.StackStatusCreateInProgress,
Expand Down
26 changes: 26 additions & 0 deletions pkg/cfn/manager/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,32 @@ import (
)

var _ = Describe("StackCollection", func() {
Context("StackStatusIsNotOperational", func() {
It("returns true for failed and rolled-back states", func() {
for _, status := range []types.StackStatus{
types.StackStatusCreateFailed,
types.StackStatusRollbackComplete,
types.StackStatusRollbackFailed,
types.StackStatusUpdateRollbackFailed,
types.StackStatusDeleteFailed,
} {
Expect(StackStatusIsNotOperational(&Stack{StackStatus: status})).To(BeTrue(), "expected %q to be not operational", status)
}
})

It("returns false for healthy and transitional states", func() {
for _, status := range []types.StackStatus{
types.StackStatusCreateComplete,
types.StackStatusUpdateComplete,
types.StackStatusUpdateRollbackComplete,
types.StackStatusCreateInProgress,
types.StackStatusRollbackInProgress,
} {
Expect(StackStatusIsNotOperational(&Stack{StackStatus: status})).To(BeFalse(), "expected %q to be operational", status)
}
})
})

Context("PropagateManagedNodeGroupTagsToASG", func() {
var (
asgName string
Expand Down
25 changes: 25 additions & 0 deletions pkg/ctl/cmdutils/filter/nodegroup_filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package filter

import (
"context"
"fmt"
"strings"

"github.com/aws/aws-sdk-go-v2/service/eks"
Expand Down Expand Up @@ -35,6 +36,7 @@ type NodeGroupFilter struct {
onlyRemote bool
localNodegroups sets.Set[string]
remoteNodegroups sets.Set[string]
nodeGroupStacks []manager.NodeGroupStack
}

// NewNodeGroupFilter creates a new NodeGroupFilter struct
Expand Down Expand Up @@ -80,11 +82,33 @@ func (f *NodeGroupFilter) AppendIncludeNames(names ...string) {
func (f *NodeGroupFilter) SetOnlyLocal(ctx context.Context, eksAPI awsapi.EKS, lister StackLister, clusterConfig *api.ClusterConfig) error {
f.onlyLocal = true

// Capture the nodegroups the user actually asked for *before* loading, since
// loadLocalAndRemoteNodegroups appends remote-only nodegroups to the config.
configNodeGroupNames := sets.New(clusterConfig.GetAllNodeGroupNames()...)

err := f.loadLocalAndRemoteNodegroups(ctx, eksAPI, lister, clusterConfig)
if err != nil {
return err
}

// Stacks stuck in a failed or rolled-back state (e.g. ROLLBACK_COMPLETE) cannot
// be used and would otherwise be silently treated as healthy existing nodegroups,
// causing `create nodegroup` to do nothing. Fail fast when a nodegroup in the
// user's config already has such a stack.
var notOperationalNodeGroups []string
for _, s := range f.nodeGroupStacks {
if configNodeGroupNames.Has(s.NodeGroupName) && s.Stack != nil && manager.StackStatusIsNotOperational(s.Stack) {
notOperationalNodeGroups = append(notOperationalNodeGroups, s.NodeGroupName)
}
}
if len(notOperationalNodeGroups) > 0 {
return fmt.Errorf(
"nodegroup(s) %q have a CloudFormation stack in a failed or rolled-back state (e.g. ROLLBACK_COMPLETE) and cannot be used; "+
"delete the failed stack(s) first with 'eksctl delete nodegroup --cluster %s --name %s' and then retry",
strings.Join(notOperationalNodeGroups, ", "), clusterConfig.Metadata.Name, notOperationalNodeGroups[0],
)
}

// Remote ones will be excluded
if f.remoteNodegroups.Len() > 0 {
logger.Info("%d existing %s(s) (%s) will be excluded", f.remoteNodegroups.Len(), "nodegroup", strings.Join(sets.List(f.remoteNodegroups), ","))
Expand Down Expand Up @@ -126,6 +150,7 @@ func (f *NodeGroupFilter) loadLocalAndRemoteNodegroups(ctx context.Context, eksA
if err != nil {
return err
}
f.nodeGroupStacks = nodeGroupsWithStacks
for _, s := range nodeGroupsWithStacks {
f.remoteNodegroups.Insert(s.NodeGroupName)
}
Expand Down
55 changes: 55 additions & 0 deletions pkg/ctl/cmdutils/filter/nodegroup_filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"

"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/service/cloudformation/types"
"github.com/aws/aws-sdk-go-v2/service/eks"
"github.com/stretchr/testify/mock"

Expand Down Expand Up @@ -117,6 +118,60 @@ var _ = Describe("nodegroup filter", func() {
Expect(excluded.HasAll("test-ng1a", "test-ng2a", "test-ng3a", "test-ng2b", "test-ng3b")).To(BeTrue())
})

It("should error when a nodegroup stack is in a failed or rolled-back state", func() {
mockLister := &mockStackLister{
nodesResult: []manager.NodeGroupStack{
{
NodeGroupName: "test-ng1a",
Stack: &manager.Stack{
StackStatus: types.StackStatusRollbackComplete,
},
},
{
NodeGroupName: "test-ng2a",
Stack: &manager.Stack{
StackStatus: types.StackStatusCreateComplete,
},
},
},
}
err := filter.SetOnlyLocal(context.Background(), mockProvider.EKS(), mockLister, cfg)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("test-ng1a"))
Expect(err.Error()).To(ContainSubstring("failed or rolled-back"))
Expect(err.Error()).To(ContainSubstring("delete nodegroup"))
})

It("should allow healthy nodegroup stacks", func() {
mockLister := &mockStackLister{
nodesResult: []manager.NodeGroupStack{
{
NodeGroupName: "test-ng1a",
Stack: &manager.Stack{
StackStatus: types.StackStatusUpdateRollbackComplete,
},
},
},
}
err := filter.SetOnlyLocal(context.Background(), mockProvider.EKS(), mockLister, cfg)
Expect(err).NotTo(HaveOccurred())
})

It("should ignore failed or rolled-back stacks for nodegroups not in the config", func() {
mockLister := &mockStackLister{
nodesResult: []manager.NodeGroupStack{
{
NodeGroupName: "remote-ng-not-in-config",
Stack: &manager.Stack{
StackStatus: types.StackStatusRollbackComplete,
},
},
},
}
err := filter.SetOnlyLocal(context.Background(), mockProvider.EKS(), mockLister, cfg)
Expect(err).NotTo(HaveOccurred())
})

It("should match only local nodegroups with exclude and include rules", func() {
err := filter.AppendIncludeGlobs(getNodeGroupNames(cfg), "test-ng?a", "test-ng?b")
Expect(err).NotTo(HaveOccurred())
Expand Down
14 changes: 13 additions & 1 deletion pkg/eks/nodegroup_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,12 @@ func ValidateExistingNodeGroupsForCompatibility(ctx context.Context, cfg *api.Cl

logger.Info("checking security group configuration for all nodegroups")
var incompatibleNodeGroups []string
var notOperationalNodeGroups []string
for ng, info := range infoByNodeGroup {
if manager.StackStatusIsNotOperational(info.Stack) {
notOperationalNodeGroups = append(notOperationalNodeGroups, ng)
continue
}
if stackManager.StackStatusIsNotTransitional(info.Stack) {
isCompatible, err := isNodeGroupCompatible(ng, info)
if err != nil {
Expand All @@ -305,8 +310,15 @@ func ValidateExistingNodeGroupsForCompatibility(ctx context.Context, cfg *api.Cl
}
}

if len(notOperationalNodeGroups) > 0 {
logger.Critical("found nodegroup(s) (%s) whose CloudFormation stack is in a failed or rolled-back state (e.g. ROLLBACK_COMPLETE) and cannot be used",
strings.Join(notOperationalNodeGroups, ", "))
logger.Critical("these nodegroups were never created successfully; delete and recreate them, e.g. 'eksctl delete nodegroup --cluster %s --name <%s>'",
cfg.Metadata.Name, notOperationalNodeGroups[0])
}

if len(incompatibleNodeGroups) == 0 {
logger.Info("all nodegroups have up-to-date cloudformation templates")
logger.Info("all nodegroups have compatible shared security group configuration")
return nil
}

Expand Down