Delete of spire-oidc might get stuck if the csi driver got removed first

[marcofranssen]

When doing a helm uninstall in some situations the spire-oidc pod gets stuck in a terminating state.
This also prevents you from deleting the namespace.

This could happen already when you do a simple helm uninstall.

The best approach to prevent this issue is to delete the oidc deployment first.

kubectl delete --namespace spire-server deployment spire-spiffe-oidc-discovery-provider
helm uninstall --namespace spire-server spire
kubectl delete ns spire-server

In case you already got stuck your namespace removal you can do following:

Run following in a separate terminal

kubectl proxy

Then run following to cleanup the namespace

namespace=spire-system
kubectl get namespace "${namespace}" -o json | jq 'del(.spec.finalizers)' >tmp.json
curl -k -H "Content-Type: application/json" -X PUT --data-binary @tmp.json "http://127.0.0.1:8001/api/v1/namespaces/${namespace}/finalize"

We probably should add a piece of documentation to inform the users of our chart. It could basically happen with any pod/namespace that depends on the csi driver.

[kfox1111]

Any pod using a csi driver will behave badly when you pull the csi driver out from under it...

It wont just affect spire-spiffe-oidc-discovery-provider, but any workload using the regular spiffe csi driver.

If nested spire is being used, spire-server will malfunction if the upstream csi driver is pulled too.

[kfox1111]

Here is a command to find out if the driver is being used by any pod:

kubectl get pods --all-namespaces -o go-template='{{range .items}}{{$nn := printf "%s %s" .metadata.namespace .metadata.name}}{{range .spec.volumes}}{{if .csi.driver}}{{if eq .csi.driver "csi.spiffe.io"}}{{printf "%s\n" $nn}}{{end}}{{end}}{{end}}{{end}}'

[kfox1111]

I don't know how safe it is to pull the kubernetes finalizer. There may be other things that get affected by that, not just the spiffe csi driver....

If the user gets into that state with their own workload, I think we should probably recommend reinstalling the csi driver so it can properly delete the other pods before removal rather then recommend they remove finalizers.

As for the workloads within our chart, while I generally don't like helm hooks much, a pre-delete hook feels pretty safe and reasonable to remove the resources that will get deleted anyway to make sure they are deleted before the csi driver gets removed. We need to test that carefully though to make sure its safe in all conditions.

[kfox1111]

I just tried purposefully failing a pre-delete hook.

$ helm delete test
Error: pod hook-preinstall failed

It leaves everything in place and the job in Error state, so the logs can be looked at.

So, I think we can have 3 jobs, weighted in the right order:

1. delete spiffe-oidc-discovery-provider in the spiffe-oidc-discovery-provider chart
2. delete spire-server if nested spire in the spire-server chart
3. check for any pod still using the csi driver. fail without retry if ones still found. This will block the driver removal.

[edwbuck]

I don't know how safe it is to pull the kubernetes finalizer. There may be other things that get affected by that, not just the spiffe csi driver....

It is generally not safe at all, the best case scenario involves the admin being fully aware of all the tasks the finalizer would have performed, and then deciding which ones still need to be performed, and then manually performing them.

Part of this blocked one of my enterprise customers from using the CSI driver, as they don't expose the ability to perform such administrative operations to the same developers they permit deployment and removal of Helm Charts, and the authorized administrative team doesn't have the tolerance to perform this manual cleanup frequently (aka once).

If the user gets into that state with their own workload, I think we should probably recommend reinstalling the CSI driver so it can properly delete the other pods before removal rather then recommend they remove finalizers.

The pre-delete hook could ensure the driver is installed, if values.yaml is configured to support it. The post-delete hook could ensure the driver is removed. This ordering should help, and will minimize future failed deployment removals.

As for the workloads within our chart, while I generally don't like helm hooks much, a pre-delete hook feels pretty safe and reasonable to remove the resources that will get deleted anyway to make sure they are deleted before the csi driver gets removed. We need to test that carefully though to make sure its safe in all conditions.

I know the hooks aren't very elegant, but this is the use case that created them. I would not recommend removing the pods in pre-delete, as ensuring the CSI is installed / reinstalled in pre-delete has the "bulk" of the application removal located where it is expected.

Once the work is done here, generally it should become easier to expand the pattern to all of the other helm hooks. The CSI driver then becomes part of the pre-install hook, and is "ensured to be present" in the post-install hook" While some of these steps might seem like busy work, they make it far easier to handle Helm rollbacks, including rollbacks from uninstallation, or rollbacks of installation that effectively remove the deployment.

Note: sub-chart ordering complicates this process significantly, because sub-charts are intended to manage independent application dependencies, not components of the same micro-service. Much like I wouldn't put a service into a sub-chart and its ConfigMap into a different sub-chart and then attempt to figure out how to get them ordered, the CSI driver should not be in a different sub-chart from its users.

[kfox1111]

I dont follow. What does csi install/reinstall have to do with deleting the chart? The issue described is when deleting the chart and helm's lack of ordering. Also, I don't think you can rollback a chart deletion? I could be wrong though.

I dont think subcharts have much effect on hooks either. All the same type of hooks are ran at the same time regardless of nesting, unless you specify an explicit ordering, in which case you have control even over multiple subcharts via priority annotation.