From 2c6f45508c602222eb003c49dd2f537c5a12f388 Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Tue, 25 Aug 2026 18:08:17 -0400 Subject: [PATCH] Report a measured zero incremental instead of erasing it (#10309) * Report a measured zero incremental instead of erasing it A CBT incremental with an exactly zero delta -- nothing changed since the parent -- was reported identically to a backup that moved the whole device. `velero backup describe --details` printed only "Moved data Size (bytes): 3221225472" with no incremental line, and status.incrementalBytes was absent, for a run that transferred nothing. The best possible CBT outcome displayed as the worst, and was indistinguishable from a genuine full, a whole-device fallback, or a backup predating incremental accounting. The zero was being erased twice. Besides the API status fields, datapath.BackupResult also carried omitempty, and that struct crosses a JSON boundary from the data mover pod to the controller (see micro_service_watcher.go), so the value was destroyed before the controller could persist it. Every uploader always reports a figure there, so 0 internally always means "transferred nothing" -- dropping omitempty is sufficient and correct for that hop. The API fields move to *int64 rather than just dropping omitempty. The field shipped in v1.18.0-v1.18.2, so backups exist whose stored volume info has no incrementalSize at all; with a plain int64 those unmarshal to 0 and would render "Incremental data Size (bytes): 0", a false claim of a perfect incremental on a run that never measured one. nil means not measured, a pointer to 0 means measured zero. Both fields already carry +optional, so the generated CRD schema is unchanged and no regeneration is required. Display gates relax from > 0 to != nil in all three places, including volumesByPod.Add, whose signature takes *int64 now; the restore describer passes nil, which is correct since restores measure no incremental. Verified live: the same zero-delta scenario that reported now reports 0 and renders "Incremental data Size (bytes): 0", while an older backup described with the new client still correctly prints no incremental line at all. Co-Authored-By: Claude Fable 5 Signed-off-by: Tiger Kaovilai (cherry picked from commit 6c7aa9d588f6d5eab134d4ce19c92b838f45557c) Signed-off-by: Tiger Kaovilai * gofmt: fix import ordering in backup_test.go Signed-off-by: Tiger Kaovilai * Regenerate CRDs for IncrementalBytes pointer type make update-crd was missed in the original commit. Regenerated with the pinned controller-gen v0.16.5 to avoid unrelated version-annotation churn across other CRDs. Signed-off-by: Tiger Kaovilai * Add changelog for #10309 Signed-off-by: Tiger Kaovilai * Address review: make IncrementalBytes a pointer to preserve backward compat Per Lyndon-Li's review on #10309: dropping omitempty on the plain int64 field breaks compatibility with a data mover from release-1.17 or earlier that predates IncrementalBytes and never writes the key -- the new controller would unmarshal a zero value ("nothing transferred") instead of recognizing the field is simply absent ("not measured"). Switch to *int64 with omitempty restored: - an old mover's omitted key unmarshals to nil ("not measured") - a current mover's genuine zero still serializes the key, unmarshaling to a non-nil pointer to 0 ("measured zero") - nonzero values work exactly as before - an old controller can still unmarshal a numeric value from a new mover pkg/controller/data_upload_controller.go and pod_volume_backup_controller.go assign the wire-struct field directly to their already-*int64,omitempty CRD status field instead of re-wrapping it with ptr.To, since both are now the same pointer type. Signed-off-by: Tiger Kaovilai * Fix CI: update marshal-fail test assertions for IncrementalBytes pointer Both backup_micro_service_test.go files hardcoded the %v-formatted zero-value BackupResult struct in an error-message assertion. Now that IncrementalBytes is *int64, its zero value prints as instead of 0. Signed-off-by: Tiger Kaovilai --------- Signed-off-by: Tiger Kaovilai Co-authored-by: Claude Fable 5 --- changelogs/unreleased/10309-kaovilai | 1 + .../v1/bases/velero.io_podvolumebackups.yaml | 8 +++- .../v2alpha1/bases/velero.io_datauploads.yaml | 8 +++- internal/volume/volumes_information.go | 12 ++++-- pkg/apis/velero/v1/pod_volume_backup_types.go | 8 +++- pkg/apis/velero/v1/zz_generated.deepcopy.go | 5 +++ pkg/apis/velero/v2alpha1/data_upload_types.go | 8 +++- .../velero/v2alpha1/zz_generated.deepcopy.go | 5 +++ pkg/backup/backup_test.go | 5 ++- pkg/builder/data_upload_builder.go | 2 +- pkg/cmd/util/output/backup_describer.go | 18 ++++++--- pkg/cmd/util/output/backup_describer_test.go | 3 +- .../output/backup_structured_describer.go | 8 +++- pkg/cmd/util/output/restore_describer.go | 2 +- pkg/datamover/backup_micro_service_test.go | 2 +- pkg/datapath/data_path.go | 3 +- pkg/datapath/data_path_test.go | 16 +++++--- pkg/datapath/micro_service_watcher_test.go | 37 +++++++++++++++++++ pkg/datapath/types.go | 14 ++++--- pkg/podvolume/backup_micro_service_test.go | 2 +- 20 files changed, 130 insertions(+), 37 deletions(-) create mode 100644 changelogs/unreleased/10309-kaovilai diff --git a/changelogs/unreleased/10309-kaovilai b/changelogs/unreleased/10309-kaovilai new file mode 100644 index 000000000..b0683330e --- /dev/null +++ b/changelogs/unreleased/10309-kaovilai @@ -0,0 +1 @@ +Report a measured zero-byte incremental instead of erasing it from status diff --git a/config/crd/v1/bases/velero.io_podvolumebackups.yaml b/config/crd/v1/bases/velero.io_podvolumebackups.yaml index 90e9f4e4a..935916db9 100644 --- a/config/crd/v1/bases/velero.io_podvolumebackups.yaml +++ b/config/crd/v1/bases/velero.io_podvolumebackups.yaml @@ -205,8 +205,12 @@ spec: nullable: true type: string incrementalBytes: - description: IncrementalBytes holds the number of bytes new or changed - since the last backup + description: |- + IncrementalBytes holds the number of bytes new or changed since the last backup. + A nil value means the uploader did not report a figure; a pointer to 0 means it + reported zero, i.e. nothing changed and nothing was transferred. The two are + distinct: erasing a measured zero makes a perfect incremental indistinguishable + from a full transfer in every downstream report. format: int64 type: integer message: diff --git a/config/crd/v2alpha1/bases/velero.io_datauploads.yaml b/config/crd/v2alpha1/bases/velero.io_datauploads.yaml index 5e1fd4124..8d03da279 100644 --- a/config/crd/v2alpha1/bases/velero.io_datauploads.yaml +++ b/config/crd/v2alpha1/bases/velero.io_datauploads.yaml @@ -192,8 +192,12 @@ spec: nullable: true type: object incrementalBytes: - description: IncrementalBytes holds the number of bytes new or changed - since the last backup + description: |- + IncrementalBytes holds the number of bytes new or changed since the last backup. + A nil value means the uploader did not report a figure; a pointer to 0 means it + reported zero, i.e. nothing changed and nothing was transferred. The two are + distinct: erasing a measured zero makes a perfect incremental indistinguishable + from a full transfer in every downstream report. format: int64 type: integer message: diff --git a/internal/volume/volumes_information.go b/internal/volume/volumes_information.go index 69214ef45..cec2922d9 100644 --- a/internal/volume/volumes_information.go +++ b/internal/volume/volumes_information.go @@ -175,8 +175,11 @@ type SnapshotDataMovementInfo struct { // Moved snapshot data size. Size int64 `json:"size"` - // Moved snapshot incremental size. - IncrementalSize int64 `json:"incrementalSize,omitempty"` + // Moved snapshot incremental size, i.e. the bytes actually transferred. Nil means + // the uploader reported no figure (including backups taken before this was + // recorded); a pointer to 0 means it transferred nothing, which is the ideal + // incremental and must stay distinguishable from "unknown". + IncrementalSize *int64 `json:"incrementalSize,omitempty"` // The DataUpload's Status.Phase value Phase velerov2alpha1.DataUploadPhase @@ -225,8 +228,9 @@ type PodVolumeInfo struct { // The snapshot corresponding volume size. Size int64 `json:"size,omitempty"` - // The incremental snapshot size. - IncrementalSize int64 `json:"incrementalSize,omitempty"` + // The incremental snapshot size, i.e. the bytes actually transferred. Nil means + // the uploader reported no figure; a pointer to 0 means it transferred nothing. + IncrementalSize *int64 `json:"incrementalSize,omitempty"` // The type of the uploader that uploads the data. The valid values are `kopia` and `restic`. UploaderType string `json:"uploaderType"` diff --git a/pkg/apis/velero/v1/pod_volume_backup_types.go b/pkg/apis/velero/v1/pod_volume_backup_types.go index 566ba3b29..c4b7f879c 100644 --- a/pkg/apis/velero/v1/pod_volume_backup_types.go +++ b/pkg/apis/velero/v1/pod_volume_backup_types.go @@ -124,9 +124,13 @@ type PodVolumeBackupStatus struct { // +optional Progress shared.DataMoveOperationProgress `json:"progress,omitempty"` - // IncrementalBytes holds the number of bytes new or changed since the last backup + // IncrementalBytes holds the number of bytes new or changed since the last backup. + // A nil value means the uploader did not report a figure; a pointer to 0 means it + // reported zero, i.e. nothing changed and nothing was transferred. The two are + // distinct: erasing a measured zero makes a perfect incremental indistinguishable + // from a full transfer in every downstream report. // +optional - IncrementalBytes int64 `json:"incrementalBytes,omitempty"` + IncrementalBytes *int64 `json:"incrementalBytes,omitempty"` // AcceptedTimestamp records the time the pod volume backup is to be prepared. // The server's time is used for AcceptedTimestamp diff --git a/pkg/apis/velero/v1/zz_generated.deepcopy.go b/pkg/apis/velero/v1/zz_generated.deepcopy.go index ffbbf0cf8..78f756640 100644 --- a/pkg/apis/velero/v1/zz_generated.deepcopy.go +++ b/pkg/apis/velero/v1/zz_generated.deepcopy.go @@ -1055,6 +1055,11 @@ func (in *PodVolumeBackupStatus) DeepCopyInto(out *PodVolumeBackupStatus) { *out = (*in).DeepCopy() } out.Progress = in.Progress + if in.IncrementalBytes != nil { + in, out := &in.IncrementalBytes, &out.IncrementalBytes + *out = new(int64) + **out = **in + } if in.AcceptedTimestamp != nil { in, out := &in.AcceptedTimestamp, &out.AcceptedTimestamp *out = (*in).DeepCopy() diff --git a/pkg/apis/velero/v2alpha1/data_upload_types.go b/pkg/apis/velero/v2alpha1/data_upload_types.go index 37e273b2b..6f28d399b 100644 --- a/pkg/apis/velero/v2alpha1/data_upload_types.go +++ b/pkg/apis/velero/v2alpha1/data_upload_types.go @@ -165,9 +165,13 @@ type DataUploadStatus struct { // +optional Progress shared.DataMoveOperationProgress `json:"progress,omitempty"` - // IncrementalBytes holds the number of bytes new or changed since the last backup + // IncrementalBytes holds the number of bytes new or changed since the last backup. + // A nil value means the uploader did not report a figure; a pointer to 0 means it + // reported zero, i.e. nothing changed and nothing was transferred. The two are + // distinct: erasing a measured zero makes a perfect incremental indistinguishable + // from a full transfer in every downstream report. // +optional - IncrementalBytes int64 `json:"incrementalBytes,omitempty"` + IncrementalBytes *int64 `json:"incrementalBytes,omitempty"` // Node is name of the node where the DataUpload is processed. // +optional diff --git a/pkg/apis/velero/v2alpha1/zz_generated.deepcopy.go b/pkg/apis/velero/v2alpha1/zz_generated.deepcopy.go index b86c573d3..0513824bd 100644 --- a/pkg/apis/velero/v2alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/velero/v2alpha1/zz_generated.deepcopy.go @@ -270,6 +270,11 @@ func (in *DataUploadStatus) DeepCopyInto(out *DataUploadStatus) { *out = (*in).DeepCopy() } out.Progress = in.Progress + if in.IncrementalBytes != nil { + in, out := &in.IncrementalBytes, &out.IncrementalBytes + *out = new(int64) + **out = **in + } if in.AcceptedTimestamp != nil { in, out := &in.AcceptedTimestamp, &out.AcceptedTimestamp *out = (*in).DeepCopy() diff --git a/pkg/backup/backup_test.go b/pkg/backup/backup_test.go index 9574aa288..5d1ed1da2 100644 --- a/pkg/backup/backup_test.go +++ b/pkg/backup/backup_test.go @@ -43,6 +43,7 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/utils/ptr" "github.com/vmware-tanzu/velero/internal/resourcepolicies" "github.com/vmware-tanzu/velero/internal/volume" @@ -5681,7 +5682,7 @@ func TestUpdateVolumeInfos(t *testing.T) { RetainedSnapshot: "vs-1", SnapshotHandle: "snapshot-id", Size: 1000, - IncrementalSize: 500, + IncrementalSize: ptr.To(int64(500)), Phase: velerov2alpha1.DataUploadPhaseFailed, }, }, @@ -5721,7 +5722,7 @@ func TestUpdateVolumeInfos(t *testing.T) { RetainedSnapshot: "vs-1", SnapshotHandle: "snapshot-id", Size: 1000, - IncrementalSize: 500, + IncrementalSize: ptr.To(int64(500)), Phase: velerov2alpha1.DataUploadPhaseCompleted, }, }, diff --git a/pkg/builder/data_upload_builder.go b/pkg/builder/data_upload_builder.go index c8fa34956..9805e71a3 100644 --- a/pkg/builder/data_upload_builder.go +++ b/pkg/builder/data_upload_builder.go @@ -147,7 +147,7 @@ func (d *DataUploadBuilder) Progress(progress shared.DataMoveOperationProgress) // IncrementalBytes sets the DataUpload's IncrementalBytes. func (d *DataUploadBuilder) IncrementalBytes(incrementalBytes int64) *DataUploadBuilder { - d.object.Status.IncrementalBytes = incrementalBytes + d.object.Status.IncrementalBytes = &incrementalBytes return d } diff --git a/pkg/cmd/util/output/backup_describer.go b/pkg/cmd/util/output/backup_describer.go index 445ce3df5..a8d43b89f 100644 --- a/pkg/cmd/util/output/backup_describer.go +++ b/pkg/cmd/util/output/backup_describer.go @@ -739,8 +739,12 @@ func describeDataMovement(d *Describer, details bool, info *volume.BackupVolumeI d.Printf("\t\t\t\tData Mover: %s\n", dataMover) d.Printf("\t\t\t\tUploader Type: %s\n", info.SnapshotDataMovementInfo.UploaderType) d.Printf("\t\t\t\tMoved data Size (bytes): %d\n", info.SnapshotDataMovementInfo.Size) - if info.SnapshotDataMovementInfo.IncrementalSize > 0 { - d.Printf("\t\t\t\tIncremental data Size (bytes): %d\n", info.SnapshotDataMovementInfo.IncrementalSize) + // Print whenever the uploader measured a figure, including zero. A zero-delta + // incremental transfers nothing, which is the whole point of CBT; hiding it + // leaves only the volume size on display and makes the best possible result + // indistinguishable from a full transfer. + if info.SnapshotDataMovementInfo.IncrementalSize != nil { + d.Printf("\t\t\t\tIncremental data Size (bytes): %d\n", *info.SnapshotDataMovementInfo.IncrementalSize) } d.Printf("\t\t\t\tResult: %s\n", info.Result) } else { @@ -915,7 +919,7 @@ type volumesByPod struct { // Add adds a pod volume with the specified pod namespace, name // and volume to the appropriate group. // Used for both backup and restore -func (v *volumesByPod) Add(namespace, name, volume, phase string, progress veleroapishared.DataMoveOperationProgress, incrementalBytes int64) { +func (v *volumesByPod) Add(namespace, name, volume, phase string, progress veleroapishared.DataMoveOperationProgress, incrementalBytes *int64) { if v.volumesByPodMap == nil { v.volumesByPodMap = make(map[string]*podVolumeGroup) } @@ -925,8 +929,12 @@ func (v *volumesByPod) Add(namespace, name, volume, phase string, progress veler // append backup progress percentage if backup is in progress if phase == "In Progress" && progress.TotalBytes != 0 { volume = fmt.Sprintf("%s (%.2f%%)", volume, float64(progress.BytesDone)/float64(progress.TotalBytes)*100) - } else if phase == string(velerov1api.PodVolumeBackupPhaseCompleted) && incrementalBytes > 0 { - volume = fmt.Sprintf("%s (size: %v, incremental size: %v)", volume, progress.TotalBytes, incrementalBytes) + } else if phase == string(velerov1api.PodVolumeBackupPhaseCompleted) && incrementalBytes != nil { + // Report the incremental figure whenever it was measured, including zero. Zero is + // the best possible outcome - nothing changed, so nothing was transferred - and + // suppressing it leaves only the volume size on display, which reads as a full + // transfer. + volume = fmt.Sprintf("%s (size: %v, incremental size: %v)", volume, progress.TotalBytes, *incrementalBytes) } else if (phase == string(velerov1api.PodVolumeBackupPhaseCompleted) || phase == string(velerov1api.PodVolumeRestorePhaseCompleted)) && progress.TotalBytes > 0 { diff --git a/pkg/cmd/util/output/backup_describer_test.go b/pkg/cmd/util/output/backup_describer_test.go index da28f6c87..c64ae04cb 100644 --- a/pkg/cmd/util/output/backup_describer_test.go +++ b/pkg/cmd/util/output/backup_describer_test.go @@ -25,6 +25,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1api "k8s.io/api/core/v1" + "k8s.io/utils/ptr" "github.com/vmware-tanzu/velero/internal/volume" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" @@ -629,7 +630,7 @@ func TestCSISnapshots(t *testing.T) { SnapshotHandle: "fake-repo-id-5", OperationID: "fake-operation-5", Size: 100, - IncrementalSize: 50, + IncrementalSize: ptr.To(int64(50)), Phase: velerov2alpha1.DataUploadPhaseFailed, }, }, diff --git a/pkg/cmd/util/output/backup_structured_describer.go b/pkg/cmd/util/output/backup_structured_describer.go index b2541df4b..1c0aefa34 100644 --- a/pkg/cmd/util/output/backup_structured_describer.go +++ b/pkg/cmd/util/output/backup_structured_describer.go @@ -467,9 +467,13 @@ func describeDataMovementInSF(details bool, info *volume.BackupVolumeInfo, snaps dataMovement["uploaderType"] = info.SnapshotDataMovementInfo.UploaderType dataMovement["result"] = string(info.Result) - if info.SnapshotDataMovementInfo.Size > 0 || info.SnapshotDataMovementInfo.IncrementalSize > 0 { + if info.SnapshotDataMovementInfo.Size > 0 { dataMovement["size"] = info.SnapshotDataMovementInfo.Size - dataMovement["incrementalSize"] = info.SnapshotDataMovementInfo.IncrementalSize + } + // Emit whenever measured, including zero - a zero-delta incremental transferred + // nothing, and that has to be reportable rather than absent. + if info.SnapshotDataMovementInfo.IncrementalSize != nil { + dataMovement["incrementalSize"] = *info.SnapshotDataMovementInfo.IncrementalSize } snapshotDetail["dataMovement"] = dataMovement diff --git a/pkg/cmd/util/output/restore_describer.go b/pkg/cmd/util/output/restore_describer.go index e94b2dedd..11e8ff4e4 100644 --- a/pkg/cmd/util/output/restore_describer.go +++ b/pkg/cmd/util/output/restore_describer.go @@ -417,7 +417,7 @@ func describePodVolumeRestores(d *Describer, restores []velerov1api.PodVolumeRes restoresByPod := new(volumesByPod) for _, restore := range restoresByPhase[phase] { - restoresByPod.Add(restore.Spec.Pod.Namespace, restore.Spec.Pod.Name, restore.Spec.Volume, phase, restore.Status.Progress, 0) + restoresByPod.Add(restore.Spec.Pod.Namespace, restore.Spec.Pod.Name, restore.Spec.Volume, phase, restore.Status.Progress, nil) } d.Printf("\t%s:\n", phase) diff --git a/pkg/datamover/backup_micro_service_test.go b/pkg/datamover/backup_micro_service_test.go index c9accdd77..69a4a1381 100644 --- a/pkg/datamover/backup_micro_service_test.go +++ b/pkg/datamover/backup_micro_service_test.go @@ -152,7 +152,7 @@ func TestOnDataUploadCompleted(t *testing.T) { { name: "marshal fail", marshalErr: errors.New("fake-marshal-error"), - expectedErr: "Failed to marshal backup result { false { } 0 0}: fake-marshal-error", + expectedErr: "Failed to marshal backup result { false { } 0 }: fake-marshal-error", }, { name: "succeed", diff --git a/pkg/datapath/data_path.go b/pkg/datapath/data_path.go index 2ec750805..647095672 100644 --- a/pkg/datapath/data_path.go +++ b/pkg/datapath/data_path.go @@ -22,6 +22,7 @@ import ( "github.com/cockroachdb/errors" "github.com/sirupsen/logrus" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/vmware-tanzu/velero/internal/credentials" @@ -220,7 +221,7 @@ func (dp *generalDataPath) StartBackup(source AccessPoint, uploaderConfig map[st } dp.callbacks.OnFailed(context.Background(), dp.namespace, dp.jobName, dataPathErr) } else { - dp.callbacks.OnCompleted(context.Background(), dp.namespace, dp.jobName, Result{Backup: BackupResult{snapshotID, emptySnapshot, source, totalBytes, incrementalBytes}}) + dp.callbacks.OnCompleted(context.Background(), dp.namespace, dp.jobName, Result{Backup: BackupResult{snapshotID, emptySnapshot, source, totalBytes, ptr.To(incrementalBytes)}}) } }() diff --git a/pkg/datapath/data_path_test.go b/pkg/datapath/data_path_test.go index 58df5d4e8..0c493645d 100644 --- a/pkg/datapath/data_path_test.go +++ b/pkg/datapath/data_path_test.go @@ -24,6 +24,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + "k8s.io/utils/ptr" velerotest "github.com/vmware-tanzu/velero/pkg/test" "github.com/vmware-tanzu/velero/pkg/uploader/provider" @@ -82,10 +83,11 @@ func TestAsyncBackup(t *testing.T) { }, result: Result{ Backup: BackupResult{ - SnapshotID: "fake-snapshot", - EmptySnapshot: false, - Source: AccessPoint{ByPath: "fake-path"}, - TotalBytes: 1000, + SnapshotID: "fake-snapshot", + EmptySnapshot: false, + Source: AccessPoint{ByPath: "fake-path"}, + TotalBytes: 1000, + IncrementalBytes: ptr.To(int64(0)), }, }, path: "fake-path", @@ -96,7 +98,11 @@ func TestAsyncBackup(t *testing.T) { t.Run(test.name, func(t *testing.T) { dp := newGeneralDataPath("job-1", "test", nil, "velero", Callbacks{}, velerotest.NewLogger()).(*generalDataPath) mockProvider := providerMock.NewProvider(t) - mockProvider.On("RunBackup", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(test.result.Backup.SnapshotID, test.result.Backup.EmptySnapshot, test.result.Backup.TotalBytes, test.result.Backup.IncrementalBytes, test.err) + var incrementalBytes int64 + if test.result.Backup.IncrementalBytes != nil { + incrementalBytes = *test.result.Backup.IncrementalBytes + } + mockProvider.On("RunBackup", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(test.result.Backup.SnapshotID, test.result.Backup.EmptySnapshot, test.result.Backup.TotalBytes, incrementalBytes, test.err) mockProvider.On("Close", mock.Anything).Return(nil) dp.uploaderProv = mockProvider dp.initialized = true diff --git a/pkg/datapath/micro_service_watcher_test.go b/pkg/datapath/micro_service_watcher_test.go index dee9560ae..315c791ea 100644 --- a/pkg/datapath/micro_service_watcher_test.go +++ b/pkg/datapath/micro_service_watcher_test.go @@ -34,6 +34,7 @@ import ( "k8s.io/client-go/kubernetes" kubeclientfake "k8s.io/client-go/kubernetes/fake" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/vmware-tanzu/velero/pkg/builder" @@ -510,6 +511,42 @@ func TestGetResultFromMessage(t *testing.T) { }, }, }, + { + // An old data mover (release-1.17 and earlier) predates IncrementalBytes and + // never writes the key at all -- this pins that its absence unmarshals to nil + // ("not measured"), not a zero value. + name: "old mover message omits incrementalBytes -> nil", + taskType: TaskTypeBackup, + message: "{\"snapshotID\":\"fake-snapshot-id\",\"emptySnapshot\":false,\"source\":{\"byPath\":\"fake-path-1\",\"volumeMode\":\"Block\"}}", + expectResult: Result{ + Backup: BackupResult{ + SnapshotID: "fake-snapshot-id", + Source: AccessPoint{ + ByPath: "fake-path-1", + VolMode: uploader.PersistentVolumeBlock, + }, + IncrementalBytes: nil, + }, + }, + }, + { + // A current mover reports a genuine zero explicitly -- this pins that the key + // being present with value 0 unmarshals to a non-nil pointer to 0 ("measured + // zero"), distinguishing it from the omitted-key case above. + name: "current mover reports measured zero incrementalBytes -> non-nil zero", + taskType: TaskTypeBackup, + message: "{\"snapshotID\":\"fake-snapshot-id\",\"emptySnapshot\":false,\"source\":{\"byPath\":\"fake-path-1\",\"volumeMode\":\"Block\"},\"incrementalBytes\":0}", + expectResult: Result{ + Backup: BackupResult{ + SnapshotID: "fake-snapshot-id", + Source: AccessPoint{ + ByPath: "fake-path-1", + VolMode: uploader.PersistentVolumeBlock, + }, + IncrementalBytes: ptr.To(int64(0)), + }, + }, + }, { name: "succeed to unmarshall restore result", taskType: TaskTypeRestore, diff --git a/pkg/datapath/types.go b/pkg/datapath/types.go index 65a6be58f..339aa6ca4 100644 --- a/pkg/datapath/types.go +++ b/pkg/datapath/types.go @@ -30,11 +30,15 @@ type Result struct { // BackupResult represents the result of a backup type BackupResult struct { - SnapshotID string `json:"snapshotID"` - EmptySnapshot bool `json:"emptySnapshot"` - Source AccessPoint `json:"source,omitempty"` - TotalBytes int64 `json:"totalBytes,omitempty"` - IncrementalBytes int64 `json:"incrementalBytes,omitempty"` + SnapshotID string `json:"snapshotID"` + EmptySnapshot bool `json:"emptySnapshot"` + Source AccessPoint `json:"source,omitempty"` + TotalBytes int64 `json:"totalBytes,omitempty"` + // IncrementalBytes is a pointer so an old data mover (release-1.17 and earlier, + // which predates this field) that omits it unmarshals to nil -- "not measured" -- + // while a current mover reporting a genuine zero still serializes the key and + // unmarshals to a non-nil zero, distinguishing "measured zero" from "not measured". + IncrementalBytes *int64 `json:"incrementalBytes,omitempty"` } // RestoreResult represents the result of a restore diff --git a/pkg/podvolume/backup_micro_service_test.go b/pkg/podvolume/backup_micro_service_test.go index 2de4705af..b83bafd8a 100644 --- a/pkg/podvolume/backup_micro_service_test.go +++ b/pkg/podvolume/backup_micro_service_test.go @@ -156,7 +156,7 @@ func TestOnDataPathCompleted(t *testing.T) { { name: "marshal fail", marshalErr: errors.New("fake-marshal-error"), - expectedErr: "Failed to marshal backup result { false { } 0 0}: fake-marshal-error", + expectedErr: "Failed to marshal backup result { false { } 0 }: fake-marshal-error", }, { name: "succeed",