Remove unused code, comments and update errors to use fmt.Errorf (#67)

controlplane/api/v1beta2/ck8scontrolplane_types.go

@@ -34,9 +34,6 @@ const (
 	// This annotation is used to detect any changes in ClusterConfiguration and trigger machine rollout in KCP.
 	CK8sServerConfigurationAnnotation = "controlplane.cluster.x-k8s.io/ck8s-server-configuration"
 
-	// SkipCoreDNSAnnotation annotation explicitly skips reconciling CoreDNS if set.
-	SkipCoreDNSAnnotation = "controlplane.cluster.x-k8s.io/skip-coredns"
-
 	// RemediationInProgressAnnotation is used to keep track that a KCP remediation is in progress, and more
 	// specifically it tracks that the system is in between having deleted an unhealthy machine and recreating its replacement.
 	// NOTE: if something external to CAPI removes this annotation the system cannot detect the above situation; this can lead to

controlplane/api/v1beta2/condition_consts.go

@@ -97,30 +97,6 @@ const (
 	PodInspectionFailedReason = "PodInspectionFailed"
 )
 
-const (
-	// EtcdClusterHealthyCondition documents the overall etcd cluster's health.
-	EtcdClusterHealthyCondition clusterv1.ConditionType = "EtcdClusterHealthyCondition"
-
-	// EtcdClusterInspectionFailedReason documents a failure in inspecting the etcd cluster status.
-	EtcdClusterInspectionFailedReason = "EtcdClusterInspectionFailed"
-
-	// EtcdClusterUnknownReason reports an etcd cluster in unknown status.
-	EtcdClusterUnknownReason = "EtcdClusterUnknown"
-
-	// EtcdClusterUnhealthyReason (Severity=Error) is set when the etcd cluster is unhealthy.
-	EtcdClusterUnhealthyReason = "EtcdClusterUnhealthy"
-
-	// MachineEtcdMemberHealthyCondition report the machine's etcd member's health status.
-	// NOTE: This conditions exists only if a stacked etcd cluster is used.
-	MachineEtcdMemberHealthyCondition clusterv1.ConditionType = "EtcdMemberHealthy"
-
-	// EtcdMemberInspectionFailedReason documents a failure in inspecting the etcd member status.
-	EtcdMemberInspectionFailedReason = "MemberInspectionFailed"
-
-	// EtcdMemberUnhealthyReason (Severity=Error) documents a Machine's etcd member is unhealthy.
-	EtcdMemberUnhealthyReason = "EtcdMemberUnhealthy"
-)
-
 const (
 	// TokenAvailableCondition documents whether the token required for nodes to join the cluster is available.
 	TokenAvailableCondition clusterv1.ConditionType = "TokenAvailable"

controlplane/controllers/ck8scontrolplane_controller.go

@@ -18,12 +18,12 @@ package controllers
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"strings"
 	"time"
 
 	"github.com/go-logr/logr"
-	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -196,7 +196,7 @@ func (r *CK8sControlPlaneReconciler) reconcileDelete(ctx context.Context, cluste
 		return reconcile.Result{}, err
 	}
 
-	// Updates conditions reporting the status of static pods and the status of the etcd cluster.
+	// Updates conditions reporting the status of static pods
 	// NOTE: Ignoring failures given that we are deleting
 	if err := r.reconcileControlPlaneConditions(ctx, controlPlane); err != nil {
 		logger.Info("failed to reconcile conditions", "error", err.Error())
@@ -498,25 +498,19 @@ func (r *CK8sControlPlaneReconciler) reconcile(ctx context.Context, cluster *clu
 	}
 
 	if err := r.syncMachines(ctx, kcp, controlPlane); err != nil {
-		return ctrl.Result{}, errors.Wrap(err, "failed to sync Machines")
+		return ctrl.Result{}, fmt.Errorf("failed to sync Machines: %w", err)
 	}
 
 	// Aggregate the operational state of all the machines; while aggregating we are adding the
 	// source ref (reason@machine/name) so the problem can be easily tracked down to its source machine.
 	conditions.SetAggregate(controlPlane.KCP, controlplanev1.MachinesReadyCondition, ownedMachines.ConditionGetters(), conditions.AddSourceRef(), conditions.WithStepCounterIf(false))
 
-	// Updates conditions reporting the status of static pods and the status of the etcd cluster.
+	// Updates conditions reporting the status of static pods
 	// NOTE: Conditions reporting KCP operation progress like e.g. Resized or SpecUpToDate are inlined with the rest of the execution.
 	if err := r.reconcileControlPlaneConditions(ctx, controlPlane); err != nil {
 		return reconcile.Result{}, err
 	}
 
-	// Ensures the number of etcd members is in sync with the number of machines/nodes.
-	// NOTE: This is usually required after a machine deletion.
-	if err := r.reconcileEtcdMembers(ctx, controlPlane); err != nil {
-		return reconcile.Result{}, err
-	}
-
 	// Reconcile unhealthy machines by triggering deletion and requeue if it is considered safe to remediate,
 	// otherwise continue with the other KCP operations.
 	if result, err := r.reconcileUnhealthyMachines(ctx, controlPlane); err != nil || !result.IsZero() {
@@ -562,26 +556,6 @@ func (r *CK8sControlPlaneReconciler) reconcile(ctx context.Context, cluster *clu
 		return r.scaleDownControlPlane(ctx, cluster, kcp, controlPlane, collections.Machines{})
 	}
 
-	// Get the workload cluster client.
-	/**
-	workloadCluster, err := r.managementCluster.GetWorkloadCluster(ctx, util.ObjectKey(cluster))
-	if err != nil {
-		logger.V(2).Info("cannot get remote client to workload cluster, will requeue", "cause", err)
-		return ctrl.Result{Requeue: true}, nil
-	}
-
-	// Update kube-proxy daemonset.
-	if err := workloadCluster.UpdateKubeProxyImageInfo(ctx, kcp); err != nil {
-		logger.Error(err, "failed to update kube-proxy daemonset")
-		return reconcile.Result{}, err
-	}
-
-	// Update CoreDNS deployment.
-	if err := workloadCluster.UpdateCoreDNS(ctx, kcp); err != nil {
-		return reconcile.Result{}, fmt.Errorf("failed to update CoreDNS deployment")
-	}
-	**/
-
 	return reconcile.Result{}, nil
 }
 
@@ -661,8 +635,7 @@ func (r *CK8sControlPlaneReconciler) reconcileKubeconfig(ctx context.Context, cl
 	return reconcile.Result{}, nil
 }
 
-// reconcileControlPlaneConditions is responsible of reconciling conditions reporting the status of static pods and
-// the status of the etcd cluster.
+// reconcileControlPlaneConditions is responsible of reconciling conditions reporting the status of static pods.
 func (r *CK8sControlPlaneReconciler) reconcileControlPlaneConditions(ctx context.Context, controlPlane *ck8s.ControlPlane) error {
 	// If the cluster is not yet initialized, there is no way to connect to the workload cluster and fetch information
 	// for updating conditions. Return early.
@@ -678,7 +651,6 @@ func (r *CK8sControlPlaneReconciler) reconcileControlPlaneConditions(ctx context
 
 	// Update conditions status
 	workloadCluster.UpdateAgentConditions(ctx, controlPlane)
-	workloadCluster.UpdateEtcdConditions(ctx, controlPlane)
 
 	// Patch machines with the updated conditions.
 	if err := controlPlane.PatchMachines(ctx); err != nil {
@@ -722,66 +694,6 @@ func (r *CK8sControlPlaneReconciler) syncMachines(ctx context.Context, kcp *cont
 	return nil
 }
 
-// reconcileEtcdMembers ensures the number of etcd members is in sync with the number of machines/nodes.
-// This is usually required after a machine deletion.
-//
-// NOTE: this func uses KCP conditions, it is required to call reconcileControlPlaneConditions before this.
-func (r *CK8sControlPlaneReconciler) reconcileEtcdMembers(ctx context.Context, controlPlane *ck8s.ControlPlane) error {
-	// NOTE(neoaggelos): Upstream uses this to reach the etcd cluster and remove any members that have not yet
-	// been removed, typically after a machine has been deleted. In the case of k8s-dqlite, this is handled automatically
-	// for us, so we do not need to do anything here.
-	//
-	// We still leave this code around in case we need to do work in the future (e.g. make sure any removed nodes do not
-	// still appear on microcluster or k8s-dqlite).
-
-	/**
-	log := ctrl.LoggerFrom(ctx)
-	// If k8s-dqlite is not managed by KCP this is a no-op.
-	if !controlPlane.IsEtcdManaged() {
-		return nil
-	}
-
-	// If there is no KCP-owned control-plane machines, then control-plane has not been initialized yet.
-	if controlPlane.Machines.Len() == 0 {
-		return nil
-	}
-
-	// Collect all the node names.
-	nodeNames := []string{}
-	for _, machine := range controlPlane.Machines {
-		if machine.Status.NodeRef == nil {
-			// If there are provisioning machines (machines without a node yet), return.
-			return nil
-		}
-		nodeNames = append(nodeNames, machine.Status.NodeRef.Name)
-	}
-
-	// Potential inconsistencies between the list of members and the list of machines/nodes are
-	// surfaced using the EtcdClusterHealthyCondition; if this condition is true, meaning no inconsistencies exists, return early.
-	if conditions.IsTrue(controlPlane.KCP, controlplanev1.EtcdClusterHealthyCondition) {
-		return nil
-	}
-
-	workloadCluster, err := r.managementCluster.GetWorkloadCluster(ctx, util.ObjectKey(controlPlane.Cluster))
-	if err != nil {
-		// Failing at connecting to the workload cluster can mean workload cluster is unhealthy for a variety of reasons such as etcd quorum loss.
-		return errors.Wrap(err, "cannot get remote client to workload cluster")
-	}
-
-	removedMembers, err := workloadCluster.ReconcileEtcdMembers(ctx, nodeNames)
-	if err != nil {
-		return errors.Wrap(err, "failed attempt to reconcile etcd members")
-	}
-
-	if len(removedMembers) > 0 {
-		log.Info("Etcd members without nodes removed from the cluster", "members", removedMembers)
-	}
-
-	**/
-
-	return nil
-}
-
 func (r *CK8sControlPlaneReconciler) upgradeControlPlane(
 	ctx context.Context,
 	cluster *clusterv1.Cluster,

controlplane/controllers/machine_controller.go

@@ -2,10 +2,10 @@ package controllers
 
 import (
 	"context"
+	"fmt"
 	"time"
 
 	"github.com/go-logr/logr"
-	"github.com/pkg/errors"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -34,15 +34,10 @@ func (r *MachineReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manag
 		For(&clusterv1.Machine{}).
 		Build(r)
 
-	// NOTE(neoaggelos): See note below
 	if r.managementCluster == nil {
 		r.managementCluster = &ck8s.Management{
 			Client:          r.Client,
 			K8sdDialTimeout: r.K8sdDialTimeout,
-			/*
-				EtcdDialTimeout: r.EtcdDialTimeout,
-				EtcdCallTimeout: r.EtcdCallTimeout,
-			*/
 		}
 	}
 
@@ -51,6 +46,7 @@ func (r *MachineReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manag
 
 // +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=clusters;clusters/status,verbs=get;list;watch
 // +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machines;machines/status,verbs=get;list;watch;create;update;patch;delete
+
 func (r *MachineReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	logger := r.Log.WithValues("namespace", req.Namespace, "machine", req.Name)
 
@@ -95,42 +91,16 @@ func (r *MachineReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 		// Note that this currently makes the annotation a no-op in the code here. However, we still keep the logic in the code is case it
 		// is needed in the future.
 
-		/**
-		cluster, err := util.GetClusterFromMetadata(ctx, r.Client, m.ObjectMeta)
-		if err != nil {
-			logger.Info("unable to get cluster.")
-			return ctrl.Result{}, errors.Wrapf(err, "unable to get cluster")
-		}
-		workloadCluster, err := r.managementCluster.GetWorkloadCluster(ctx, util.ObjectKey(cluster))
-		if err != nil {
-			logger.Error(err, "failed to create client to workload cluster")
-			return ctrl.Result{}, errors.Wrapf(err, "failed to create client to workload cluster")
-		}
-
-		etcdRemoved, err := workloadCluster.RemoveEtcdMemberForMachine(ctx, m)
-		if err != nil {
-			logger.Error(err, "failed to remove etcd member for machine")
-			return ctrl.Result{}, err
-		}
-		if !etcdRemoved {
-			logger.Info("wait embedded etcd controller to remove etcd")
-			return ctrl.Result{Requeue: true}, err
-		}
-
-		// It is possible that the machine has no machine ref yet, will record the machine name in log
-		logger.Info("etcd remove etcd member succeeded", "machine name", m.Name)
-		**/
-
 		patchHelper, err := patch.NewHelper(m, r.Client)
 		if err != nil {
-			return ctrl.Result{}, errors.Wrapf(err, "failed to create patch helper for machine")
+			return ctrl.Result{}, fmt.Errorf("failed to create patch helper for machine: %w", err)
 		}
 
 		mAnnotations := m.GetAnnotations()
 		delete(mAnnotations, clusterv1.PreTerminateDeleteHookAnnotationPrefix)
 		m.SetAnnotations(mAnnotations)
 		if err := patchHelper.Patch(ctx, m); err != nil {
-			return ctrl.Result{}, errors.Wrapf(err, "failed patch machine")
+			return ctrl.Result{}, fmt.Errorf("failed to patch machine: %w", err)
 		}
 	}