Cluster-api-provider-azure: MachinePool K8s upgrade should provide MinAvailable, MaxSurge, with safe rolling updates

[CecileRobertMichon]

/kind bug

https://github.com/kubernetes-sigs/cluster-api/issues/3160

Create a cluster with a machine pool, eg. :

NAME                                   STATUS   ROLES    AGE     VERSION
capz-vmss-master-control-plane-7k9ql   Ready    master   15m     v1.17.5
capz-vmss-master-control-plane-bx7vl   Ready    master   18m     v1.17.5
capz-vmss-master-control-plane-jt4z5   Ready    master   19m     v1.17.5
capz-vmss-master-mp-0000000            Ready    <none>   4m10s   v1.17.5
capz-vmss-master-mp-0000002            Ready    <none>   4m14s   v1.17.5

Try to edit the mp resource and change Spec.Version to another k8s version, such as v1.18.3. Nothing happens. The mp stays in phase "running" and no update is made to amp.

NAME                    PHASE
capz-vmss-master-mp-0   Running

NAME                    REPLICAS   READY   STATE
capz-vmss-master-mp-0   2          true    Succeeded

The upgrade policy is currently set to "manual". In order to implement rolling upgrade, we'd have to loop over the instances and drain/update them one by one.

In the future, we might be able to leverage vmss rolling upgrade by setting MaxUnavailable (supported) and MaxSurge (not currently supported) and make those configurable / exposed to the user.

See also https://github.com/awesomenix/drainsafe by @awesomenix which we might be able to leverage to drain the nodes before they get deleted.

[CecileRobertMichon]

/priority important-longterm
/milestone next

[awesomenix]

Also want to add that there is a small setting change required on VMSS

https://docs.microsoft.com/en-us/azure/virtual-machine-scale-sets/virtual-machine-scale-sets-terminate-notification#rest-api

to allow drainsafe to receive termination events. If we can integrate drainsafe (or similar mechanism), we can treat upgrades as any other maintenance operation which would mean that there shouldnt be any special code for upgrade (optionally just include VMSS rolling upgrade directly). If its not yet assigned to anybody i can work on this issue

[awesomenix]

/assign @awesomenix

[CecileRobertMichon]

@awesomenix go for it!

[CecileRobertMichon]

@awesomenix any updates on this? We'd like to move MachinePool out of exp/ as part of v1alpha4 which is planned for Q4 and this is a blocker for that.

[awesomenix]

Sorry have been swamped with work past few weeks. I will get to it this weekend and send PR in stages.

[awesomenix]

Sorry for the delay to get this completed, I am playing around rolling upgrade + scheduled events and i see multiple problems

1. When terminationNotificationTimeout is set, I dont see it being applied to VMSS, seems like a bug from my PR, investigation needed
2. During rolling updated, VMSS doesnt seem to emit scheduled events at all. Scheduled events seems to be only emitted on reboot or delete of instance.
3. I thought rolling update would be creating new instance and deleting old one. Instead VMSS applies update in-place which means this would be downtime for workloads, since after cordon and drain, there may not be additional vms to run workloads.

I can see few options going forward.

1. Followup with Microsoft VMSS team to see if surge behaviour can be implemented.
2. Implement above workflow ourselves. Which includes
   
   
   
   1. ScaleUp VMSS instances + (Surge Value)
   
   
   2. Update each instances manually while performing cordon and drain
   
   

I dont have necessary contacts for VMSS team, if @CecileRobertMichon or @devigned could follow up would be really helpful. If we choose to go for option 2 I can start working on design and code.

I will be offline for next 3 weeks on personal agenda, if someone else wants to take up this work please do reassign.

[CecileRobertMichon]

Thanks @awesomenix. I know that vmss MaxSurge is planned but I don't know about the exact timeline for it so I think we should start with option 2 for now with the plan to change it to 1 once we can rely on built-in surge.

I will also follow up on "During rolling updated, VMSS doesnt seem to emit scheduled events at all. Scheduled events seems to be only emitted on reboot or delete of instance.", that doesn't seem right.

[devigned]

Perhaps, we should rethink upgrade. What if CAPZ updates the model, then deletes an instance and allows VMSS to recreate the instance with the new model?

Seems like we would get the delete instance event, and we could control surge / max unavailable ourselves.

Thoughts?

Thank you for working on this @awesomenix!

[CecileRobertMichon]

Seems like we would get the delete instance event, and we could control surge / max unavailable ourselves.

That's interesting. At that point, we wouldn't even need to delete instance event since we would be the ones triggering the delete, so we would know when it is getting deleted. Although there might be some value in having a generic "drain before delete" implementation where the nodes get drained no matter what/who triggered the delete.

[devigned]

After doing a bit of experimentation with behavior of scaling configuration and instance delete I found that if manual scaling is enabled on the VMSS (a manual set number of replicas) and you delete an instance, the number of replicas decreases by 1. That is to say, if you have 3 replicas on the VMSS and you delete an instance, the VMSS will now only have 2 replicas.

On the other hand, if you set scaling rules to be minimum number of replicas = 3 and maximum number of replicas = 3, then delete an instance, VMSS will rebuild the instance that was deleted. That is, VMSS will respect the minimum number of replicas, 3, by replacing the deleted instance.

I believe we could use the min and max behavior to managed our own upgrade flow.

[CecileRobertMichon]

/milestone v0.4.10

[devigned]

Based on #983 and pending resolution to #1000, we can complete rolling upgrade via the AzureMachinePool controller. I believe our first implementation should attempt to cordon and drain nodes with out dependence on Azure platform features (host events: there is an ongoing discussion about adding new host events to unblock this scenario, but it won't be around for a while).

Since we will likely not have host events, we should implement min, max surge and controller driven cordon and drain with the goal of moving it to depend on platform features in the future.