search

couchbase / service-broker

An Open Service Broker Based Kubernetes Templating Engine
Apache License 2.0
17 stars 11 forks source link

Deprovisioning fails on client-side when using eden #49

Closed toabi closed 3 years ago

toabi commented 4 years ago

Describe the bug A clear and concise description of what the bug is.

To Reproduce

  1. Provision service
  2. Deprovision service

=> Service-Instances gets deleted => Created Templates get deleted => Existing service-binding secrets are not deleted (might be okay, because most OSB clients might not let you delete things which have active bindings, eden just does…) but that doesn't matter

Expected behavior Deprovisioning ends a successful response.

But after some digging it might be more an eden issue. I'll have to later test it with the CloudFoundry Marketplace.

https://github.com/starkandwayne/eden/blob/6f84e1d02e8a63edf59802708921684e0b3598dd/apiclient/open_service_broker.go#L198-L239 is the code on edens side.

The OSB Spec looks like eden should fall into a "Polling Last Operation" when getting the 202 which it actually does.

But the last-operation then returns a 410 instead of a 200 with the right state.

Environment:

Additional context

service-broker 

I0708 09:54:40.908224       1 broker.go:227] HTTP req: "DELETE /v2/service_instances/6702849d-711b-4286-8063-25060d4a7422?service_id=8522e991-07bc-4225-a859-1eec1e333154&plan_id=ec0f2c9b-0277-46d7-985f-ba1fbf3b068f&accepts_incomplete=true HTTP/2.0" 10.1.6.193:34428 
I0708 09:54:40.918865       1 broker.go:236] HTTP rsp: "202 Accepted" 10.64104ms
I0708 09:54:45.132809       1 broker.go:227] HTTP req: "GET /readyz HTTP/2.0" 10.1.6.76:49780 
I0708 09:54:45.135213       1 broker.go:236] HTTP rsp: "200 OK" 229.773µs
I0708 09:54:45.981203       1 broker.go:227] HTTP req: "GET /v2/service_instances/6702849d-711b-4286-8063-25060d4a7422/last_operation?operation=3ffe0869-6aed-469d-a27b-4a5aefb255b7&service_id=8522e991-07bc-4225-a859-1eec1e333154&plan_id=ec0f2c9b-0277-46d7-985f-ba1fbf3b068f HTTP/2.0" 10.1.6.193:34428 
I0708 09:54:45.986748       1 broker.go:236] HTTP rsp: "410 Gone" 5.546728ms

eden

❯ eden d -i arangodb-deployment-6702849d-711b-4286-8063-25060d4a7422
deprovision: arangodb/deployment - guid: 6702849d-711b-4286-8063-25060d4a7422
deprovision: in-progress
API request error 410: &{ResourceGone service instance does not exist}
toabi commented 4 years ago

Ah, it's probably here: https://github.com/couchbase/service-broker/blob/873ed42595f7db90d4f9af5f2d40c1116f455ad2/pkg/broker/handlers.go#L653-L656

But hard to know at that point if it should say "Ok it's gone now" or "It never existed…"

toabi commented 4 years ago

CloudFoundry doesn’t care about this issue and shows no error after deprovision.

toabi commented 4 years ago

Just stumbled upon https://github.com/openservicebrokerapi/servicebroker/issues/706 so current implementation should be good enough for this broker

spjmurray commented 4 years ago

From the specification:

The platform MUST consider this response a success

So it's working as per the specification, and eden is at "fault". Cloud Foundry invented the spec, so I'm hardly surprised it works for them! That said, now you've drawn my attention to it:

The expected response body is {}.

So we shouldn't be returning an error string in this case.

spjmurray commented 3 years ago

As stated above, eden doesn't follow the API correctly, or uses a weird version. But, there was a bug in my implementation of the API that got fixed here.