Skip to content

Commit

Permalink
Include plugin name in the error message by operations
Browse files Browse the repository at this point in the history
fixes #6512

Signed-off-by: Daniel Jiang <[email protected]>
  • Loading branch information
reasonerjt committed Nov 20, 2023
1 parent 9b5678f commit ca57756
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 4 deletions.
1 change: 1 addition & 0 deletions changelogs/unreleased/7115-reasonerjt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Include plugin name in the error message by operations
23 changes: 19 additions & 4 deletions pkg/controller/backup_operations_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,11 @@ package controller
import (
"bytes"
"context"
"fmt"
"time"

v2 "github.com/vmware-tanzu/velero/pkg/plugin/velero/backupitemaction/v2"

"github.com/pkg/errors"
"github.com/sirupsen/logrus"
apierrors "k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -291,7 +294,7 @@ func getBackupItemOperationProgress(
if err != nil {
operation.Status.Phase = itemoperation.OperationPhaseFailed
operation.Status.Error = err.Error()
errs = append(errs, err.Error())
errs = append(errs, wrapErrMsg(err.Error(), bia))

Check warning on line 297 in pkg/controller/backup_operations_controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controller/backup_operations_controller.go#L297

Added line #L297 was not covered by tests
changes = true
failedCount++
continue
Expand All @@ -300,7 +303,7 @@ func getBackupItemOperationProgress(
if err != nil {
operation.Status.Phase = itemoperation.OperationPhaseFailed
operation.Status.Error = err.Error()
errs = append(errs, err.Error())
errs = append(errs, wrapErrMsg(err.Error(), bia))

Check warning on line 306 in pkg/controller/backup_operations_controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controller/backup_operations_controller.go#L306

Added line #L306 was not covered by tests
changes = true
failedCount++
continue
Expand Down Expand Up @@ -338,7 +341,7 @@ func getBackupItemOperationProgress(
if operationProgress.Err != "" {
operation.Status.Phase = itemoperation.OperationPhaseFailed
operation.Status.Error = operationProgress.Err
errs = append(errs, operationProgress.Err)
errs = append(errs, wrapErrMsg(operationProgress.Err, bia))
changes = true
failedCount++
continue
Expand All @@ -353,7 +356,7 @@ func getBackupItemOperationProgress(
_ = bia.Cancel(operation.Spec.OperationID, backup)
operation.Status.Phase = itemoperation.OperationPhaseFailed
operation.Status.Error = "Asynchronous action timed out"
errs = append(errs, operation.Status.Error)
errs = append(errs, wrapErrMsg(operation.Status.Error, bia))

Check warning on line 359 in pkg/controller/backup_operations_controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controller/backup_operations_controller.go#L359

Added line #L359 was not covered by tests
changes = true
failedCount++
continue
Expand All @@ -373,3 +376,15 @@ func getBackupItemOperationProgress(
}
return inProgressOperations, changes, completedCount, failedCount, errs
}

// wrap the error message to include the BIA name
func wrapErrMsg(errMsg string, bia v2.BackupItemAction) string {
plugin := "unknown"
if bia != nil {
plugin = bia.Name()
}
if len(errMsg) > 0 {
errMsg += ", "
}
return fmt.Sprintf("%splugin: %s", errMsg, plugin)
}
40 changes: 40 additions & 0 deletions pkg/controller/backup_operations_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (
"testing"
"time"

v2 "github.com/vmware-tanzu/velero/pkg/plugin/velero/backupitemaction/v2"

"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
Expand Down Expand Up @@ -286,6 +288,7 @@ func TestBackupOperationsReconcile(t *testing.T) {
backupStore.On("PutBackupItemOperations", mock.Anything, mock.Anything).Return(nil)
backupStore.On("PutBackupMetadata", mock.Anything, mock.Anything).Return(nil)
for _, operation := range test.backupOperations {
bia.On("Name").Return("test")
bia.On("Progress", operation.Spec.OperationID, mock.Anything).
Return(velero.OperationProgress{
Completed: test.operationComplete,
Expand All @@ -308,3 +311,40 @@ func TestBackupOperationsReconcile(t *testing.T) {
})
}
}

func TestWrapErrMsg(t *testing.T) {
bia2 := &biav2mocks.BackupItemAction{}
bia2.On("Name").Return("test-bia")
cases := []struct {
name string
inputErr string
plugin v2.BackupItemAction
expect string
}{
{
name: "empty error message",
inputErr: "",
plugin: bia2,
expect: "plugin: test-bia",
},
{
name: "nil bia",
inputErr: "some error happened",
plugin: nil,
expect: "some error happened, plugin: unknown",
},
{
name: "regular error and bia",
inputErr: "some error happened",
plugin: bia2,
expect: "some error happened, plugin: test-bia",
},
}

for _, test := range cases {
t.Run(test.name, func(t *testing.T) {
got := wrapErrMsg(test.inputErr, test.plugin)
assert.Equal(t, test.expect, got)
})
}
}

0 comments on commit ca57756

Please sign in to comment.