-
Notifications
You must be signed in to change notification settings - Fork 1.4k
📖 Add implementation notes to the in-place update proposal #12880
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
📖 Add implementation notes to the in-place update proposal #12880
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great overall!
Only left a couple of non-blocking comments for typos and minor text/diagram wording.
| @@ -0,0 +1,88 @@ | |||
| # In-place updates in Cluster API - Implementations notes | |||
|
|
|||
| This document is an collection of notes about implementation details for the in-place update proposal. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| This document is an collection of notes about implementation details for the in-place update proposal. | |
| This document is a collection of notes about implementation details for the in-place update proposal. |
| MS Controller->>M1: Move M1 to MS2 (NewMS)<br/>Apply annotation ".../pending-acknowledge-move": ""<br/>Apply annotation ".../update-in-progress": "" | ||
| ``` | ||
|
|
||
| Workflow #3: MD controller recongnizes the newMS being moved to the newMS and it scales up newMS to acknowledge the operation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Workflow #3: MD controller recongnizes the newMS being moved to the newMS and it scales up newMS to acknowledge the operation | |
| Workflow #3: MD controller recognizes that a Machine has been moved to the new MachineSet and scales up the new MachineSet to acknowledge the operation. | |
| autonumber | ||
| participant MS Controller as MS Controller<br/>when reconciling<br/>MS2 (NewMS) | ||
| participant MS2 (NewMS) | ||
| participant M1 as M1<br/>now controller by<br/>MS2 (NewMS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| participant M1 as M1<br/>now controller by<br/>MS2 (NewMS) | |
| participant M1 as M1<br/>now controlled by<br/>MS2 (NewMS) |
| MD Controller->>MS2 (NewMS): Scale up to acknowledge M1<br/>Apply annotation ".../acknowledged-move": "M1" | ||
| ``` | ||
|
|
||
| Workflow #4: MS controller, when reconciling newMS, detects a machine has been acknoledged; it cleanups annotation on the machine and this unblocks the in-place upgrade to start |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Workflow #4: MS controller, when reconciling newMS, detects a machine has been acknoledged; it cleanups annotation on the machine and this unblocks the in-place upgrade to start | |
| Workflow #4: MS controller, when reconciling newMS, detects that a machine has been acknowledged; it cleans up annotations on the machine, allowing the in-place upgrade to begin. | |
| participant M1 as M1<br/>now controlled by<br/>MS2 (NewMS) | ||
| MD Controller-->>M1: Are you pending acknowledge? | ||
| M1-->>MD Controller: Yes! | ||
| MD Controller->>MS2 (NewMS): Scale up to acknowledge M1<br/>Apply annotation ".../acknowledged-move": "M1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| MD Controller->>MS2 (NewMS): Scale up to acknowledge M1<br/>Apply annotation ".../acknowledged-move": "M1" | |
| MD Controller->>MS2 (NewMS): Scale up to acknowledge receipt of M1<br/>Apply annotation ".../acknowledged-move": "M1" |
| - MS controller, when reconciling the newMS, will take over the moved machine and start the actual in-place upgrade operation | ||
|
|
||
| - Orchestration of in-place upgrades between MD controller, MS controller, and Machine controller is implemented using annotations. | ||
| Following schemas provide a overview of how new annotation are used |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Following schemas provide a overview of how new annotation are used | |
| Following schemas provide an overview of how new annotations are used. |
| - Old MS will be informed to move machines to the newMS, and newMS will be informed it will receive machines from oldMS. | ||
| - MS controller manages a subset of Machines | ||
| - When scaling down the old MS, if required to move, MS controller is responsible for moving a Machine to newMS | ||
| - MS controller, when reconciling the newMS, will take over the moved machine and start the actual in-place upgrade operation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - MS controller, when reconciling the newMS, will take over the moved machine and start the actual in-place upgrade operation | |
| - When reconciling the new MachineSet, the MS controller takes ownership of the moved machine and begins the actual in-place upgrade. |
| - The implementation respects the existing set of responsibilities of each controller | ||
| - MD controller manages MS | ||
| - MD controller enforces maxUnavailable, maxSurge | ||
| - MD controller decides when to scale up newMS, when to scale down oldMS | ||
| - When there is a decision to scale down, MD controller should check if this can be done via in-place vs delete/recreate. If in-place is possible: | ||
| - Old MS will be informed to move machines to the newMS, and newMS will be informed it will receive machines from oldMS. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - The implementation respects the existing set of responsibilities of each controller | |
| - MD controller manages MS | |
| - MD controller enforces maxUnavailable, maxSurge | |
| - MD controller decides when to scale up newMS, when to scale down oldMS | |
| - When there is a decision to scale down, MD controller should check if this can be done via in-place vs delete/recreate. If in-place is possible: | |
| - Old MS will be informed to move machines to the newMS, and newMS will be informed it will receive machines from oldMS. | |
| - The implementation respects the existing set of responsibilities of each controller: | |
| - MD controller manages MS: | |
| - MD controller enforces maxUnavailable, maxSurge | |
| - MD controller decides when to scale up newMS, when to scale down oldMS | |
| - When scaling down, the MD controller checks whether the operation can be performed in-place instead of delete/recreate. If in-place is possible: | |
| - Old MS is instructed to move machines to the newMS, and newMS is informed to receive machines from oldMS. |
|
|
||
| - In place is always considered as potentially disruptive | ||
| - in place must respect maxUnavailable | ||
| - if maxUnavailable is zero, a new machine must be created, then as soon as there is “buffer” for in-place, in-place update is done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - if maxUnavailable is zero, a new machine must be created, then as soon as there is “buffer” for in-place, in-place update is done | |
| - if maxUnavailable is zero, a new machine must be created first, then as soon as there is a “buffer” for in-place, the in-place update can proceed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - if maxUnavailable is zero, a new machine must be created, then as soon as there is “buffer” for in-place, in-place update is done | |
| - if maxUnavailable is zero, a new machine must be created, then as soon as there is “buffer” for in-place, in-place update can proceed |
Just to avoid interpretation of "done" as "finished".
| This document is an collection of notes about implementation details for the in-place update proposal. | ||
|
|
||
| As soon as the implementation will be completed, some of the notes in this document will be moved back | ||
| into the proposal or moved to the user facing documentation about this feature. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| into the proposal or moved to the user facing documentation about this feature. | |
| into the proposal or into the user-facing documentation for this feature. |
|
Unknown CLA label state. Rechecking for CLA labels. Send feedback to sig-contributor-experience at kubernetes/community. /check-cla |
|
@lentzi90 @furkatgofurov7 thanks for feedback, everything should be addressed now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
|
LGTM label has been added. Git tree hash: 72a05da489f4d135173f986fa6ad8a30c72a2f97
|
|
/assign |
| participant RX | ||
| participant MS1 (OldMS) | ||
| participant MS2 (NewMS) | ||
| MD Controller-->>+RX: Can you update in-place from MS1 (OldMS) to MD2 (NewMS)? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| MD Controller-->>+RX: Can you update in-place from MS1 (OldMS) to MD2 (NewMS)? | |
| MD Controller-->>+RX: Can you update in-place from MS1 (OldMS) to MS2 (NewMS)? |
| participant MS Controller as MS Controller<br/>when reconciling<br/>MS1 (OldMS) | ||
| participant MS1 (OldMS) | ||
| participant MS2 (NewMS) | ||
| participant M1 as M1<br/>controlled by<br/>MS1 (OldMS),<br/>selected to be moved to MS2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| participant M1 as M1<br/>controlled by<br/>MS1 (OldMS),<br/>selected to be moved to MS2 | |
| participant M1 as M1<br/>controlled by<br/>MS1 (OldMS),<br/>selected to be moved to MS2 (NewMS) |
|
Thx! /lgtm |
|
LGTM label has been added. Git tree hash: 912e9ef1456764b16e97aca8738a85e0a1f80af6
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbueringer The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What this PR does / why we need it:
Start collecting some notes about the ongoing work for in-place
Which issue(s) this PR fixes:
Fixes #
Part of #12291
/area documentation