From e86d26795f67747d1c9baa6e6a2037ad56d27f81 Mon Sep 17 00:00:00 2001
From: Allen Ray <allentray125@gmail.com>
Date: Mon, 14 Oct 2024 11:34:52 -0400
Subject: [PATCH] Revert "Merge pull request #1350 from
 jubittajohn/remove-stability-check"

This reverts commit 28467760b688785bf62a463b5ba545892cbd9471, reversing
changes made to 52cac6a07da30a239e370311fac9052ba393505a.
---
 pkg/operator/ceohelpers/bootstrap.go          |  7 +++++
 pkg/operator/ceohelpers/bootstrap_test.go     |  7 +++++
 .../etcdendpointscontroller_test.go           | 31 +++++++++++++++++++
 3 files changed, 45 insertions(+)

diff --git a/pkg/operator/ceohelpers/bootstrap.go b/pkg/operator/ceohelpers/bootstrap.go
index 2d63e223f..42ed5cf7f 100644
--- a/pkg/operator/ceohelpers/bootstrap.go
+++ b/pkg/operator/ceohelpers/bootstrap.go
@@ -158,6 +158,7 @@ func IsBootstrapComplete(configmapLister corev1listers.ConfigMapLister, staticPo
 		return isBootstrapComplete, err
 	}
 
+	// now run check to stability of revisions
 	_, status, _, err := staticPodClient.GetStaticPodOperatorState()
 	if err != nil {
 		return false, fmt.Errorf("failed to get static pod operator state: %w", err)
@@ -165,6 +166,12 @@ func IsBootstrapComplete(configmapLister corev1listers.ConfigMapLister, staticPo
 	if status.LatestAvailableRevision == 0 {
 		return false, nil
 	}
+	for _, curr := range status.NodeStatuses {
+		if curr.CurrentRevision != status.LatestAvailableRevision {
+			klog.V(4).Infof("bootstrap considered incomplete because revision %d is still in progress", status.LatestAvailableRevision)
+			return false, nil
+		}
+	}
 
 	// check if etcd-bootstrap member is still present within the etcd cluster membership
 	membersList, err := etcdClient.MemberList(context.Background())
diff --git a/pkg/operator/ceohelpers/bootstrap_test.go b/pkg/operator/ceohelpers/bootstrap_test.go
index 973c9a603..6d9237f32 100644
--- a/pkg/operator/ceohelpers/bootstrap_test.go
+++ b/pkg/operator/ceohelpers/bootstrap_test.go
@@ -197,6 +197,13 @@ func Test_IsBootstrapComplete(t *testing.T) {
 			expectComplete:     true,
 			expectError:        nil,
 		},
+		"bootstrap complete, node progressing": {
+			bootstrapConfigMap: bootstrapComplete,
+			nodes:              twoNodesProgressingTowardsCurrentRevision,
+			etcdMembers:        u.DefaultEtcdMembers(),
+			expectComplete:     false,
+			expectError:        nil,
+		},
 		"bootstrap complete, etcd-bootstrap removed": {
 			bootstrapConfigMap: bootstrapComplete,
 			nodes:              twoNodesAtCurrentRevision,
diff --git a/pkg/operator/etcdendpointscontroller/etcdendpointscontroller_test.go b/pkg/operator/etcdendpointscontroller/etcdendpointscontroller_test.go
index b8c4064a7..c1ea81968 100644
--- a/pkg/operator/etcdendpointscontroller/etcdendpointscontroller_test.go
+++ b/pkg/operator/etcdendpointscontroller/etcdendpointscontroller_test.go
@@ -133,6 +133,37 @@ func TestBootstrapAnnotationRemoval(t *testing.T) {
 				}
 			},
 		},
+		{
+			// The configmap should remain intact because although bootstrapping
+			// reports complete, the nodes are still progressing towards a revision.
+			name: "NewClusterBootstrapNodesProgressing",
+			objects: []runtime.Object{
+				u.BootstrapConfigMap(u.WithBootstrapStatus("complete")),
+				u.EndpointsConfigMap(
+					u.WithBootstrapIP("192.0.2.1"),
+					u.WithEndpoint(etcdMembers[0].ID, etcdMembers[0].PeerURLs[0]),
+					u.WithEndpoint(etcdMembers[1].ID, etcdMembers[1].PeerURLs[0]),
+					u.WithEndpoint(etcdMembers[2].ID, etcdMembers[2].PeerURLs[0]),
+				),
+			},
+			staticPodStatus: u.StaticPodOperatorStatus(
+				u.WithLatestRevision(3),
+				u.WithNodeStatusAtCurrentRevision(3),
+				u.WithNodeStatusAtCurrentRevision(2),
+				u.WithNodeStatusAtCurrentRevision(3),
+			),
+			expectBootstrap: true,
+			etcdMembers:     etcdMembers,
+			validateFunc: func(ts *testing.T, endpoints []func(*corev1.ConfigMap), actions []clientgotesting.Action) {
+				for _, action := range actions {
+					if action.Matches("update", "configmaps") {
+						updateAction := action.(clientgotesting.UpdateAction)
+						actual := updateAction.GetObject().(*corev1.ConfigMap)
+						ts.Errorf("unexpected configmap update: %#v", actual)
+					}
+				}
+			},
+		},
 		{
 			// The configmap should remain intact because although nodes appear
 			// to have converged on a revision, bootstrap reports incomplete.