Skip to content

Commit

Permalink
Stop updating ClusterStatus for KubernetesVersion >= v1.22.0
Browse files Browse the repository at this point in the history
  • Loading branch information
@fabriziopandini
fabriziopandini committed May 20, 2021
1 parent f5dec18 commit 6d60650
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 49 deletions.
15 changes: 14 additions & 1 deletion controlplane/kubeadm/internal/workload_cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,17 @@ const (
)

var (
// Starting from v1.22.0 kubeadm dropped usage of the ClusterStatus entry from the kubeadm-config ConfigMap
// so it isn't necessary anymore to remove API endpoints for control plane nodes after deletion.
// NOTE: This assume kubeadm version equals to Kubernetes version.
minKubernetesVersionWithoutClusterStatus = semver.MustParse("1.22.0")

// Starting from v1.21.0 kubeadm defaults to systemdCGroup driver, as well as images built with ImageBuilder,
// so it is necessary to mutate the kubelet-config-xx ConfigMap.
// NOTE: This assume kubeadm version equals to Kubernetes version.
minVerKubeletSystemdDriver = semver.MustParse("1.21.0")
ErrControlPlaneMinNodes = errors.New("cluster has fewer than 2 control plane nodes; removing an etcd member is not supported")

ErrControlPlaneMinNodes = errors.New("cluster has fewer than 2 control plane nodes; removing an etcd member is not supported")
)

