mirrors/velero
1
0
Fork 0
mirror of https://github.com/vmware-tanzu/velero.git synced 2026-09-13 11:34:54 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge pull request #10464 from chlins/fix/error-message-context

Browse Source 
Add operation context to user-facing error messages
This commit is contained in:
Chlins Zhang
2026-09-03 10:34:21 +08:00
committed by GitHub
parent 85c660612b e03ff894ff
commit 5bcac16213
8 changed files with 27 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
changelogs/unreleased/10464-chlins
+1
View File
@@ -0,0 +1 @@
Add operation context to user-facing error messages in CR statuses and CLI output
pkg/cmd/errors.go
+1 -1
View File
@@ -27,7 +27,7 @@ import (
func CheckError(err error) {
if err != nil {
if err != context.Canceled {
fmt.Fprintf(os.Stderr, "An error occurred: %v\n", err)
fmt.Fprintf(os.Stderr, "velero: %v\n", err)
}
os.Exit(1)
}
pkg/controller/backup_controller.go
+2 -2
View File
@@ -362,7 +362,7 @@ func (b *backupReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
// result in the backup being Failed.
log.WithError(err).Error("backup failed")
request.Status.Phase = velerov1api.BackupPhaseFailed
request.Status.FailureReason = err.Error()
request.Status.FailureReason = fmt.Sprintf("backup execution failed: %v", err)
}
switch request.Status.Phase {
@@ -619,7 +619,7 @@ func (b *backupReconciler) prepareBackupRequest(ctx context.Context, backup *vel
resourcePolicies, err := resourcepolicies.GetResourcePoliciesFromBackupWithGlobal(
*request.Backup, b.kbClient, b.globalVolumePoliciesConfigMap, request.Namespace, logger)
if err != nil {
request.Status.ValidationErrors = append(request.Status.ValidationErrors, err.Error())
request.Status.ValidationErrors = append(request.Status.ValidationErrors, fmt.Sprintf("invalid resource policies: %v", err))
} else if b.globalVolumePoliciesConfigMap != "" {
// Record the contributing global volume policies ConfigMap so `velero backup describe` can surface it.
request.Annotations[velerov1api.GlobalBackupVolumePolicyConfigMapAnnotation] = b.globalVolumePoliciesConfigMap
pkg/controller/backup_controller_test.go
+2 -2
View File
@@ -1205,7 +1205,7 @@ func TestProcessBackupCompletions(t *testing.T) {
},
Status: velerov1api.BackupStatus{
Phase: velerov1api.BackupPhaseFailed,
FailureReason: "backup already exists in object storage",
FailureReason: "backup execution failed: backup already exists in object storage",
Version: 1,
FormatVersion: "1.1.0",
StartTimestamp: &timestamp,
@@ -1250,7 +1250,7 @@ func TestProcessBackupCompletions(t *testing.T) {
},
Status: velerov1api.BackupStatus{
Phase: velerov1api.BackupPhaseFailed,
FailureReason: "error checking if backup already exists in object storage: Backup already exists in object storage",
FailureReason: "backup execution failed: error checking if backup already exists in object storage: Backup already exists in object storage",
Version: 1,
FormatVersion: "1.1.0",
StartTimestamp: &timestamp,
pkg/controller/backup_deletion_controller.go
+7 -7
View File
@@ -315,7 +315,7 @@ func (r *backupDeletionReconciler) Reconcile(ctx context.Context, req ctrl.Reque
volumeSnapshotter, ok := volumeSnapshotters[snapshot.Spec.Location]
if !ok {
if volumeSnapshotter, err = r.volumeSnapshottersForVSL(ctx, backup.Namespace, snapshot.Spec.Location, pluginManager); err != nil {
errs = append(errs, err.Error())
errs = append(errs, errors.Wrapf(err, "error getting volume snapshotter for location %q", snapshot.Spec.Location).Error())
continue
}
volumeSnapshotters[snapshot.Spec.Location] = volumeSnapshotter
@@ -334,7 +334,7 @@ func (r *backupDeletionReconciler) Reconcile(ctx context.Context, req ctrl.Reque
log.Info("Removing pod volume snapshots")
if deleteErrs := r.deletePodVolumeSnapshots(ctx, backup); len(deleteErrs) > 0 {
for _, err := range deleteErrs {
errs = append(errs, err.Error())
errs = append(errs, errors.Wrap(err, "error deleting pod volume snapshots").Error())
}
}
@@ -342,7 +342,7 @@ func (r *backupDeletionReconciler) Reconcile(ctx context.Context, req ctrl.Reque
log.Info("Removing snapshot data by data mover")
if deleteErrs := r.deleteMovedSnapshots(ctx, backup); len(deleteErrs) > 0 {
for _, err := range deleteErrs {
errs = append(errs, err.Error())
errs = append(errs, errors.Wrap(err, "error deleting moved snapshot data").Error())
}
}
duList := &velerov2alpha1.DataUploadList{}
@@ -354,12 +354,12 @@ func (r *backupDeletionReconciler) Reconcile(ctx context.Context, req ctrl.Reque
}),
}); err != nil {
log.WithError(err).Error("Error listing datauploads")
errs = append(errs, err.Error())
errs = append(errs, errors.Wrap(err, "error listing datauploads for backup").Error())
} else {
for i := range duList.Items {
du := duList.Items[i]
if err := r.Delete(ctx, &du); err != nil {
errs = append(errs, err.Error())
errs = append(errs, errors.Wrapf(err, "error deleting dataupload %q", du.Name).Error())
}
}
}
@@ -368,7 +368,7 @@ func (r *backupDeletionReconciler) Reconcile(ctx context.Context, req ctrl.Reque
if backupStore != nil && len(errs) == 0 {
log.Info("Removing backup from backup storage")
if err := backupStore.DeleteBackup(backup.Name); err != nil {
errs = append(errs, err.Error())
errs = append(errs, errors.Wrap(err, "error removing backup from backup storage").Error())
}
} else if len(errs) > 0 {
log.Info("Skipping removal of backup from backup storage due to previous errors")
@@ -631,7 +631,7 @@ func (r *backupDeletionReconciler) patchDeleteBackupRequest(ctx context.Context,
func (r *backupDeletionReconciler) patchDeleteBackupRequestWithError(ctx context.Context, req *velerov1api.DeleteBackupRequest, err error) error {
_, err = r.patchDeleteBackupRequest(ctx, req, func(r *velerov1api.DeleteBackupRequest) {
r.Status.Phase = velerov1api.DeleteBackupRequestPhaseProcessed
r.Status.Errors = []string{err.Error()}
r.Status.Errors = []string{errors.WithMessage(err, "backup deletion failed").Error()}
})
return err
}
pkg/controller/backup_deletion_controller_test.go
+8 -8
View File
@@ -137,7 +137,7 @@ func TestBackupDeletionControllerReconcile(t *testing.T) {
td.fakeClient.Get(ctx, td.req.NamespacedName, res)
assert.Equal(t, "Processed", string(res.Status.Phase))
assert.Len(t, res.Status.Errors, 1)
assert.True(t, strings.HasPrefix(res.Status.Errors[0], "error getting the backup store"))
assert.True(t, strings.HasPrefix(res.Status.Errors[0], "backup deletion failed: error getting the backup store"))
})
t.Run("missing spec.backupName", func(t *testing.T) {
@@ -153,7 +153,7 @@ func TestBackupDeletionControllerReconcile(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, "Processed", string(res.Status.Phase))
assert.Len(t, res.Status.Errors, 1)
assert.Equal(t, "spec.backupName is required", res.Status.Errors[0])
assert.Equal(t, "backup deletion failed: spec.backupName is required", res.Status.Errors[0])
})
t.Run("existing deletion requests for the backup are deleted", func(t *testing.T) {
@@ -221,7 +221,7 @@ func TestBackupDeletionControllerReconcile(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, "Processed", string(res.Status.Phase))
assert.Len(t, res.Status.Errors, 1)
assert.Equal(t, "backup is still in progress", res.Status.Errors[0])
assert.Equal(t, "backup deletion failed: backup is still in progress", res.Status.Errors[0])
})
t.Run("unable to find backup", func(t *testing.T) {
@@ -235,7 +235,7 @@ func TestBackupDeletionControllerReconcile(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, "Processed", string(res.Status.Phase))
assert.Len(t, res.Status.Errors, 1)
assert.Equal(t, "backup not found", res.Status.Errors[0])
assert.Equal(t, "backup deletion failed: backup not found", res.Status.Errors[0])
})
t.Run("unable to find backup storage location", func(t *testing.T) {
backup := builder.ForBackup(velerov1api.DefaultNamespace, "foo").StorageLocation("default").Result()
@@ -250,7 +250,7 @@ func TestBackupDeletionControllerReconcile(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, "Processed", string(res.Status.Phase))
assert.Len(t, res.Status.Errors, 1)
assert.Equal(t, "backup storage location default not found", res.Status.Errors[0])
assert.Equal(t, "backup deletion failed: backup storage location default not found", res.Status.Errors[0])
})
t.Run("backup storage location is in read-only mode", func(t *testing.T) {
@@ -267,7 +267,7 @@ func TestBackupDeletionControllerReconcile(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, "Processed", string(res.Status.Phase))
assert.Len(t, res.Status.Errors, 1)
assert.Equal(t, "cannot delete backup because backup storage location default is currently in read-only mode", res.Status.Errors[0])
assert.Equal(t, "backup deletion failed: cannot delete backup because backup storage location default is currently in read-only mode", res.Status.Errors[0])
})
t.Run("backup storage location is in unavailable state", func(t *testing.T) {
@@ -284,7 +284,7 @@ func TestBackupDeletionControllerReconcile(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, "Processed", string(res.Status.Phase))
assert.Len(t, res.Status.Errors, 1)
assert.Equal(t, "cannot delete backup because backup storage location default is currently in Unavailable state", res.Status.Errors[0])
assert.Equal(t, "backup deletion failed: cannot delete backup because backup storage location default is currently in Unavailable state", res.Status.Errors[0])
})
t.Run("full delete, no errors", func(t *testing.T) {
@@ -877,7 +877,7 @@ func TestBackupDeletionControllerReconcile(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, "Processed", string(res.Status.Phase))
assert.Len(t, res.Status.Errors, 1)
assert.Equal(t, "backup not found", res.Status.Errors[0])
assert.Equal(t, "backup deletion failed: backup not found", res.Status.Errors[0])
})
}
pkg/controller/pod_volume_backup_controller.go
+2 -3
View File
@@ -784,10 +784,9 @@ func UpdatePVBStatusToFailed(ctx context.Context, c client.Client, pvb *velerov1
pvb.Status.SnapshotID = dataPathError.GetSnapshotID()
}
if len(strings.TrimSpace(msg)) == 0 {
pvb.Status.Message = errOut.Error()
} else {
pvb.Status.Message = errors.WithMessage(errOut, msg).Error()
msg = "pod volume backup failed"
}
pvb.Status.Message = errors.WithMessage(errOut, msg).Error()
if pvb.Status.StartTimestamp.IsZero() {
pvb.Status.StartTimestamp = &metav1.Time{Time: time}
}
pkg/controller/restore_controller.go
+4 -4
View File
@@ -275,7 +275,7 @@ func (r *restoreReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
if err := r.runValidatedRestore(restore, info, resourceModifiers, restoreResPolicies); err != nil {
log.WithError(err).Debug("Restore failed")
restore.Status.Phase = api.RestorePhaseFailed
restore.Status.FailureReason = err.Error()
restore.Status.FailureReason = fmt.Sprintf("restore execution failed: %v", err)
r.metrics.RegisterRestoreFailed(backupScheduleName)
}
@@ -349,7 +349,7 @@ func (r *restoreReconciler) validateAndComplete(ctx context.Context, restore *ap
// validate Restore Init Hook's InitContainers
restoreHooks, err := hook.GetRestoreHooksFromSpec(&restore.Spec.Hooks)
if err != nil {
restore.Status.ValidationErrors = append(restore.Status.ValidationErrors, err.Error())
restore.Status.ValidationErrors = append(restore.Status.ValidationErrors, fmt.Sprintf("invalid restore hooks: %v", err))
}
for _, resource := range restoreHooks {
for _, h := range resource.RestoreHooks {
@@ -357,7 +357,7 @@ func (r *restoreReconciler) validateAndComplete(ctx context.Context, restore *ap
for _, container := range h.Init.InitContainers {
err = hook.ValidateContainer(container.Raw)
if err != nil {
restore.Status.ValidationErrors = append(restore.Status.ValidationErrors, err.Error())
restore.Status.ValidationErrors = append(restore.Status.ValidationErrors, fmt.Sprintf("invalid init container in restore hook %q: %v", resource.Name, err))
}
}
}
@@ -433,7 +433,7 @@ func (r *restoreReconciler) validateAndComplete(ctx context.Context, restore *ap
)
if err != nil {
restore.Status.ValidationErrors = append(
restore.Status.ValidationErrors, err.Error(),
restore.Status.ValidationErrors, fmt.Sprintf("invalid restore resource policies: %v", err),
)
return backupInfo{}, nil, nil
}