Repository: keikoproj/instance-manager (go)

Issue:
CR Deletion: Will not reconcile if stack is in finite-state
It seems that if a stack is in an error state where stack is finite (ROLLBACK_COMPLETE), it will not delete the stack as a result of deleting the CR.
While it's good to leave the stack for a human to inspect it, we should respond to the act of CR deletion as this is an explicit operation

Pull request:
Deletion error state
Fixes #24 

This PR adds distinction between UnrecoverableError and UnrecoverableDeletionError,  when an IG is deleted, if the state is `UnrecoverableError` (e.g. `ROLLBACK_COMPLETE`), it should allow the deletion of the stack, however if the stack is in `UnrecoverableDeletionError` (e.g. `DELETE_FAILED`) only a human can fix this, so stack will be in err state.

Added a test to validate this behavior 

Patch:
diff --git a/controllers/providers/aws/aws.go b/controllers/providers/aws/aws.go
index 52b0b6f..ce8f8b6 100644
--- a/controllers/providers/aws/aws.go
+++ b/controllers/providers/aws/aws.go
@@ -273,11 +273,12 @@ func (w *AwsWorker) DeriveEksVpcID(clusterName string) (string, error) {
 }
 
 type CloudformationReconcileState struct {
-	OngoingState           bool
-	FiniteState            bool
-	FiniteDeleted          bool
-	UpdateRecoverableError bool
-	UnrecoverableError     bool
+	OngoingState             bool
+	FiniteState              bool
+	FiniteDeleted            bool
+	UpdateRecoverableError   bool
+	UnrecoverableError       bool
+	UnrecoverableDeleteError bool
 }
 
 var OngoingState = CloudformationReconcileState{OngoingState: true}
@@ -285,6 +286,7 @@ var FiniteState = CloudformationReconcileState{FiniteState: true}
 var FiniteDeleted = CloudformationReconcileState{FiniteDeleted: true}
 var UpdateRecoverableError = CloudformationReconcileState{UpdateRecoverableError: true}
 var UnrecoverableError = CloudformationReconcileState{UnrecoverableError: true}
