Modify GetMachineDeploymentNodes to return unregistered nodes

[jsravn]

This is needed so the autoscaler can track nodes that fail to provision. With this change the autoscaler will correctly try other node groups if a node group is failing, such as due to InsufficentInstanceCapacity.

Related to #37 and #35.

Release note:

MCM provider now also returns unregistered nodes to Autoscaler. This change enables autoscaler to pick up an alternate worker-pool if the chosen one can't be scaled-up.

[gardener-robot]

@jsravn Thank you for your contribution.

[CLAassistant]

All committers have signed the CLA.

[gardener-robot-ci-2]

Thank you @jsravn for your contribution. Before I can start building your PR, a member of the organization must set the required label(s) {'reviewed/ok-to-test'}. Once started, you can check the build status in the PR checks section below.

[jsravn]

I've been testing this a bunch, and it seems to fix the main issue I was experiencing where the autoscaler would get stuck if any MachineDeployment fails to scale. By returning the unregistered nodes, the autoscaler will eventually time out the failing node pools (using the -max-node-provision-time) and scale up other node pools instead. This should be a good step to supporting spot instance pools which have worse availability than the on demand instance types.

[hardikdr]

Thanks a lot for the PR @jsravn .
A minor doubt/query looks great anyways.

[hardikdr]

Gave it a quick try, and it solves the fundamental issue, that another machine-deployment is chosen after ~15mins.[MaxNodeProvisionTime].

Basically, scale-up is backed-off for the failed machine-deployment.
The failed-machine deployment is reconsidered for scale-up after ~30mins, and then it can again be used for the usual workload.

• I am curious if there could be an elegant solution, where we could fail faster when insufficient-capacity is seen. But that'll definitely bring a bit of complexity.

[hardikdr]

@prashanth26 Would you want to take a quick look at the PR?

[jsravn]

I am curious if there could be an elegant solution, where we could fail faster when insufficient-capacity is seen. But that'll definitely bring a bit of complexity.

I think it's possible but will need to be done on the MCM side. It needs to report the machine deployment as unable to scale further and then the MCM autoscaler cloud provider should adjust its max size appropriately (freezing it if it can't scale up). Something like this.

[hardikdr]

I think it's possible but will need to be done on the MCM side. It needs to report the machine deployment as unable to scale further and then the MCM autoscaler cloud provider should adjust its max size appropriately (freezing it if it can't scale up). Something like this.

We implemented something very similar at MCM in past, but the PR couldn't make it through. We felt it's bringing complexity, but maybe something to reconsider on that front. See: gardener/machine-controller-manager#454
@prashanth26 had a few concerns and a proposal for another approach.

• MCM also post FailedMachineSummary in the Status, which can be parsed by Autoscaler, but that summary is also short-lived and will be gone once machines are healthy. An annotation that stays for a long time[~5-6 hours] could actually be useful.

[hardikdr]

/assign
/lgtm

[prashanth26]

/lgtm apologies for the delay in review.