Deleting statefulset fails with "Forbidden: updates to statefulset spec for fields other than 'replicas', 'template', and 'updateStrategy' are forbidden"

[k-wall]

Testing 4.6.2 in OCP 3.11, we have discovered that using the following API to delete a statefulset with persistent volume claim ends with the following error:

client.resource(statefulSet).cascading(true).delete();

Forbidden: updates to statefulset spec for fields other than 'replicas', 'template', and 'updateStrategy' are forbidden

Turning up logging you see that it is the attempt to scale the statefulset prior to the delete that is actually failing. The patch generated internally is actually invalid and includes a spurious mutation of the template namespaces, which is rejected by the server.

INFO: [{"op":"replace","path":"/spec/replicas","value":0},{"op":"add","path":"/spec/template/metadata/namespace","value":"test"},{"op":"add","path":"/spec/volumeClaimTemplates/0/metadata/namespace","value":"test"}]

There is a reproducer for this issue here:

https://github.com/k-wall/kc-del-statefulset

I dug into the issue a little, and can see that internally the NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableImpl$ChangeNamespace are actually erroneously mutating the namespaces of the template and volumeClaimTemplates. This is coming from a call chain start at: NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableImpl#delete. This causes the patch sent to be server to be incorrect.

I don't understand the code well, but this looks fishy. I suspect those visitors shouldn't be touching those.

Now with 4.6.3 (or 4.6.4), the issue actually disappears. This is because #1856 prevents the consideration of the reaper (reloadingFromServer is false). However, I think this is merely side stepping this defect. I worry is that it will show up elsewhere, so I make the report.

[rohanKanojia]

@k-wall : ah, You're still facing issues with deletion :-( . I would look into your report tomorrow.

[rohanKanojia]

Hmm, I checked and this namespace mutation also doesn't look right to me. These were added in 2016 in #508 . As far as I know, these deletion options were added since Kubernetes 1.7/1.8 and the client needed to handle it before these. But since we now have moved on to propagationPolicy on DeleteOptions, I think we can get rid of these visitors/reapers and leave it up to Kubernetes server to clean up.

I'll try removing them and test if it is breaking anything.

[rohanKanojia]

I'm afraid we can't remove ChangeNamespace visitors since they seem to be solving the use case of overriding namespace passed explicitly in DSL(like client.apps().statefulSets().inNamespace("explicitNamespace").get() over namespace in metadata(statefulSetObj.getMetadata().getNamespace().

Although I think we can remove reapers.