Skip to content

fix(aws): implement cleanup on partial creation failures #612

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.

Sign up for GitHub

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

Merged
ArangoGutierrez merged 1 commit into from
Feb 6, 2026
Merged

fix(aws): implement cleanup on partial creation failures #612

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
68 changes: 55 additions & 13 deletions pkg/provider/aws/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ const (
defaultWaiterTimeout = 15 * time.Minute
)

type cleanupFunc func() error

// Create creates an EC2 instance with proper Network configuration
// VPC, Subnet, Internet Gateway, Route Table, Security Group
// If the environment specifies a cluster configuration, it delegates to CreateCluster()
Expand All @@ -49,70 +51,110 @@ func (p *Provider) Create() error {

// Single-node deployment
cache := new(AWS)
var cleanupStack []cleanupFunc
var err error

// Defer cleanup on failure - execute cleanup functions in reverse order
defer func() {
if err != nil {
p.log.Warning("Creation failed, rolling back created resources...")
for i := len(cleanupStack) - 1; i >= 0; i-- {
if cleanupErr := cleanupStack[i](); cleanupErr != nil {
p.log.Warning("Cleanup failed: %v", cleanupErr)
}
}
}
}()

if err := p.updateProgressingCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Creating AWS resources"); err != nil {
if err = p.updateProgressingCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Creating AWS resources"); err != nil {
p.log.Warning("Failed to update progressing condition: %v", err)
}

if err := p.createVPC(cache); err != nil {
if err = p.createVPC(cache); err != nil {
if updateErr := p.updateDegradedCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Error creating VPC"); updateErr != nil {
p.log.Warning("Failed to update degraded condition: %v", updateErr)
}
return fmt.Errorf("error creating VPC: %w", err)
}
if err := p.updateProgressingCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "VPC created"); err != nil {
cleanupStack = append(cleanupStack, func() error {
cleanupCache := &AWS{Vpcid: cache.Vpcid}
return p.deleteVPC(cleanupCache)
})
if err = p.updateProgressingCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "VPC created"); err != nil {
p.log.Warning("Failed to update progressing condition: %v", err)
}

if err := p.createSubnet(cache); err != nil {
if err = p.createSubnet(cache); err != nil {
if updateErr := p.updateDegradedCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Error creating subnet"); updateErr != nil {
p.log.Warning("Failed to update degraded condition: %v", updateErr)
}
return fmt.Errorf("error creating subnet: %w", err)
}
if err := p.updateProgressingCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Subnet created"); err != nil {
cleanupStack = append(cleanupStack, func() error {
cleanupCache := &AWS{Subnetid: cache.Subnetid}
return p.deleteSubnet(cleanupCache)
})
if err = p.updateProgressingCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Subnet created"); err != nil {
p.log.Warning("Failed to update progressing condition: %v", err)
}

if err := p.createInternetGateway(cache); err != nil {
if err = p.createInternetGateway(cache); err != nil {
if updateErr := p.updateDegradedCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Error creating Internet Gateway"); updateErr != nil {
p.log.Warning("Failed to update degraded condition: %v", updateErr)
}
return fmt.Errorf("error creating Internet Gateway: %w", err)
}
if err := p.updateProgressingCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Internet Gateway created"); err != nil {
cleanupStack = append(cleanupStack, func() error {
cleanupCache := &AWS{
InternetGwid: cache.InternetGwid,
Vpcid: cache.Vpcid,
}
return p.deleteInternetGateway(cleanupCache)
})
if err = p.updateProgressingCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Internet Gateway created"); err != nil {
p.log.Warning("Failed to update progressing condition: %v", err)
}

if err := p.createRouteTable(cache); err != nil {
if err = p.createRouteTable(cache); err != nil {
if updateErr := p.updateDegradedCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Error creating route table"); updateErr != nil {
p.log.Warning("Failed to update degraded condition: %v", updateErr)
}
return fmt.Errorf("error creating route table: %w", err)
}
if err := p.updateProgressingCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Route Table created"); err != nil {
cleanupStack = append(cleanupStack, func() error {
cleanupCache := &AWS{
RouteTable: cache.RouteTable,
Vpcid: cache.Vpcid,
}
return p.deleteRouteTable(cleanupCache)
})
if err = p.updateProgressingCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Route Table created"); err != nil {
p.log.Warning("Failed to update progressing condition: %v", err)
}

if err := p.createSecurityGroup(cache); err != nil {
if err = p.createSecurityGroup(cache); err != nil {
if updateErr := p.updateDegradedCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Error creating security group"); updateErr != nil {
p.log.Warning("Failed to update degraded condition: %v", updateErr)
}
return fmt.Errorf("error creating security group: %w", err)
}
if err := p.updateProgressingCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Security Group created"); err != nil {
cleanupStack = append(cleanupStack, func() error {
cleanupCache := &AWS{SecurityGroupid: cache.SecurityGroupid}
return p.deleteSecurityGroups(cleanupCache)
})
if err = p.updateProgressingCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Security Group created"); err != nil {
p.log.Warning("Failed to update progressing condition: %v", err)
}

if err := p.createEC2Instance(cache); err != nil {
if err = p.createEC2Instance(cache); err != nil {
if updateErr := p.updateDegradedCondition(*p.DeepCopy(), cache, "v1alpha1.Creating", "Error creating EC2 instance"); updateErr != nil {
p.log.Warning("Failed to update degraded condition: %v", updateErr)
}
return fmt.Errorf("error creating EC2 instance: %w", err)
}

// Save objects ID's into a cache file
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: If updateAvailableCondition fails after all resources are successfully created, the defer cleanup will be triggered and roll back all successfully created resources. This would leave the AWS resources in limbo - created but not tracked. Consider setting a flag to disable cleanup after all resources are successfully created, or clearing the cleanup stack before this call.

Suggested change
// Save objects ID's into a cache file
// Save objects ID's into a cache file. At this point all AWS resources
// have been successfully created, so we must not run the deferred
// cleanup logic on failure, otherwise we would delete live resources
// that are no longer tracked. Clear the cleanup stack before updating
// the available condition.
cleanupStack = nil

Copilot uses AI. Check for mistakes.
if err := p.updateAvailableCondition(*p.Environment, cache); err != nil {
if err = p.updateAvailableCondition(*p.Environment, cache); err != nil {
return fmt.Errorf("error creating cache file: %w", err)
}
return nil
Expand Down
Loading