Skip to content

Commit

Permalink
Move OperationInProgress to util
Browse files Browse the repository at this point in the history
We want to use it more in ramen, to remove bugus errors for logs.
To encourage use this this pattern and make it easier to use correctly,
add util.IsOperationInProgress() helper.

Improve the test to use the new IsOperationInProgress() and test also
wrapped OperationInProgress error.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
  • Loading branch information
nirs committed Dec 9, 2024
1 parent 6bb00d8 commit db43d7d
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 54 deletions.
3 changes: 0 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,6 @@ test-util: generate manifests envtest ## Run util tests.
test-util-pvc: generate manifests envtest ## Run util-pvc tests.
go test ./internal/controller/util -coverprofile cover.out -ginkgo.focus PVCS_Util

test-kubeobjects: ## Run kubeobjects tests.
go test ./internal/controller/kubeobjects -coverprofile cover.out -ginkgo.focus Kubeobjects

test-drenv: ## Run drenv tests.
$(MAKE) -C test

Expand Down
13 changes: 0 additions & 13 deletions internal/controller/kubeobjects/requests.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,19 +136,6 @@ type Operation struct {
InverseOp string `json:"inverseOp,omitempty"`
}

// OperationInProgress error is return when an operation is in progress and we wait for the desired state. The error
// string should describe the operation for logging the error.
type OperationInProgress string

func (e OperationInProgress) Error() string { return string(e) }

// Called by errors.Is() to match target.
func (OperationInProgress) Is(target error) bool {
_, ok := target.(OperationInProgress)

return ok
}

type RequestsManager interface {
ProtectsPath() string
RecoversPath() string
Expand Down
30 changes: 0 additions & 30 deletions internal/controller/kubeobjects/requests_test.go

This file was deleted.

13 changes: 7 additions & 6 deletions internal/controller/kubeobjects/velero/requests.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (

"github.com/go-logr/logr"
"github.com/ramendr/ramen/internal/controller/kubeobjects"
"github.com/ramendr/ramen/internal/controller/util"
velero "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
corev1 "k8s.io/api/core/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -249,11 +250,11 @@ func backupDummyStatusProcessAndRestore(
velero.BackupPhaseUploading,
velero.BackupPhaseUploadingPartialFailure,
velero.BackupPhaseDeleting:
return nil, kubeobjects.OperationInProgress("backup" + string(backup.Status.Phase))
return nil, util.OperationInProgress("backup" + string(backup.Status.Phase))
case velero.BackupPhaseFailedValidation:
return nil, errors.New("backup" + string(backup.Status.Phase))
default:
return nil, kubeobjects.OperationInProgress("backup.status.phase absent")
return nil, util.OperationInProgress("backup.status.phase absent")
}
}

Expand Down Expand Up @@ -283,13 +284,13 @@ func restoreStatusProcess(
return nil
case velero.RestorePhaseNew,
velero.RestorePhaseInProgress:
return kubeobjects.OperationInProgress("restore" + string(restore.Status.Phase))
return util.OperationInProgress("restore" + string(restore.Status.Phase))
case velero.RestorePhaseFailed,
velero.RestorePhaseFailedValidation,
velero.RestorePhasePartiallyFailed:
return errors.New("restore" + string(restore.Status.Phase))
default:
return kubeobjects.OperationInProgress("restore.status.phase absent")
return util.OperationInProgress("restore.status.phase absent")
}
}

Expand Down Expand Up @@ -399,13 +400,13 @@ func backupRealStatusProcess(
velero.BackupPhaseUploading,
velero.BackupPhaseUploadingPartialFailure,
velero.BackupPhaseDeleting:
return kubeobjects.OperationInProgress("backup" + string(backup.Status.Phase))
return util.OperationInProgress("backup" + string(backup.Status.Phase))
case velero.BackupPhaseFailedValidation,
velero.BackupPhasePartiallyFailed,
velero.BackupPhaseFailed:
return errors.New("backup" + string(backup.Status.Phase))
default:
return kubeobjects.OperationInProgress("backup.status.phase absent")
return util.OperationInProgress("backup.status.phase absent")
}
}

Expand Down
24 changes: 24 additions & 0 deletions internal/controller/util/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// SPDX-FileCopyrightText: The RamenDR authors
// SPDX-License-Identifier: Apache-2.0package util

package util

import "errors"

// OperationInProgress error is return when an operation is in progress and we wait for the desired state. The error
// string should describe the operation for logging the error.
type OperationInProgress string

func (e OperationInProgress) Error() string { return string(e) }

// Called by errors.Is() to match target.
func (OperationInProgress) Is(target error) bool {
_, ok := target.(OperationInProgress)

return ok
}

// IsOperationInProgress returns true if err or error wrapped by it is an OperationInProgress error.
func IsOperationInProgress(err error) bool {
return errors.Is(err, OperationInProgress(""))
}
33 changes: 33 additions & 0 deletions internal/controller/util/errors_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// SPDX-FileCopyrightText: The RamenDR authors
// SPDX-License-Identifier: Apache-2.0

package util_test

import (
"errors"
"fmt"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/ramendr/ramen/internal/controller/util"
)

var _ = Describe("OperationInProgress", func() {
Context("comparing errors", func() {
err := util.OperationInProgress("this operation")

It("is not equal", func() {
Expect(err == util.OperationInProgress("other operation")).To(Equal(false))
})
It("match error", func() {
Expect(util.IsOperationInProgress(err)).To(Equal(true))
})
It("match wrapped error", func() {
wrapped := fmt.Errorf("wrapping operation in progress: %w", err)
Expect(util.IsOperationInProgress(wrapped)).To(Equal(true))
})
It("does not match other errors", func() {
Expect(util.IsOperationInProgress(errors.New("other error"))).To(Equal(false))
})
})
})
4 changes: 2 additions & 2 deletions internal/controller/vrg_kubeobjects.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ func (v *VRGInstance) kubeObjectsGroupCapture(
continue
}

if errors.Is(err, kubeobjects.OperationInProgress("")) {
if util.IsOperationInProgress(err) {
log1.Info("Kube objects group capturing", "state", err.Error())

continue
Expand Down Expand Up @@ -695,7 +695,7 @@ func (v *VRGInstance) executeRecoverGroup(result *ctrl.Result, s3StoreAccessor s
}
}

if errors.Is(err, kubeobjects.OperationInProgress("")) {
if util.IsOperationInProgress(err) {
log1.Info("Kube objects group recovering", "state", err.Error())

return err
Expand Down

0 comments on commit db43d7d

Please sign in to comment.