When the k8s API Server is unreachable, KCP.status.ready is always in true

[Levi080513]

What steps did you take and what happened?

When I was verifying the failure scenario, I manually deleted the CP Machine, and the apiserver was unreachable, KCP.status.readyReplicas is 0 but KCP.status.ready is always in true.

kc get kcp,machine,cluster  -l cluster.x-k8s.io/cluster-name=hw-sks-test-1-25-11-decrease
NAME                                                                                          CLUSTER                        INITIALIZED   API SERVER AVAILABLE   REPLICAS   READY   UPDATED   UNAVAILABLE   AGE   VERSION
kubeadmcontrolplane.controlplane.cluster.x-k8s.io/hw-sks-test-1-25-11-decrease-controlplane   hw-sks-test-1-25-11-decrease   true          true                   1          0       1         1             22h   v1.25.11

NAME                                                                                       CLUSTER                        NODENAME                                          PROVIDERID                                   PHASE      AGE   VERSION
machine.cluster.x-k8s.io/hw-sks-test-1-25-11-decrease-controlplane-pcg2n                   hw-sks-test-1-25-11-decrease   hw-sks-test-1-25-11-decrease-controlplane-xqnx2   elf://14e897cc-d407-4cb0-8b63-945f5c080e52   Deleting   22h   v1.25.11
machine.cluster.x-k8s.io/hw-sks-test-1-25-11-decrease-workergroup1-78998b57b8x8kg9ctpwjk   hw-sks-test-1-25-11-decrease   hw-sks-test-1-25-11-decrease-workergroup1-htn4d   elf://a6fa5db2-a90d-4ae5-96c9-3131fc42af85   Running    22h   v1.25.11

NAME                                                    PHASE         AGE   VERSION
cluster.cluster.x-k8s.io/hw-sks-test-1-25-11-decrease   Provisioned   22h

kc get kcp hw-sks-test-1-25-11-decrease-controlplane -ojson | jq .status
{
  "conditions": [
    {
      "lastTransitionTime": "2023-06-27T04:16:26Z",
      "message": "failed to delete K8s node hw-sks-test-1-25-11-decrease-controlplane-xqnx2 for Cluster default/hw-sks-test-1-25-11-decrease: Delete \"https://10.255.9.30:6443/api/v1/nodes/hw-sks-test-1-25-11-decrease-controlplane-xqnx2?timeout=10s\": dial tcp 10.255.9.30:6443: connect: no route to host",
      "reason": "DeletionFailed @ Machine/hw-sks-test-1-25-11-decrease-controlplane-pcg2n",
      "severity": "Warning",
      "status": "False",
      "type": "Ready"
    },
    {
      "lastTransitionTime": "2023-06-26T05:41:31Z",
      "status": "True",
      "type": "Available"
    },
    {
      "lastTransitionTime": "2023-06-26T05:38:09Z",
      "status": "True",
      "type": "CertificatesAvailable"
    },
    {
      "lastTransitionTime": "2023-06-27T04:15:11Z",
      "message": "Following machines are reporting control plane info: hw-sks-test-1-25-11-decrease-controlplane-pcg2n",
      "reason": "ControlPlaneComponentsUnhealthy",
      "severity": "Info",
      "status": "False",
      "type": "ControlPlaneComponentsHealthy"
    },
    {
      "lastTransitionTime": "2023-06-27T04:15:11Z",
      "message": "Following machines are reporting etcd member info: hw-sks-test-1-25-11-decrease-controlplane-pcg2n",
      "reason": "EtcdClusterUnhealthy",
      "severity": "Info",
      "status": "False",
      "type": "EtcdClusterHealthy"
    },
    {
      "lastTransitionTime": "2023-06-26T05:38:24Z",
      "status": "True",
      "type": "MachinesCreated"
    },
    {
      "lastTransitionTime": "2023-06-27T04:16:26Z",
      "message": "failed to delete K8s node hw-sks-test-1-25-11-decrease-controlplane-xqnx2 for Cluster default/hw-sks-test-1-25-11-decrease: Delete \"https://10.255.9.30:6443/api/v1/nodes/hw-sks-test-1-25-11-decrease-controlplane-xqnx2?timeout=10s\": dial tcp 10.255.9.30:6443: connect: no route to host",
      "reason": "DeletionFailed @ Machine/hw-sks-test-1-25-11-decrease-controlplane-pcg2n",
      "severity": "Warning",
      "status": "False",
      "type": "MachinesReady"
    },
    {
      "lastTransitionTime": "2023-06-26T05:40:59Z",
      "status": "True",
      "type": "Resized"
    }
  ],
  "initialized": true,
  "observedGeneration": 1,
  "ready": true,
  "readyReplicas": 0,
  "replicas": 1,
  "selector": "cluster.x-k8s.io/cluster-name=hw-sks-test-1-25-11-decrease,cluster.x-k8s.io/control-plane",
  "unavailableReplicas": 1,
  "updatedReplicas": 1,
  "version": "v1.25.11"
}

