bugfix: fix data operation failure when upgrading cache engine implementation with same Dataset name #4329
Conversation
…igrate utils Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
…data operations Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
|
|
@TrafalgarZZZ I read through the code and figure out the reason for #4328. fluid/pkg/controllers/operation_controller.go Line 147 in d23bf2c I know the DataMigrate must get the dataset object for checking runtime type so we can not set the NamespacedName in ctx before ReconcileDeletion. But, do we still have a way to merge the |
@xliuqq Thanks for the advice. But I am currently not sure how to merge these two functions. func GetTargetDataset() (namespacedNames []types.NamespacedName, targetDataset *datav1alpha1.Dataset) {
// If dataset object cannot be found -> returns full namespacedNames
// If dataset object can be found -> returns a namespacedName and the corresponding Dataset
} |
|
|
||
| dataset, innerErr := GetDataset(client, toCheck.Name, namespace) | ||
| if innerErr != nil { | ||
| err = errors.Wrapf(innerErr, "failed to get dataset \"%s/%s\" from DataMigrate \"%s/%s\"'s spec", namespace, toCheck.Name, dataMigrate.Namespace, dataMigrate.Name) |
There was a problem hiding this comment.
we can not wrap the error, as the code in
calls
fluid/pkg/ddc/base/operation_lock.go
Line 92 in d23bf2c
and later code will check the not found error.
There was a problem hiding this comment.
@xliuqq Thank you! Very careful advice. I've tested such case and I think that would not be a problem.
In vendor/k8s.io/apimachinery/pkg/api/errors/errors.go, you can see the function automatically unwraps the error and checks if it's a NotFound error.
// IsNotFound returns true if the specified error was created by NewNotFound.
// It supports wrapped errors and returns false when the error is nil.
func IsNotFound(err error) bool {
reason, code := reasonAndCodeForError(err)
if reason == metav1.StatusReasonNotFound {
return true
}
if _, ok := knownReasons[reason]; !ok && code == http.StatusNotFound {
return true
}
return false
}I've also tried something like this, and it prints "Wrapped error supported". From my understanding, the code now should works fine.
func main() {
err := apierrors.NewNotFound(schema.ParseGroupResource("datasets.data.fluid.io"), "dataset not found")
wrappedErr := errors.Wrap(err, "A wrapped error")
if apierrors.IsNotFound(wrappedErr) {
fmt.Println("Wrapped error supported")
} else {
fmt.Println("Wrapped error is not support")
}
}|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cheyang The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test fluid-e2e |
|
/retest |
Ⅰ. Describe what this PR does
GetPossibleTargetDatasetNamespacedNames()for all data operations. It's currently only used when cleaning data operations. The function returns a list of possible datasets'types.NamespacedName, after then the operation reconciler can correctly remove engines with the namespace and name.func GetTargetDatasetOfMigrateto make it easier to read.Ⅱ. Does this pull request fix one issue?
fixes #4328
Ⅲ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews