Files
velero/pkg/datapath/data_path.go
T
2c6f45508c
Run the E2E test on kind / setup-test-matrix (push) Failing after 4s
e2e-test-kind.yaml / extract (push) Failing after 11s
Run the E2E test on kind / get-go-version (push) Failing after 12s
Run the E2E test on kind / build (push) Skipped
Run the E2E test on kind / run-e2e-test (push) Skipped
push.yml / extract (push) Failing after 7s
Main CI / get-go-version (push) Failing after 8s
Main CI / Build (push) Skipped
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 <none> 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 <noreply@anthropic.com>
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
(cherry picked from commit 6c7aa9d588f6d5eab134d4ce19c92b838f45557c)
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>

* gofmt: fix import ordering in backup_test.go

Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>

* 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 <tkaovila@redhat.com>

* Add changelog for #10309

Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>

* 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 <tkaovila@redhat.com>

* 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 <nil> instead of 0.

Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>

---------

Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-08-25 18:08:17 -04:00

287 lines
8.5 KiB
Go

/*
Copyright The Velero Contributors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package datapath
import (
"context"
"sync"
"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"
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
"github.com/vmware-tanzu/velero/pkg/cbtservice"
"github.com/vmware-tanzu/velero/pkg/repository"
repokey "github.com/vmware-tanzu/velero/pkg/repository/keys"
repoProvider "github.com/vmware-tanzu/velero/pkg/repository/provider"
"github.com/vmware-tanzu/velero/pkg/uploader"
"github.com/vmware-tanzu/velero/pkg/uploader/provider"
"github.com/vmware-tanzu/velero/pkg/util/filesystem"
)
// InitParam define the input param for data path init
type InitParam struct {
BSLName string
SourceNamespace string
UploaderType string
RepositoryType string
RepoIdentifier string
RepositoryEnsurer *repository.Ensurer
CredentialGetter *credentials.CredentialGetter
Filesystem filesystem.Interface
CacheDir string
}
// BackupStartParam define the input param for backup start
type BackupStartParam struct {
RealSource string
ParentSnapshot string
ForceFull bool
Tags map[string]string
VolumeID string
ChangeID string
SnapshotID string
CBTService cbtservice.Service
}
// RestoreStartParam define the input param for restore start
type RestoreStartParam struct {
}
type generalDataPath struct {
ctx context.Context
cancel context.CancelFunc
backupRepo *velerov1api.BackupRepository
uploaderProv provider.Provider
log logrus.FieldLogger
client client.Client
backupLocation *velerov1api.BackupStorageLocation
namespace string
initialized bool
callbacks Callbacks
jobName string
requestorType string
wgDataPath sync.WaitGroup
dataPathLock sync.Mutex
}
func newGeneralDataPath(jobName string, requestorType string, client client.Client, namespace string, callbacks Callbacks, log logrus.FieldLogger) AsyncBR {
dp := &generalDataPath{
jobName: jobName,
requestorType: requestorType,
client: client,
namespace: namespace,
callbacks: callbacks,
wgDataPath: sync.WaitGroup{},
log: log,
}
return dp
}
func (dp *generalDataPath) Init(ctx context.Context, param any) error {
initParam := param.(*InitParam)
var err error
defer func() {
if err != nil {
dp.Close(ctx)
}
}()
dp.ctx, dp.cancel = context.WithCancel(ctx)
backupLocation := &velerov1api.BackupStorageLocation{}
if err = dp.client.Get(ctx, client.ObjectKey{
Namespace: dp.namespace,
Name: initParam.BSLName,
}, backupLocation); err != nil {
return errors.Wrapf(err, "error getting backup storage location %s", initParam.BSLName)
}
dp.backupLocation = backupLocation
dp.backupRepo, err = initParam.RepositoryEnsurer.EnsureRepo(ctx, dp.namespace, initParam.SourceNamespace, initParam.BSLName, initParam.RepositoryType)
if err != nil {
return errors.Wrapf(err, "error to ensure backup repository %s-%s-%s", initParam.BSLName, initParam.SourceNamespace, initParam.RepositoryType)
}
err = dp.boostRepoConnect(ctx, initParam.RepositoryType, initParam.CredentialGetter, initParam.CacheDir)
if err != nil {
return errors.Wrapf(err, "error to boost backup repository connection %s-%s-%s", initParam.BSLName, initParam.SourceNamespace, initParam.RepositoryType)
}
dp.uploaderProv, err = provider.NewUploaderProvider(ctx, dp.client, initParam.UploaderType, dp.requestorType, initParam.RepoIdentifier,
dp.backupLocation, dp.backupRepo, initParam.CredentialGetter, repokey.RepoKeySelector(), dp.log)
if err != nil {
return errors.Wrapf(err, "error creating uploader %s", initParam.UploaderType)
}
dp.initialized = true
dp.log.WithFields(
logrus.Fields{
"jobName": dp.jobName,
"bsl": initParam.BSLName,
"source namespace": initParam.SourceNamespace,
"uploader": initParam.UploaderType,
"repository": initParam.RepositoryType,
}).Info("Data path is initialized")
return nil
}
func (dp *generalDataPath) Close(ctx context.Context) {
if dp.cancel != nil {
dp.cancel()
}
dp.log.WithField("user", dp.jobName).Info("Closing data path")
dp.wgDataPath.Wait()
dp.close(ctx)
dp.log.WithField("user", dp.jobName).Info("Data path is closed")
}
func (dp *generalDataPath) close(ctx context.Context) {
dp.dataPathLock.Lock()
defer dp.dataPathLock.Unlock()
if dp.uploaderProv != nil {
if err := dp.uploaderProv.Close(ctx); err != nil {
dp.log.Errorf("failed to close uploader provider with error %v", err)
}
dp.uploaderProv = nil
}
}
func (dp *generalDataPath) StartBackup(source AccessPoint, uploaderConfig map[string]string, param any) error {
if !dp.initialized {
return errors.New("file system data path is not initialized")
}
dp.wgDataPath.Add(1)
backupParam := param.(*BackupStartParam)
go func() {
dp.log.Info("Start data path backup")
defer func() {
dp.close(context.Background())
dp.wgDataPath.Done()
}()
snapshotID, emptySnapshot, totalBytes, incrementalBytes, err := dp.uploaderProv.RunBackup(
dp.ctx,
source.ByPath,
backupParam.RealSource,
backupParam.Tags,
backupParam.ForceFull,
backupParam.ParentSnapshot,
provider.CBTParam{
Source: cbtservice.SourceInfo{
Snapshot: backupParam.SnapshotID,
VolumeID: backupParam.VolumeID,
ChangeID: backupParam.ChangeID,
},
Service: backupParam.CBTService,
},
source.VolMode,
uploaderConfig,
dp,
)
if err == provider.ErrorCanceled {
dp.callbacks.OnCancelled(context.Background(), dp.namespace, dp.jobName)
} else if err != nil {
dataPathErr := DataPathError{
snapshotID: snapshotID,
err: err,
}
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, ptr.To(incrementalBytes)}})
}
}()
return nil
}
func (dp *generalDataPath) StartRestore(snapshotID string, target AccessPoint, uploaderConfigs map[string]string, param any) error {
if !dp.initialized {
return errors.New("data path is not initialized")
}
dp.wgDataPath.Add(1)
go func() {
dp.log.Info("Start data path restore")
defer func() {
dp.close(context.Background())
dp.wgDataPath.Done()
}()
totalBytes, err := dp.uploaderProv.RunRestore(dp.ctx, snapshotID, target.ByPath, target.VolMode, uploaderConfigs, dp)
if err == provider.ErrorCanceled {
dp.callbacks.OnCancelled(context.Background(), dp.namespace, dp.jobName)
} else if err != nil {
dataPathErr := DataPathError{
snapshotID: snapshotID,
err: err,
}
dp.callbacks.OnFailed(context.Background(), dp.namespace, dp.jobName, dataPathErr)
} else {
dp.callbacks.OnCompleted(context.Background(), dp.namespace, dp.jobName, Result{Restore: RestoreResult{Target: target, TotalBytes: totalBytes}})
}
}()
return nil
}
// UpdateProgress which implement ProgressUpdater interface to update progress status
func (dp *generalDataPath) UpdateProgress(p *uploader.Progress) {
if dp.callbacks.OnProgress != nil {
dp.callbacks.OnProgress(context.Background(), dp.namespace, dp.jobName, &uploader.Progress{TotalBytes: p.TotalBytes, BytesDone: p.BytesDone})
}
}
func (dp *generalDataPath) Cancel() {
dp.cancel()
dp.log.WithField("user", dp.jobName).Info("FileSystemBR is canceled")
}
func (dp *generalDataPath) boostRepoConnect(ctx context.Context, repositoryType string, credentialGetter *credentials.CredentialGetter, cacheDir string) error {
if repositoryType == velerov1api.BackupRepositoryTypeKopia {
if err := repoProvider.NewUnifiedRepoProvider(*credentialGetter, repositoryType, dp.log).BoostRepoConnect(ctx, repoProvider.RepoParam{BackupLocation: dp.backupLocation, BackupRepo: dp.backupRepo, CacheDir: cacheDir}); err != nil {
return err
}
return nil
}
return errors.Errorf("error getting provider for repo %s", repositoryType)
}