Is this the expected?

What did you expect to happen?

I expected KCP.status.ready is false when KCP.status.readyReplicas is 0.

Cluster API version

v1.4.3

Kubernetes version

v1.25.11

Anything else you would like to add?

Related issue:

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

Related discussion:

https://kubernetes.slack.com/archives/C8TSNPY4T/p1632762597003500

Label(s) to be applied

/kind bug 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 determines 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/test-infra repository.

[killianmuldoon]

From the above I see that the Ready appears to be false, so these two are definitely in disagreement and this is a bug that should be fixed IMO.

/triage-accepted

[fabriziopandini]

I'm not sure this is an issue. KCP.status.ready is part of the CAPI contract and it is used to orchestrate provisioning across CAPI and the control plane provider (KCP in this case).

More specifically, according the contract KCP.status.ready denotes when the control plane is ready to serve requests, and if I got it right from this example, the control plane was ready.

However KCP.status.ready doesn't have any other uses than orchestrate provisioning. It is not expected to change afterward to reflect the cluster state.

Conditions are used instead for reflecting the cluster operational state at any moment.

Given this context, it is acceptable they are in disagreement, even if I agree we should think if/how to make this more explicit

[jessehu]

In this case the KubeadmControlPlane API Server is not able to receive requests, so setting KCP.status.ready to false is reasonable ? https://github.com/kubernetes-sigs/cluster-api/blob/355348c26e6128c37ec35e3cd2ff1b24892ad96c/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_types.go#L197C1-L201C27

	// Ready denotes that the KubeadmControlPlane API Server is ready to
	// receive requests.
	// +optional
	Ready bool `json:"ready"`

[fabriziopandini]

However KCP.status.ready doesn't have any other uses than orchestrate provisioning. It is not expected to change afterward to reflect the cluster state.

That's my current understanding of the CAPI contract, once set to true this flag should never flip back to false.

[jessehu]

I see. By searching for kcp.Status.Ready (find by reference gives same result), kcp.Status.Ready is ready only in 1 place. kcp.Status.Ready is set to true when kcp.Status.ReadyReplicas > 0.

So if kcp.Status.ReadyReplicas <= 0, it makes sense to set kcp.Status.Ready to false ? If we need a flag to show it was ready once, how about using the kcp.Status.Initialized as flag or something else? Using kcp.Status.Ready as ready once flag is not so good and leads to confusion.

[fabriziopandini]

I'm not sure I still follow what problem you are trying to solve at this stage. We already have conditions that are responsible to represent the current state of the controlplane, we can improve them if necessary, but personally, I don't see the need of changing the function of KCP.status.ready to provide another, different representation of the current state of the controlplane. KCP.status.ready simply has a different function.

Also, this is an API/contract change that should be agreed by the community, and probably it requires a proposal or some form of document to keep track of the decision and the rationale behind it, but given the info I have on this thread, I'm -1 to this change

[jessehu]

