diff --git a/docs/reference/operator_parameters.md b/docs/reference/operator_parameters.md index 5b56d98af..f120dfce0 100644 --- a/docs/reference/operator_parameters.md +++ b/docs/reference/operator_parameters.md @@ -556,9 +556,10 @@ configuration they are grouped under the `kubernetes` key. * **master_pod_move_timeout** The period of time to wait for the success of migration of master pods from an unschedulable node. The migration includes Patroni switchovers to - respective replicas on healthy nodes. The situation where master pods still - exist on the old node after this timeout expires has to be fixed manually. - The default is 20 minutes. + respective replicas on healthy nodes. A failed migration attempt is retried + every minute until this timeout expires. The situation where master pods + still exist on the old node after this timeout expires has to be fixed + manually. The default is 20 minutes. * **enable_pod_antiaffinity** toggles [pod anti affinity](https://kubernetes.io/docs/concepts/configuration/assign-pod-node/) diff --git a/pkg/controller/node.go b/pkg/controller/node.go index d962190be..416e2f6c0 100644 --- a/pkg/controller/node.go +++ b/pkg/controller/node.go @@ -156,7 +156,8 @@ func (c *Controller) moveMasterPodsOffNode(node *v1.Node) { func() (bool, error) { err := c.attemptToMoveMasterPodsOffNode(node) if err != nil { - return false, err + c.logger.Warningf("attempt to move master pods off node %q failed, will retry: %v", node.Name, err) + return false, nil } return true, nil }, diff --git a/pkg/controller/node_test.go b/pkg/controller/node_test.go index b9326d9ef..9afaee040 100644 --- a/pkg/controller/node_test.go +++ b/pkg/controller/node_test.go @@ -1,11 +1,18 @@ package controller import ( + "fmt" + "strings" "testing" + "time" + logrustest "github.com/sirupsen/logrus/hooks/test" "github.com/zalando/postgres-operator/v2/pkg/spec" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/fake" + k8stesting "k8s.io/client-go/testing" ) const ( @@ -93,3 +100,32 @@ func TestNodeIsReady(t *testing.T) { } } } + +// TestMoveMasterPodsOffNodeRetriesOnError ensures a failed attempt to move +// master pods off a node is retried rather than aborting the whole retry +// loop on the first error. +func TestMoveMasterPodsOffNodeRetriesOnError(t *testing.T) { + clientSet := fake.NewSimpleClientset() + clientSet.PrependReactor("list", "pods", func(action k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, fmt.Errorf("could not list pods") + }) + + controller := newNodeTestController() + controller.KubeClient.PodsGetter = clientSet.CoreV1() + // timeout == the retry interval hardcoded in moveMasterPodsOffNode, so + // the single retry attempt resolves synchronously without a real sleep. + controller.opConfig.MasterPodMoveTimeout = &metav1.Duration{Duration: 1 * time.Minute} + + logger, hook := logrustest.NewNullLogger() + controller.logger = logger.WithField("pkg", "controller") + + controller.moveMasterPodsOffNode(makeNode(map[string]string{}, false)) + + lastEntry := hook.LastEntry() + if lastEntry == nil { + t.Fatal("expected moveMasterPodsOffNode to log a warning") + } + if !strings.Contains(lastEntry.Message, "still failing after") { + t.Errorf("expected the retry loop to run out of attempts instead of aborting on the first error, got log message: %q", lastEntry.Message) + } +}