Unexpected rollout when using KCP and updating to v1.11

[chrischdi]

What steps did you take and what happened?

make kind-cluster
export CLUSTER_TOPOLOGY=true
clusterctl init --wait-providers --core cluster-api:v1.10.8 --bootstrap kubeadm:v1.10.8 --control-plane kubeadm:v1.10.8 --infrastructure docker:v1.10.8

clusterctl-v1.10.5 generate cluster capi-quickstart --flavor development --kubernetes-version v1.32.8 --control-plane-machine-count=1 --worker-machine-count=1 > cluster.yaml

# pre-create custom KubeadmControlPlaneTemplate
cat <<EOF | kubectl create -f -
apiVersion: controlplane.cluster.x-k8s.io/v1beta1
kind: KubeadmControlPlaneTemplate
metadata:
  name: quick-start-control-plane
  namespace: default
spec:
  template:
    spec:
      kubeadmConfigSpec:
        clusterConfiguration:
          apiServer:
            timeoutForControlPlane: 20m
            certSANs:
            - localhost
            - 127.0.0.1
            - 0.0.0.0
            - host.docker.internal
        initConfiguration:
          nodeRegistration:
            kubeletExtraArgs:
              v: "4"
        joinConfiguration:
          nodeRegistration:
            kubeletExtraArgs:
              v: "4"
EOF

# Use `kubectl create` to not replace the above created template
kubectl create -f cluster.yaml

# remove mhc
kubectl edit clusterclass quick-start

kubectl wait --timeout=5m --for='jsonpath={.status.conditions[?(@.type=="Ready")].status}=True' cluster/capi-quickstart

kind get kubeconfig --name capi-quickstart > quickstart.kubeconfig
kubectl apply -f test/e2e/data/cni/kindnet/kindnet.yaml --kubeconfig quickstart.kubeconfig


# This command triggers a rollout for KCP.
clusterctl upgrade apply --wait-providers --contract v1beta2

What did you expect to happen?

No rollout to happen

Cluster API version

Update from v1.10.8 to v1.11.3

Kubernetes version

No response

Anything else you would like to add?

The diff happens because the v1beta1 version of KCP (before the upgrade):

• has a value set at spec.kubeadmConfigSpec.clusterConfiguration.apiServer.timeoutForControlPlane

Conversion seems to lead that in v1beta2 it then detects a diff, because in the joinConfiguration there is nothing set for .timeouts.controlPlaneComponentsHealthCheckSeconds.

diff happening:

I1120 13:08:35.689312       1 controller.go:477] "Rolling out Control Plane machines: Machine capi-quickstart-shntc-8c4xj needs rollout: Machine KubeadmConfig InitConfiguration or JoinConfiguration are outdated: diff: &v1beta2.KubeadmConfigSpec{\n    ClusterConfiguration: {},\n    InitConfiguration: v1beta2.InitConfiguration{\n      BootstrapTokens: nil,\n      NodeRegistration: v1beta2.NodeRegistrationOptions{\n        Name:                  \"\",\n        CRISocket:             \"\",\n        Taints:                nil,\n-       KubeletExtraArgs:      []v1beta2.Arg{{Name: \"v\", Value: &\"4\"}},\n+       KubeletExtraArgs:      nil,\n        IgnorePreflightErrors: nil,\n        ImagePullPolicy:       \"IfNotPresent\",\n        ImagePullSerial:       nil,\n      },\n      LocalAPIEndpoint: {},\n      SkipPhases:       nil,\n      Patches:          {},\n      Timeouts: v1beta2.Timeouts{\n-       ControlPlaneComponentHealthCheckSeconds: &1200,\n+       ControlPlaneComponentHealthCheckSeconds: nil,\n        KubeletHealthCheckSeconds:               nil,\n        KubernetesAPICallSeconds:                nil,\n        ... // 3 identical fields\n      },\n    },\n    JoinConfiguration: v1beta2.JoinConfiguration{\n      NodeRegistration: v1beta2.NodeRegistrationOptions{\n        Name:                  \"\",\n        CRISocket:             \"\",\n        Taints:                nil,\n-       KubeletExtraArgs:      nil,\n+       KubeletExtraArgs:      []v1beta2.Arg{{Name: \"v\", Value: &\"4\"}},\n        IgnorePreflightErrors: nil,\n        ImagePullPolicy:       \"IfNotPresent\",\n        ImagePullSerial:       nil,\n      },\n      CACertPath: \"\",\n      Discovery:  {},\n      ... // 4 identical fields\n    },\n    Files:     {{Path: \"/etc/kubernetes/kube-apiserver-admission-pss.yaml\", Content: \"apiVersion: apiserver.config.k8s.io/v1\\nkind: AdmissionConfigurat\"...}},\n    DiskSetup: {},\n    ... // 9 identical fields\n  }" controller="kubeadmcontrolplane" controllerGroup="controlplane.cluster.x-k8s.io" controllerKind="KubeadmControlPlane" KubeadmControlPlane="default/capi-quickstart-shntc" namespace="default" name="capi-quickstart-shntc" reconcileID="b267a0d5-e02c-4d65-ab36-c1009ce59c53" Cluster="default/capi-quickstart" machinesNeedingRollout=["capi-quickstart-shntc-8c4xj"]

