K8SPG-772: Fix panic if backup completedAt is nil

cmd/postgres-operator/main.go

@@ -194,11 +194,8 @@ func addControllersToManager(ctx context.Context, mgr manager.Manager) error {
 	if err := mgr.GetFieldIndexer().IndexField(
 		context.Background(),
 		&v2.PerconaPGBackup{},
-		"spec.pgCluster",
-		func(rawObj client.Object) []string {
-			backup := rawObj.(*v2.PerconaPGBackup)
-			return []string{backup.Spec.PGCluster}
-		},
+		v2.IndexFieldPGCluster,
+		v2.PGClusterIndexerFunc,
 	); err != nil {
 		return err
 	}

percona/testutils/client.go

@@ -0,0 +1,41 @@
+package testutils
+
+import (
+	"k8s.io/apimachinery/pkg/runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+
+	pgv2 "github.com/percona/percona-postgresql-operator/pkg/apis/pgv2.percona.com/v2"
+	crunchyv1beta1 "github.com/percona/percona-postgresql-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
+)
+
+func BuildFakeClient(initObjs ...client.Object) client.Client {
+	scheme := runtime.NewScheme()
+
+	perconaTypes := []runtime.Object{
+		new(pgv2.PerconaPGCluster),
+		new(pgv2.PerconaPGClusterList),
+		new(pgv2.PerconaPGBackup),
+		new(pgv2.PerconaPGBackupList),
+		new(pgv2.PerconaPGRestore),
+		new(pgv2.PerconaPGRestoreList),
+		new(pgv2.PerconaPGUpgrade),
+		new(pgv2.PerconaPGUpgradeList),
+	}
+
+	crunchyTypes := []runtime.Object{
+		new(crunchyv1beta1.PostgresCluster),
+		new(crunchyv1beta1.PostgresClusterList),
+		new(crunchyv1beta1.PGUpgrade),
+		new(crunchyv1beta1.PGUpgradeList),
+	}
+
+	scheme.AddKnownTypes(pgv2.GroupVersion, perconaTypes...)
+	scheme.AddKnownTypes(crunchyv1beta1.GroupVersion, crunchyTypes...)
+
+	return fake.NewClientBuilder().
+		WithScheme(scheme).
+		WithObjects(initObjs...).
+		WithIndex(new(pgv2.PerconaPGBackup), pgv2.IndexFieldPGCluster, pgv2.PGClusterIndexerFunc).
+		Build()
+}

percona/watcher/wal.go

@@ -131,9 +131,19 @@ func getLatestBackup(ctx context.Context, cli client.Client, cr *pgv2.PerconaPGC
 	runningBackupExists := false
 	for _, backup := range backupList.Items {
 		backup := backup
+
 		switch backup.Status.State {
 		case pgv2.BackupSucceeded:
-			if latest.Status.CompletedAt == nil || backup.Status.CompletedAt.After(latest.Status.CompletedAt.Time) {
+			var completedAt *metav1.Time
+
+			if backup.Status.CompletedAt != nil {
+				completedAt = backup.Status.CompletedAt
+			}
+			if completedAt == nil {
+				completedAt = &backup.CreationTimestamp
+			}
+
+			if latest.Status.CompletedAt == nil || completedAt.After(latest.Status.CompletedAt.Time) {
 				latest = &backup
 			}
 		case pgv2.BackupFailed:

percona/watcher/wal_test.go

@@ -0,0 +1,312 @@
+package watcher
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/percona/percona-postgresql-operator/percona/testutils"
+	pgv2 "github.com/percona/percona-postgresql-operator/pkg/apis/pgv2.percona.com/v2"
+)
+
+func mustParseTime(layout string, value string) time.Time {
+	time, err := time.Parse(layout, value)
+	if err != nil {
+		panic(err)
+	}
+	return time
+}
+
+func TestGetLatestBackup(t *testing.T) {
+	tests := []struct {
+		name             string
+		backups          []client.Object
+		latestBackupName string
+		expectedErr      error
+	}{
+		{
+			name:             "no backups",
+			backups:          []client.Object{},
+			latestBackupName: "",
+			expectedErr:      errNoBackups,
+		},
+		{
+			name: "single backup",
+			backups: []client.Object{
+				&pgv2.PerconaPGBackup{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "backup1",
+						Namespace: "test-ns",
+						CreationTimestamp: metav1.Time{
+							Time: mustParseTime(time.RFC3339, "2024-02-04T21:00:57Z"),
+						},
+					},
+					Spec: pgv2.PerconaPGBackupSpec{
+						PGCluster: "test-cluster",
+					},
+					Status: pgv2.PerconaPGBackupStatus{
+						State: pgv2.BackupSucceeded,
+						CompletedAt: &metav1.Time{
+							Time: mustParseTime(time.RFC3339, "2024-02-04T21:23:38Z"),
+						},
+					},
+				},
+			},
+			latestBackupName: "backup1",
+			expectedErr:      nil,
+		},
+		{
+			name: "multiple backups, same cluster",
+			backups: []client.Object{
+				&pgv2.PerconaPGBackup{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "backup1",
+						Namespace: "test-ns",
+						CreationTimestamp: metav1.Time{
+							Time: mustParseTime(time.RFC3339, "2024-02-04T21:00:57Z"),
+						},
+					},
+					Spec: pgv2.PerconaPGBackupSpec{
+						PGCluster: "test-cluster",
+					},
+					Status: pgv2.PerconaPGBackupStatus{
+						State: pgv2.BackupSucceeded,
+						CompletedAt: &metav1.Time{
+							Time: mustParseTime(time.RFC3339, "2024-02-04T21:23:38Z"),
+						},
+					},
+				},
+				&pgv2.PerconaPGBackup{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "backup2",
+						Namespace: "test-ns",
+						CreationTimestamp: metav1.Time{
+							Time: mustParseTime(time.RFC3339, "2024-02-04T22:00:57Z"),
+						},
+					},
+					Spec: pgv2.PerconaPGBackupSpec{
+						PGCluster: "test-cluster",
+					},
+					Status: pgv2.PerconaPGBackupStatus{
+						State: pgv2.BackupSucceeded,
+						CompletedAt: &metav1.Time{
+							Time: mustParseTime(time.RFC3339, "2024-02-04T22:24:12Z"),
+						},
+					},
+				},
+			},
+			latestBackupName: "backup2",
+			expectedErr:      nil,
+		},
+		{
+			name: "multiple backups, different clusters",
+			backups: []client.Object{
+				&pgv2.PerconaPGBackup{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "backup1",
+						Namespace: "test-ns",
+						CreationTimestamp: metav1.Time{
+							Time: mustParseTime(time.RFC3339, "2024-02-04T21:00:57Z"),
+						},
+					},
+					Spec: pgv2.PerconaPGBackupSpec{
+						PGCluster: "test-cluster",
+					},
+					Status: pgv2.PerconaPGBackupStatus{
+						State: pgv2.BackupSucceeded,
+						CompletedAt: &metav1.Time{
+							Time: mustParseTime(time.RFC3339, "2024-02-04T21:23:38Z"),
+						},
+					},
+				},
+				&pgv2.PerconaPGBackup{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "backup2",
+						Namespace: "test-ns",
+						CreationTimestamp: metav1.Time{
+							Time: mustParseTime(time.RFC3339, "2024-02-04T22:00:57Z"),
+						},
+					},
+					Spec: pgv2.PerconaPGBackupSpec{
+						PGCluster: "test-cluster",
+					},
+					Status: pgv2.PerconaPGBackupStatus{
+						State: pgv2.BackupSucceeded,
+						CompletedAt: &metav1.Time{
+							Time: mustParseTime(time.RFC3339, "2024-02-04T22:24:12Z"),
+						},
+					},
+				},
+				&pgv2.PerconaPGBackup{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "backup-from-different-cluster",
+						Namespace: "test-ns",
+						CreationTimestamp: metav1.Time{
+							Time: mustParseTime(time.RFC3339, "2024-02-04T22:10:57Z"),
+						},
+					},
+					Spec: pgv2.PerconaPGBackupSpec{
+						PGCluster: "different-cluster",
+					},
+					Status: pgv2.PerconaPGBackupStatus{
+						State: pgv2.BackupSucceeded,
+						CompletedAt: &metav1.Time{
+							Time: mustParseTime(time.RFC3339, "2024-02-04T22:14:12Z"),
+						},
+					},
+				},
+			},
+			latestBackupName: "backup2",
+			expectedErr:      nil,
+		},
+		{
+			name: "single running backup",
+			backups: []client.Object{
+				&pgv2.PerconaPGBackup{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "backup1",
+						Namespace: "test-ns",
+						CreationTimestamp: metav1.Time{
+							Time: mustParseTime(time.RFC3339, "2024-02-04T21:00:57Z"),
+						},
+					},
+					Spec: pgv2.PerconaPGBackupSpec{
+						PGCluster: "test-cluster",
+					},
+					Status: pgv2.PerconaPGBackupStatus{
+						State: pgv2.BackupRunning,
+					},
+				},
+			},
+			latestBackupName: "",
+			expectedErr:      errRunningBackup,
+		},
+		{
+			name: "running backup but a backup is already succeeded",
+			backups: []client.Object{
+				&pgv2.PerconaPGBackup{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "backup1",
+						Namespace: "test-ns",
+						CreationTimestamp: metav1.Time{
+							Time: mustParseTime(time.RFC3339, "2024-02-04T21:00:57Z"),
+						},
+					},
+					Spec: pgv2.PerconaPGBackupSpec{
+						PGCluster: "test-cluster",
+					},
+					Status: pgv2.PerconaPGBackupStatus{
+						State: pgv2.BackupSucceeded,
+						CompletedAt: &metav1.Time{
+							Time: mustParseTime(time.RFC3339, "2024-02-04T21:23:38Z"),
+						},
+					},
+				},
+				&pgv2.PerconaPGBackup{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "backup2",
+						Namespace: "test-ns",
+						CreationTimestamp: metav1.Time{
+							Time: mustParseTime(time.RFC3339, "2024-02-04T22:00:57Z"),
+						},
+					},
+					Spec: pgv2.PerconaPGBackupSpec{
+						PGCluster: "test-cluster",
+					},
+					Status: pgv2.PerconaPGBackupStatus{
+						State: pgv2.BackupRunning,
+					},
+				},
+			},
+			latestBackupName: "backup1",
+			expectedErr:      nil,
+		},
+		{
+			name: "K8SPG-772: multiple backups, some has no CompletedAt",
+			backups: []client.Object{
+				&pgv2.PerconaPGBackup{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "backup1",
+						Namespace: "test-ns",
+						CreationTimestamp: metav1.Time{
+							Time: mustParseTime(time.RFC3339, "2024-02-04T21:00:57Z"),
+						},
+					},
+					Spec: pgv2.PerconaPGBackupSpec{
+						PGCluster: "test-cluster",
+					},
+					Status: pgv2.PerconaPGBackupStatus{
+						State: pgv2.BackupSucceeded,
+						CompletedAt: &metav1.Time{
+							Time: mustParseTime(time.RFC3339, "2024-02-04T21:24:12Z"),
+						},
+					},
+				},
+				&pgv2.PerconaPGBackup{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "backup2",
+						Namespace: "test-ns",
+						CreationTimestamp: metav1.Time{
+							Time: mustParseTime(time.RFC3339, "2024-02-04T22:00:57Z"),
+						},
+					},
+					Spec: pgv2.PerconaPGBackupSpec{
+						PGCluster: "test-cluster",
+					},
+					Status: pgv2.PerconaPGBackupStatus{
+						State: pgv2.BackupSucceeded,
+					},
+				},
+				&pgv2.PerconaPGBackup{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "backup3",
+						Namespace: "test-ns",
+						CreationTimestamp: metav1.Time{
+							Time: mustParseTime(time.RFC3339, "2024-02-04T23:00:57Z"),
+						},
+					},
+					Spec: pgv2.PerconaPGBackupSpec{
+						PGCluster: "test-cluster",
+					},
+					Status: pgv2.PerconaPGBackupStatus{
+						State: pgv2.BackupSucceeded,
+						CompletedAt: &metav1.Time{
+							Time: mustParseTime(time.RFC3339, "2024-02-04T23:24:12Z"),
+						},
+					},
+				},
+			},
+			latestBackupName: "backup3",
+			expectedErr:      nil,
+		},
+	}
+
+	ctx := context.Background()
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			client := testutils.BuildFakeClient(tt.backups...)
+
+			cluster := &pgv2.PerconaPGCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-cluster",
+					Namespace: "test-ns",
+				},
+			}
+
+			latest, err := getLatestBackup(ctx, client, cluster)
+			if tt.expectedErr != nil {
+				require.EqualError(t, err, tt.expectedErr.Error())
+				assert.Nil(t, latest)
+			} else {
+				require.NoError(t, err)
+				assert.NotNil(t, latest)
+				assert.Equal(t, latest.Name, tt.latestBackupName)
+			}
+		})
+	}
+}