kapp-controller hangs waiting for StatefulSet PVCs deletion

[beltran-rubo]

The team is working on creating a Carvel repository that includes most of the Bitnami Helm charts using the App.spec helmTemplate.

Based on https://github.com/vmware-tanzu/carvel-kapp/issues/36, the StatefulSets are the responsible of creating the PVCs, so kapp-controller wont delete them, but only will wait for it to be deleted. That hangs the process to fully remove the packageInstall as there is not any process that would remove them. This is also a known issue in Helm and that project decided not to remove them.

Based on other tickets, it seems the suggested approach is to use the annotation kapp.k14s.io/owned-for-deletion for removing those PVCs, so kapp-controller will not hang and it will remove those volumes.

This solution would work for users who can modify the yaml files and are specific built for Carvel packages. In our case, we are using the Helm charts as the source-of-truth for building the Carvel packages so adding that annotation to the Helm templates would mean to mix Carvel annotations there.

What would be the recommended approach for kapp-controller to avoid hanging the deletion process when there is a PVC created by a StatefulSet?

[danielhelfand]

@beltran-rubo Thanks for opening this.

In reviewing this issue, is there any way you could use an additional templating step with ytt to add this annotation to the helm chart? We were thinking you could use .spec.template.inline.paths to add an overlay that will add this annotation to the chart yaml. Then this information would be passed on to kapp.

You may also be able to reach out to our slack channel to get quicker advice on this kind of question.

[beltran-rubo]

Thanks, that is an option but will require to write a YTT file per application with all the PVCs annotations. Any users who use the helmTemplate feature with StatefulSets will require an extra YTT for handling this.

Did you consider that kapp-controller would not hang the deletion process and just keep the volumes in the cluster if they are not tracked? That would be similar to the Helm approach.

[joe-kimmel-vmw]

@beltran-rubo kapp-controller delegates the deletion logic to kapp, but from the user perspective of course it all looks like kapp-controller. We've had previous discussion around whether kapp should have a flag for basically "delete all the things you know how to delete, ignoring or removing any finalizers that carvel owns (so that those resources gets deleted), and not waiting on cleaning up dependencies you know about but don't own (such as PVCs created outside of kapp)."

If that flag existed and there were a way to invoke it through kapp-controller, would that meet the desires you're describing?

[cppforlife]

(back from vacation)

kapp has a default ownership rules that get applied to PVCs -- https://github.com/vmware-tanzu/carvel-kapp/blob/develop/pkg/kapp/config/default.go#L223-L237. you can disable it per resource by adding kapp.k14s.io/disable-default-ownership-label-rules annotation. (if you do want to own them for deletion, then as you mentioned you can also use another annotation to mark them as such)

This solution would work for users who can modify the yaml files and are specific built for Carvel packages. In our case, we are using the Helm charts as the source-of-truth for building the Carvel packages so adding that annotation to the Helm templates would mean to mix Carvel annotations there.

since kapp is the shared deploy mechanism, annotation mentioned above are not expected to be only used for ytt templates. they are equally useful for configuration coming form other places including helm templates. you can for example add ytt step after helm template that would add such annotation to statefulset if you do not want to modify helm chart directly.

Did you consider that kapp-controller would not hang the deletion process and just keep the volumes in the cluster if they are not tracked?

this is not a kapp-controller choice since this behaviour is coming from kapp. (for the record, it is not a hang -- kapp will fail waiting for them to be gone after 15min by default.). we'll need to see if it makes sense to change this deafult behaviour in kapp, but there would be concerns around breaking backwards compatibility.

[beltran-rubo]

Thanks for your detailed notes and thanks for reconsidering that behaviour. @joe-kimmel-vmw if that flag is implemented it would be a valid 'workaround' for the PVCs created by StatefulSets.

[neil-hickey]

This is a behaviour of kapp that right now is as expected. Anything futher to add on this one @joe-kimmel-vmw / @cppforlife ?