// WorkloadCluster defines all behaviors necessary to upgrade kubernetes on a workload cluster
Expand Down Expand Up @@ -247,6 +256,10 @@ func (w *Workload) RemoveMachineFromKubeadmConfigMap(ctx context.Context, machin

// RemoveNodeFromKubeadmConfigMap removes the entry for the node from the kubeadm configmap.
func (w *Workload) RemoveNodeFromKubeadmConfigMap(ctx context.Context, name string, version semver.Version) error {
if version.GTE(minKubernetesVersionWithoutClusterStatus) {
return nil
}

return w.updateClusterStatus(ctx, func(s *bootstrapv1.ClusterStatus) {
delete(s.APIEndpoints, name)
}, version)
Expand Down
89 changes: 68 additions & 21 deletions controlplane/kubeadm/internal/workload_cluster_etcd_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -449,21 +449,24 @@ func TestReconcileEtcdMembers(t *testing.T) {
Namespace: metav1.NamespaceSystem,
},
Data: map[string]string{
clusterStatusKey: `apiEndpoints:
ip-10-0-0-1.ec2.internal:
advertiseAddress: 10.0.0.1
bindPort: 6443
ip-10-0-0-2.ec2.internal:
advertiseAddress: 10.0.0.2
bindPort: 6443
someFieldThatIsAddedInTheFuture: bar
ip-10-0-0-3.ec2.internal:
advertiseAddress: 10.0.0.3
bindPort: 6443
apiVersion: kubeadm.k8s.io/v1beta2
kind: ClusterStatus`,
clusterStatusKey: "apiEndpoints:\n" +
" ip-10-0-0-1.ec2.internal:\n" +
" advertiseAddress: 10.0.0.1\n" +
" bindPort: 6443\n" +
" ip-10-0-0-2.ec2.internal:\n" +
" advertiseAddress: 10.0.0.2\n" +
" bindPort: 6443\n" +
" someFieldThatIsAddedInTheFuture: bar\n" +
" ip-10-0-0-3.ec2.internal:\n" +
" advertiseAddress: 10.0.0.3\n" +
" bindPort: 6443\n" +
"apiVersion: kubeadm.k8s.io/v1beta2\n" +
"kind: ClusterStatus\n",
},
}
kubeadmConfigWithoutClusterStatus := kubeadmConfig.DeepCopy()
delete(kubeadmConfigWithoutClusterStatus.Data, clusterStatusKey)

node1 := &corev1.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "ip-10-0-0-1.ec2.internal",
Expand Down Expand Up @@ -491,26 +494,70 @@ kind: ClusterStatus`,

tests := []struct {
name string
kubernetesVersion semver.Version
objs []client.Object
nodes []string
etcdClientGenerator etcdClientFor
expectErr bool
assert func(*WithT)
assert func(*WithT, client.Client)
}{
{
// the node to be removed is ip-10-0-0-3.ec2.internal since the
// other two have nodes
name: "successfully removes the etcd member without a node and removes the node from kubeadm config",
objs: []client.Object{node1.DeepCopy(), node2.DeepCopy(), kubeadmConfig.DeepCopy()},
nodes: []string{node1.Name, node2.Name},
name: "successfully removes the etcd member without a node and removes the node from kubeadm config for Kubernetes version < 1.22.0",
kubernetesVersion: semver.MustParse("1.19.1"), // Kubernetes version < 1.22.0 has ClusterStatus
objs: []client.Object{node1.DeepCopy(), node2.DeepCopy(), kubeadmConfig.DeepCopy()},
nodes: []string{node1.Name, node2.Name},
etcdClientGenerator: &fakeEtcdClientGenerator{
forNodesClient: &etcd.Client{
EtcdClient: fakeEtcdClient,
},
},
expectErr: false,
assert: func(g *WithT, c client.Client) {
g.Expect(fakeEtcdClient.RemovedMember).To(Equal(uint64(3)))

var actualConfig corev1.ConfigMap
g.Expect(c.Get(
ctx,
client.ObjectKey{Name: kubeadmConfigKey, Namespace: metav1.NamespaceSystem},
&actualConfig,
)).To(Succeed())

g.Expect(actualConfig.Data[clusterStatusKey]).To(Equal("apiEndpoints:\n" +
" ip-10-0-0-1.ec2.internal:\n" +
" advertiseAddress: 10.0.0.1\n" +
" bindPort: 6443\n" +
" ip-10-0-0-2.ec2.internal:\n" +
" advertiseAddress: 10.0.0.2\n" +
" bindPort: 6443\n" +
"apiVersion: kubeadm.k8s.io/v1beta2\n" +
"kind: ClusterStatus\n"))
},
},
{
// the node to be removed is ip-10-0-0-3.ec2.internal since the
// other two have nodes
name: "successfully removes the etcd member without a node for Kubernetes version >= 1.22.0",
kubernetesVersion: minKubernetesVersionWithoutClusterStatus, // Kubernetes version >= 1.22.0 does not have ClusterStatus
objs: []client.Object{node1.DeepCopy(), node2.DeepCopy(), kubeadmConfigWithoutClusterStatus.DeepCopy()},
nodes: []string{node1.Name, node2.Name},
etcdClientGenerator: &fakeEtcdClientGenerator{
forNodesClient: &etcd.Client{
EtcdClient: fakeEtcdClient,
},
},
expectErr: false,
assert: func(g *WithT) {
assert: func(g *WithT, c client.Client) {
g.Expect(fakeEtcdClient.RemovedMember).To(Equal(uint64(3)))

var actualConfig corev1.ConfigMap
g.Expect(c.Get(
ctx,
client.ObjectKey{Name: kubeadmConfigKey, Namespace: metav1.NamespaceSystem},
&actualConfig,
)).To(Succeed())
g.Expect(actualConfig.Data).ToNot(HaveKey(clusterStatusKey))
},
},
{
Expand Down Expand Up @@ -538,19 +585,19 @@ kind: ClusterStatus`,
}

w := &Workload{
Client: testEnv,
Client: testEnv.Client,
etcdClientGenerator: tt.etcdClientGenerator,
}
ctx := context.TODO()
_, err := w.ReconcileEtcdMembers(ctx, tt.nodes, semver.MustParse("1.19.1"))
_, err := w.ReconcileEtcdMembers(ctx, tt.nodes, tt.kubernetesVersion)
if tt.expectErr {
g.Expect(err).To(HaveOccurred())
return
}
g.Expect(err).ToNot(HaveOccurred())

