Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More orphanDependents changes between 4.1.3 and 4.6.1 #1840

Closed
k-wall opened this issue Oct 29, 2019 · 6 comments
Closed

More orphanDependents changes between 4.1.3 and 4.6.1 #1840

k-wall opened this issue Oct 29, 2019 · 6 comments

Comments

@k-wall
Copy link
Contributor

k-wall commented Oct 29, 2019

Following on from #1775 which reported an API change in list().delete(), I notice other places where the semantics of the delete() have changed since 4.1.3.

Running this simple Main which calls client.configMaps().delete() with OkHttp tracing turned on:

against 4.1.3 I see:

INFO: {"apiVersion":"v1","kind":"DeleteOptions","orphanDependents":false}

against 4.6.1:

INFO: {"apiVersion":"v1","kind":"DeleteOptions","gracePeriodSeconds":0,"orphanDependents":true}

Bisecting, I see that the change was introduced in 4.2.0. Was this API change intentional? This will be breaking for existing users.

@rohanKanojia
Copy link
Member

We changed deletion policy in order to align with kubectl defaults. I haven't checked for ConfigMap but isn't it the same when done using kubectl?

@k-wall
Copy link
Contributor Author

k-wall commented Oct 29, 2019

Not for me. kubectl v1.14.1 sends Background which I believe corresponds to a cascading delete. This tallies with what I read.

$ kubectl -v=8 delete cm kwtest
I1029 14:10:48.323689   28292 loader.go:359] Config loaded from file /Users/keith/.kube/config
I1029 14:10:48.347667   28292 request.go:942] Request Body: {"propagationPolicy":"Background"}

If the intent was to change the API, it should be called out in the release notes. I don't think I see it.

@rohanKanojia
Copy link
Member

@k-wall : ah, you're right. It's a mistake on our side that we haven't provided API change information in release notes. Apologies for that. I'll update the release note whenever I get time.

@k-wall
Copy link
Contributor Author

k-wall commented Oct 30, 2019

Ok, our project will make a change to make explicit use of cascading(true) or withPropagationPolicy() so we get the behaviour it needs.

k-wall added a commit to k-wall/enmasse that referenced this issue Oct 30, 2019
* Bump sundrio.version from 0.16.4 to 0.17.2
* Remove KubernetesDeserializer thread safety workaround.
* Simplify code used to turn off HTTP/2, utilizing new method in KC 4.4.0
* Remove generic parameter from KubernetesResource (relates to fabric8io/kubernetes-client#1661)
* Workaround Fabric8 API change by making cascade desire explict (see fabric8io/kubernetes-client#1840)
k-wall added a commit to EnMasseProject/enmasse that referenced this issue Nov 4, 2019
* Bump fabric8 kubernetes-client from 4.1.3 to 4.6.2

* Bump sundrio.version from 0.16.4 to 0.17.2
* Remove KubernetesDeserializer thread safety workaround.
* Simplify code used to turn off HTTP/2, utilizing new method in KC 4.4.0
* Remove generic parameter from KubernetesResource (relates to fabric8io/kubernetes-client#1661)
* Workaround Fabric8 API change by making cascade desire explict (see fabric8io/kubernetes-client#1840)
@k-wall
Copy link
Contributor Author

k-wall commented Nov 5, 2019

@rohanKanojia from our perspective, this issue can be closed. We changed our code to adapt to the new behaviour.

@Aitozi
Copy link

Aitozi commented Jul 12, 2020

Following on from #1775 which reported an API change in list().delete(), I notice other places where the semantics of the delete() have changed since 4.1.3.

Running this simple Main which calls client.configMaps().delete() with OkHttp tracing turned on:

against 4.1.3 I see:

INFO: {"apiVersion":"v1","kind":"DeleteOptions","orphanDependents":false}

against 4.6.1:

INFO: {"apiVersion":"v1","kind":"DeleteOptions","gracePeriodSeconds":0,"orphanDependents":true}

Bisecting, I see that the change was introduced in 4.2.0. Was this API change intentional? This will be breaking for existing users.

Hi, @k-wall I may have encountered the same issue, can you tell me how to enable the okhttp tracing log, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants