WIP

cloudfoundry · Sep 25, 2024 · 73b8cca · 73b8cca
1 parent f2e63ea
commit 73b8cca
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 98 deletions.
diff --git a/controllers/controllers/services/instances/managed/controller.go b/controllers/controllers/services/instances/managed/controller.go
@@ -238,96 +238,71 @@ func (r *Reconciler) finalizeCFServiceInstance(
 		return ctrl.Result{}, nil
 	}
 
-	servicePlan, err := r.getServicePlan(ctx, serviceInstance.Spec.PlanGUID)
-	if err != nil {
-		log.Error(err, "failed to get service plan")
-
+	if isDeprovisionRequested(serviceInstance) {
 		if controllerutil.RemoveFinalizer(serviceInstance, korifiv1alpha1.CFManagedServiceInstanceFinalizerName) {
 			log.V(1).Info("finalizer removed")
 		}
 
+		return ctrl.Result{}, nil
+	}
+
+	err := r.deprovisionServiceinstance(ctx, serviceInstance)
+	if err != nil {
 		return ctrl.Result{}, err
 	}
 
+	return ctrl.Result{Requeue: true}, nil
+}
+
+func (r *Reconciler) deprovisionServiceinstance(ctx context.Context, serviceInstance *korifiv1alpha1.CFServiceInstance) error {
+	log := logr.FromContextOrDiscard(ctx).WithName("finalizeCFServiceInstance")
+
+	servicePlan, err := r.getServicePlan(ctx, serviceInstance.Spec.PlanGUID)
+	if err != nil {
+		log.Error(err, "failed to get service plan")
+		return nil
+	}
+
 	serviceBroker, err := r.getServiceBroker(ctx, servicePlan.Labels[korifiv1alpha1.RelServiceBrokerGUIDLabel])
 	if err != nil {
 		log.Error(err, "failed to get service broker")
-
-		if controllerutil.RemoveFinalizer(serviceInstance, korifiv1alpha1.CFManagedServiceInstanceFinalizerName) {
-			log.V(1).Info("finalizer removed")
-		}
-
-		return ctrl.Result{}, err
+		return nil
 	}
 
 	serviceOffering, err := r.getServiceOffering(ctx, servicePlan.Labels[korifiv1alpha1.RelServiceOfferingGUIDLabel])
 	if err != nil {
 		log.Error(err, "failed to get service offering")
-
-		if controllerutil.RemoveFinalizer(serviceInstance, korifiv1alpha1.CFManagedServiceInstanceFinalizerName) {
-			log.V(1).Info("finalizer removed")
-		}
-		return ctrl.Result{}, err
+		return nil
 	}
 
 	osbapiClient, err := r.osbapiClientFactory.CreateClient(ctx, serviceBroker)
 	if err != nil {
 		log.Error(err, "failed to create broker client", "broker", serviceBroker.Name)
-
-		if controllerutil.RemoveFinalizer(serviceInstance, korifiv1alpha1.CFManagedServiceInstanceFinalizerName) {
-			log.V(1).Info("finalizer removed")
-		}
-
-		return ctrl.Result{}, fmt.Errorf("failed to create client for broker %q: %w", serviceBroker.Name, err)
-	}
-
-	if !isDeprovisionRequested(serviceInstance) {
-
-		var deprovisionResponse osbapi.ServiceInstanceOperationResponse
-		deprovisionResponse, err = osbapiClient.Deprovision(ctx, osbapi.InstanceDeprovisionPayload{
-			ID: serviceInstance.Name,
-			InstanceDeprovisionRequest: osbapi.InstanceDeprovisionRequest{
-				ServiceId: serviceOffering.Spec.BrokerCatalog.ID,
-				PlanID:    servicePlan.Spec.BrokerCatalog.ID,
-			},
-		})
-		if err != nil {
-			log.Error(err, "failed to deprovision service instance")
-			return ctrl.Result{}, k8s.NewNotReadyError().WithReason("DeprovisionFailed")
-		}
-
-		serviceInstance.Status.DeprovisionOperation = deprovisionResponse.Operation
-		meta.SetStatusCondition(&serviceInstance.Status.Conditions, metav1.Condition{
-			Type:               korifiv1alpha1.DeprovisionRequestedCondition,
-			Status:             metav1.ConditionTrue,
-			ObservedGeneration: serviceInstance.Generation,
-			LastTransitionTime: metav1.NewTime(time.Now()),
-			Reason:             "DeprovisionRequested",
-		})
+		return nil
 	}
-
-	lastOpResponse, err := osbapiClient.GetServiceInstanceLastOperation(ctx, osbapi.GetLastOperationPayload{
+	var deprovisionResponse osbapi.ServiceInstanceOperationResponse
+	deprovisionResponse, err = osbapiClient.Deprovision(ctx, osbapi.InstanceDeprovisionPayload{
 		ID: serviceInstance.Name,
-		GetLastOperationRequest: osbapi.GetLastOperationRequest{
+		InstanceDeprovisionRequest: osbapi.InstanceDeprovisionRequest{
 			ServiceId: serviceOffering.Spec.BrokerCatalog.ID,
 			PlanID:    servicePlan.Spec.BrokerCatalog.ID,
-			Operation: serviceInstance.Status.DeprovisionOperation,
 		},
 	})
 	if err != nil {
-		log.Error(err, "getting service instance last operation failed")
-		return ctrl.Result{}, k8s.NewNotReadyError().WithCause(err).WithReason("GetLastOperationFailed")
-	}
-
-	if lastOpResponse.State == "in progress" {
-		return ctrl.Result{}, k8s.NewNotReadyError().WithReason("DeprovisionInProgress").WithRequeue()
+		log.Error(err, "failed to deprovision service instance")
+		return k8s.NewNotReadyError().WithReason("DeprovisionFailed")
 	}
 
-	if controllerutil.RemoveFinalizer(serviceInstance, korifiv1alpha1.CFManagedServiceInstanceFinalizerName) {
-		log.V(1).Info("finalizer removed", "deprovision-operation-result", lastOpResponse)
-	}
+	serviceInstance.Status.DeprovisionOperation = deprovisionResponse.Operation
+	meta.SetStatusCondition(&serviceInstance.Status.Conditions, metav1.Condition{
+		Type:               korifiv1alpha1.DeprovisionRequestedCondition,
+		Status:             metav1.ConditionTrue,
+		ObservedGeneration: serviceInstance.Generation,
+		LastTransitionTime: metav1.NewTime(time.Now()),
+		Reason:             "DeprovisionRequested",
+	})
 
-	return ctrl.Result{}, nil
+	return k8s.NewNotReadyError().WithReason("DeprovisionRequested").WithRequeue()
 }
 
 func (r *Reconciler) getNamespace(ctx context.Context, namespaceName string) (*corev1.Namespace, error) {

diff --git a/controllers/controllers/services/instances/managed/controller_test.go b/controllers/controllers/services/instances/managed/controller_test.go
@@ -516,12 +516,8 @@ var _ = Describe("CFServiceInstance", func() {
 
 		It("deprovisions the instance with the broker", func() {
 			Eventually(func(g Gomega) {
-				g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed())
-				g.Expect(instance.Status.DeprovisionOperation).To(Equal("deprovision-op"))
-				g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll(
-					HasType(Equal(korifiv1alpha1.DeprovisionRequestedCondition)),
-					HasStatus(Equal(metav1.ConditionTrue)),
-				)))
+				err := adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)
+				g.Expect(k8serrors.IsNotFound(err)).To(BeTrue())
 
 				g.Expect(brokerClient.DeprovisionCallCount()).To(Equal(1))
 				_, actualDeprovisionRequest := brokerClient.DeprovisionArgsForCall(0)
@@ -535,44 +531,12 @@ var _ = Describe("CFServiceInstance", func() {
 			}).Should(Succeed())
 		})
 
-		It("checks deprovision operation status", func() {
-			Eventually(func(g Gomega) {
-				g.Expect(brokerClient.GetServiceInstanceLastOperationCallCount()).To(BeNumerically(">", 0))
-				_, lastOp := brokerClient.GetServiceInstanceLastOperationArgsForCall(brokerClient.GetServiceInstanceLastOperationCallCount() - 1)
-				Expect(lastOp).To(Equal(osbapi.GetLastOperationPayload{
-					ID: instance.Name,
-					GetLastOperationRequest: osbapi.GetLastOperationRequest{
-						ServiceId: "service-offering-id",
-						PlanID:    "service-plan-id",
-						Operation: "deprovision-op",
-					},
-				}))
-			}).Should(Succeed())
-		})
-
 		It("does not delete the service instance", func() {
 			Consistently(func(g Gomega) {
 				g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed())
 			}).Should(Succeed())
 		})
 
-		When("checking the deprovision operation fails", func() {
-			BeforeEach(func() {
-				brokerClient.GetServiceInstanceLastOperationReturns(osbapi.LastOperationResponse{}, errors.New("last-op-error"))
-			})
-
-			It("sets ready condition to false", func() {
-				Eventually(func(g Gomega) {
-					g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)).To(Succeed())
-					g.Expect(instance.Status.Conditions).To(ContainElement(SatisfyAll(
-						HasType(Equal(korifiv1alpha1.StatusConditionReady)),
-						HasStatus(Equal(metav1.ConditionFalse)),
-						HasReason(Equal("GetLastOperationFailed")),
-					)))
-				}).Should(Succeed())
-			})
-		})
-
 		When("deprovision fails", func() {
 			BeforeEach(func() {
 				brokerClient.DeprovisionReturns(osbapi.ServiceInstanceOperationResponse{}, errors.New("deprovision-failed"))