Merge pull request #4076 from Skarlso/always-clean-roles-and-resources

chore(ref): always clean roles and resources on cf stack failure

Author: k8s-ci-robot

cmd/clusterawsadm/cloudformation/service/service.go

@@ -49,7 +49,7 @@ func NewService(i cloudformationiface.CloudFormationAPI) *Service {
 }
 
 // ReconcileBootstrapStack creates or updates bootstrap CloudFormation.
-func (s *Service) ReconcileBootstrapStack(stackName string, t go_cfn.Template, tags map[string]string, deleteOnFailure bool) error {
+func (s *Service) ReconcileBootstrapStack(stackName string, t go_cfn.Template, tags map[string]string) error {
 	yaml, err := t.YAML()
 	processedYaml := string(yaml)
 	if err != nil {
@@ -64,7 +64,7 @@ func (s *Service) ReconcileBootstrapStack(stackName string, t go_cfn.Template, t
 		})
 	}
 	//nolint:nestif
-	if err := s.createStack(stackName, processedYaml, stackTags, deleteOnFailure); err != nil {
+	if err := s.createStack(stackName, processedYaml, stackTags); err != nil {
 		if code, _ := awserrors.Code(errors.Cause(err)); code == "AlreadyExistsException" {
 			klog.Infof("AWS Cloudformation stack %q already exists, updating", klog.KRef("", stackName))
 			updateErr := s.updateStack(stackName, processedYaml, stackTags)
@@ -97,29 +97,26 @@ func (s *Service) ReconcileBootstrapNoUpdate(stackName string, t go_cfn.Template
 		})
 	}
 	//nolint:nestif
-	if err := s.createStack(stackName, processedYaml, stackTags, true); err != nil {
+	if err := s.createStack(stackName, processedYaml, stackTags); err != nil {
 		if code, _ := awserrors.Code(errors.Cause(err)); code == "AlreadyExistsException" {
 			desInput := &cfn.DescribeStacksInput{StackName: aws.String(stackName)}
 			if err := s.CFN.WaitUntilStackCreateComplete(desInput); err != nil {
 				return errors.Wrap(err, "failed to wait for AWS CloudFormation stack to be CreateComplete")
 			}
 			return nil
 		}
-		return err
+		return fmt.Errorf("failed to create CF stack: %w", err)
 	}
 	return nil
 }
 
-func (s *Service) createStack(stackName, yaml string, tags []*cfn.Tag, deleteOnFailure bool) error {
+func (s *Service) createStack(stackName, yaml string, tags []*cfn.Tag) error {
 	input := &cfn.CreateStackInput{
 		Capabilities: aws.StringSlice([]string{cfn.CapabilityCapabilityIam, cfn.CapabilityCapabilityNamedIam}),
 		TemplateBody: aws.String(yaml),
 		StackName:    aws.String(stackName),
 		Tags:         tags,
 	}
-	if deleteOnFailure {
-		input.OnFailure = aws.String(cfn.OnFailureDelete)
-	}
 	klog.V(2).Infof("creating AWS CloudFormation stack %q", stackName)
 	if _, err := s.CFN.CreateStack(input); err != nil {
 		return errors.Wrap(err, "failed to create AWS CloudFormation stack")

cmd/clusterawsadm/cmd/bootstrap/iam/cloudformation.go

@@ -109,7 +109,7 @@ func createCloudFormationStackCmd() *cobra.Command {
 
 			cfnSvc := cloudformation.NewService(cfn.New(sess))
 
-			err = cfnSvc.ReconcileBootstrapStack(t.Spec.StackName, *t.RenderCloudFormation(), t.Spec.StackTags, false)
+			err = cfnSvc.ReconcileBootstrapStack(t.Spec.StackName, *t.RenderCloudFormation(), t.Spec.StackTags)
 			if err != nil {
 				fmt.Printf("Error: %v\n", err)
 				return err

test/e2e/shared/aws.go

@@ -410,30 +410,19 @@ func createCloudFormationStack(prov client.ConfigProvider, t *cfn_bootstrap.Temp
 	// CloudFormation stack will clean up on a failure, we don't need an Eventually here.
 	// The `create` already does a WaitUntilStackCreateComplete.
 	cfnSvc := cloudformation.NewService(cfnClient)
-	err := cfnSvc.ReconcileBootstrapNoUpdate(t.Spec.StackName, *renderCustomCloudFormation(t), tags)
-	if err != nil {
+	if err := cfnSvc.ReconcileBootstrapNoUpdate(t.Spec.StackName, *renderCustomCloudFormation(t), tags); err != nil {
 		By(fmt.Sprintf("Error reconciling Cloud formation stack %v", err))
 		spewCloudFormationResources(cfnClient, t)
 
-		stack, derr := cfnClient.DescribeStacks(&cfn.DescribeStacksInput{StackName: aws.String(t.Spec.StackName)})
-		if derr == nil && len(stack.Stacks) > 0 {
-			if aws.StringValue(stack.Stacks[0].StackStatus) == cfn.StackStatusRollbackFailed ||
-				aws.StringValue(stack.Stacks[0].StackStatus) == cfn.StackStatusRollbackComplete ||
-				aws.StringValue(stack.Stacks[0].StackStatus) == cfn.StackStatusRollbackInProgress ||
-				aws.StringValue(stack.Stacks[0].StackStatus) == cfn.StackStatusCreateFailed ||
-				aws.StringValue(stack.Stacks[0].StackStatus) == cfn.StackStatusDeleteFailed {
-				// If cloudformation stack creation fails due to resources that already exist, stack stays in rollback status and must be manually deleted.
-				// Delete resources that failed because they already exists.
-				By("Starting cleanup process as the stack failed to create")
-				deleteMultitenancyRoles(prov)
-				deleteResourcesInCloudFormation(prov, t)
-			}
-		}
+		// always clean up on a failure because we could leak these resources and the next cloud formation create would
+		// fail with the same problem.
+		deleteMultitenancyRoles(prov)
+		deleteResourcesInCloudFormation(prov, t)
 		return err
 	}
 
 	spewCloudFormationResources(cfnClient, t)
-	return err
+	return nil
 }
 
 func spewCloudFormationResources(cfnClient *cfn.CloudFormation, t *cfn_bootstrap.Template) {