From 60d165a8308645704c5e07d122fb5d83efc84f2a Mon Sep 17 00:00:00 2001 From: Hongchao Deng Date: Sun, 24 Jul 2016 20:21:17 -0700 Subject: [PATCH] storage error: precondition failure should return invalid object error --- pkg/api/errors/storage/storage.go | 4 ++-- pkg/storage/errors.go | 10 +++------- pkg/storage/etcd/etcd_helper.go | 9 +++++---- pkg/storage/etcd/etcd_helper_test.go | 4 ++-- 4 files changed, 12 insertions(+), 15 deletions(-) diff --git a/pkg/api/errors/storage/storage.go b/pkg/api/errors/storage/storage.go index d900c170e0..7f84cfcbc4 100644 --- a/pkg/api/errors/storage/storage.go +++ b/pkg/api/errors/storage/storage.go @@ -65,7 +65,7 @@ func InterpretCreateError(err error, qualifiedResource unversioned.GroupResource // operation into the appropriate API error. func InterpretUpdateError(err error, qualifiedResource unversioned.GroupResource, name string) error { switch { - case storage.IsTestFailed(err), storage.IsNodeExist(err): + case storage.IsTestFailed(err), storage.IsNodeExist(err), storage.IsInvalidObj(err): return errors.NewConflict(qualifiedResource, name, err) case storage.IsUnreachable(err): return errors.NewServerTimeout(qualifiedResource, "update", 2) // TODO: make configurable or handled at a higher level @@ -86,7 +86,7 @@ func InterpretDeleteError(err error, qualifiedResource unversioned.GroupResource return errors.NewNotFound(qualifiedResource, name) case storage.IsUnreachable(err): return errors.NewServerTimeout(qualifiedResource, "delete", 2) // TODO: make configurable or handled at a higher level - case storage.IsTestFailed(err), storage.IsNodeExist(err): + case storage.IsTestFailed(err), storage.IsNodeExist(err), storage.IsInvalidObj(err): return errors.NewConflict(qualifiedResource, name, err) case storage.IsInternalError(err): return errors.NewInternalError(err) diff --git a/pkg/storage/errors.go b/pkg/storage/errors.go index 26985fa073..3ae0a8893c 100644 --- a/pkg/storage/errors.go +++ b/pkg/storage/errors.go @@ -107,7 +107,7 @@ func IsUnreachable(err error) bool { // IsTestFailed returns true if and only if err is a write conflict. func IsTestFailed(err error) bool { - return isErrCode(err, ErrCodeResourceVersionConflicts, ErrCodeInvalidObj) + return isErrCode(err, ErrCodeResourceVersionConflicts) } // IsInvalidUID returns true if and only if err is invalid UID error @@ -115,16 +115,12 @@ func IsInvalidObj(err error) bool { return isErrCode(err, ErrCodeInvalidObj) } -func isErrCode(err error, codes ...int) bool { +func isErrCode(err error, code int) bool { if err == nil { return false } if e, ok := err.(*StorageError); ok { - for _, code := range codes { - if e.Code == code { - return true - } - } + return e.Code == code } return false } diff --git a/pkg/storage/etcd/etcd_helper.go b/pkg/storage/etcd/etcd_helper.go index 62d01aa409..3912c4e86f 100644 --- a/pkg/storage/etcd/etcd_helper.go +++ b/pkg/storage/etcd/etcd_helper.go @@ -150,7 +150,7 @@ func (h *etcdHelper) Create(ctx context.Context, key string, obj, out runtime.Ob return err } -func checkPreconditions(preconditions *storage.Preconditions, out runtime.Object) error { +func checkPreconditions(key string, preconditions *storage.Preconditions, out runtime.Object) error { if preconditions == nil { return nil } @@ -159,7 +159,8 @@ func checkPreconditions(preconditions *storage.Preconditions, out runtime.Object return storage.NewInternalErrorf("can't enforce preconditions %v on un-introspectable object %v, got error: %v", *preconditions, out, err) } if preconditions.UID != nil && *preconditions.UID != objMeta.UID { - return etcd.Error{Code: etcd.ErrorCodeTestFailed, Message: fmt.Sprintf("the UID in the precondition (%s) does not match the UID in record (%s). The object might have been deleted and then recreated", *preconditions.UID, objMeta.UID)} + errMsg := fmt.Sprintf("Precondition failed: UID in precondition: %v, UID in object meta: %v", preconditions.UID, objMeta.UID) + return storage.NewInvalidObjError(key, errMsg) } return nil } @@ -195,7 +196,7 @@ func (h *etcdHelper) Delete(ctx context.Context, key string, out runtime.Object, if err != nil { return toStorageErr(err, key, 0) } - if err := checkPreconditions(preconditions, obj); err != nil { + if err := checkPreconditions(key, preconditions, obj); err != nil { return toStorageErr(err, key, 0) } index := uint64(0) @@ -471,7 +472,7 @@ func (h *etcdHelper) GuaranteedUpdate(ctx context.Context, key string, ptrToType if err != nil { return toStorageErr(err, key, 0) } - if err := checkPreconditions(preconditions, obj); err != nil { + if err := checkPreconditions(key, preconditions, obj); err != nil { return toStorageErr(err, key, 0) } meta := storage.ResponseMeta{} diff --git a/pkg/storage/etcd/etcd_helper_test.go b/pkg/storage/etcd/etcd_helper_test.go index c178fc860b..952d972d3b 100644 --- a/pkg/storage/etcd/etcd_helper_test.go +++ b/pkg/storage/etcd/etcd_helper_test.go @@ -460,7 +460,7 @@ func TestGuaranteedUpdateUIDMismatch(t *testing.T) { err = helper.GuaranteedUpdate(context.TODO(), "/some/key", podPtr, true, storage.NewUIDPreconditions("B"), storage.SimpleUpdate(func(in runtime.Object) (runtime.Object, error) { return obj, nil })) - if !storage.IsTestFailed(err) { + if !storage.IsInvalidObj(err) { t.Fatalf("Expect a Test Failed (write conflict) error, got: %v", err) } } @@ -499,7 +499,7 @@ func TestDeleteUIDMismatch(t *testing.T) { t.Fatalf("Unexpected error %#v", err) } err = helper.Delete(context.TODO(), "/some/key", obj, storage.NewUIDPreconditions("B")) - if !storage.IsTestFailed(err) { + if !storage.IsInvalidObj(err) { t.Fatalf("Expect a Test Failed (write conflict) error, got: %v", err) } }