diff --git a/internal/backupcontroller/redis_artifact_size_test.go b/internal/backupcontroller/redis_artifact_size_test.go index ade7cbc9b..47d357c4e 100644 --- a/internal/backupcontroller/redis_artifact_size_test.go +++ b/internal/backupcontroller/redis_artifact_size_test.go @@ -17,6 +17,7 @@ import ( "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" clientfake "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" backupsv1alpha1 "github.com/cozystack/cozystack/api/backups/v1alpha1" ) @@ -367,16 +368,39 @@ func TestCreateRedisBackupArtifact_Artifact(t *testing.T) { } }) - t.Run("an existing Backup is returned without probing", func(t *testing.T) { + t.Run("an existing Backup is returned without probing or creating", func(t *testing.T) { existing := &backupsv1alpha1.Backup{ObjectMeta: metav1.ObjectMeta{Namespace: "tenant", Name: "bj-1"}} l := &fakeS3Lister{statSize: 777} r := redisReconciler(t, l, j, existing, projectedCredsSecret(nil)) + creates := 0 + r.Client = interceptor.NewClient(r.Client.(client.WithWatch), interceptor.Funcs{ + Create: func(ctx context.Context, cl client.WithWatch, obj client.Object, opts ...client.CreateOption) error { + creates++ + return cl.Create(ctx, obj, opts...) + }, + }) if _, err := r.createRedisBackupArtifact(context.Background(), j, &ResolvedBackupConfig{}, redisTestKey, nil); err != nil { t.Fatalf("createRedisBackupArtifact: %v", err) } if l.calls != 0 { t.Errorf("S3 probed %d times for a Backup that already exists", l.calls) } + if creates != 0 { + t.Errorf("Create called %d times for a Backup that already exists", creates) + } + }) + + t.Run("an empty snapshot is recorded and warned about", func(t *testing.T) { + r := redisReconciler(t, &fakeS3Lister{statSize: 0}, j, projectedCredsSecret(nil)) + rec := record.NewFakeRecorder(4) + r.Recorder = rec + got, err := r.createRedisBackupArtifact(context.Background(), j, &ResolvedBackupConfig{}, redisTestKey, nil) + if err != nil || got == nil || got.Status.Artifact == nil || got.Status.Artifact.URI != "s3://cozy-backups/"+redisTestKey { + t.Fatalf("createRedisBackupArtifact = %#v, %v; want the artifact recorded", got, err) + } + if ev := nextEvent(rec); !strings.HasPrefix(ev, "Warning EmptySnapshot ") { + t.Errorf("event = %q, want Warning EmptySnapshot", ev) + } }) } diff --git a/internal/backupcontroller/redisstrategy_controller.go b/internal/backupcontroller/redisstrategy_controller.go index 036d1218a..4169e6cef 100644 --- a/internal/backupcontroller/redisstrategy_controller.go +++ b/internal/backupcontroller/redisstrategy_controller.go @@ -526,8 +526,15 @@ func (r *BackupJobReconciler) createRedisBackupArtifact( } // Only once the Backup exists: a terminal Create error fails the // BackupJob, and a collision returns a Backup another pass measured. - if unmeasured != "" && r.Recorder != nil { - r.Recorder.Eventf(j, corev1.EventTypeNormal, "ArtifactNotMeasured", "The backup succeeded, but its location and size were not recorded: %s", unmeasured) + if r.Recorder != nil { + switch { + case unmeasured != "": + r.Recorder.Eventf(j, corev1.EventTypeNormal, "ArtifactNotMeasured", "The backup succeeded, but its location and size were not recorded: %s", unmeasured) + case backup.Status.Artifact != nil && backup.Status.Artifact.SizeBytes == 0: + // sizeBytes is omitempty, so a 0 reads as "not measured" on the + // Backup; a valid RDB snapshot always carries a header. + r.Recorder.Event(j, corev1.EventTypeWarning, "EmptySnapshot", "The backup's RDB snapshot is empty (0 bytes); a valid snapshot is never empty, so this backup is likely broken.") + } } return backup, nil }