Kubeadm: Remove ClusterStatus from kubeadm-config

[fabriziopandini]

This KEP is proposing a new mode for tracking the list of the API endpoints in a cluster, thus allowing to remove the ClusterStatus entry in the kubeadm-config ConfigMap and solve the problems that arise when, for any reasons, such entry does not reflect anymore the real status of the cluster.

/sig cluster-lifecycle
/assign @neolit123 @rosti @ereslibre @ncdc @timothysc

[ereslibre]

I like removing shared state.

I guess one of the motivations for this KEP is to improve the multiple control plane join at once case. Is this correct? Removing shared state is nice, but I think it's harder to block on a known shared resource (as we could with a shared configmap -- with leader election).

Here, when a control plane node joins, it will have to:

• List nodes, retrieve annotations
• Perform the etcd join with the list of current endpoints
• Let the kubelet register with TLS bootstrap
• Annotate node

I see potential race conditions if several control planes are joining at the same time, and I believe it's harder to sync on several annotations on different resources (nodes), than to sync on a shared config map. What do you think about this? Should it be mentioned under risks?

[ereslibre]

Currently, we are composing this information with:

func GetClientURL(localEndpoint *kubeadmapi.APIEndpoint) string {
	return "https://" + net.JoinHostPort(localEndpoint.AdvertiseAddress, strconv.Itoa(constants.EtcdListenClientPort))
}

So, we are today setting etcd's advertise-client-urls forcing the port to the etcd listen client one, reusing the current local endpoint advertise address.

What's the goal of this new annotation as opposed to kubeadm.kubernetes.io/kube-apiserver.advertise-address? Couldn't we theoretically compose the advertise-client-urls based on kubeadm.kubernetes.io/kube-apiserver.advertise-address?

[fabriziopandini]

The goal is to build the list of the etcd peers looking at the current etcd pods (instead of looking at the kube-apiserver pods, which can be somehow confusing or eventually also error-prone)

This is especially helpful in the current join --control-plane sequence because the kube-apiserver and the etcd static pods are created in two separated moments

[rosti]

Thanks @fabriziopandini !
I like the idea very much! However, we must also ensure, that we don't break anyone who's relying on ClusterStatus-es.

[rosti]

We need to guarantee that ClusterStatus-es are kept together with annotations for some time (they can be considered beta, so at least 9 months/3 cycles) not to break people.

[fabriziopandini]

see [1]
The whole point of this KEP is that it is not possible to keep ClusterStatus up to date with the current status.

[neolit123]

we discussed during kubeadm office hours that we would like to keep the ClusterStatus for a while.
@fabriziopandini please confirm. does this section needs a minor update?

[fabriziopandini]

@ereslibre

Here, when a control plane node joins, it will have to:

• List nodes, retrieve annotations
• Perform the etcd join with the list of current endpoints
• Let the kubelet register with TLS bootstrap
• Annotate node

I think that there is some misunderstanding here.
This KEP is proposing to add annotations on the kube-apiserver static pod manifest and on the etcd static pod manifest. No annotation at node level is part of this proposal

The join process will be:

• create control-plane static pod manifests (as of today, only with an additional annotation on the kubea-api server static pod manifest)
• start kubelet (as of today)
• get the list of current etcd pods/kubeadm.kubernetes.io/etcd.advertise-client-urls (replacement of get ClusterStatus)
• create control-plane static pod manifests (as of today, only with an additional annotation)

Considering that I don't see potential race conditions, but happy to discuss this if this is not clear yet

[ereslibre]

Thank you for the clarifications @fabriziopandini. This LGTM.

[timothysc]

I like it. Sorry for my late response, pre-holidays was nuts.
I have some minor questions but I'm unblocking approval.

/approve

[timothysc]

I have not checked, but if a static pod goes down is that always removed from the pod list? Also LBs would still need to update periodically.

[neolit123]

if the static manifest is removed the mirror Pod should be removed too.
but if the manifest is in place the kubelet will try to create the Pod. it would then be possible do kubectl describe po on it to get the annotation.

[timothysc]

Do we do a destructive LB update test?

[timothysc]

/hold
for other comments and lgtms

[ereslibre]

/lgtm

Feel free to unhold.

[rosti]

My comments were non-blocking too.
/lgtm

[fabriziopandini]

@neolit123 @timothysc all the comments are addressed, please PTAL

[neolit123]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ereslibre, fabriziopandini, neolit123, rosti, timothysc

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:

• keps/sig-cluster-lifecycle/OWNERS [fabriziopandini,neolit123,timothysc]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[neolit123]

please "hold cancel" before Tuesday next week.

[ereslibre]

Tim's comments were non blocking and there's consensus on this KEP. Thank you everyone!

/hold cancel