Merge pull request #2711 from chuckha/remove-etcd-removal-annotation

🐛 Remove a troublesome annotation
kubernetes-sigs · Mar 18, 2020 · 2e98141 · 2e98141
2 parents 5f14f88 + 877800f
commit 2e98141
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 22 deletions.
diff --git a/controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_types.go b/controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_types.go
@@ -30,7 +30,6 @@ const (
 	SelectedForUpgradeAnnotation             = "kubeadm.controlplane.cluster.x-k8s.io/selected-for-upgrade"
 	UpgradeReplacementCreatedAnnotation      = "kubeadm.controlplane.cluster.x-k8s.io/upgrade-replacement-created"
 	DeleteForScaleDownAnnotation             = "kubeadm.controlplane.cluster.x-k8s.io/delete-for-scale-down"
-	ScaleDownEtcdMemberRemovedAnnotation     = "kubeadm.controlplane.cluster.x-k8s.io/scale-down-etcd-member-removed"
 	ScaleDownConfigMapEntryRemovedAnnotation = "kubeadm.controlplane.cluster.x-k8s.io/scale-down-configmap-entry-removed"
 )
 

diff --git a/controlplane/kubeadm/controllers/kubeadm_control_plane_controller.go b/controlplane/kubeadm/controllers/kubeadm_control_plane_controller.go
@@ -532,26 +532,21 @@ func (r *KubeadmControlPlaneReconciler) scaleDownControlPlane(ctx context.Contex
 		return ctrl.Result{}, errors.New("failed to pick control plane Machine to delete")
 	}
 
-	if !machinefilters.HasAnnotationKey(controlplanev1.ScaleDownEtcdMemberRemovedAnnotation)(machineToDelete) {
-		// Ensure etcd is healthy prior to attempting to remove the member
-		if err := r.managementCluster.TargetClusterEtcdIsHealthy(ctx, util.ObjectKey(cluster), kcp.Name); err != nil {
-			logger.Error(err, "waiting for control plane to pass etcd health check before removing a control plane machine")
-			r.recorder.Eventf(kcp, corev1.EventTypeWarning, "ControlPlaneUnhealthy", "Waiting for control plane to pass etcd health check before removing a control plane machine: %v", err)
-			return ctrl.Result{}, &capierrors.RequeueAfterError{RequeueAfter: HealthCheckFailedRequeueAfter}
-		}
-		// If etcd leadership is on machine that is about to be deleted, move it to the newest member available.
-		etcdLeaderCandidate := ownedMachines.Newest()
-		if err := workloadCluster.ForwardEtcdLeadership(ctx, machineToDelete, etcdLeaderCandidate); err != nil {
-			logger.Error(err, "failed to move leadership to another machine")
-			return ctrl.Result{}, err
-		}
-		if err := workloadCluster.RemoveEtcdMemberForMachine(ctx, machineToDelete); err != nil {
-			logger.Error(err, "failed to remove etcd member for machine")
-			return ctrl.Result{}, err
-		}
-		if err := r.markWithAnnotationKey(ctx, machineToDelete, controlplanev1.ScaleDownEtcdMemberRemovedAnnotation); err != nil {
-			return ctrl.Result{}, errors.Wrapf(err, "failed to mark machine %s as having etcd membership removed", machineToDelete.Name)
-		}
+	// Ensure etcd is healthy prior to attempting to remove the member
+	if err := r.managementCluster.TargetClusterEtcdIsHealthy(ctx, util.ObjectKey(cluster), kcp.Name); err != nil {
+		logger.Error(err, "waiting for control plane to pass etcd health check before removing a control plane machine")
+		r.recorder.Eventf(kcp, corev1.EventTypeWarning, "ControlPlaneUnhealthy", "Waiting for control plane to pass etcd health check before removing a control plane machine: %v", err)
+		return ctrl.Result{}, &capierrors.RequeueAfterError{RequeueAfter: HealthCheckFailedRequeueAfter}
+	}
+	// If etcd leadership is on machine that is about to be deleted, move it to the newest member available.
+	etcdLeaderCandidate := ownedMachines.Newest()
+	if err := workloadCluster.ForwardEtcdLeadership(ctx, machineToDelete, etcdLeaderCandidate); err != nil {
+		logger.Error(err, "failed to move leadership to another machine")
+		return ctrl.Result{}, err
+	}
+	if err := workloadCluster.RemoveEtcdMemberForMachine(ctx, machineToDelete); err != nil {
+		logger.Error(err, "failed to remove etcd member for machine")
+		return ctrl.Result{}, err
 	}
 
 	if !machinefilters.HasAnnotationKey(controlplanev1.ScaleDownConfigMapEntryRemovedAnnotation)(machineToDelete) {

diff --git a/test/infrastructure/docker/e2e/docker_test.go b/test/infrastructure/docker/e2e/docker_test.go
@@ -208,7 +208,7 @@ var _ = Describe("Docker", func() {
 				By("upgrading the control plane object to a new version")
 				patchHelper, err := patch.NewHelper(controlPlane, client)
 				Expect(err).ToNot(HaveOccurred())
-				controlPlane.Spec.Version = "1.17.2"
+				controlPlane.Spec.Version = "v1.17.2"
 				Expect(patchHelper.Patch(ctx, controlPlane)).To(Succeed())
 				By("waiting for all control plane nodes to exist")
 				inClustersNamespaceListOption := ctrlclient.InNamespace(cluster.Namespace)