Skip to content

🌱 KCP: remove code handling Kubernetes <= v1.21 #11146

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions controlplane/kubeadm/internal/controllers/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -802,12 +802,7 @@ func (r *KubeadmControlPlaneReconciler) reconcileEtcdMembers(ctx context.Context
return errors.Wrap(err, "cannot get remote client to workload cluster")
}

parsedVersion, err := semver.ParseTolerant(controlPlane.KCP.Spec.Version)
if err != nil {
return errors.Wrapf(err, "failed to parse kubernetes version %q", controlPlane.KCP.Spec.Version)
}

removedMembers, err := workloadCluster.ReconcileEtcdMembers(ctx, nodeNames, parsedVersion)
removedMembers, err := workloadCluster.ReconcileEtcdMembers(ctx, nodeNames)
if err != nil {
return errors.Wrap(err, "failed attempt to reconcile etcd members")
}
Expand Down
6 changes: 1 addition & 5 deletions controlplane/kubeadm/internal/controllers/fakes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func (f *fakeWorkloadCluster) ForwardEtcdLeadership(_ context.Context, _ *cluste
return nil
}

func (f *fakeWorkloadCluster) ReconcileEtcdMembers(_ context.Context, _ []string, _ semver.Version) ([]string, error) {
func (f *fakeWorkloadCluster) ReconcileEtcdMembers(_ context.Context, _ []string) ([]string, error) {
return nil, nil
}

Expand Down Expand Up @@ -129,10 +129,6 @@ func (f *fakeWorkloadCluster) RemoveEtcdMemberForMachine(_ context.Context, _ *c
return nil
}

func (f *fakeWorkloadCluster) RemoveMachineFromKubeadmConfigMap(_ context.Context, _ *clusterv1.Machine, _ semver.Version) error {
return nil
}

func (f *fakeWorkloadCluster) EtcdMembers(_ context.Context) ([]string, error) {
return f.EtcdMembersResult, nil
}
Expand Down
11 changes: 0 additions & 11 deletions controlplane/kubeadm/internal/controllers/remediation.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"fmt"
"time"

"github.com/blang/semver/v4"
"github.com/go-logr/logr"
"github.com/pkg/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -227,16 +226,6 @@ func (r *KubeadmControlPlaneReconciler) reconcileUnhealthyMachines(ctx context.C

// NOTE: etcd member removal will be performed by the kcp-cleanup hook after machine completes drain & all volumes are detached.
}

parsedVersion, err := semver.ParseTolerant(controlPlane.KCP.Spec.Version)
if err != nil {
return ctrl.Result{}, errors.Wrapf(err, "failed to parse kubernetes version %q", controlPlane.KCP.Spec.Version)
}

if err := workloadCluster.RemoveMachineFromKubeadmConfigMap(ctx, machineToBeRemediated, parsedVersion); err != nil {
log.Error(err, "Failed to remove machine from kubeadm ConfigMap")
return ctrl.Result{}, err
}
}

// Delete the machine
Expand Down
11 changes: 0 additions & 11 deletions controlplane/kubeadm/internal/controllers/scale.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"context"
"strings"

"github.com/blang/semver/v4"
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -139,16 +138,6 @@ func (r *KubeadmControlPlaneReconciler) scaleDownControlPlane(
// NOTE: etcd member removal will be performed by the kcp-cleanup hook after machine completes drain & all volumes are detached.
}

parsedVersion, err := semver.ParseTolerant(controlPlane.KCP.Spec.Version)
if err != nil {
return ctrl.Result{}, errors.Wrapf(err, "failed to parse kubernetes version %q", controlPlane.KCP.Spec.Version)
}

if err := workloadCluster.RemoveMachineFromKubeadmConfigMap(ctx, machineToDelete, parsedVersion); err != nil {
logger.Error(err, "Failed to remove machine from kubeadm ConfigMap")
return ctrl.Result{}, err
}

