Skip to content

Commit fea1996

Browse files
Fix failures in cluster deletion in regions without capability support (#8631)
1 parent efcb779 commit fea1996

File tree

3 files changed

+74
-17
lines changed

3 files changed

+74
-17
lines changed

pkg/actions/capability/remover.go

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,20 +70,34 @@ func (r *Remover) DeleteTasks(ctx context.Context, capabilities []Summary) (*tas
7070
return nil, fmt.Errorf("failed to fetch IAM role stacks for capabilities: %w", err)
7171
}
7272

73+
capabilityNames := make(map[string]bool)
74+
if len(capabilities) == 0 { // Passed in on cluster deletion case.
75+
for _, capStack := range capabilityStacks {
76+
capabilityNames[manager.GetCapabilityNameFromStack(capStack)] = true
77+
}
78+
for _, iamStack := range iamStacks {
79+
capabilityNames[manager.GetCapabilityNameFromIAMStack(iamStack)] = true
80+
}
81+
} else {
82+
for _, cap := range capabilities {
83+
capabilityNames[cap.Name] = true
84+
}
85+
}
86+
7387
// Create parallel tasks for capability deletion and waiting
7488
capabilityTasks := &tasks.TaskTree{
7589
Parallel: true,
7690
}
7791

78-
for _, cap := range capabilities {
92+
for capName := range capabilityNames {
7993
capabilityTasks.Append(&tasks.GenericTask{
80-
Description: fmt.Sprintf("delete and wait for capability %s", cap.Name),
94+
Description: fmt.Sprintf("delete and wait for capability %s", capName),
8195
Doer: func() error {
82-
if err := r.deleteCapabilityStack(ctx, cap.Name, capabilityStacks); err != nil {
96+
if err := r.deleteCapabilityStack(ctx, capName, capabilityStacks); err != nil {
8397
return err
8498
}
8599

86-
return r.deleteCapabilityIAMRoleStack(ctx, cap.Name, iamStacks)
100+
return r.deleteCapabilityIAMRoleStack(ctx, capName, iamStacks)
87101
},
88102
})
89103
}

pkg/actions/capability/remover_test.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"context"
55
"testing"
66

7+
"github.com/aws/aws-sdk-go-v2/aws"
8+
79
cfntypes "github.com/aws/aws-sdk-go-v2/service/cloudformation/types"
810
"github.com/stretchr/testify/mock"
911

@@ -57,6 +59,59 @@ func TestRemover_DeleteTasks(t *testing.T) {
5759
}
5860
}
5961

62+
func TestRemover_DeleteTasks_ClusterDeleteCase(t *testing.T) {
63+
mockStackRemover := mocks.NewStackRemover(t)
64+
65+
// Create mock stacks for ListCapabilityStacks (cap-a, cap-b)
66+
capAStack := &cfntypes.Stack{
67+
StackName: aws.String("eksctl-test-cluster-capability-cap-a"),
68+
Tags: []cfntypes.Tag{
69+
{Key: aws.String(api.CapabilityNameTag), Value: aws.String("cap-a")},
70+
},
71+
}
72+
capBStack := &cfntypes.Stack{
73+
StackName: aws.String("eksctl-test-cluster-capability-cap-b"),
74+
Tags: []cfntypes.Tag{
75+
{Key: aws.String(api.CapabilityNameTag), Value: aws.String("cap-b")},
76+
},
77+
}
78+
79+
// Create mock stacks for ListCapabilitiesIAMStacks (cap-b, cap-c)
80+
capBIAMStack := &cfntypes.Stack{
81+
StackName: aws.String("eksctl-test-cluster-capability-iam-cap-b"),
82+
Tags: []cfntypes.Tag{
83+
{Key: aws.String(api.CapabilityNameTag), Value: aws.String("cap-b")},
84+
},
85+
}
86+
capCIAMStack := &cfntypes.Stack{
87+
StackName: aws.String("eksctl-test-cluster-capability-iam-cap-c"),
88+
Tags: []cfntypes.Tag{
89+
{Key: aws.String(api.CapabilityNameTag), Value: aws.String("cap-c")},
90+
},
91+
}
92+
93+
// Mock stack listing calls
94+
mockStackRemover.EXPECT().ListCapabilityStacks(mock.Anything).Return([]*cfntypes.Stack{capAStack, capBStack}, nil)
95+
mockStackRemover.EXPECT().ListCapabilitiesIAMStacks(mock.Anything).Return([]*cfntypes.Stack{capBIAMStack, capCIAMStack}, nil)
96+
97+
remover := capability.NewRemover("test-cluster", mockStackRemover)
98+
99+
// Pass in 0 capabilities - cluster delete case
100+
taskTree, err := remover.DeleteTasks(context.Background(), []capability.Summary{})
101+
if err != nil {
102+
t.Fatalf("Expected no error, got %v", err)
103+
}
104+
105+
// Should have 3 tasks (cap-a, cap-b, cap-c - deduplicated from stacks)
106+
if taskTree.Len() != 3 {
107+
t.Errorf("Expected 3 tasks, got %d", taskTree.Len())
108+
}
109+
// Should be parallel execution
110+
if !taskTree.Parallel {
111+
t.Error("Expected parallel task execution, got sequential")
112+
}
113+
}
114+
60115
func TestRemover_DeleteWithWait(t *testing.T) {
61116
// This test is simplified to avoid complex mock setup
62117
// The functionality is tested through integration tests

pkg/actions/cluster/owned.go

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -126,20 +126,8 @@ func (c *OwnedCluster) Delete(ctx context.Context, _, podEvictionWaitPeriod time
126126
newTasksToDeleteAddonIAM := addon.NewRemover(c.stackManager).DeleteAddonIAMTasks
127127

128128
newTasksToDeleteCapability := func() (*tasks.TaskTree, error) {
129-
capabilityGetter := &capability.Getter{
130-
ClusterName: c.cfg.Metadata.Name,
131-
EKSAPI: c.ctl.AWSProvider.EKS(),
132-
}
133-
134-
// Get all capabilities
135-
capabilities, err := capabilityGetter.Get(ctx, "")
136-
137-
if err != nil {
138-
return &tasks.TaskTree{}, err
139-
}
140-
141129
capabilityRemover := capability.NewRemover(c.cfg.Metadata.Name, c.stackManager)
142-
return capabilityRemover.DeleteTasks(ctx, capabilities)
130+
return capabilityRemover.DeleteTasks(ctx, nil) // Delete all capabilities.
143131
}
144132

145133
newTasksToDeletePodIdentityRoles := func() (*tasks.TaskTree, error) {

0 commit comments

Comments
 (0)