make 'InstallPlanPhaseInstalling' come before 'InstallPlanPhaseRequir…

…esApproval'
operator-framework · Jun 7, 2018 · 9daa86c · 9daa86c
1 parent cef0745
commit 9daa86c
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 14 deletions.
diff --git a/pkg/controller/operators/catalog/operator.go b/pkg/controller/operators/catalog/operator.go
@@ -203,24 +203,12 @@ type installPlanTransitioner interface {
 var _ installPlanTransitioner = &Operator{}
 
 func transitionInstallPlanState(transitioner installPlanTransitioner, plan *v1alpha1.InstallPlan) error {
-	if plan.Spec.Approval == v1alpha1.ApprovalManual && plan.Spec.Approved != true {
-		// FIXME(alecmerdler): Need to add `status.plan` even if not approved to show dry run UI
-		log.Debugf("plan %s is not approved, skipping sync", plan.SelfLink)
-		plan.Status.Phase = v1alpha1.InstallPlanPhaseRequiresApproval
-		return nil
-	}
-
 	switch plan.Status.Phase {
 	case v1alpha1.InstallPlanPhaseNone:
 		log.Debugf("plan %s phase unrecognized, setting to Planning", plan.SelfLink)
 		plan.Status.Phase = v1alpha1.InstallPlanPhasePlanning
 		return nil
 
-	case v1alpha1.InstallPlanPhaseRequiresApproval:
-		log.Debugf("plan %s approved, setting to Planning", plan.SelfLink)
-		plan.Status.Phase = v1alpha1.InstallPlanPhasePlanning
-		return nil
-
 	case v1alpha1.InstallPlanPhasePlanning:
 		log.Debugf("plan %s phase Planning, attempting to resolve", plan.SelfLink)
 		if err := transitioner.ResolvePlan(plan); err != nil {
@@ -230,7 +218,21 @@ func transitionInstallPlanState(transitioner installPlanTransitioner, plan *v1al
 			return err
 		}
 		plan.Status.SetCondition(v1alpha1.ConditionMet(v1alpha1.InstallPlanResolved))
-		plan.Status.Phase = v1alpha1.InstallPlanPhaseInstalling
+
+		if plan.Spec.Approval == v1alpha1.ApprovalManual && plan.Spec.Approved != true {
+			plan.Status.Phase = v1alpha1.InstallPlanPhaseRequiresApproval
+		} else {
+			plan.Status.Phase = v1alpha1.InstallPlanPhaseInstalling
+		}
+		return nil
+
+	case v1alpha1.InstallPlanPhaseRequiresApproval:
+		if plan.Spec.Approved {
+			log.Debugf("plan %s approved, setting to Planning", plan.SelfLink)
+			plan.Status.Phase = v1alpha1.InstallPlanPhaseInstalling
+		} else {
+			log.Debugf("plan %s is not approved, skipping sync", plan.SelfLink)
+		}
 		return nil
 
 	case v1alpha1.InstallPlanPhaseInstalling:

diff --git a/pkg/controller/operators/catalog/operator_test.go b/pkg/controller/operators/catalog/operator_test.go
@@ -70,6 +70,8 @@ func TestTransitionInstallPlan(t *testing.T) {
 
 		{v1alpha1.InstallPlanPhasePlanning, nil, v1alpha1.ApprovalAutomatic, false, v1alpha1.InstallPlanPhaseInstalling, resolved},
 		{v1alpha1.InstallPlanPhasePlanning, nil, v1alpha1.ApprovalAutomatic, true, v1alpha1.InstallPlanPhaseInstalling, resolved},
+		{v1alpha1.InstallPlanPhasePlanning, nil, v1alpha1.ApprovalManual, false, v1alpha1.InstallPlanPhaseRequiresApproval, resolved},
+		{v1alpha1.InstallPlanPhasePlanning, nil, v1alpha1.ApprovalManual, true, v1alpha1.InstallPlanPhaseInstalling, resolved},
 		{v1alpha1.InstallPlanPhasePlanning, err, v1alpha1.ApprovalAutomatic, false, v1alpha1.InstallPlanPhaseFailed, unresolved},
 		{v1alpha1.InstallPlanPhasePlanning, err, v1alpha1.ApprovalAutomatic, true, v1alpha1.InstallPlanPhaseFailed, unresolved},
 
@@ -79,7 +81,7 @@ func TestTransitionInstallPlan(t *testing.T) {
 		{v1alpha1.InstallPlanPhaseInstalling, err, v1alpha1.ApprovalAutomatic, true, v1alpha1.InstallPlanPhaseFailed, failed},
 
 		{v1alpha1.InstallPlanPhaseRequiresApproval, nil, v1alpha1.ApprovalManual, false, v1alpha1.InstallPlanPhaseRequiresApproval, nil},
-		{v1alpha1.InstallPlanPhaseRequiresApproval, nil, v1alpha1.ApprovalManual, true, v1alpha1.InstallPlanPhasePlanning, nil},
+		{v1alpha1.InstallPlanPhaseRequiresApproval, nil, v1alpha1.ApprovalManual, true, v1alpha1.InstallPlanPhaseInstalling, nil},
 	}
 	for _, tt := range table {
 		// Create a plan in the provided initial phase.