logger = logger.WithValues("Machine", klog.KObj(machineToDelete))
if err := r.Client.Delete(ctx, machineToDelete); err != nil && !apierrors.IsNotFound(err) {
logger.Error(err, "Failed to delete control plane machine")
Expand Down
84 changes: 23 additions & 61 deletions controlplane/kubeadm/internal/workload_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ import (
"sigs.k8s.io/cluster-api/util/certs"
containerutil "sigs.k8s.io/cluster-api/util/container"
"sigs.k8s.io/cluster-api/util/patch"
"sigs.k8s.io/cluster-api/util/version"
)

const (
Expand All @@ -68,18 +67,6 @@ const (
)

var (
// Starting from v1.22.0 kubeadm dropped the usage of the ClusterStatus entry from the kubeadm-config ConfigMap
// so we're not anymore required to remove API endpoints for control plane nodes after deletion.
//
// NOTE: The following assumes that 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: The following assumes that kubeadm version equals to Kubernetes version.
minVerKubeletSystemdDriver = semver.MustParse("1.21.0")

// Starting from v1.24.0 kubeadm uses "kubelet-config" a ConfigMap name for KubeletConfiguration,
// Dropping the X-Y suffix.
//
Expand Down Expand Up @@ -124,15 +111,13 @@ type WorkloadCluster interface {
UpdateKubeProxyImageInfo(ctx context.Context, kcp *controlplanev1.KubeadmControlPlane, version semver.Version) error
UpdateCoreDNS(ctx context.Context, kcp *controlplanev1.KubeadmControlPlane, version semver.Version) error
RemoveEtcdMemberForMachine(ctx context.Context, machine *clusterv1.Machine) error
RemoveMachineFromKubeadmConfigMap(ctx context.Context, machine *clusterv1.Machine, version semver.Version) error
RemoveNodeFromKubeadmConfigMap(ctx context.Context, nodeName string, version semver.Version) error
ForwardEtcdLeadership(ctx context.Context, machine *clusterv1.Machine, leaderCandidate *clusterv1.Machine) error
AllowBootstrapTokensToGetNodes(ctx context.Context) error
AllowClusterAdminPermissions(ctx context.Context, version semver.Version) error
UpdateClusterConfiguration(ctx context.Context, version semver.Version, mutators ...func(*bootstrapv1.ClusterConfiguration)) error

// State recovery tasks.
ReconcileEtcdMembers(ctx context.Context, nodeNames []string, version semver.Version) ([]string, error)
ReconcileEtcdMembers(ctx context.Context, nodeNames []string) ([]string, error)
}

// Workload defines operations on workload clusters.
Expand Down Expand Up @@ -277,33 +262,31 @@ func (w *Workload) UpdateKubeletConfigMap(ctx context.Context, version semver.Ve
// NOTE: It is considered safe to update the kubelet-config-1.21 ConfigMap
// because only new nodes using v1.21 images will pick up the change during
// kubeadm join.
if version.GE(minVerKubeletSystemdDriver) {
data, ok := cm.Data[kubeletConfigKey]
if !ok {
return errors.Errorf("unable to find %q key in %s", kubeletConfigKey, cm.Name)
}
kubeletConfig, err := yamlToUnstructured([]byte(data))
if err != nil {
return errors.Wrapf(err, "unable to decode kubelet ConfigMap's %q content to Unstructured object", kubeletConfigKey)
}
cgroupDriver, _, err := unstructured.NestedString(kubeletConfig.UnstructuredContent(), cgroupDriverKey)
if err != nil {
return errors.Wrapf(err, "unable to extract %q from Kubelet ConfigMap's %q", cgroupDriverKey, cm.Name)
}
data, ok := cm.Data[kubeletConfigKey]
Copy link
Member

@neolit123 neolit123 Sep 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wondering if the cgroup driver override can be removed at this point.
existing clusters were already mutated by this code, while for new clusters kubeadm does the defaulting.

it might be safer to do this once:
https://github.com/kubernetes/enhancements/blob/7e1c131dc50fa84caa5e99da2b8bf67db32054ab/keps/sig-node/4033-group-driver-detection-over-cri/kep.yaml
goes GA.

however CRs need to adapt it first too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added it to #8190 (to consider this later)

if !ok {
return errors.Errorf("unable to find %q key in %s", kubeletConfigKey, cm.Name)
}
kubeletConfig, err := yamlToUnstructured([]byte(data))
if err != nil {
return errors.Wrapf(err, "unable to decode kubelet ConfigMap's %q content to Unstructured object", kubeletConfigKey)
}
cgroupDriver, _, err := unstructured.NestedString(kubeletConfig.UnstructuredContent(), cgroupDriverKey)
if err != nil {
return errors.Wrapf(err, "unable to extract %q from Kubelet ConfigMap's %q", cgroupDriverKey, cm.Name)
}

// If the value is not already explicitly set by the user, change according to kubeadm/image builder new requirements.
if cgroupDriver == "" {
cgroupDriver = "systemd"
// If the value is not already explicitly set by the user, change according to kubeadm/image builder new requirements.
if cgroupDriver == "" {
cgroupDriver = "systemd"

if err := unstructured.SetNestedField(kubeletConfig.UnstructuredContent(), cgroupDriver, cgroupDriverKey); err != nil {
return errors.Wrapf(err, "unable to update %q on Kubelet ConfigMap's %q", cgroupDriverKey, cm.Name)
}
updated, err := yaml.Marshal(kubeletConfig)
if err != nil {
return errors.Wrapf(err, "unable to encode Kubelet ConfigMap's %q to YAML", cm.Name)
}
cm.Data[kubeletConfigKey] = string(updated)
if err := unstructured.SetNestedField(kubeletConfig.UnstructuredContent(), cgroupDriver, cgroupDriverKey); err != nil {
return errors.Wrapf(err, "unable to update %q on Kubelet ConfigMap's %q", cgroupDriverKey, cm.Name)
}
updated, err := yaml.Marshal(kubeletConfig)
if err != nil {
return errors.Wrapf(err, "unable to encode Kubelet ConfigMap's %q to YAML", cm.Name)
}
cm.Data[kubeletConfigKey] = string(updated)
}

// Update the name to the new name
Expand Down Expand Up @@ -338,27 +321,6 @@ func (w *Workload) UpdateSchedulerInKubeadmConfigMap(scheduler bootstrapv1.Contr
}
}

// RemoveMachineFromKubeadmConfigMap removes the entry for the machine from the kubeadm configmap.
func (w *Workload) RemoveMachineFromKubeadmConfigMap(ctx context.Context, machine *clusterv1.Machine, version semver.Version) error {
if machine == nil || machine.Status.NodeRef == nil {
// Nothing to do, no node for Machine
return nil
}

return w.RemoveNodeFromKubeadmConfigMap(ctx, machine.Status.NodeRef.Name, version)
}

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

return w.updateClusterStatus(ctx, func(s *bootstrapv1.ClusterStatus) {
delete(s.APIEndpoints, name)
}, v)
}

// updateClusterStatus gets the ClusterStatus kubeadm-config ConfigMap, converts it to the
// Cluster API representation, and then applies a mutation func; if changes are detected, the
// data are converted back into the Kubeadm API version in use for the target Kubernetes version and the
Expand Down
11 changes: 3 additions & 8 deletions controlplane/kubeadm/internal/workload_cluster_etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package internal
import (
"context"

"github.com/blang/semver/v4"
"github.com/pkg/errors"
kerrors "k8s.io/apimachinery/pkg/util/errors"

Expand All @@ -36,19 +35,19 @@ type etcdClientFor interface {

// ReconcileEtcdMembers iterates over all etcd members and finds members that do not have corresponding nodes.
// If there are any such members, it deletes them from etcd and removes their nodes from the kubeadm configmap so that kubeadm does not run etcd health checks on them.
func (w *Workload) ReconcileEtcdMembers(ctx context.Context, nodeNames []string, version semver.Version) ([]string, error) {
func (w *Workload) ReconcileEtcdMembers(ctx context.Context, nodeNames []string) ([]string, error) {
allRemovedMembers := []string{}
allErrs := []error{}
for _, nodeName := range nodeNames {
removedMembers, errs := w.reconcileEtcdMember(ctx, nodeNames, nodeName, version)
removedMembers, errs := w.reconcileEtcdMember(ctx, nodeNames, nodeName)
allRemovedMembers = append(allRemovedMembers, removedMembers...)
allErrs = append(allErrs, errs...)
}

return allRemovedMembers, kerrors.NewAggregate(allErrs)
}

func (w *Workload) reconcileEtcdMember(ctx context.Context, nodeNames []string, nodeName string, version semver.Version) ([]string, []error) {
func (w *Workload) reconcileEtcdMember(ctx context.Context, nodeNames []string, nodeName string) ([]string, []error) {
// Create the etcd Client for the etcd Pod scheduled on the Node
etcdClient, err := w.etcdClientGenerator.forFirstAvailableNode(ctx, []string{nodeName})
if err != nil {
Expand Down Expand Up @@ -84,10 +83,6 @@ loopmembers:
if err := w.removeMemberForNode(ctx, member.Name); err != nil {
errs = append(errs, err)
}

if err := w.RemoveNodeFromKubeadmConfigMap(ctx, member.Name, version); err != nil {
errs = append(errs, err)
}
}
return removedMembers, errs
}
Expand Down
Loading
Loading