Change Deleting to Warning for ServiceInstancesAndBindingsNotCleaned …

…reason (#657) * Change Deletin to Warning for ServiceInstancesAndBindingsNotCleaned reason * wip * wip * Change reconcile * wip * Refactor * Change state when updating manager * Refactor * E2E test * Better logs
kyma-project · Apr 23, 2024 · 0191eba · 0191eba
1 parent 65263cf
commit 0191eba
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 12 deletions.
diff --git a/controllers/btpoperator_controller.go b/controllers/btpoperator_controller.go
@@ -220,7 +220,7 @@ func (r *BtpOperatorReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		return ctrl.Result{}, r.Update(ctx, reconcileCr)
 	}
 
-	if !reconcileCr.ObjectMeta.DeletionTimestamp.IsZero() && reconcileCr.Status.State != v1alpha1.StateDeleting {
+	if !reconcileCr.ObjectMeta.DeletionTimestamp.IsZero() && reconcileCr.Status.State != v1alpha1.StateDeleting && !reconcileCr.IsReasonStringEqual(string(conditions.ServiceInstancesAndBindingsNotCleaned)) {
 		return ctrl.Result{}, r.UpdateBtpOperatorStatus(ctx, reconcileCr, v1alpha1.StateDeleting, conditions.HardDeleting, "BtpOperator is to be deleted")
 	}
 
@@ -229,7 +229,9 @@ func (r *BtpOperatorReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		return ctrl.Result{}, r.HandleInitialState(ctx, reconcileCr)
 	case v1alpha1.StateProcessing:
 		return ctrl.Result{RequeueAfter: ProcessingStateRequeueInterval}, r.HandleProcessingState(ctx, reconcileCr)
-	case v1alpha1.StateError, v1alpha1.StateWarning:
+	case v1alpha1.StateWarning:
+		return r.HandleWarningState(ctx, reconcileCr)
+	case v1alpha1.StateError:
 		return ctrl.Result{}, r.HandleErrorState(ctx, reconcileCr)
 	case v1alpha1.StateDeleting:
 		err := r.HandleDeletingState(ctx, reconcileCr)
@@ -701,6 +703,21 @@ func (r *BtpOperatorReconciler) checkResourceExistence(ctx context.Context, u *u
 	}
 }
 
+func (r *BtpOperatorReconciler) HandleWarningState(ctx context.Context, cr *v1alpha1.BtpOperator) (ctrl.Result, error) {
+	logger := log.FromContext(ctx)
+	logger.Info("Handling Warning state")
+
+	if cr.IsReasonStringEqual(string(conditions.ServiceInstancesAndBindingsNotCleaned)) {
+		err := r.handleDeleting(ctx, cr)
+		if cr.IsReasonStringEqual(string(conditions.ServiceInstancesAndBindingsNotCleaned)) {
+			return ctrl.Result{RequeueAfter: ReadyStateRequeueInterval}, err
+		}
+		return ctrl.Result{}, err
+	}
+
+	return ctrl.Result{}, r.UpdateBtpOperatorStatus(ctx, cr, v1alpha1.StateProcessing, conditions.Updated, "CR has been updated")
+}
+
 func (r *BtpOperatorReconciler) HandleErrorState(ctx context.Context, cr *v1alpha1.BtpOperator) error {
 	logger := log.FromContext(ctx)
 	logger.Info("Handling Error state")
@@ -712,6 +729,12 @@ func (r *BtpOperatorReconciler) HandleDeletingState(ctx context.Context, cr *v1a
 	logger := log.FromContext(ctx)
 	logger.Info("Handling Deleting state")
 
+	return r.handleDeleting(ctx, cr)
+}
+
+func (r *BtpOperatorReconciler) handleDeleting(ctx context.Context, cr *v1alpha1.BtpOperator) error {
+	logger := log.FromContext(ctx)
+
 	if len(cr.GetFinalizers()) == 0 {
 		logger.Info("BtpOperator CR without finalizers - nothing to do, waiting for deletion")
 		return nil
@@ -788,12 +811,12 @@ func (r *BtpOperatorReconciler) handleDeprovisioning(ctx context.Context, cr *v1
 			logger.Info(msg)
 
 			// if the reason is already set, do nothing
-			if cr.IsReasonStringEqual(string(conditions.ServiceInstancesAndBindingsNotCleaned)) {
+			if cr.IsReasonStringEqual(string(conditions.ServiceInstancesAndBindingsNotCleaned)) && cr.Status.State == v1alpha1.StateWarning {
 				return nil
 			}
 
 			if updateStatusErr := r.UpdateBtpOperatorStatus(ctx, cr,
-				v1alpha1.StateDeleting, conditions.ServiceInstancesAndBindingsNotCleaned, msg); updateStatusErr != nil {
+				v1alpha1.StateWarning, conditions.ServiceInstancesAndBindingsNotCleaned, msg); updateStatusErr != nil {
 				return updateStatusErr
 			}
 			return nil

diff --git a/controllers/btpoperator_controller_cr_rotation_test.go b/controllers/btpoperator_controller_cr_rotation_test.go
@@ -137,7 +137,7 @@ var _ = Describe("BTP Operator CR leader replacement", func() {
 				}).WithTimeout(k8sOpsTimeout).WithPolling(k8sOpsPollingInterval).Should(BeTrue())
 
 				Expect(k8sClient.Delete(ctx, btpOperator1)).To(Succeed())
-				Eventually(updateCh).Should(Receive(matchReadyCondition(v1alpha1.StateDeleting, metav1.ConditionFalse, conditions.ServiceInstancesAndBindingsNotCleaned)))
+				Eventually(updateCh).Should(Receive(matchReadyCondition(v1alpha1.StateWarning, metav1.ConditionFalse, conditions.ServiceInstancesAndBindingsNotCleaned)))
 				Expect(k8sClient.Get(ctx, client.ObjectKey{Namespace: btpOperator1.GetNamespace(), Name: btpOperator1.GetName()}, btpOperator1)).To(Succeed())
 				btpOperator1.SetFinalizers([]string{})
 				Expect(k8sClient.Update(ctx, btpOperator1)).To(Succeed())

diff --git a/controllers/btpoperator_controller_deprovisioning_test.go b/controllers/btpoperator_controller_deprovisioning_test.go
@@ -58,7 +58,7 @@ var _ = Describe("BTP Operator controller - deprovisioning", func() {
 			Expect(k8sClient.Get(ctx, client.ObjectKey{Namespace: defaultNamespace, Name: btpOperatorName}, cr)).To(Succeed())
 			Expect(k8sClient.Delete(ctx, cr)).To(Succeed())
 
-			Eventually(updateCh).Should(Receive(matchReadyCondition(v1alpha1.StateDeleting, metav1.ConditionFalse, conditions.ServiceInstancesAndBindingsNotCleaned)))
+			Eventually(updateCh).Should(Receive(matchReadyCondition(v1alpha1.StateWarning, metav1.ConditionFalse, conditions.ServiceInstancesAndBindingsNotCleaned)))
 		})
 
 	})

diff --git a/controllers/serviceinstance_controller_test.go b/controllers/serviceinstance_controller_test.go
@@ -70,7 +70,7 @@ var _ = Describe("Service Instance and Bindings controller", Ordered, func() {
 
 				//  - trigger BTP operator deletion
 				Expect(k8sClient.Delete(ctx, btpOperatorResource)).To(Succeed())
-				Eventually(updateCh).Should(Receive(matchReadyCondition(v1alpha1.StateDeleting, metav1.ConditionFalse, conditions.ServiceInstancesAndBindingsNotCleaned)))
+				Eventually(updateCh).Should(Receive(matchReadyCondition(v1alpha1.StateWarning, metav1.ConditionFalse, conditions.ServiceInstancesAndBindingsNotCleaned)))
 
 				// WHEN
 				Expect(k8sClient.Delete(ctx, siUnstructured)).To(Succeed())
@@ -93,7 +93,7 @@ var _ = Describe("Service Instance and Bindings controller", Ordered, func() {
 
 				//  - trigger BTP operator deletion
 				Expect(k8sClient.Delete(ctx, btpOperatorResource)).To(Succeed())
-				Eventually(updateCh).Should(Receive(matchReadyCondition(v1alpha1.StateDeleting, metav1.ConditionFalse, conditions.ServiceInstancesAndBindingsNotCleaned)))
+				Eventually(updateCh).Should(Receive(matchReadyCondition(v1alpha1.StateWarning, metav1.ConditionFalse, conditions.ServiceInstancesAndBindingsNotCleaned)))
 
 				// WHEN
 				Expect(k8sClient.Delete(ctx, sbUnstructured)).To(Succeed())

diff --git a/docs/contributor/02-10-operations.md b/docs/contributor/02-10-operations.md
@@ -102,7 +102,7 @@ Only one Condition of type `Ready` is used.
 | 6                    | Processing           | Ready                | false                | UpdateCheck                                     | Checking for updates                                                                          |
 | 7                    | Processing           | Ready                | false                | Updated                                         | Resource has been updated                                                                     |
 | 8                    | Deleting             | Ready                | false                | HardDeleting                                    | Trying to hard delete                                                                         |
-| 9                    | Deleting             | Ready                | false                | ServiceInstancesAndBindingsNotCleaned           | Deprovisioning blocked because of ServiceInstances and/or ServiceBindings existence           |
+| 9                    | Warning             | Ready                | false                | ServiceInstancesAndBindingsNotCleaned           | Deprovisioning blocked because of ServiceInstances and/or ServiceBindings existence           |
 | 10                   | Deleting             | Ready                | false                | SoftDeleting                                    | Trying to soft delete after hard delete failed                                                |
 | 11                   | Error                | Ready                | false                | ChartInstallFailed                              | Failure during chart installation                                                             |
 | 12                   | Error                | Ready                | false                | ChartPathEmpty                                  | No chart path available for processing                                                        |

diff --git a/internal/conditions/conditions.go b/internal/conditions/conditions.go
@@ -72,7 +72,7 @@ var Reasons = map[Reason]Metadata{
 	StoringChartDetailsFailed:             {Status: metav1.ConditionFalse, State: v1alpha1.StateError},      //Error;Failure of storing chart details
 	GettingConfigMapFailed:                {Status: metav1.ConditionFalse, State: v1alpha1.StateError},      //Error;Getting Config Map failed
 	ProvisioningFailed:                    {Status: metav1.ConditionFalse, State: v1alpha1.StateError},      //Error;Provisioning failed
-	ServiceInstancesAndBindingsNotCleaned: {Status: metav1.ConditionFalse, State: v1alpha1.StateDeleting},   //Deleting;Deprovisioning blocked because of ServiceInstances and/or ServiceBindings existence
+	ServiceInstancesAndBindingsNotCleaned: {Status: metav1.ConditionFalse, State: v1alpha1.StateWarning},    //Warning;Deprovisioning blocked because of ServiceInstances and/or ServiceBindings existence
 }
 
 // gophers_metadata_section_end

diff --git a/scripts/testing/run_e2e_module_tests.sh b/scripts/testing/run_e2e_module_tests.sh
@@ -68,8 +68,17 @@ kubectl delete btpoperators/e2e-test-btpoperator &
 
 echo -e "\n--- Checking deprovisioning without force delete label"
 
-while [[ $(kubectl get btpoperators/e2e-test-btpoperator -o json| jq '.status.conditions[] | select(.type=="Ready") |.status+.reason'|xargs)  != "FalseServiceInstancesAndBindingsNotCleaned" ]];
-do echo -e "\n--- Waiting for ServiceInstancesAndBindingsNotCleaned reason"; sleep 5; done
+while true; do
+  operator_status=$(kubectl get btpoperators/e2e-test-btpoperator -o json)
+  condition_status=$(echo $operator_status | jq -r '.status.conditions[] | select(.type=="Ready") | .status+.reason')
+  state_status=$(echo $operator_status | jq -r '.status.state')
+
+  if [[ $condition_status == "FalseServiceInstancesAndBindingsNotCleaned" && $state_status == "Warning" ]]; then
+    break
+  else
+    echo -e "\n--- Waiting for ServiceInstancesAndBindingsNotCleaned reason and state"; sleep 5;
+  fi
+done
 
 echo -e "\n--- Condition reason is correct"