diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index e15b5fedc..a7c4e37ca 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -745,6 +745,21 @@ func (c *Cluster) syncStatefulSet() error { // statefulset or those that got their configuration from the outdated statefulset) if len(podsToRecreate) > 0 { if isSafeToRecreatePods { + // Ensure we have enough "accounted for" pods before starting a rolling update + // A pod is considered "accounted for" if it is either running OR marked for recreation + numReplicas := 1 + if c.Statefulset.Spec.Replicas != nil { + numReplicas = int(*c.Statefulset.Spec.Replicas) + } else { + c.logger.Warningf("replicas number in statefulset spec is nil") + } + + res := checkClusterStableForRollout(pods, podsToRecreate, numReplicas) + if !res.isEnough { + c.logger.Warningf("postpone pod recreation until next sync because %s", res.reason) + return fmt.Errorf("pod recreation cannot be started: %s", res.reason) + } + c.logger.Info("performing rolling update") c.eventRecorder.Event(c.GetReference(), v1.EventTypeNormal, "Update", "Performing rolling update") if err := c.recreatePods(podsToRecreate, switchoverCandidates); err != nil { @@ -1813,3 +1828,56 @@ func (c *Cluster) syncLogicalBackupJob() error { return nil } + +// podsAccountingResult holds the result of checking whether there are enough +// "accounted for" pods to safely start a rolling update +type podsAccountingResult struct { + accountedPods int + unaccountedPods []string + numReplicas int + isEnough bool + reason string +} + +// checkClusterStableForRollout verifies we have enough "accounted for" pods before +// starting a rolling update. A pod is "accounted for" if it is either running +// OR explicitly scheduled for recreation +func checkClusterStableForRollout(pods []v1.Pod, podsToRecreate []v1.Pod, numReplicas int) podsAccountingResult { + result := podsAccountingResult{ + numReplicas: numReplicas, + unaccountedPods: []string{}, + } + + podsToRecreateMap := make(map[string]struct{}, len(podsToRecreate)) + for _, p := range podsToRecreate { + podsToRecreateMap[p.Name] = struct{}{} + } + + for i := range pods { + isRunning := !podIsNotRunning(&pods[i]) + _, inRecreateList := podsToRecreateMap[pods[i].Name] + + if isRunning || inRecreateList { + result.accountedPods++ + } else { + result.unaccountedPods = append(result.unaccountedPods, pods[i].Name) + } + } + + result.isEnough = result.accountedPods >= numReplicas + + if !result.isEnough { + if len(result.unaccountedPods) > 0 { + result.reason = fmt.Sprintf( + "only %d/%d pods are healthy or scheduled for recreation, unaccounted/not running pods: %s", + result.accountedPods, numReplicas, strings.Join(result.unaccountedPods, ", "), + ) + } else { + result.reason = fmt.Sprintf( + "only %d/%d pods are healthy or scheduled for recreation, %d pod(s) are completely missing", + result.accountedPods, numReplicas, numReplicas-result.accountedPods, + ) + } + } + return result +} diff --git a/pkg/cluster/sync_test.go b/pkg/cluster/sync_test.go index f7d46d427..f0bd8c9e5 100644 --- a/pkg/cluster/sync_test.go +++ b/pkg/cluster/sync_test.go @@ -1053,3 +1053,193 @@ func TestUpdateSecretNameConflict(t *testing.T) { expectedError := fmt.Sprintf("syncing secret %s failed: error while checking for password rotation: could not update secret because of user name mismatch", "default/prepared-owner-user.acid-test-cluster.credentials") assert.Contains(t, err.Error(), expectedError) } + +func Test_checkClusterStableForRollout(t *testing.T) { + createRunningPod := func(name string) v1.Pod { + return v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Status: v1.PodStatus{ + Phase: v1.PodRunning, + ContainerStatuses: []v1.ContainerStatus{ + {State: v1.ContainerState{Running: &v1.ContainerStateRunning{}}}, + }, + }, + } + } + + createFailingPod := func(name, reason string) v1.Pod { + return v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Status: v1.PodStatus{ + Phase: v1.PodRunning, + ContainerStatuses: []v1.ContainerStatus{ + {State: v1.ContainerState{Waiting: &v1.ContainerStateWaiting{Reason: reason}}}, + }, + }, + } + } + + createTerminatedPod := func(name string, exitCode int32) v1.Pod { + return v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Status: v1.PodStatus{ + Phase: v1.PodRunning, + ContainerStatuses: []v1.ContainerStatus{ + {State: v1.ContainerState{Terminated: &v1.ContainerStateTerminated{ExitCode: exitCode}}}, + }, + }, + } + } + + createPendingPod := func(name string) v1.Pod { + return v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Status: v1.PodStatus{Phase: v1.PodPending}, + } + } + + createUnreportedPod := func(name string) v1.Pod { + return v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Status: v1.PodStatus{}, // no status reported yet + } + } + + tests := []struct { + name string + numReplicas int + allPods []v1.Pod + podsToRecreate []v1.Pod + expectedResult podsAccountingResult + }{ + { + name: "All pods running", + numReplicas: 3, + allPods: []v1.Pod{createRunningPod("pod-0"), createRunningPod("pod-1"), createRunningPod("pod-2")}, + expectedResult: podsAccountingResult{ + accountedPods: 3, + unaccountedPods: []string{}, + numReplicas: 3, + isEnough: true, + reason: "", + }, + }, + { + name: "One pod failing, but in recreate list", + numReplicas: 3, + allPods: []v1.Pod{createRunningPod("pod-0"), createRunningPod("pod-1"), createFailingPod("pod-2", "CrashLoop")}, + podsToRecreate: []v1.Pod{createFailingPod("pod-2", "CrashLoop")}, + expectedResult: podsAccountingResult{ + accountedPods: 3, + unaccountedPods: []string{}, + numReplicas: 3, + isEnough: true, + reason: "", + }, + }, + { + name: "One pod failing, not in recreate list", + numReplicas: 3, + allPods: []v1.Pod{createRunningPod("pod-0"), createRunningPod("pod-1"), createFailingPod("pod-2", "ImagePullBackOff")}, + podsToRecreate: []v1.Pod{}, + expectedResult: podsAccountingResult{ + accountedPods: 2, + unaccountedPods: []string{"pod-2"}, + numReplicas: 3, + isEnough: false, + reason: "only 2/3 pods are healthy or scheduled for recreation, unaccounted/not running pods: pod-2", + }, + }, + { + name: "One pod missing from API", + numReplicas: 3, + allPods: []v1.Pod{createRunningPod("pod-0"), createRunningPod("pod-1")}, + expectedResult: podsAccountingResult{ + accountedPods: 2, + unaccountedPods: []string{}, + numReplicas: 3, + isEnough: false, + reason: "only 2/3 pods are healthy or scheduled for recreation, 1 pod(s) are completely missing", + }, + }, + { + name: "Minimal cluster: numReplicas=1", + numReplicas: 1, + allPods: []v1.Pod{createRunningPod("master")}, + expectedResult: podsAccountingResult{ + accountedPods: 1, + unaccountedPods: []string{}, + numReplicas: 1, + isEnough: true, + reason: "", + }, + }, + { + name: "Edge case: numReplicas=0", + numReplicas: 0, + allPods: []v1.Pod{}, + expectedResult: podsAccountingResult{ + accountedPods: 0, + unaccountedPods: []string{}, + numReplicas: 0, + isEnough: true, + reason: "", + }, + }, + { + name: "Pod with empty status counts as accounted", + numReplicas: 1, + allPods: []v1.Pod{createUnreportedPod("pod-0")}, + expectedResult: podsAccountingResult{ + accountedPods: 1, + unaccountedPods: []string{}, + numReplicas: 1, + isEnough: true, + reason: "", + }, + }, + { + name: "Multiple unaccounted pods", + numReplicas: 3, + allPods: []v1.Pod{createRunningPod("pod-0"), createFailingPod("pod-1", "CrashLoop"), createPendingPod("pod-2")}, + expectedResult: podsAccountingResult{ + accountedPods: 1, + unaccountedPods: []string{"pod-1", "pod-2"}, + numReplicas: 3, + isEnough: false, + reason: "only 1/3 pods are healthy or scheduled for recreation, unaccounted/not running pods: pod-1, pod-2", + }, + }, + { + name: "Scale-down in progress: more pods than numReplicas", + numReplicas: 2, + allPods: []v1.Pod{createRunningPod("pod-0"), createRunningPod("pod-1"), createRunningPod("pod-2")}, + expectedResult: podsAccountingResult{ + accountedPods: 3, + unaccountedPods: []string{}, + numReplicas: 2, + isEnough: true, + reason: "", + }, + }, + { + name: "Terminated pod is not running", + numReplicas: 2, + allPods: []v1.Pod{createRunningPod("pod-0"), createTerminatedPod("pod-1", 137)}, + expectedResult: podsAccountingResult{ + accountedPods: 1, + unaccountedPods: []string{"pod-1"}, + numReplicas: 2, + isEnough: false, + reason: "only 1/2 pods are healthy or scheduled for recreation, unaccounted/not running pods: pod-1", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := checkClusterStableForRollout(tt.allPods, tt.podsToRecreate, tt.numReplicas) + assert.Equal(t, tt.expectedResult, result) + }) + } +}