From d154ca9c00a3a4676ec691e69bf43b7de8f51e42 Mon Sep 17 00:00:00 2001 From: Maciej Szulik Date: Fri, 4 Aug 2023 15:27:07 +0200 Subject: [PATCH] Statefulset should wait for new replicas when removing .start.ordinal --- test/e2e/apps/statefulset.go | 55 +++++++++++++++++++++--------------- test/e2e/apps/wait.go | 12 ++++++++ 2 files changed, 45 insertions(+), 22 deletions(-) diff --git a/test/e2e/apps/statefulset.go b/test/e2e/apps/statefulset.go index 7f355f2433e..4e1f1da9fd9 100644 --- a/test/e2e/apps/statefulset.go +++ b/test/e2e/apps/statefulset.go @@ -1500,7 +1500,8 @@ var _ = SIGDescribe("StatefulSet", func() { ginkgo.By("Confirming 2 replicas, with start ordinal 0") pods := e2estatefulset.GetPodList(ctx, c, ss) - expectPodNames(pods, []string{"ss-0", "ss-1"}) + err = expectPodNames(pods, []string{"ss-0", "ss-1"}) + framework.ExpectNoError(err) ginkgo.By("Setting .spec.replicas = 3 .spec.ordinals.start = 2") ss, err = updateStatefulSetWithRetries(ctx, c, ns, ss.Name, func(update *appsv1.StatefulSet) { @@ -1510,12 +1511,14 @@ var _ = SIGDescribe("StatefulSet", func() { *(update.Spec.Replicas) = 3 }) framework.ExpectNoError(err) + + // we need to ensure we wait for all the new ones to show up, not + // just for any random 3 + waitForStatus(ctx, c, ss) + waitForPodNames(ctx, c, ss, []string{"ss-2", "ss-3", "ss-4"}) + ginkgo.By("Confirming 3 replicas, with start ordinal 2") e2estatefulset.WaitForStatusReplicas(ctx, c, ss, 3) e2estatefulset.WaitForStatusReadyReplicas(ctx, c, ss, 3) - - ginkgo.By("Confirming 3 replicas, with start ordinal 2") - pods = e2estatefulset.GetPodList(ctx, c, ss) - expectPodNames(pods, []string{"ss-2", "ss-3", "ss-4"}) }) ginkgo.It("Increasing .start.ordinal", func(ctx context.Context) { @@ -1532,7 +1535,8 @@ var _ = SIGDescribe("StatefulSet", func() { ginkgo.By("Confirming 2 replicas, with start ordinal 2") pods := e2estatefulset.GetPodList(ctx, c, ss) - expectPodNames(pods, []string{"ss-2", "ss-3"}) + err = expectPodNames(pods, []string{"ss-2", "ss-3"}) + framework.ExpectNoError(err) ginkgo.By("Increasing .spec.ordinals.start = 4") ss, err = updateStatefulSetWithRetries(ctx, c, ns, ss.Name, func(update *appsv1.StatefulSet) { @@ -1541,13 +1545,14 @@ var _ = SIGDescribe("StatefulSet", func() { } }) framework.ExpectNoError(err) + + // since we are replacing 2 pods for 2, we need to ensure we wait + // for the new ones to show up, not just for any random 2 + ginkgo.By("Confirming 2 replicas, with start ordinal 4") waitForStatus(ctx, c, ss) + waitForPodNames(ctx, c, ss, []string{"ss-4", "ss-5"}) e2estatefulset.WaitForStatusReplicas(ctx, c, ss, 2) e2estatefulset.WaitForStatusReadyReplicas(ctx, c, ss, 2) - - ginkgo.By("Confirming 2 replicas, with start ordinal 4") - pods = e2estatefulset.GetPodList(ctx, c, ss) - expectPodNames(pods, []string{"ss-4", "ss-5"}) }) ginkgo.It("Decreasing .start.ordinal", func(ctx context.Context) { @@ -1564,7 +1569,8 @@ var _ = SIGDescribe("StatefulSet", func() { ginkgo.By("Confirming 2 replicas, with start ordinal 3") pods := e2estatefulset.GetPodList(ctx, c, ss) - expectPodNames(pods, []string{"ss-3", "ss-4"}) + err = expectPodNames(pods, []string{"ss-3", "ss-4"}) + framework.ExpectNoError(err) ginkgo.By("Decreasing .spec.ordinals.start = 2") ss, err = updateStatefulSetWithRetries(ctx, c, ns, ss.Name, func(update *appsv1.StatefulSet) { @@ -1573,13 +1579,14 @@ var _ = SIGDescribe("StatefulSet", func() { } }) framework.ExpectNoError(err) + + // since we are replacing 2 pods for 2, we need to ensure we wait + // for the new ones to show up, not just for any random 2 + ginkgo.By("Confirming 2 replicas, with start ordinal 2") waitForStatus(ctx, c, ss) + waitForPodNames(ctx, c, ss, []string{"ss-2", "ss-3"}) e2estatefulset.WaitForStatusReplicas(ctx, c, ss, 2) e2estatefulset.WaitForStatusReadyReplicas(ctx, c, ss, 2) - - ginkgo.By("Confirming 2 replicas, with start ordinal 2") - pods = e2estatefulset.GetPodList(ctx, c, ss) - expectPodNames(pods, []string{"ss-2", "ss-3"}) }) ginkgo.It("Removing .start.ordinal", func(ctx context.Context) { @@ -1595,19 +1602,22 @@ var _ = SIGDescribe("StatefulSet", func() { ginkgo.By("Confirming 2 replicas, with start ordinal 3") pods := e2estatefulset.GetPodList(ctx, c, ss) - expectPodNames(pods, []string{"ss-3", "ss-4"}) + err = expectPodNames(pods, []string{"ss-3", "ss-4"}) + framework.ExpectNoError(err) ginkgo.By("Removing .spec.ordinals") ss, err = updateStatefulSetWithRetries(ctx, c, ns, ss.Name, func(update *appsv1.StatefulSet) { update.Spec.Ordinals = nil }) framework.ExpectNoError(err) + + // since we are replacing 2 pods for 2, we need to ensure we wait + // for the new ones to show up, not just for any random 2 + framework.Logf("Confirming 2 replicas, with start ordinal 0") + waitForStatus(ctx, c, ss) + waitForPodNames(ctx, c, ss, []string{"ss-0", "ss-1"}) e2estatefulset.WaitForStatusReplicas(ctx, c, ss, 2) e2estatefulset.WaitForStatusReadyReplicas(ctx, c, ss, 2) - - ginkgo.By("Confirming 2 replicas, with start ordinal 0") - pods = e2estatefulset.GetPodList(ctx, c, ss) - expectPodNames(pods, []string{"ss-0", "ss-1"}) }) }) }) @@ -2174,7 +2184,7 @@ func verifyStatefulSetPVCsExistWithOwnerRefs(ctx context.Context, c clientset.In // expectPodNames compares the names of the pods from actualPods with expectedPodNames. // actualPods can be in any list, since we'll sort by their ordinals and filter // active ones. expectedPodNames should be ordered by statefulset ordinals. -func expectPodNames(actualPods *v1.PodList, expectedPodNames []string) { +func expectPodNames(actualPods *v1.PodList, expectedPodNames []string) error { e2estatefulset.SortStatefulPods(actualPods) pods := []string{} for _, pod := range actualPods.Items { @@ -2186,6 +2196,7 @@ func expectPodNames(actualPods *v1.PodList, expectedPodNames []string) { } if !reflect.DeepEqual(expectedPodNames, pods) { diff := cmp.Diff(expectedPodNames, pods) - framework.Failf("Pod names don't match. Diff (- for expected, + for actual):\n%s", diff) + return fmt.Errorf("pod names don't match, diff (- for expected, + for actual):\n%s", diff) } + return nil } diff --git a/test/e2e/apps/wait.go b/test/e2e/apps/wait.go index 92c25f067e5..99392fcb2a5 100644 --- a/test/e2e/apps/wait.go +++ b/test/e2e/apps/wait.go @@ -97,6 +97,18 @@ func waitForStatus(ctx context.Context, c clientset.Interface, set *appsv1.State return set } +// waitForPodNames waits for the StatefulSet's pods to match expected names. +func waitForPodNames(ctx context.Context, c clientset.Interface, set *appsv1.StatefulSet, expectedPodNames []string) { + e2estatefulset.WaitForState(ctx, c, set, + func(intSet *appsv1.StatefulSet, pods *v1.PodList) (bool, error) { + if err := expectPodNames(pods, expectedPodNames); err != nil { + framework.Logf("Currently %v", err) + return false, nil + } + return true, nil + }) +} + // waitForStatus waits for the StatefulSetStatus's CurrentReplicas to be equal to expectedReplicas // The returned StatefulSet contains such a StatefulSetStatus func waitForStatusCurrentReplicas(ctx context.Context, c clientset.Interface, set *appsv1.StatefulSet, expectedReplicas int32) *appsv1.StatefulSet {