Add KubeadmControlPlanePhase to CRD

[sivchari]

What would you like to be added (User Story)?

I suggest to add KubeadmControlPlanePhase to CRD. Currently, Cluster and MachineDeployment has individual phase, but KCP doesn't have it. KCP is same as MachineDeployment, because ControlPlane is machine, too.

Detailed Description

TBW

Anything else you would like to add?

No response

Label(s) to be applied

/kind feature One or more /area label. See https://github.com/kubernetes-sigs/cluster-api/labels?q=area for the list of labels.

[k8s-ci-robot]

This issue is currently awaiting triage.

If CAPI contributors determine this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions 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.

[fabriziopandini]

@sivchari which phases do you envision for KCP? how they are defined?

[sivchari]

I want Scaling Up, Scaling Down and Running phase. This phase is changed following and changing by replicas and other field.

[chrischdi]

You should be able to take a look at the ResizedCondition of a KCP:

https://github.com/kubernetes-sigs/cluster-api/blob/88905d4349f793db458b719e9844ba7960ed2004/controlplane/kubeadm/api/v1beta1/condition_consts.go#L61

It is getting set here:

https://github.com/kubernetes-sigs/cluster-api/blob/88905d4349f793db458b719e9844ba7960ed2004/controlplane/kubeadm/internal/controllers/status.go#L65-L86

And from quickly reading it, the condition should either be:

• True: if all kcp machines are ready
• False with controlplanev1.ScalingUpReason for scaling up.
• False with controlplanev1.ScalingDownReason for scaling down.

[sivchari]

I didn't know this condition so far, thanks. Then why does MachineDeployment has conditions and phase ?

[chrischdi]

Good question, maybe searching the PRs which introduced the one or the other or the ones for the proposals and reading through them may have context on that.

[fabriziopandini]

I was looking into API conventions and they are stating:

Some resources in the v1 API contain fields called phase, and associated message, reason, and other status fields. The pattern of using phase is deprecated. Newer API types should use conditions instead. Phase was essentially a state-machine enumeration field, that contradicted system-design principles and hampered evolution, since adding new enum values breaks backward compatibility. Rather than encouraging clients to infer implicit properties from phases, we prefer to explicitly expose the individual conditions that clients need to monitor. Conditions also have the benefit that it is possible to create some conditions with uniform meaning across all resource types, while still exposing others that are unique to specific resource types. See #7856 for more details and discussion.

So I think we should close this issue because we should align to API conventions (that' means also dropping existing phase fields from other resources in a future API version)

/close

[k8s-ci-robot]

@fabriziopandini: Closing this issue.

In response to this:

I was looking into API conventions and they are stating:

Some resources in the v1 API contain fields called phase, and associated message, reason, and other status fields. The pattern of using phase is deprecated. Newer API types should use conditions instead. Phase was essentially a state-machine enumeration field, that contradicted system-design principles and hampered evolution, since adding new enum values breaks backward compatibility. Rather than encouraging clients to infer implicit properties from phases, we prefer to explicitly expose the individual conditions that clients need to monitor. Conditions also have the benefit that it is possible to create some conditions with uniform meaning across all resource types, while still exposing others that are unique to specific resource types. See #7856 for more details and discussion.

So I think we should close this issue because we should align to API conventions (that' means also dropping existing phase fields from other resources in a future API version)

/close

Instructions 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.