Provide an option to retain failed machine

[aylei]

What would you like to be added:
An --machine-delete-policy option that support retaining failed machine. Two strategies are proposed:

• Delete: current behavior
• Orphan: remove failed machine from the machine set (orphan it)

Why is this needed:
I run stateful applications on gardener and using local persistent volume, machine deletion is a critical operation because it also delete all data in the local PV. And I've witnessed that all nodes were get replaces when the load balancer of shoot apiserver was unhealthy and lost all local data.
As a more conservative strategy, the machineset controller could orphan the failed machine from the machineset without actually deleting it. The orphaned machines can then be deleted by human operators with their manual confirmation.

[hardikdr]

Overall sounds like a good idea to me.
The only part, we might want to consider is, if there are constant failures, such machines could stack up, and lead to unnecessary costs if not seen by the user. We have a freeze mechanism in place, where if number of machine-objects goes beyond a certain number MCM stops scaling-up.

• You might want to complement the above-suggested feature with freeze, at some point we stop creating the new machines.

Also, out of curiosity, if you are not willing to replace the unhealthy machine, you can simply set a large health-timeout flag, and MCM should not touch it. There could be an improvement to set -1 to signify infinite timeout.

• Or is it that, you do want to spawn new machines in the place of unhealthy ones, but also willing to keep the old ones?

[hardikdr]

And I've witnessed that all nodes were get replaces when the load balancer of shoot apiserver was unhealthy and lost all local data.

I think we have not faced it but thought about this case. Basically, kubelets can't reach the APIServer, but all the rest of the control-plane components[kcm, mcm] can reach it. Hence, node-objects go stale, KCM starts evicting and, for MCM machines are unhealthy hence will be replaced.

@amshuman-kr @ggaurav10 you worked on this case and also implemented the mitigation by disabling the control-plane if the load-balancer is unhealthy, right?
Can you please elaborate a little around, how can this be avoided?

[dguendisch]

Out of curiosity: what is important on that local volume data? I ask as pods managing state typically do this on PVCs. I mean your pod can be evicted and respawn on another node anytime, also with no machine failures, the pods also "lose" the local data in such cases.

[aylei]

• You might want to complement the above-suggested feature with freeze, at some point we stop creating the new machines.

Good call! I've considered something similar and it is great to know we already have this.

• Or is it that, you do want to spawn new machines in the place of unhealthy ones, but also willing to keep the old ones?

Yes, I do want to spawn new machines to keep the healthy nodes count match the spec

[aylei]

Out of curiosity: what is important on that local volume data? I ask as pods managing state typically do this on PVCs. I mean your pod can be evicted and respawn on another node anytime, also with no machine failures, the pods also "lose" the local data in such cases.

Actually we use local PV and all the data of our database goes to the local volume, the Pods do assume the local PV via PVCs, but the PVC topology constraint will no longer be satisfied if the Node of the local PV is down.

BTW, I'm going to elaborate this in depth in today's Gardener community meeting. Hope that will make things clear.

[amshuman-kr]

And I've witnessed that all nodes were get replaces when the load balancer of shoot apiserver was unhealthy and lost all local data.

@aylei Gardener deploys the dependency-watchdog probe as part of the seed bootstrap which probes the shoot apiservers via the loadbalancer and scales down the kube-controller-manager to avoid loosing nodes in that scenario. Did that not work in your case? The probe was explicitly introduced to manage the above case you mentioned.

[vasu1124]

Minor sidenote on naming convention: I think we should reuse keywords from storageClass, i.e. someting like --machine-reclaimPolicy:

• Delete: default behavior.
• Retain: remove from working pool, but retain the machine.

This way, we do not create new terminology and can attach to already existing concepts.

[btw. maybe then we could have other tools that somehow could still operate on the machine (depends on the infrastructure, I know) and extract data or re-animate the machine and so on.]

[vlerenc]

I like the feature as well, something like a "quarantine". If we do that, we should also specify a max number of such machines that shall be "parked"/"ignored". We don't want them to pile up unbound like retained volumes. It's a debugging feature, so 1 or 2 is what I would people to normally desire.