fix: remove node driver registrar lifecycle hook

[vadasambar]

• was causing /bin/sh not found error
• new node driver registrar version automatically removes the socket before terminating

Problem

We see the following error message every time csi driver pod restarts

"Dec  6 18:33:03 ip-xxx-xx-x-xx kubelet: I1206 18:33:03.442938    4937 handlers.go:67] Exec lifecycle hook ([/bin/sh -c rm -rf /registration/csi-image.warm-metal.tech /registration/csi-image.warm-metal.tech-reg.sock]) for Container \"node-driver-registrar\" in Pod \"csi-image-warm-metal-p9zzf_kube-system(f77a9f24-2f8a-46f6-a413-c6f1bfa7ea83)\" failed - error: command '/bin/sh -c rm -rf /registration/csi-image.warm-metal.tech /registration/csi-image.warm-metal.tech-reg.sock' exited with 126: , message: \"OCI runtime exec failed: exec failed: unable to start container process: exec: \\\"/bin/sh\\\": stat /bin/sh: no such file or directory: unknown\

Solution

This problem happens because node-driver-registrar does not have /bin/sh in it because it uses distroless image. This problem should happen every time warm metal pod shuts down.

node-driver-registrar versions < v2.0.0-rc1 don't remove the socket (/registration/csi-image.warm-metal.tech and /registration/csi-image.warm-metal.tech-reg.sock) automatically and we had to use the lifecycle hook. Our current warm-metal chart in master uses v2.8.0 version of node-driver-registrar which automatically removes the sockets for us (kubernetes-csi/node-driver-registrar#21 (comment)). Hence, we don't need the lifecycle hook anymore.

Related

• Ceph CSI Lifecycle Hook fails rook/rook#4427
• node-driver-registrar preStop hook fails cert-manager/csi-driver#109
• node-driver-registrar preStop fails because /bin/sh doesn't exist kubernetes-sigs/aws-ebs-csi-driver#747

Checks

Helm lint passes without any issues:

[vadasambar]

We need another PR to update our node-driver-registrar version in the installer and samples from v1.1.0 to v2.8.0.

More info on the installer: https://github.com/warm-metal/csi-driver-image/tree/master#installation

[mugdha-adhav]

We need another PR to update our node-driver-registrar version in the installer and samples from v1.1.0 to v2.8.0.

More info on the installer: https://github.com/warm-metal/csi-driver-image/tree/master#installation

Could we update them in this PR itself?

[vadasambar]

We need another PR to update our node-driver-registrar version in the installer and samples from v1.1.0 to v2.8.0.
More info on the installer: https://github.com/warm-metal/csi-driver-image/tree/master#installation

Could we update them in this PR itself?

I think it could be a change that can be handled in a separate PR because the change is related to the installer while this PR is more concerned with the chart. Ideally, it would be nice to have both the changes in the same PR but I think we can split the changes into 2 PRs. Let me know what you think.

[mugdha-adhav]

I think it could be a change that can be handled in a separate PR because the change is related to the installer while this PR is more concerned with the chart. Ideally, it would be nice to have both the changes in the same PR but I think we can split the changes into 2 PRs. Let me know what you think.

Yeah, I think we should fix it in a separate PR. Could you please create an issue to update the installer?

[mugdha-adhav]

@vadasambar we should be updating our master with this change soon. Could you please update this PR and get it ready for review again?

[vadasambar]

@vadasambar we should be updating our master with this change soon. Could you please update this PR and get it ready for review again?

Will do and mention you here again. Thanks for the reminder.

[vadasambar]

@mugdha-adhav updated the PR but forgot to mention you. Can you take a look? I think the PR is ready for review.

[mugdha-adhav]

Could you please also remove the remaining references for the hook?

And also could we update the Makefile and Chart.yaml with v1.0.1 so that we don't need another PR for releasing this change?

[mbtamuli]

The PR looks good. 👍🏼