+var UnrecoverableDeleteError = CloudformationReconcileState{UnrecoverableDeleteError: true}
 
 func IsStackInConditionState(key string, condition string) bool {
 	conditionStates := map[string]CloudformationReconcileState{
@@ -301,7 +303,7 @@ func IsStackInConditionState(key string, condition string) bool {
 		"UPDATE_ROLLBACK_COMPLETE":                     UpdateRecoverableError,
 		"UPDATE_ROLLBACK_FAILED":                       UnrecoverableError,
 		"CREATE_FAILED":                                UnrecoverableError,
-		"DELETE_FAILED":                                UnrecoverableError,
+		"DELETE_FAILED":                                UnrecoverableDeleteError,
 		"ROLLBACK_COMPLETE":                            UnrecoverableError,
 	}
 	state := conditionStates[key]
@@ -317,6 +319,8 @@ func IsStackInConditionState(key string, condition string) bool {
 		return state.UpdateRecoverableError
 	case "UnrecoverableError":
 		return state.UnrecoverableError
+	case "UnrecoverableDeleteError":
+		return state.UnrecoverableDeleteError
 	default:
 		return false
 	}
diff --git a/controllers/provisioners/ekscloudformation/ekscloudformation.go b/controllers/provisioners/ekscloudformation/ekscloudformation.go
index db084f2..c5426ec 100644
--- a/controllers/provisioners/ekscloudformation/ekscloudformation.go
+++ b/controllers/provisioners/ekscloudformation/ekscloudformation.go
@@ -46,11 +46,12 @@ var (
 )
 
 const (
-	OngoingStateString           = "OngoingState"
-	FiniteStateString            = "FiniteState"
-	FiniteDeletedString          = "FiniteDeleted"
-	UpdateRecoverableErrorString = "UpdateRecoverableError"
-	UnrecoverableErrorString     = "UnrecoverableError"
+	OngoingStateString             = "OngoingState"
+	FiniteStateString              = "FiniteState"
+	FiniteDeletedString            = "FiniteDeleted"
+	UpdateRecoverableErrorString   = "UpdateRecoverableError"
+	UnrecoverableErrorString       = "UnrecoverableError"
+	UnrecoverableDeleteErrorString = "UnrecoverableDeleteError"
 )
 
 // New constructs a new instance group provisioner of EKS Cloudformation type
@@ -220,17 +221,23 @@ func (ctx *EksCfInstanceGroupContext) StateDiscovery() {
 			// resource is being deleted
 			if provisioned {
 				if awsprovider.IsStackInConditionState(stackStatus, OngoingStateString) {
-					// stack is in an ongoing state
+					// deleting stack is in an ongoing state
 					instanceGroup.SetState(v1alpha1.ReconcileDeleting)
 				} else if awsprovider.IsStackInConditionState(stackStatus, FiniteStateString) {
-					// stack is in a finite state
+					// deleting stack is in a finite state
+					instanceGroup.SetState(v1alpha1.ReconcileInitDelete)
+				} else if awsprovider.IsStackInConditionState(stackStatus, UpdateRecoverableErrorString) {
+					// deleting stack is in an update recoverable state
 					instanceGroup.SetState(v1alpha1.ReconcileInitDelete)
 				} else if awsprovider.IsStackInConditionState(stackStatus, FiniteDeletedString) {
-					// stack is in a finite-deleted state
+					// deleting stack is in a finite-deleted state
 					instanceGroup.SetState(v1alpha1.ReconcileDeleted)
-				} else if awsprovider.IsStackInConditionState(stackStatus, UnrecoverableErrorString) {
-					// stack is in unrecoverable error state
+				} else if awsprovider.IsStackInConditionState(stackStatus, UnrecoverableDeleteErrorString) {
+					// deleting stack is in a unrecoverable delete error state
 					instanceGroup.SetState(v1alpha1.ReconcileErr)
+				} else if awsprovider.IsStackInConditionState(stackStatus, UnrecoverableErrorString) {
+					// deleting stack is in a unrecoverable error state - allow it to delete
+					instanceGroup.SetState(v1alpha1.ReconcileInitDelete)
 				}
 			} else {
 				// Stack does not exist


Test patch:
diff --git a/controllers/provisioners/ekscloudformation/ekscloudformation_test.go b/controllers/provisioners/ekscloudformation/ekscloudformation_test.go
index 0d84dd3..f9fab00 100644
--- a/controllers/provisioners/ekscloudformation/ekscloudformation_test.go
+++ b/controllers/provisioners/ekscloudformation/ekscloudformation_test.go
@@ -802,6 +802,36 @@ func TestStateDiscoveryUnrecoverableError(t *testing.T) {
 }
 
 func TestStateDiscoveryUnrecoverableErrorDelete(t *testing.T) {
+	ig := FakeIG{
+		IsDeleting: true,
+	}
+	testCase := EksCfUnitTest{
+		Description:       "StateDiscovery - when stack in err state, allow deletes",
+		InstanceGroup:     ig.getInstanceGroup(),
+		StackExist:        true,
+		StackUpdateNeeded: true,
+		StackState:        "ROLLBACK_COMPLETE",
+		ExpectedState:     v1alpha1.ReconcileInitDelete,
+	}
+	testCase.Run(t)
+}
+
+func TestStateDiscoveryRecoverableErrorDelete(t *testing.T) {
+	ig := FakeIG{
+		IsDeleting: true,
+	}
+	testCase := EksCfUnitTest{
+		Description:       "StateDiscovery - when stack in err state, allow deletes",
+		InstanceGroup:     ig.getInstanceGroup(),
+		StackExist:        true,
+		StackUpdateNeeded: true,
+		StackState:        "UPDATE_ROLLBACK_COMPLETE",
+		ExpectedState:     v1alpha1.ReconcileInitDelete,
+	}
+	testCase.Run(t)
+}
+
+func TestStateDiscoveryUnrecoverableErrorDeleteFailure(t *testing.T) {
 	ig := FakeIG{
 		IsDeleting: true,
 	}