if tt.assert != nil {
tt.assert(g)
tt.assert(g, testEnv.Client)
}
})
}
Expand Down
70 changes: 43 additions & 27 deletions controlplane/kubeadm/internal/workload_cluster_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,28 +181,29 @@ func TestRemoveMachineFromKubeadmConfigMap(t *testing.T) {
Namespace: metav1.NamespaceSystem,
},
Data: map[string]string{
clusterStatusKey: `apiEndpoints:
ip-10-0-0-1.ec2.internal:
advertiseAddress: 10.0.0.1
bindPort: 6443
ip-10-0-0-2.ec2.internal:
advertiseAddress: 10.0.0.2
bindPort: 6443
apiVersion: kubeadm.k8s.io/v1beta2
kind: ClusterStatus`,
clusterStatusKey: "apiEndpoints:\n" +
" ip-10-0-0-1.ec2.internal:\n" +
" advertiseAddress: 10.0.0.1\n" +
" bindPort: 6443\n" +
" ip-10-0-0-2.ec2.internal:\n" +
" advertiseAddress: 10.0.0.2\n" +
" bindPort: 6443\n" +
"apiVersion: kubeadm.k8s.io/v1beta2\n" +
"kind: ClusterStatus\n",
},
BinaryData: map[string][]byte{
"": nil,
},
}
kconfWithoutKey := kubeadmConfig.DeepCopy()
delete(kconfWithoutKey.Data, clusterStatusKey)
kubeadmConfigWithoutClusterStatus := kubeadmConfig.DeepCopy()
delete(kubeadmConfigWithoutClusterStatus.Data, clusterStatusKey)

g := NewWithT(t)
scheme := runtime.NewScheme()
g.Expect(corev1.AddToScheme(scheme)).To(Succeed())
tests := []struct {
name string
kubernetesVersion semver.Version
machine *clusterv1.Machine
objs []client.Object
expectErr bool
Expand All @@ -227,23 +228,38 @@ kind: ClusterStatus`,
expectErr: true,
},
{
name: "returns error if unable to remove api endpoint",
machine: machine,
objs: []client.Object{kconfWithoutKey},
expectErr: true,
name: "returns error if unable to find kubeadm-config for Kubernetes version < 1.22.0",
kubernetesVersion: semver.MustParse("1.19.1"),
machine: machine,
objs: []client.Object{kubeadmConfigWithoutClusterStatus},
expectErr: true,
},
{
name: "removes the machine node ref from kubeadm config",
machine: machine,
objs: []client.Object{kubeadmConfig},
expectErr: false,
expectedEndpoints: `apiEndpoints:
ip-10-0-0-2.ec2.internal:
advertiseAddress: 10.0.0.2
bindPort: 6443
apiVersion: kubeadm.k8s.io/v1beta2
kind: ClusterStatus
`,
name: "returns error if unable to remove api endpoint for Kubernetes version < 1.22.0",
kubernetesVersion: semver.MustParse("1.19.1"), // Kubernetes version < 1.22.0 has ClusterStatus
machine: machine,
objs: []client.Object{kubeadmConfigWithoutClusterStatus},
expectErr: true,
},
{
name: "removes the machine node ref from kubeadm config for Kubernetes version < 1.22.0",
kubernetesVersion: semver.MustParse("1.19.1"), // Kubernetes version < 1.22.0 has ClusterStatus
machine: machine,
objs: []client.Object{kubeadmConfig},
expectErr: false,
expectedEndpoints: "apiEndpoints:\n" +
" ip-10-0-0-2.ec2.internal:\n" +
" advertiseAddress: 10.0.0.2\n" +
" bindPort: 6443\n" +
"apiVersion: kubeadm.k8s.io/v1beta2\n" +
"kind: ClusterStatus\n",
},
{
name: "no op for Kubernetes version >= 1.22.0",
kubernetesVersion: minKubernetesVersionWithoutClusterStatus, // Kubernetes version >= 1.22.0 should not manage ClusterStatus
machine: machine,
objs: []client.Object{kubeadmConfigWithoutClusterStatus},
expectErr: false,
},
}

Expand All @@ -254,7 +270,7 @@ kind: ClusterStatus
w := &Workload{
Client: fakeClient,
}
err := w.RemoveMachineFromKubeadmConfigMap(ctx, tt.machine, semver.MustParse("1.19.1"))
err := w.RemoveMachineFromKubeadmConfigMap(ctx, tt.machine, tt.kubernetesVersion)
if tt.expectErr {
g.Expect(err).To(HaveOccurred())
return
Expand Down

0 comments on commit 6d60650

Please sign in to comment.