Thanks for explaining the workflow! Is there an API contract description to mention KCP.status.ready only means KCP was ready once but do not guarantee it's currently ready? CAPI 1.4.x code doesn't say it. From the code, KCP.status.initialized is more silimiar to what you want to express.

https://github.com/kubernetes-sigs/cluster-api/blob/8b2541151f049ae975591cb0921c72cc6b022326/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_types.go#L268-L276

https://github.com/kubernetes-sigs/cluster-api/blob/8b2541151f049ae975591cb0921c72cc6b022326/controlplane/kubeadm/internal/controllers/status.go#L109-L117

[enxebre]

Thanks @jessehu! The .status.ready condition for the Control Plane resource gets bubbled up into the Cluster resource https://github.com/kubernetes-sigs/cluster-api/blob/5982e18ddd13ad02ee3fdc0d88dce93ff77780c8/internal/controllers/cluster/cluster_controller_phases.go#L251-L258 as both a field controlPlaneReady and a condition ControlPlaneReadyCondition. And that is reflected in the Cluster resource status. The stronger contract for CAPI core workflow comes via .spec.controlPlaneEndPoint which needs to flow as in Infra -> Cluster -> ControlPlane. See also https://cluster-api.sigs.k8s.io/developer/architecture/controllers/cluster.html#contracts

Your proposed change makes sense to me. However we've traditionally considered this field set and forget and therefore changes like this are sensitive while providing very limited value. So I agree with @fabriziopandini that relying on the existing conditions on the control plane resource would be preferred. We are on discussions to evolve the CAPI contract to become more meaningful, solid and to provide first class support for other use cases like managed Kubernetes. I suggest we defer the proposed change until those discussions are resolved and meanwhile we can possibly document the field better.

[jessehu]

Thank you both very much 👍 According to the above code cluster.Status.ControlPlaneReady = ready, cluster.Status.ControlPlaneReady will never be set to false even if KCP becomes unavailable. I agree we can possibly document the field better to mention this issue and tell users to reply on ControlPlaneReadyCondition instead of status.controlPlaneReady before fixing this issue.

Currently the CAPI code and doc don't say cluster.Status.ControlPlaneReady won't become false when KCP becomes unavailable ? This means CAPI is not changing API contract if we deliver this fix. So IMHO this should be an easy fix as long as we go through the change review flow @fabriziopandini mentioned.

[fabriziopandini]

However we've traditionally considered this field set and forget and therefore changes like this are sensitive while providing very limited value. So I agree with @fabriziopandini that relying on the existing conditions on the control plane resource would be preferred.

@enxebre thanks for summarizing this very well, +1 to improve doc and defer

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle rotten
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[jessehu]

/remove-lifecycle rotten

[fabriziopandini]

As per the discussion above: /remove kind/bug /kind documentation /title Improve documentation for KCP.status.ready and the cluster.Status.ControlPlaneReady /triage accepted /priority backlog

Let's add the following note to the godoc description of KCP.status.ready and cluster.Status.ControlPlaneReady

// NOTE: this field is part of the Cluster API contract and it is used to orchestrate provisioning. // The value of this field is never updated after provisioning is completed. Please use conditions to check the operational state of the control plane.

/good-first-issue

[k8s-ci-robot]

@fabriziopandini: This request has been marked as suitable for new contributors.

Guidelines

Please ensure that the issue body includes answers to the following questions:

• Why are we solving this issue?
• To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
• Does this issue have zero to low barrier of entry?
• How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-good-first-issue command.

In response to this:

As per the discussion above: /remove kind/bug /kind documentation /title Improve documentation for KCP.status.ready and the cluster.Status.ControlPlaneReady /triage accepted /priority backlog

Let's add the following note to the godoc description of KCP.status.ready and cluster.Status.ControlPlaneReady

// NOTE: this field is part of the Cluster API contract and it is used to orchestrate provisioning. // The value of this field is never updated after provisioning is completed. Please use conditions to check the operational state of the control plane.

/good-first-issue

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/test-infra repository.

[pravarag]

I would like to work on this issue /assign

[pravarag]

@fabriziopandini, I've raised the following PR.