add azure machine pool machine proposal

[devigned]

What type of PR is this?
/kind design

What this PR does / why we need it:
The scope of work for implementing safe rolling upgrades for AzureMachinePool with MaxUnavailable and MaxSurge has grown to warrant a proposal. This proposal provides background, tradeoffs and the design strategy to explain why we want to break out AzureMachinePoolMachines from AzureMachinePools.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Related to #1105

TODOs:

• squashed commits
• includes documentation
• adds unit tests

Release note:

Add AzureMachinePoolMachine proposal which describes the design strategy for implementing rolling upgrades for AzureMachinePools

[CecileRobertMichon]

Thank you so much for taking the time to write this down. It helped me understand better what the goals are in #1105, and I'm sure it will help future contributors and users as well.

Overall this looks good to me but I challenged some of the assumptions / implied design choices that might not be obvious to others. My main concern is around the delete scenario and how the sync works between Azure resource deletions and k8s CR deletions.

[CecileRobertMichon]

How do we make it clear to the users that they are not responsible for creating the AzureMachinePoolMachines?

[devigned]

That's a great question. Do you have any suggestions?

My first inclination is to add documentation.

There really isn't anything that stops someone from creating an AMPM. It won't hurt anything.

[nader-ziada]

Thank you @devigned for this doc, very helpful in understanding how all this works

[fiunchinho]

Looking at how the upgrade process is implemented in KCP we could try to make the two look even more similar. There are two fields that I believe could be useful here:

• nodeDrainTimeout introduced in KCP here, it allows the user to specify the amount of time we want to wait for a node to be drained. If capz is taking care of draining, I believe it should allow users to specify this.
• upgradeAfter introduced in KCP here, is a field to indicate an upgrade should be performed after the specified time, to allow users to schedule upgrades.

What do you think?

[devigned]

upgradeAfter introduced in KCP here, is a field to indicate an upgrade should be performed after the specified time, to allow users to schedule upgrades.

@fiunchinho I don't think that upgradeAfter makes sense for AzureMachinePool since if there are no changes to the underlying model, then there will be nothing to upgrade.

I added the NodeDrainTimeout. Thank you!

[fiunchinho]

upgradeAfter introduced in KCP here, is a field to indicate an upgrade should be performed after the specified time, to allow users to schedule upgrades.

@fiunchinho I don't think that upgradeAfter makes sense for AzureMachinePool since if there are no changes to the underlying model, then there will be nothing to upgrade.

I added the NodeDrainTimeout. Thank you!

Totally with you. I miss understood the upgradeAfter field. Thanks.

[CecileRobertMichon]

nit: indent

[devigned]

fixed

[CecileRobertMichon]

/lgtm

[nader-ziada]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nader-ziada

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [nader-ziada]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment