fix: improve logging & safety of statefulSetReady

Confirm that the current and updated revision numbers also match as part
of the readiness check. Add coverage for readiness scenarios where
StatefulSet status does not reflect the most recent generation of the
StatefulSet yet.

Also add additional logging around the sts transitions from non-ready to
ready.

Fixes: #10163

Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>

pkg/action/upgrade.go

@@ -391,6 +391,9 @@ func (u *Upgrade) releasingUpgrade(c chan<- resultMessage, upgradedRelease *rele
 	}
 
 	if u.Wait {
+		u.cfg.Log(
+			"waiting for release %s resources (created: %d updated: %d  deleted: %d)",
+			upgradedRelease.Name, len(results.Created), len(results.Updated), len(results.Deleted))
 		if u.WaitForJobs {
 			if err := u.cfg.KubeClient.WaitWithJobs(target, u.Timeout); err != nil {
 				u.cfg.recordRelease(originalRelease)

pkg/kube/ready.go

@@ -353,9 +353,16 @@ func (c *ReadyChecker) crdReady(crd apiextv1.CustomResourceDefinition) bool {
 func (c *ReadyChecker) statefulSetReady(sts *appsv1.StatefulSet) bool {
 	// If the update strategy is not a rolling update, there will be nothing to wait for
 	if sts.Spec.UpdateStrategy.Type != appsv1.RollingUpdateStatefulSetStrategyType {
+		c.log("StatefulSet skipped ready check: %s/%s. updateStrategy is %v", sts.Namespace, sts.Name, sts.Spec.UpdateStrategy.Type)
 		return true
 	}
 
+	// Make sure the status is up-to-date with the StatefulSet changes
+	if sts.Status.ObservedGeneration < sts.Generation {
+		c.log("StatefulSet is not ready: %s/%s. update has not yet been observed", sts.Namespace, sts.Name)
+		return false
+	}
+
 	// Dereference all the pointers because StatefulSets like them
 	var partition int
 	// 1 is the default for replicas if not set
@@ -386,6 +393,13 @@ func (c *ReadyChecker) statefulSetReady(sts *appsv1.StatefulSet) bool {
 		c.log("StatefulSet is not ready: %s/%s. %d out of %d expected pods are ready", sts.Namespace, sts.Name, sts.Status.ReadyReplicas, replicas)
 		return false
 	}
+
+	if sts.Status.CurrentRevision != sts.Status.UpdateRevision {
+		c.log("StatefulSet is not ready: %s/%s. currentRevision %s does not yet match updateRevision %s", sts.Namespace, sts.Name, sts.Status.CurrentRevision, sts.Status.UpdateRevision)
+		return false
+	}
+
+	c.log("StatefulSet is ready: %s/%s. %d out of %d expected pods are ready", sts.Namespace, sts.Name, sts.Status.ReadyReplicas, replicas)
 	return true
 }
 

pkg/kube/ready_test.go

@@ -175,6 +175,20 @@ func Test_ReadyChecker_statefulSetReady(t *testing.T) {
 			},
 			want: true,
 		},
+		{
+			name: "statefulset is not ready when status of latest generation has not yet been observed",
+			args: args{
+				sts: newStatefulSetWithNewGeneration("foo", 1, 0, 1, 1),
+			},
+			want: false,
+		},
+		{
+			name: "statefulset is not ready when current revision for current replicas does not match update revision for updated replicas",
+			args: args{
+				sts: newStatefulSetWithUpdateRevision("foo", 1, 0, 1, 1, "foo-bbbbbbb"),
+			},
+			want: false,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -377,8 +391,9 @@ func newDaemonSet(name string, maxUnavailable, numberReady, desiredNumberSchedul
 func newStatefulSet(name string, replicas, partition, readyReplicas, updatedReplicas int) *appsv1.StatefulSet {
 	return &appsv1.StatefulSet{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      name,
-			Namespace: defaultNamespace,
+			Name:       name,
+			Namespace:  defaultNamespace,
+			Generation: int64(1),
 		},
 		Spec: appsv1.StatefulSetSpec{
 			UpdateStrategy: appsv1.StatefulSetUpdateStrategy{
@@ -404,12 +419,27 @@ func newStatefulSet(name string, replicas, partition, readyReplicas, updatedRepl
 			},
 		},
 		Status: appsv1.StatefulSetStatus{
-			UpdatedReplicas: int32(updatedReplicas),
-			ReadyReplicas:   int32(readyReplicas),
+			ObservedGeneration: int64(1),
+			CurrentRevision:    name + "-aaaaaaa",
+			UpdateRevision:     name + "-aaaaaaa",
+			UpdatedReplicas:    int32(updatedReplicas),
+			ReadyReplicas:      int32(readyReplicas),
 		},
 	}
 }
 
+func newStatefulSetWithNewGeneration(name string, replicas, partition, readyReplicas, updatedReplicas int) *appsv1.StatefulSet {
+	ss := newStatefulSet(name, replicas, partition, readyReplicas, updatedReplicas)
+	ss.Generation++
+	return ss
+}
+
+func newStatefulSetWithUpdateRevision(name string, replicas, partition, readyReplicas, updatedReplicas int, updateRevision string) *appsv1.StatefulSet {
+	ss := newStatefulSet(name, replicas, partition, readyReplicas, updatedReplicas)
+	ss.Status.UpdateRevision = updateRevision
+	return ss
+}
+
 func newDeployment(name string, replicas, maxSurge, maxUnavailable int) *appsv1.Deployment {
 	return &appsv1.Deployment{
 		ObjectMeta: metav1.ObjectMeta{