Better formatted:

I1120 13:08:35.689312       1 controller.go:477] "Rolling out Control Plane machines: Machine capi-quickstart-shntc-8c4xj needs rollout: Machine KubeadmConfig InitConfiguration or JoinConfiguration are outdated: diff:
&v1beta2.KubeadmConfigSpec{
    ClusterConfiguration: {},
    InitConfiguration: v1beta2.InitConfiguration{
      BootstrapTokens: nil,
      NodeRegistration: v1beta2.NodeRegistrationOptions{
        Name:                  \"\",
        CRISocket:             \"\",
        Taints:                nil,
-       KubeletExtraArgs:      []v1beta2.Arg{{Name: \"v\", Value: &\"4\"}},
+       KubeletExtraArgs:      nil,
        IgnorePreflightErrors: nil,
        ImagePullPolicy:       \"IfNotPresent\",
        ImagePullSerial:       nil,
      },
      LocalAPIEndpoint: {},
      SkipPhases:       nil,
      Patches:          {},
      Timeouts: v1beta2.Timeouts{
-       ControlPlaneComponentHealthCheckSeconds: &1200,
+       ControlPlaneComponentHealthCheckSeconds: nil,
        KubeletHealthCheckSeconds:               nil,
        KubernetesAPICallSeconds:                nil,
        ... // 3 identical fields
      },
    },
    JoinConfiguration: v1beta2.JoinConfiguration{
      NodeRegistration: v1beta2.NodeRegistrationOptions{
        Name:                  \"\",
        CRISocket:             \"\",
        Taints:                nil,
-       KubeletExtraArgs:      nil,
+       KubeletExtraArgs:      []v1beta2.Arg{{Name: \"v\", Value: &\"4\"}},
        IgnorePreflightErrors: nil,
        ImagePullPolicy:       \"IfNotPresent\",
        ImagePullSerial:       nil,
      },
      CACertPath: \"\",
      Discovery:  {},
      ... // 4 identical fields
    },
    Files:     {{Path: \"/etc/kubernetes/kube-apiserver-admission-pss.yaml\", Content: \"apiVersion: apiserver.config.k8s.io/v1\
kind: AdmissionConfigurat\"...}},
    DiskSetup: {},
    ... // 9 identical fields
  }" controller="kubeadmcontrolplane" controllerGroup="controlplane.cluster.x-k8s.io" controllerKind="KubeadmControlPlane" KubeadmControlPlane="default/capi-quickstart-shntc" namespace="default" name="capi-quickstart-shntc" reconcileID="b267a0d5-e02c-4d65-ab36-c1009ce59c53" Cluster="default/capi-quickstart" machinesNeedingRollout=["capi-quickstart-shntc-8c4xj"]

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

[chrischdi]

Workaround:

Set spec.kubeadmConfigSpec.joinConfiguration.discovery.timeout to the same value as spec.kubeadmConfigSpec.clusterConfiguration.apiServer.timeoutForControlPlane

[chrischdi]

Also reproducible in upstream CI using the following diff:

❯ git diff test/e2e/data/infrastructure-docker/v1.10/clusterclass-quick-start.yaml
diff --git a/test/e2e/data/infrastructure-docker/v1.10/clusterclass-quick-start.yaml b/test/e2e/data/infrastructure-docker/v1.10/clusterclass-quick-start.yaml
index e3b77e871f..10083febe8 100644
--- a/test/e2e/data/infrastructure-docker/v1.10/clusterclass-quick-start.yaml
+++ b/test/e2e/data/infrastructure-docker/v1.10/clusterclass-quick-start.yaml
@@ -554,6 +554,7 @@ spec:
           apiServer:
             extraArgs:
               v: "0"
+            timeoutForControlPlane: 4m
             # host.docker.internal is required by kubetest when running on MacOS because of the way ports are proxied.
             certSANs: [localhost, host.docker.internal, "::", "::1", "127.0.0.1", "0.0.0.0"]
         initConfiguration:

And by running the e2e for When testing clusterctl upgrades using ClusterClass (v1.10=>current) [ClusterClass] Should create a management cluster and then upgrade all the providers [ClusterClass]

[sbueringer]

Maybe let's open a PR with that change? Then we can take a closer look together at logs etc. and see what we have to fix to get that test to green

EDIT: #13026
(not sure if I'll have enough time to debug this before I'm on PTO)

[sbueringer]

I think the explanation might be the following

On KCP:

• conversion will convert ClusterConfiguration.APIServer.TimeoutForControlPlane <=> InitConfiguration/JoinConfiguration.Timeouts.ControlPlaneComponentHealthCheckSeconds
• the v1beta2 KubeadmConfig that we compute for comparison accordingly has the ControlPlaneComponentHealthCheckSeconds field
• the existing KubeadmConfig from a Machine does not have a ClusterConfiguration (as we didn't set the ClusterConfiguration in these KubeadmConfigs in older CAPI versions). Accordingly without ClusterConfiguration we also won't get the ControlPlaneComponentHealthCheckSeconds field

Just doesn't make sense with the diff, but no idea why

-       ControlPlaneComponentHealthCheckSeconds: &1200,
+       ControlPlaneComponentHealthCheckSeconds: nil,

(+ should be the field from KCP)

I think it would be good to understand / confirm that last detail.

Independent of that I think we have a easy solution. If I see correctly all timeouts are only used by kubeadm itself (and not to configure Kubernetes components). If this is correct then changes to timeouts should not trigger any rollouts at all. So we should just drop the timeouts before we compare.

[sbueringer]

@neolit123 The timeout API struct has this godoc:

// Timeouts holds various timeouts that apply to kubeadm commands.

Am I interpreting this correctly that timeouts in this struct only apply to kubeadm commands and will not be used to configure Kubernetes components in any way? (also for timeouts that might be added to that struct in the future).

Just trying to decide if it makes sense to "blank out" the entire timeout field for the rollout decision or if we should only ignore the currently existing timeouts in that struct.

(In general our idea is that we do not want to trigger rollouts of KCP machines if the delta is just to run kubeadm in a slightly different way and the change would not have any impact on the resulting Node/Machine))

[neolit123]

The diff happens because the v1beta1 version of KCP (before the upgrade):
has a value set at spec.kubeadmConfigSpec.clusterConfiguration.apiServer.timeoutForControlPlane
Conversion seems to lead that in v1beta2 it then detects a diff, because in the joinConfiguration there is nothing set for .timeouts.controlPlaneComponentsHealthCheckSeconds.

in kubeadm v1beta4 the field was moved from ClusterConfiguration.apiServer.timeoutForControlPlane to Timeouts.controlPlaneComponentsHealthCheckSeconds. this Timeouts struct is present under Init/Join/Reset/Upgrade configuration. it's a 'bucket' struct for various timeouts.

[sbueringer]

Yup. I know :)

My question: is it safe to assume that the timouts are only used for kubeadm, or could it be that some of the timeouts also end up as part of the "Kubernetes configuration" (e.g. config for the apiserver etc)

[neolit123]

@neolit123 The timeout API struct has this godoc:

// Timeouts holds various timeouts that apply to kubeadm commands.

Am I interpreting this correctly that timeouts in this struct only apply to kubeadm commands and will not be used to configure Kubernetes components in any way? (also for timeouts that might be added to that struct in the future).

as it stands the comment is valid. the timeouts in the struct are only relevant to actions by kubeadm commands.
e.g. constructing a k8s client and setting a retry timeout for it, constructing an etcd client and applying a timeout for it, timeout for action X.

Just trying to decide if it makes sense to "blank out" the entire timeout field for the rollout decision or if we should only ignore the currently existing timeouts in that struct.

(In general our idea is that we do not want to trigger rollouts of KCP machines if the delta is just to run kubeadm in a slightly different way and the change would not have any impact on the resulting Node/Machine))

i don't think kubeadm has guarantees to never include non-kubeadm command timeouts in the Timeouts struct. if relevant it might do so in the future. e.g. some new etcd flag that we want to control can be exposed via a timeout. maybe never, but there is no such guarantee.

i think the controlplane timeout rollout bug that happened here should be special cased.
this is caused by the kubeadm v1beta4 field being moved. and maybe 'blanking out' the entire struct because of it is not ideal for all CAPI users.

[neolit123]

i think the controlplane timeout rollout bug that happened here should be special cased.
this is caused by the kubeadm v1beta4 field being moved. and maybe 'blanking out' the entire struct because of it is not ideal for all CAPI users.

i recall downstream issues specifically for the etcd timeout being too short on slower Edge infra with big etcd DBs, so leaving the control for that is not unwise.