Updating etcdcluster manifest does not upgrade the cluster unless the number of members is also changed

[jberkus]

To reproduce:

1. Install etcd-operator 0.2
2. Create a 3-node etcdcluster with etcd image 3.6.8
3. Update etcdcluster.yaml to use image 3.6.10
4. kubectl apply the new manifest from sample_autocert_etcdcluster.yaml
5. etcdcluster definition is updated
6. The Statefulset does NOT get updated. No members actually get upgraded.
7. Try modifying etcdcluster-sample using kubectl edit
8. Statefulset still not updated
9. Edit the YAML file one more time, changing the number of members also
10. The statefulset will be updated, including the image version

It appears that changes made to the manifest for an existing cluster are not applied to the statefulset unless the user also changes the number of members. This is a missing logic loop somewhere.

[ahrtr]

It turns out that the etcd-operator only reconciles the cluster size, and ignore all other changes.
On the other hand, if users update the image version(upgrade), cluster size and other fields together, then we shouldn't reconcile them together (such mix-operations are not recommended); instead we should reconcile them in a certain order, e.g.

• apply normal options change first
• resize if needed
• upgrade

etcd-operator/internal/controller/etcdcluster_controller.go

Lines 258 to 387 in 0ca09d9

	func (r *EtcdClusterReconciler) reconcileClusterState(ctx context.Context, s *reconcileState) (ctrl.Result, error) {
	logger := log.FromContext(ctx)
	memberCnt := 0
	if s.memberListResp != nil {
	memberCnt = len(s.memberListResp.Members)
	}
	targetReplica := *s.sts.Spec.Replicas
	var err error
	// The number of replicas in the StatefulSet doesn't match the number of etcd members in the cluster.
	if int(targetReplica) != memberCnt {
	logger.Info("The expected number of replicas doesn't match the number of etcd members in the cluster", "targetReplica", targetReplica, "memberCnt", memberCnt)
	if int(targetReplica) < memberCnt {
	logger.Info("An etcd member was added into the cluster, but the StatefulSet hasn't scaled out yet")
	newReplicaCount := targetReplica + 1
	logger.Info("Increasing StatefulSet replicas to match the etcd cluster member count", "oldReplicaCount", targetReplica, "newReplicaCount", newReplicaCount)
	if _, err := reconcileStatefulSet(ctx, logger, s.cluster, r.Client, newReplicaCount, r.Scheme); err != nil {
	return ctrl.Result{}, err
	}
	} else {
	logger.Info("An etcd member was removed from the cluster, but the StatefulSet hasn't scaled in yet")
	newReplicaCount := targetReplica - 1
	logger.Info("Decreasing StatefulSet replicas to remove the unneeded Pod.", "oldReplicaCount", targetReplica, "newReplicaCount", newReplicaCount)
	if _, err := reconcileStatefulSet(ctx, logger, s.cluster, r.Client, newReplicaCount, r.Scheme); err != nil {
	return ctrl.Result{}, err
	}
	}
	return ctrl.Result{RequeueAfter: requeueDuration}, nil
	}
	var (
	learnerStatus *clientv3.StatusResponse
	learner uint64
	leaderStatus *clientv3.StatusResponse
	)
	if memberCnt > 0 {
	// Find the leader status
	_, leaderStatus = etcdutils.FindLeaderStatus(s.memberHealth, logger)
	if leaderStatus == nil {
	// If the leader is not available, wait for the leader to be elected
	return ctrl.Result{}, fmt.Errorf("couldn't find leader, memberCnt: %d", memberCnt)
	}
	learner, learnerStatus = etcdutils.FindLearnerStatus(s.memberHealth, logger)
	if learner > 0 {
	// There is at least one learner. Try to promote it if it's ready; otherwise requeue and wait.
	logger.Info("Learner found", "learnedID", learner, "learnerStatus", learnerStatus)
	if etcdutils.IsLearnerReady(leaderStatus, learnerStatus) {
	logger.Info("Learner is ready to be promoted to voting member", "learnerID", learner)
	logger.Info("Promoting the learner member", "learnerID", learner)
	eps := clientEndpointsFromStatefulsets(s.sts)
	eps = eps[:(len(eps) - 1)]
	if err := etcdutils.PromoteLearner(eps, learner); err != nil {
	// The member is not promoted yet, so we error out and requeue via the caller.
	return ctrl.Result{}, err
	}
	} else {
	// Learner is not yet ready. We can't add another learner or proceed further until this one is promoted.
	logger.Info("The learner member isn't ready to be promoted yet", "learnerID", learner)
	return ctrl.Result{RequeueAfter: requeueDuration}, nil
	}
	}
	}
	if targetReplica == int32(s.cluster.Spec.Size) {
	logger.Info("EtcdCluster is already up-to-date")
	return ctrl.Result{}, nil
	}
	eps := clientEndpointsFromStatefulsets(s.sts)
	// If there are no learners left, we can proceed to scale the cluster towards the desired size.
	// When there are no members to add, the controller will requeue above and this block won't execute.
	if targetReplica < int32(s.cluster.Spec.Size) {
	// scale out
	_, peerURL := peerEndpointForOrdinalIndex(s.cluster, int(targetReplica))
	targetReplica++
	logger.Info("[Scale out] adding a new learner member to etcd cluster", "peerURLs", peerURL)
	if _, err := etcdutils.AddMember(eps, []string{peerURL}, true); err != nil {
	return ctrl.Result{}, err
	}
	logger.Info("Learner member added successfully", "peerURLs", peerURL)
	// gofail: var exceptionAfterMemberAdd struct{}
	if s.sts, err = reconcileStatefulSet(ctx, logger, s.cluster, r.Client, targetReplica, r.Scheme); err != nil {
	return ctrl.Result{}, err
	}
	return ctrl.Result{RequeueAfter: requeueDuration}, nil
	}
	if targetReplica > int32(s.cluster.Spec.Size) {
	// scale in
	targetReplica--
	logger = logger.WithValues("targetReplica", targetReplica, "expectedSize", s.cluster.Spec.Size)
	memberID := s.memberHealth[memberCnt-1].Status.Header.MemberId
	logger.Info("[Scale in] removing one member", "memberID", memberID)
	eps = eps[:targetReplica]
	if err := etcdutils.RemoveMember(eps, memberID); err != nil {
	return ctrl.Result{}, err
	}
	// gofail: var exceptionAfterMemberDelete struct{}
	if s.sts, err = reconcileStatefulSet(ctx, logger, s.cluster, r.Client, targetReplica, r.Scheme); err != nil {
	return ctrl.Result{}, err
	}
	return ctrl.Result{RequeueAfter: requeueDuration}, nil
	}
	// Ensure every etcd member reports itself healthy before declaring success.
	allMembersHealthy, err := areAllMembersHealthy(s.sts, logger)
	if err != nil {
	return ctrl.Result{}, err
	}
	if !allMembersHealthy {
	// Requeue until the StatefulSet settles and all members are healthy.
	return ctrl.Result{RequeueAfter: requeueDuration}, nil
	}
	logger.Info("EtcdCluster reconciled successfully")
	return ctrl.Result{}, nil
	}

[lavishpal]

/assign

[jberkus]

If we get this fixed before the rest of 0.3, I think we should update it as 0.2.1.

[lavishpal]

Will raise the PR shortly.