From ad6e0a84b32888e3ea8441f4436a9c2b7e0f33f7 Mon Sep 17 00:00:00 2001 From: Yusmen Zabanov Date: Fri, 27 Sep 2024 09:39:01 +0000 Subject: [PATCH] Support synchronous service provisioning from OSBAPI Brokers fixes #3481 --- .../services/instances/managed/controller.go | 4 ++ .../instances/managed/controller_test.go | 26 +++++++++++++ .../controllers/services/osbapi/client.go | 6 ++- .../services/osbapi/client_test.go | 37 +++++++++++++++---- .../controllers/services/osbapi/types.go | 1 + tests/helpers/broker/broker.go | 3 +- 6 files changed, 67 insertions(+), 10 deletions(-) diff --git a/controllers/controllers/services/instances/managed/controller.go b/controllers/controllers/services/instances/managed/controller.go index a0095db4a..cc540e948 100644 --- a/controllers/controllers/services/instances/managed/controller.go +++ b/controllers/controllers/services/instances/managed/controller.go @@ -202,6 +202,10 @@ func (r *Reconciler) provisionServiceInstance( return ctrl.Result{}, fmt.Errorf("failed to provision service: %w", err) } + if provisionResponse.Complete { + return ctrl.Result{}, nil + } + serviceInstance.Status.ProvisionOperation = provisionResponse.Operation meta.SetStatusCondition(&serviceInstance.Status.Conditions, metav1.Condition{ Type: korifiv1alpha1.ProvisionRequestedCondition, diff --git a/controllers/controllers/services/instances/managed/controller_test.go b/controllers/controllers/services/instances/managed/controller_test.go index fe0f8a5fc..8e2093bd4 100644 --- a/controllers/controllers/services/instances/managed/controller_test.go +++ b/controllers/controllers/services/instances/managed/controller_test.go @@ -261,6 +261,32 @@ var _ = Describe("CFServiceInstance", func() { }) }) + When("the provisioning is synchronous", func() { + BeforeEach(func() { + brokerClient.ProvisionReturns(osbapi.ServiceInstanceOperationResponse{ + Operation: "operation-1", + Complete: true, + }, nil) + }) + + It("does not check last operation", func() { + Consistently(func(g Gomega) { + g.Expect(brokerClient.GetServiceInstanceLastOperationCallCount()).To(Equal(0)) + }).Should(Succeed()) + }) + + It("set sets ready condition to true", 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.ConditionTrue)), + ))) + }).Should(Succeed()) + }) + }) + When("service provisioning fails", func() { BeforeEach(func() { brokerClient.ProvisionReturns(osbapi.ServiceInstanceOperationResponse{}, errors.New("provision-failed")) diff --git a/controllers/controllers/services/osbapi/client.go b/controllers/controllers/services/osbapi/client.go index 9cdb493b1..eb99cc9c2 100644 --- a/controllers/controllers/services/osbapi/client.go +++ b/controllers/controllers/services/osbapi/client.go @@ -70,7 +70,11 @@ func (c *Client) Provision(ctx context.Context, payload InstanceProvisionPayload return ServiceInstanceOperationResponse{}, fmt.Errorf("provision request failed with status code: %d", statusCode) } - var response ServiceInstanceOperationResponse + response := ServiceInstanceOperationResponse{} + if statusCode == http.StatusCreated { + response.Complete = true + } + err = json.Unmarshal(respBytes, &response) if err != nil { return ServiceInstanceOperationResponse{}, fmt.Errorf("failed to unmarshal response: %w", err) diff --git a/controllers/controllers/services/osbapi/client_test.go b/controllers/controllers/services/osbapi/client_test.go index a13d47688..d52f6da6d 100644 --- a/controllers/controllers/services/osbapi/client_test.go +++ b/controllers/controllers/services/osbapi/client_test.go @@ -138,7 +138,7 @@ var _ = Describe("OSBAPI Client", func() { ) BeforeEach(func() { - brokerServer.WithResponse( + brokerServer = broker.NewServer().WithResponse( "/v2/service_instances/{id}", map[string]any{ "operation": "provision_op1", @@ -162,13 +162,6 @@ var _ = Describe("OSBAPI Client", func() { }) }) - It("provisions the service", func() { - Expect(provisionErr).NotTo(HaveOccurred()) - Expect(provisionResp).To(Equal(osbapi.ServiceInstanceOperationResponse{ - Operation: "provision_op1", - })) - }) - It("sends async provision request to broker", func() { Expect(provisionErr).NotTo(HaveOccurred()) requests := brokerServer.ServedRequests() @@ -203,6 +196,34 @@ var _ = Describe("OSBAPI Client", func() { })) }) + It("provisions the service synchronously", func() { + Expect(provisionErr).NotTo(HaveOccurred()) + Expect(provisionResp).To(Equal(osbapi.ServiceInstanceOperationResponse{ + Operation: "provision_op1", + Complete: true, + })) + }) + + When("the broker accepts the provision request", func() { + BeforeEach(func() { + brokerServer = broker.NewServer().WithResponse( + "/v2/service_instances/{id}", + map[string]any{ + "operation": "provision_op1", + }, + http.StatusAccepted, + ) + }) + + It("provisions the service asynchronously", func() { + Expect(provisionErr).NotTo(HaveOccurred()) + Expect(provisionResp).To(Equal(osbapi.ServiceInstanceOperationResponse{ + Operation: "provision_op1", + Complete: false, + })) + }) + }) + When("the provision request fails", func() { BeforeEach(func() { brokerServer = broker.NewServer().WithHandler("/v2/service_instances/{id}", http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { diff --git a/controllers/controllers/services/osbapi/types.go b/controllers/controllers/services/osbapi/types.go index 93d31310f..d3dacb9e3 100644 --- a/controllers/controllers/services/osbapi/types.go +++ b/controllers/controllers/services/osbapi/types.go @@ -76,6 +76,7 @@ type Plan struct { type ServiceInstanceOperationResponse struct { Operation string `json:"operation"` + Complete bool } type LastOperationResponse struct { diff --git a/tests/helpers/broker/broker.go b/tests/helpers/broker/broker.go index bf5a2de95..3228f671d 100644 --- a/tests/helpers/broker/broker.go +++ b/tests/helpers/broker/broker.go @@ -32,8 +32,9 @@ func (b *BrokerServer) WithResponse(pattern string, response map[string]any, sta Expect(err).NotTo(HaveOccurred()) return b.WithHandler(pattern, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - _, _ = w.Write(respBytes) w.WriteHeader(statusCode) + _, err := w.Write(respBytes) + Expect(err).NotTo(HaveOccurred()) })) }