Clear remove annotations on cluster reset; refuse to delete last member from cluster

pkg/etcd/member_controller.go

@@ -7,8 +7,10 @@ import (
 
 	"github.com/k3s-io/k3s/pkg/util"
 	"github.com/k3s-io/k3s/pkg/version"
+	"github.com/pkg/errors"
 	controllerv1 "github.com/rancher/wrangler/pkg/generated/controllers/core/v1"
 	"github.com/sirupsen/logrus"
+	"go.etcd.io/etcd/api/v3/v3rpc/rpctypes"
 	v1 "k8s.io/api/core/v1"
 )
 
@@ -49,52 +51,58 @@ func (e *etcdMemberHandler) sync(key string, node *v1.Node) (*v1.Node, error) {
 	node = node.DeepCopy()
 
 	if removalRequested, ok := node.Annotations[removalAnnotation]; ok {
+		// removal requires node name and address annotations; fail if either are not found
+		name, ok := node.Annotations[NodeNameAnnotation]
+		if !ok {
+			return node, fmt.Errorf("node name annotation for node %s not found", key)
+		}
+		address, ok := node.Annotations[NodeAddressAnnotation]
+		if !ok {
+			return node, fmt.Errorf("node address annotation for node %s not found", key)
+		}
+		lf := logrus.Fields{"name": name, "address": address}
+
+		// Check to see if the node was previously removed from the cluster
 		if removed, ok := node.Annotations[removedNodeNameAnnotation]; ok {
-			// check to see if removed is true. if it is, nothing to do.
-			if currentNodeName, ok := node.Annotations[NodeNameAnnotation]; ok {
-				if currentNodeName != removed {
-					// If the current node name is not the same as the removed node name, reset the tainted annotation and removed node name
-					logrus.Infof("Resetting removed node flag as removed node name (did not match current node name")
-					delete(node.Annotations, removedNodeNameAnnotation)
-					node.Annotations[removalAnnotation] = "false"
-					return e.nodeController.Update(node)
-				}
-				// this is the case where the current node name matches the removed node name. We have already removed the
-				// node, so no need to perform any action. Fallthrough to the non-op below.
+			if removed != name {
+				// If the current node name is not the same as the removed node name, clear the removal annotations,
+				// as this indicates that the node has been re-added with a new name.
+				logrus.WithFields(lf).Info("Resetting removed node flag as removed node name does not match current node name")
+				delete(node.Annotations, removedNodeNameAnnotation)
+				delete(node.Annotations, removalAnnotation)
+				return e.nodeController.Update(node)
 			}
-			// This is the edge case where the removed annotation exists, but there is not a current node name annotation.
-			// This should be a non-op, as we can't remove the node anyway.
-			logrus.Debugf("etcd member %s was already marked via annotations as removed", key)
+			// Current node name matches removed node name; don't need to do anything
 			return node, nil
 		}
-		if strings.ToLower(removalRequested) == "true" {
-			// remove the member.
-			name, ok := node.Annotations[NodeNameAnnotation]
-			if !ok {
-				return node, fmt.Errorf("node name annotation for node %s not found", key)
-			}
-			address, ok := node.Annotations[NodeAddressAnnotation]
-			if !ok {
-				return node, fmt.Errorf("node address annotation for node %s not found", key)
-			}
 
-			logrus.Debugf("removing etcd member from cluster name: %s address: %s", name, address)
+		if strings.ToLower(removalRequested) == "true" {
+			// Removal requested, attempt to remove it from the cluster
+			logrus.WithFields(lf).Info("Removing etcd member from cluster due to remove annotation")
 			if err := e.etcd.RemovePeer(e.ctx, name, address, true); err != nil {
+				// etcd will reject the removal if this is the only voting member; abort the removal by removing
+				// the annotation if this is the case. The requesting controller can re-request removal by setting
+				// the annotation again, once there are more cluster members.
+				if errors.Is(err, rpctypes.ErrMemberNotEnoughStarted) {
+					logrus.WithFields(lf).Errorf("etcd member removal rejected, clearing remove annotation: %v", err)
+					delete(node.Annotations, removalAnnotation)
+					return e.nodeController.Update(node)
+				}
 				return node, err
 			}
-			logrus.Debugf("etcd member removal successful for name: %s address: %s", name, address)
-			// Set the removed node name annotation and clean up the other etcd node annotations.
-			// These will be set if the tombstone file is then created and the etcd member is re-added, to their new
-			// respective values.
+
+			logrus.WithFields(lf).Info("etcd emember removed successfully")
+			// Set the removed node name annotation and delete the etcd name and address annotations.
+			// These will be re-set to their new value when the member rejoins the cluster.
 			node.Annotations[removedNodeNameAnnotation] = name
 			delete(node.Annotations, NodeNameAnnotation)
 			delete(node.Annotations, NodeAddressAnnotation)
 			return e.nodeController.Update(node)
 		}
-		// In the event that we had an unexpected removal value, simply return.
+		// In the event that we had an unexpected removal annotation value, simply return.
 		// Fallthrough to the non-op below.
 	}
-	// This is a non-op, as we don't have a tainted annotation to worry about.
+	// This is a non-op, as we don't have a deleted annotation to worry about.
 	return node, nil
 }
 
@@ -103,19 +111,13 @@ func (e *etcdMemberHandler) onRemove(key string, node *v1.Node) (*v1.Node, error
 		logrus.Debugf("Node %s was not labeled etcd node, skipping etcd member removal", key)
 		return node, nil
 	}
-	logrus.Infof("Removing etcd member %s from cluster", key)
-	if removalRequested, ok := node.Annotations[removalAnnotation]; ok {
-		if strings.ToLower(removalRequested) == "true" {
-			if removedNodeName, ok := node.Annotations[removedNodeNameAnnotation]; ok {
-				if len(removedNodeName) > 0 {
-					// If we received a node to delete that has already been removed via annotation, it will be missing
-					// the corresponding node name and address annotations.
-					logrus.Infof("etcd member %s was already removed as member name %s via annotation from the cluster", key, removedNodeName)
-					return node, nil
-				}
-			}
-		}
+
+	if removedNodeName, ok := node.Annotations[removedNodeNameAnnotation]; ok && len(removedNodeName) > 0 {
+		logrus.Debugf("Node %s was already removed from the cluster, skipping etcd member removal", key)
+		return node, nil
 	}
+
+	// removal requires node name and address annotations; fail if either are not found
 	name, ok := node.Annotations[NodeNameAnnotation]
 	if !ok {
 		return node, fmt.Errorf("node name annotation for node %s not found", key)
@@ -124,5 +126,8 @@ func (e *etcdMemberHandler) onRemove(key string, node *v1.Node) (*v1.Node, error
 	if !ok {
 		return node, fmt.Errorf("node address annotation for node %s not found", key)
 	}
+	lf := logrus.Fields{"name": name, "address": address}
+
+	logrus.WithFields(lf).Info("Removing etcd member from cluster due to node delete")
 	return node, e.etcd.RemovePeer(e.ctx, name, address, true)
 }

pkg/etcd/metadata_controller.go

@@ -3,12 +3,16 @@ package etcd
 import (
 	"context"
 	"os"
+	"sync"
 	"time"
 
 	"github.com/k3s-io/k3s/pkg/util"
 	controllerv1 "github.com/rancher/wrangler/pkg/generated/controllers/core/v1"
 	"github.com/sirupsen/logrus"
 	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/client-go/util/retry"
 )
 
 func registerMetadataHandlers(ctx context.Context, etcd *ETCD) {
@@ -17,6 +21,7 @@ func registerMetadataHandlers(ctx context.Context, etcd *ETCD) {
 		etcd:           etcd,
 		nodeController: nodes,
 		ctx:            ctx,
+		once:           &sync.Once{},
 	}
 
 	logrus.Infof("Starting managed etcd node metadata controller")
@@ -27,9 +32,11 @@ type metadataHandler struct {
 	etcd           *ETCD
 	nodeController controllerv1.NodeController
 	ctx            context.Context
+	once           *sync.Once
 }
 
 func (m *metadataHandler) sync(key string, node *v1.Node) (*v1.Node, error) {
+
 	if node == nil {
 		return nil, nil
 	}
@@ -48,6 +55,43 @@ func (m *metadataHandler) sync(key string, node *v1.Node) (*v1.Node, error) {
 	return node, nil
 }
 
+// checkReset ensures that member removal annotations are cleared when the cluster is reset.
+// This is done here instead of in the member controller, as the member removal controller is
+// not guaranteed to run on the node that was reset.
+func (m *metadataHandler) checkReset() {
+	if resetDone, _ := m.etcd.IsReset(); resetDone {
+		labelSelector := labels.Set{util.ETCDRoleLabelKey: "true"}.String()
+		nodes, err := m.nodeController.List(metav1.ListOptions{LabelSelector: labelSelector})
+		if err != nil {
+			logrus.Errorf("Failed to list etcd nodes: %v", err)
+			return
+		}
+		for _, n := range nodes.Items {
+			node := &n
+			err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
+				_, remove := node.Annotations[removalAnnotation]
+				_, removed := node.Annotations[removedNodeNameAnnotation]
+				if remove || removed {
+					node = node.DeepCopy()
+					delete(node.Annotations, removalAnnotation)
+					delete(node.Annotations, removedNodeNameAnnotation)
+					node, err = m.nodeController.Update(node)
+					return err
+				}
+				return nil
+			})
+			if err != nil {
+				logrus.Errorf("Failed to clear removal annotations from node %s after cluster reset: %v", node.Name, err)
+			} else {
+				logrus.Infof("Cleared etcd member removal annotations from node %s after cluster reset", node.Name)
+			}
+		}
+		if err := m.etcd.clearReset(); err != nil {
+			logrus.Errorf("Failed to delete etcd cluster-reset file: %v", err)
+		}
+	}
+}
+
 func (m *metadataHandler) handleSelf(node *v1.Node) (*v1.Node, error) {
 	if m.etcd.config.DisableETCD {
 		if node.Annotations[NodeNameAnnotation] == "" &&
@@ -71,6 +115,8 @@ func (m *metadataHandler) handleSelf(node *v1.Node) (*v1.Node, error) {
 		return m.nodeController.Update(node)
 	}
 
+	m.once.Do(m.checkReset)
+
 	if node.Annotations[NodeNameAnnotation] == m.etcd.name &&
 		node.Annotations[NodeAddressAnnotation] == m.etcd.address &&
 		node.Labels[util.ETCDRoleLabelKey] == "true" {