馃尡 feat: add support for "initialization completed" to machine pools - #14030
馃尡 feat: add support for "initialization completed" to machine pools#14030richardcase wants to merge 3 commits into
Conversation
kubernetes-prow
Bot
added
the
area/machinepool
kubernetes-prow
Bot
added
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
For initial review: /assign mboersma |
Add InfrastructureMachinePool contract helpers for the v1beta2 "initialization completed" contract change: Provisioned (contract-version aware, falling back to status.ready for v1beta1), ProviderIDList, Replicas and InfrastructureMachineKind, plus a StringSlice accessor type used by ProviderIDList.
Replace the hardcoded status.ready read (external.IsReady) in reconcileInfrastructure with the contract-version aware InfrastructureMachinePool().Provisioned() accessor, and migrate the providerIDList, replicas and infrastructureMachineKind reads to the corresponding contract accessors. Align initialization.infrastructureProvisioned with the documented API semantics: the field is never flipped back to false once provisioning completed (same behavior as the Machine controller).
Add status.initialization.provisioned to the v1beta2 DevMachinePool and mark status.ready as deprecated; the controller sets both fields until compatibility with the deprecated v1beta1 contract is dropped.
bd6a150 to
c1643b5
Compare
|
/lgtm |
kubernetes-prow
Bot
added
the
lgtm
|
LGTM label has been added. DetailsGit tree hash: 86a5ac75c179e5e5e9a3549f91eb2321ec386d1c |
kubernetes-prow
Bot
added
the
needs-rebase
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
| // Ready denotes that the machine pool is ready | ||
| // | ||
| // Deprecated: This field is deprecated and is going to be removed in a future apiVersion. Use initialization.provisioned instead. | ||
| // +optional | ||
| Ready bool `json:"ready"` |
There was a problem hiding this comment.
I think we should drop this field in v1beta2 directly. IIRC we just kept it because we couldn't migrate to the v1beta2 contract yet
|
I think this PR should be orthogonal but I'll wait with review until #13803 is merged just to be safe. |
4 participants
What this PR does / why we need it:
This adds support for the "initialization completed" status signal as per the updated infra MachinePool contract (here).
As part of this, we have added machine pools to the
contractpackage. One difference to the contract functions is that for machine pools a provider will declare they are compatible to a contract version as a while (i.e.v1beta2). However, the contract for MachinePools was updated afterv1beta2and so the contract methods have the concept of a "fallback" field that will be used if the primary contract field isn't available. To give an example:status.initialization.provisionedto its infra MachinePools yetready.statuswhich was the v1beta1 contractcontractfunction will trystatus.initialization.provisionedfirst and the fallback toready.statusThe internal infra providers (dev & docker) have had their APIs changed to add
status.initialization.provisioned.Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Relates #12178
/area machinepool