[Meta] Deletion of TPRs via catalog API Server fails

[kibbles-n-bytes]

Note: this issue is made up of several smaller ones. Ideally a single patch should close this and all of the smaller ones. See https://github.com/kubernetes-incubator/service-catalog/issues/600#issuecomment-300330144 for details.

Upon trying to run kubectl delete -n test-ns bindings ups-binding as per the walkthrough, I get:

The ThirdPartyResourceData "ups-binding" is invalid: 
* metadata.deletionGracePeriodSeconds: Invalid value: 0: field is immutable; may only be changed via deletion
* metadata.deletionTimestamp: Invalid value: "2017-03-21T20:20:05Z": field is immutable; may only be changed via deletion

API server logs: http://pastebin.com/5BFFMsuv Catalog logs: http://pastebin.com/VTZPP5rz

[arschles]

@kibbles-n-bytes the workaround is to delete the TPRs directly on kubernetes core. you could use that workaround on https://github.com/kubernetes-incubator/service-catalog/pull/602, but obviously it won't close this issue

[arschles]

may be fixed by work that comes out of https://github.com/kubernetes-incubator/service-catalog/issues/599

cc/ @krancour

[krancour]

@arschles #612 (which is meant to close #599) also seems to have this problem.

[arschles]

@krancour good to know, thanks

[arschles]

cc/ @simonleung8

[arschles]

@simonleung8 is currently working on this. I am moving this into 0.0.4

[simonleung8]

A workaround has been posted to resolve this issue. see #718

[simonleung8]

I have to close the PR because it is breaking integration test for etcd. Will further investigate to find a proper remedy.

[arschles]

Moving to the beta cycle, as this is still in progress

[krancour]

https://github.com/kubernetes-incubator/service-catalog/pull/738 just recently fixed the update for tpr-backed storage. I'm not 100% of the root cause behind this issue, but delete via apiserver is a soft-delete-- which is really an update. I suspect that this issue can be closed, but I really need to try the walkthrough using the new and improved (and better tested) tpr storage. I will try and get that done tomorrow if not sooner.

[arschles]

I confirm that this is still an issue on the canary as of right now. The logs that are returned on the API server are as follows:

I0509 21:46:23.684728       1 request.go:991] Response Body: {"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"ThirdPartyResourceData.extensions \"ups-broker\" is invalid: [metadata.deletionGracePeriodSeconds: Invalid value: 0: field is immutable; may only be changed via deletion, metadata.deletionTimestamp: Invalid value: 2017-05-09 21:46:23 +0000 UTC: field is immutable; may only be changed via deletion]","reason":"Invalid","details":{"name":"ups-broker","group":"extensions","kind":"ThirdPartyResourceData","causes":[{"reason":"FieldValueInvalid","message":"Invalid value: 0: field is immutable; may only be changed via deletion","field":"metadata.deletionGracePeriodSeconds"},{"reason":"FieldValueInvalid","message":"Invalid value: 2017-05-09 21:46:23 +0000 UTC: field is immutable; may only be changed via deletion","field":"metadata.deletionTimestamp"},{"reason":"FieldValueInvalid","message":"Invalid value: 0: field is immutable; may only be changed via deletion","field":"metadata.deletionGracePeriodSeconds"},{"reason":"FieldValueInvalid","message":"Invalidvalue: 2017-05-09 21:46:23 +0000 UTC: field is immutable; may only be changed via deletion","field":"metadata.deletionTimestamp"}]},"code":422}
E0509 21:46:23.685068       1 storage_interface.go:556] executing PUT to servicecatalog/ups-broker (ThirdPartyResourceData.extensions "ups-broker" is invalid: [metadata.deletionGracePeriodSeconds: Invalid value: 0: field is immutable; may only be changed via deletion, metadata.deletionTimestamp: Invalid value: 2017-05-09 21:46:23 +0000 UTC: field is immutable; may only be changed via deletion])

It looks like this is related to not setting the resource version properly (#723)

[simonleung8]

@arschles The rewrite from @krancour seems to have fixed the issue, I can successfully delete TPR resource going through our walk through with the canary build. Your failure might be due to another issue.

With v0.04

$ kubectl --context=service-catalog delete -n test-ns bindings ups-binding
The ThirdPartyResourceData "ups-binding" is invalid:
* metadata.deletionGracePeriodSeconds: Invalid value: 0: field is immutable; may only be changed via deletion
* metadata.deletionTimestamp: Invalid value: "2017-05-09T21:57:07Z": field is immutable; may only be changed via deletion

With canary

$ kubectl --context=service-catalog delete -n test-ns bindings ups-binding
binding "ups-binding" deleted

Edit: nevertheless, this issue probably still stand since you still have problem deleting TPR resource

[arschles]

I've changed the title of this issue to include [Meta], because there are several other issues that describe sub-problems of this one:

• 720: since delete uses GuaranteedUpdate to do soft-deletes, that function needs to be fixed

• 732: we'll likely need to change resource version handling to make GuaranteedUpdate work properly

[arschles]

Moving to 0.1.0 as this has not been completed

[arschles]

The issue is as follows:

When the API registry gets a DELETE, it sets the deletion timestamp and deletion grace period on the resource and calls GuaranteedUpdate. The TPR storage implementation fails because it tries to PUT those changes on the upstream API, but those fields are read-only.

My proposed solution is to try intercepting calls to update those fields in GuaranteedUpdate and set a finalizer on the TPR instead.

[arschles]

Moving to 0.1.0, as #836 is not yet finished