test(operator): cover backup retention and health evaluation

Verolop · Verolop · commit d83b81b08bd6 · 2026-04-03T04:48:01.000-04:00
Add focused tests for retention merge and defaulting, extended backup health evaluation, and repository health condition handling.
diff --git a/api/v1alpha1/common_types_test.go b/api/v1alpha1/common_types_test.go
@@ -0,0 +1,144 @@
+package v1alpha1
+
+import (
+	"testing"
+
+	"k8s.io/utils/ptr"
+)
+
+func TestMergeBackupConfig_Retention(t *testing.T) {
+	t.Parallel()
+
+	t.Run("child overrides parent", func(t *testing.T) {
+		t.Parallel()
+		parent := &BackupConfig{
+			Type:      BackupTypeS3,
+			Retention: &RetentionPolicy{FullCount: ptr.To(int32(4)), DifferentialCount: ptr.To(int32(1))},
+		}
+		child := &BackupConfig{
+			Type:      BackupTypeS3,
+			Retention: &RetentionPolicy{FullCount: ptr.To(int32(7))},
+		}
+		merged := MergeBackupConfig(child, parent)
+		if merged.Retention == nil {
+			t.Fatal("expected Retention to be set")
+		}
+		if *merged.Retention.FullCount != 7 {
+			t.Errorf("FullCount = %d, want 7", *merged.Retention.FullCount)
+		}
+		if *merged.Retention.DifferentialCount != 1 {
+			t.Errorf("DifferentialCount = %d, want 1 (from parent)", *merged.Retention.DifferentialCount)
+		}
+	})
+
+	t.Run("parent preserved when child unset", func(t *testing.T) {
+		t.Parallel()
+		parent := &BackupConfig{
+			Type:      BackupTypeS3,
+			S3:        &S3BackupConfig{Bucket: "b", Region: "r"},
+			Retention: &RetentionPolicy{FullCount: ptr.To(int32(5)), DifferentialCount: ptr.To(int32(3))},
+		}
+		child := &BackupConfig{
+			Type: BackupTypeS3,
+			S3:   &S3BackupConfig{Bucket: "b", Region: "r"},
+		}
+		merged := MergeBackupConfig(child, parent)
+		if merged.Retention == nil {
+			t.Fatal("expected parent Retention to be preserved")
+		}
+		if *merged.Retention.FullCount != 5 {
+			t.Errorf("FullCount = %d, want 5", *merged.Retention.FullCount)
+		}
+		if *merged.Retention.DifferentialCount != 3 {
+			t.Errorf("DifferentialCount = %d, want 3", *merged.Retention.DifferentialCount)
+		}
+	})
+
+	t.Run("nil child retention preserves parent", func(t *testing.T) {
+		t.Parallel()
+		parent := &BackupConfig{
+			Type:      BackupTypeS3,
+			Retention: &RetentionPolicy{FullCount: ptr.To(int32(4)), DifferentialCount: ptr.To(int32(2))},
+		}
+		child := &BackupConfig{
+			Type: BackupTypeS3,
+			S3:   &S3BackupConfig{Bucket: "child-bucket", Region: "us-west-2"},
+		}
+		merged := MergeBackupConfig(child, parent)
+		if merged.Retention == nil {
+			t.Fatal("expected Retention from parent to be preserved")
+		}
+		if *merged.Retention.FullCount != 4 {
+			t.Errorf("FullCount = %d, want 4", *merged.Retention.FullCount)
+		}
+	})
+}
+
+func TestMergeBackupConfig_S3PointerBools(t *testing.T) {
+	t.Parallel()
+
+	t.Run("parent preserved when child unset", func(t *testing.T) {
+		t.Parallel()
+		parent := &BackupConfig{
+			Type: BackupTypeS3,
+			S3: &S3BackupConfig{
+				Bucket:                     "parent-bucket",
+				Region:                     "us-east-1",
+				CleanupOnDelete:            ptr.To(true),
+				AllowStaleMetadataRecovery: ptr.To(true),
+			},
+		}
+		child := &BackupConfig{
+			Type: BackupTypeS3,
+			S3:   &S3BackupConfig{Bucket: "child-bucket", Region: "us-east-1"},
+		}
+		merged := MergeBackupConfig(child, parent)
+		if merged.S3.CleanupOnDelete == nil || !*merged.S3.CleanupOnDelete {
+			t.Error("expected parent CleanupOnDelete=true to be preserved")
+		}
+		if merged.S3.AllowStaleMetadataRecovery == nil || !*merged.S3.AllowStaleMetadataRecovery {
+			t.Error("expected parent AllowStaleMetadataRecovery=true to be preserved")
+		}
+		if merged.S3.Bucket != "child-bucket" {
+			t.Errorf("Bucket = %s, want child-bucket", merged.S3.Bucket)
+		}
+	})
+
+	t.Run("child overrides parent", func(t *testing.T) {
+		t.Parallel()
+		parent := &BackupConfig{
+			Type: BackupTypeS3,
+			S3: &S3BackupConfig{
+				Bucket:          "b",
+				Region:          "r",
+				CleanupOnDelete: ptr.To(true),
+			},
+		}
+		child := &BackupConfig{
+			Type: BackupTypeS3,
+			S3: &S3BackupConfig{
+				Bucket:          "b",
+				Region:          "r",
+				CleanupOnDelete: ptr.To(false),
+			},
+		}
+		merged := MergeBackupConfig(child, parent)
+		if merged.S3.CleanupOnDelete == nil || *merged.S3.CleanupOnDelete {
+			t.Error("expected child CleanupOnDelete=false to override parent true")
+		}
+	})
+}
+
+func TestBackupConfig_DeepCopyPreservesRetention(t *testing.T) {
+	t.Parallel()
+	original := &BackupConfig{
+		Type:      BackupTypeS3,
+		S3:        &S3BackupConfig{Bucket: "b", Region: "r"},
+		Retention: &RetentionPolicy{FullCount: ptr.To(int32(4))},
+	}
+	copied := original.DeepCopy()
+	*copied.Retention.FullCount = 99
+	if *original.Retention.FullCount != 4 {
+		t.Errorf("DeepCopy leaked pointer: original FullCount changed to %d", *original.Retention.FullCount)
+	}
+}
diff --git a/pkg/data-handler/backuphealth/extended_test.go b/pkg/data-handler/backuphealth/extended_test.go
@@ -0,0 +1,236 @@
+package backuphealth_test
+
+import (
+	"testing"
+	"time"
+
+	multipoolermanagerdatapb "github.com/multigres/multigres/go/pb/multipoolermanagerdata"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/ptr"
+
+	multigresv1alpha1 "github.com/multigres/multigres-operator/api/v1alpha1"
+	"github.com/multigres/multigres-operator/pkg/data-handler/backuphealth"
+)
+
+func newTestShard() *multigresv1alpha1.Shard {
+	return &multigresv1alpha1.Shard{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:       "test-shard",
+			Namespace:  "default",
+			Generation: 1,
+			Labels:     map[string]string{"multigres.com/cluster": "test-cluster"},
+		},
+	}
+}
+
+// recentBackupID generates a pgBackRest-style backup ID for a backup that
+// completed `age` ago. Uses UTC because ParseTime (which uses time.Parse
+// without a location) returns UTC timestamps.
+func recentBackupID(age time.Duration) string {
+	return time.Now().UTC().Add(-age).Format("20060102-150405")
+}
+
+func TestEvaluateBackupsExtended_BackupCounts(t *testing.T) {
+	t.Parallel()
+	shard := newTestShard()
+	backups := []*multipoolermanagerdatapb.BackupMetadata{
+		{BackupId: recentBackupID(1 * time.Hour), Status: multipoolermanagerdatapb.BackupMetadata_COMPLETE, Type: "full"},
+		{BackupId: recentBackupID(2 * time.Hour), Status: multipoolermanagerdatapb.BackupMetadata_COMPLETE, Type: "full"},
+		{BackupId: recentBackupID(30 * time.Minute), Status: multipoolermanagerdatapb.BackupMetadata_COMPLETE, Type: "diff"},
+		{BackupId: recentBackupID(3 * time.Hour), Status: multipoolermanagerdatapb.BackupMetadata_INCOMPLETE, Type: "full"},
+	}
+	retention := &multigresv1alpha1.RetentionPolicy{FullCount: ptr.To(int32(4))}
+
+	result := backuphealth.EvaluateBackupsExtended(shard, backups, retention, nil)
+
+	if result.FullBackupCount != 2 {
+		t.Errorf("expected 2 full backups, got %d", result.FullBackupCount)
+	}
+	if result.DiffBackupCount != 1 {
+		t.Errorf("expected 1 diff backup, got %d", result.DiffBackupCount)
+	}
+}
+
+func TestEvaluateBackupsExtended_OldestBackupAge(t *testing.T) {
+	t.Parallel()
+	shard := newTestShard()
+	backups := []*multipoolermanagerdatapb.BackupMetadata{
+		{BackupId: recentBackupID(1 * time.Hour), Status: multipoolermanagerdatapb.BackupMetadata_COMPLETE, Type: "full"},
+		{BackupId: recentBackupID(48 * time.Hour), Status: multipoolermanagerdatapb.BackupMetadata_COMPLETE, Type: "full"},
+	}
+
+	result := backuphealth.EvaluateBackupsExtended(shard, backups, nil, nil)
+
+	if result.OldestBackupAge < 47*time.Hour || result.OldestBackupAge > 50*time.Hour {
+		t.Errorf("expected oldest age ~48h, got %v", result.OldestBackupAge)
+	}
+}
+
+func TestEvaluateBackupsExtended_NoBackups_OldestAgeZero(t *testing.T) {
+	t.Parallel()
+	shard := newTestShard()
+
+	result := backuphealth.EvaluateBackupsExtended(shard, nil, nil, nil)
+
+	if result.OldestBackupAge != 0 {
+		t.Errorf("expected zero oldest age with no backups, got %v", result.OldestBackupAge)
+	}
+}
+
+func TestEvaluateBackupsExtended_IntegrityCheckPass(t *testing.T) {
+	t.Parallel()
+	shard := newTestShard()
+	backups := []*multipoolermanagerdatapb.BackupMetadata{
+		{BackupId: recentBackupID(1 * time.Hour), Status: multipoolermanagerdatapb.BackupMetadata_COMPLETE, Type: "full"},
+	}
+	check := &backuphealth.IntegrityCheckResult{Passed: true}
+
+	result := backuphealth.EvaluateBackupsExtended(shard, backups, nil, check)
+
+	if result.RepositoryHealthy == nil || !*result.RepositoryHealthy {
+		t.Error("expected RepositoryHealthy=true when integrity check passes")
+	}
+	if result.RepositoryReason != backuphealth.ReasonHealthy {
+		t.Errorf("expected reason %q, got %q", backuphealth.ReasonHealthy, result.RepositoryReason)
+	}
+}
+
+func TestEvaluateBackupsExtended_IntegrityCheckFail(t *testing.T) {
+	t.Parallel()
+	shard := newTestShard()
+	backups := []*multipoolermanagerdatapb.BackupMetadata{
+		{BackupId: recentBackupID(1 * time.Hour), Status: multipoolermanagerdatapb.BackupMetadata_COMPLETE, Type: "full"},
+	}
+	check := &backuphealth.IntegrityCheckResult{Passed: false, Error: "stanza mismatch"}
+
+	result := backuphealth.EvaluateBackupsExtended(shard, backups, nil, check)
+
+	if result.RepositoryHealthy == nil || *result.RepositoryHealthy {
+		t.Error("expected RepositoryHealthy=false when integrity check fails")
+	}
+	if result.RepositoryReason != backuphealth.ReasonIntegrityCheckFailed {
+		t.Errorf("expected reason %q, got %q", backuphealth.ReasonIntegrityCheckFailed, result.RepositoryReason)
+	}
+}
+
+func TestEvaluateBackupsExtended_NoIntegrityCheck_Nil(t *testing.T) {
+	t.Parallel()
+	shard := newTestShard()
+	backups := []*multipoolermanagerdatapb.BackupMetadata{
+		{BackupId: recentBackupID(1 * time.Hour), Status: multipoolermanagerdatapb.BackupMetadata_COMPLETE, Type: "full"},
+	}
+
+	result := backuphealth.EvaluateBackupsExtended(shard, backups, nil, nil)
+
+	if result.RepositoryHealthy != nil {
+		t.Errorf("expected RepositoryHealthy=nil when no integrity check, got %v", *result.RepositoryHealthy)
+	}
+}
+
+func TestEvaluateBackupsExtended_RetentionCountWarning(t *testing.T) {
+	t.Parallel()
+	shard := newTestShard()
+	backups := []*multipoolermanagerdatapb.BackupMetadata{
+		{BackupId: recentBackupID(1 * time.Hour), Status: multipoolermanagerdatapb.BackupMetadata_COMPLETE, Type: "full"},
+	}
+	retention := &multigresv1alpha1.RetentionPolicy{FullCount: ptr.To(int32(4))}
+
+	result := backuphealth.EvaluateBackupsExtended(shard, backups, retention, nil)
+
+	if !result.RetentionCountWarning {
+		t.Error("expected retention count warning when 1 full backup vs retention=4")
+	}
+}
+
+func TestEvaluateBackupsExtended_RetentionCountOK(t *testing.T) {
+	t.Parallel()
+	shard := newTestShard()
+	backups := []*multipoolermanagerdatapb.BackupMetadata{
+		{BackupId: recentBackupID(1 * time.Hour), Status: multipoolermanagerdatapb.BackupMetadata_COMPLETE, Type: "full"},
+		{BackupId: recentBackupID(25 * time.Hour), Status: multipoolermanagerdatapb.BackupMetadata_COMPLETE, Type: "full"},
+		{BackupId: recentBackupID(49 * time.Hour), Status: multipoolermanagerdatapb.BackupMetadata_COMPLETE, Type: "full"},
+	}
+	retention := &multigresv1alpha1.RetentionPolicy{FullCount: ptr.To(int32(3))}
+
+	result := backuphealth.EvaluateBackupsExtended(shard, backups, retention, nil)
+
+	if result.RetentionCountWarning {
+		t.Error("expected no retention count warning when count matches retention")
+	}
+}
+
+func TestApplyExtended_SetsConditionTrue(t *testing.T) {
+	t.Parallel()
+	shard := newTestShard()
+	healthy := true
+	result := &backuphealth.ExtendedResult{
+		RepositoryHealthy: &healthy,
+		RepositoryReason:  backuphealth.ReasonHealthy,
+		RepositoryMessage: "OK",
+	}
+
+	backuphealth.ApplyExtended(shard, result)
+
+	found := false
+	for _, c := range shard.Status.Conditions {
+		if c.Type == backuphealth.ConditionRepositoryHealthy {
+			found = true
+			if c.Status != metav1.ConditionTrue {
+				t.Errorf("expected True, got %s", c.Status)
+			}
+		}
+	}
+	if !found {
+		t.Error("BackupRepositoryHealthy condition not set")
+	}
+}
+
+func TestApplyExtended_SetsConditionFalse(t *testing.T) {
+	t.Parallel()
+	shard := newTestShard()
+	unhealthy := false
+	result := &backuphealth.ExtendedResult{
+		RepositoryHealthy: &unhealthy,
+		RepositoryReason:  backuphealth.ReasonIntegrityCheckFailed,
+		RepositoryMessage: "check failed",
+	}
+
+	backuphealth.ApplyExtended(shard, result)
+
+	for _, c := range shard.Status.Conditions {
+		if c.Type == backuphealth.ConditionRepositoryHealthy {
+			if c.Status != metav1.ConditionFalse {
+				t.Errorf("expected False, got %s", c.Status)
+			}
+			if c.Reason != backuphealth.ReasonIntegrityCheckFailed {
+				t.Errorf("expected reason %q, got %q", backuphealth.ReasonIntegrityCheckFailed, c.Reason)
+			}
+			return
+		}
+	}
+	t.Error("BackupRepositoryHealthy condition not set")
+}
+
+func TestApplyExtended_RemovesStaleCondition(t *testing.T) {
+	t.Parallel()
+	shard := newTestShard()
+	// Pre-set a condition
+	shard.Status.Conditions = []metav1.Condition{
+		{Type: backuphealth.ConditionRepositoryHealthy, Status: metav1.ConditionTrue, Reason: "Healthy"},
+		{Type: "Available", Status: metav1.ConditionTrue, Reason: "AllPodsReady"},
+	}
+
+	// Apply with nil RepositoryHealthy — should remove the stale condition
+	result := &backuphealth.ExtendedResult{RepositoryHealthy: nil}
+	backuphealth.ApplyExtended(shard, result)
+
+	for _, c := range shard.Status.Conditions {
+		if c.Type == backuphealth.ConditionRepositoryHealthy {
+			t.Error("expected BackupRepositoryHealthy condition to be removed, but it's still present")
+		}
+	}
+	// Other conditions should be preserved
+	if len(shard.Status.Conditions) != 1 {
+		t.Errorf("expected 1 remaining condition, got %d", len(shard.Status.Conditions))
+	}
+}
diff --git a/pkg/resolver/shard_test.go b/pkg/resolver/shard_test.go
