From a428246960b83eb568bef1056dcc30d8ca0254bd Mon Sep 17 00:00:00 2001 From: "Timothy St. Clair" Date: Thu, 3 Dec 2015 10:09:45 -0600 Subject: [PATCH] Abstract the error handling for the storage layer to eliminate the direct etcd dependency. --- .gitignore | 3 ++ pkg/api/errors/etcd/etcd.go | 32 +++++++-------- pkg/apiserver/errors.go | 4 +- pkg/master/master.go | 1 + pkg/master/master_test.go | 5 +-- pkg/registry/service/allocator/etcd/etcd.go | 3 +- pkg/storage/errors.go | 45 +++++++++++++++++++++ 7 files changed, 70 insertions(+), 23 deletions(-) create mode 100644 pkg/storage/errors.go diff --git a/.gitignore b/.gitignore index 8eb4dd32785..950e035bc7c 100644 --- a/.gitignore +++ b/.gitignore @@ -13,6 +13,9 @@ .idea/ *.iml +# Vscode files +.vscode/** + # This is where the result of the go build goes /output/** /output diff --git a/pkg/api/errors/etcd/etcd.go b/pkg/api/errors/etcd/etcd.go index 45569fa4583..d0ad3e11df3 100644 --- a/pkg/api/errors/etcd/etcd.go +++ b/pkg/api/errors/etcd/etcd.go @@ -18,68 +18,68 @@ package etcd import ( "k8s.io/kubernetes/pkg/api/errors" - etcdutil "k8s.io/kubernetes/pkg/storage/etcd/util" + "k8s.io/kubernetes/pkg/storage" ) -// InterpretListError converts a generic etcd error on a retrieval +// InterpretListError converts a generic error on a retrieval // operation into the appropriate API error. func InterpretListError(err error, kind string) error { switch { - case etcdutil.IsEtcdNotFound(err): + case storage.IsNotFound(err): return errors.NewNotFound(kind, "") - case etcdutil.IsEtcdUnreachable(err): + case storage.IsUnreachable(err): return errors.NewServerTimeout(kind, "list", 2) // TODO: make configurable or handled at a higher level default: return err } } -// InterpretGetError converts a generic etcd error on a retrieval +// InterpretGetError converts a generic error on a retrieval // operation into the appropriate API error. func InterpretGetError(err error, kind, name string) error { switch { - case etcdutil.IsEtcdNotFound(err): + case storage.IsNotFound(err): return errors.NewNotFound(kind, name) - case etcdutil.IsEtcdUnreachable(err): + case storage.IsUnreachable(err): return errors.NewServerTimeout(kind, "get", 2) // TODO: make configurable or handled at a higher level default: return err } } -// InterpretCreateError converts a generic etcd error on a create +// InterpretCreateError converts a generic error on a create // operation into the appropriate API error. func InterpretCreateError(err error, kind, name string) error { switch { - case etcdutil.IsEtcdNodeExist(err): + case storage.IsNodeExist(err): return errors.NewAlreadyExists(kind, name) - case etcdutil.IsEtcdUnreachable(err): + case storage.IsUnreachable(err): return errors.NewServerTimeout(kind, "create", 2) // TODO: make configurable or handled at a higher level default: return err } } -// InterpretUpdateError converts a generic etcd error on a update +// InterpretUpdateError converts a generic error on a update // operation into the appropriate API error. func InterpretUpdateError(err error, kind, name string) error { switch { - case etcdutil.IsEtcdTestFailed(err), etcdutil.IsEtcdNodeExist(err): + case storage.IsTestFailed(err), storage.IsNodeExist(err): return errors.NewConflict(kind, name, err) - case etcdutil.IsEtcdUnreachable(err): + case storage.IsUnreachable(err): return errors.NewServerTimeout(kind, "update", 2) // TODO: make configurable or handled at a higher level default: return err } } -// InterpretDeleteError converts a generic etcd error on a delete +// InterpretDeleteError converts a generic error on a delete // operation into the appropriate API error. func InterpretDeleteError(err error, kind, name string) error { switch { - case etcdutil.IsEtcdNotFound(err): + case storage.IsNotFound(err): return errors.NewNotFound(kind, name) - case etcdutil.IsEtcdUnreachable(err): + case storage.IsUnreachable(err): return errors.NewServerTimeout(kind, "delete", 2) // TODO: make configurable or handled at a higher level default: return err diff --git a/pkg/apiserver/errors.go b/pkg/apiserver/errors.go index f83704a4f2b..d3d485e7d09 100644 --- a/pkg/apiserver/errors.go +++ b/pkg/apiserver/errors.go @@ -21,7 +21,7 @@ import ( "net/http" "k8s.io/kubernetes/pkg/api/unversioned" - etcdutil "k8s.io/kubernetes/pkg/storage/etcd/util" + "k8s.io/kubernetes/pkg/storage" "k8s.io/kubernetes/pkg/util" ) @@ -52,7 +52,7 @@ func errToAPIStatus(err error) *unversioned.Status { status := http.StatusInternalServerError switch { //TODO: replace me with NewConflictErr - case etcdutil.IsEtcdTestFailed(err): + case storage.IsTestFailed(err): status = http.StatusConflict } // Log errors that were not converted to an error status diff --git a/pkg/master/master.go b/pkg/master/master.go index d9f201793c1..57c877c5534 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -836,6 +836,7 @@ func (m *Master) getServersToValidate(c *Config) map[string]apiserver.Server { addr = etcdUrl.Host port = 4001 } + // TODO: etcd health checking should be abstracted in the storage tier serversToValidate[fmt.Sprintf("etcd-%d", ix)] = apiserver.Server{Addr: addr, Port: port, Path: "/health", Validate: etcdutil.EtcdHealthCheck} } return serversToValidate diff --git a/pkg/master/master_test.go b/pkg/master/master_test.go index 24c213aa0d0..72a4eb8f63b 100644 --- a/pkg/master/master_test.go +++ b/pkg/master/master_test.go @@ -47,7 +47,6 @@ import ( etcdstorage "k8s.io/kubernetes/pkg/storage/etcd" "k8s.io/kubernetes/pkg/storage/etcd/etcdtest" etcdtesting "k8s.io/kubernetes/pkg/storage/etcd/testing" - etcdutil "k8s.io/kubernetes/pkg/storage/etcd/util" "k8s.io/kubernetes/pkg/util" "k8s.io/kubernetes/pkg/util/intstr" @@ -789,7 +788,7 @@ func testInstallThirdPartyAPIDeleteVersion(t *testing.T, version string) { thirdPartyObj := extensions.ThirdPartyResourceData{} err = master.thirdPartyStorage.Get( context.TODO(), expectedDeletedKey, &thirdPartyObj, false) - if !etcdutil.IsEtcdNotFound(err) { + if !storage.IsNotFound(err) { t.Errorf("expected deletion didn't happen: %v", err) } } @@ -876,7 +875,7 @@ func testInstallThirdPartyResourceRemove(t *testing.T, version string) { for _, key := range expectedDeletedKeys { thirdPartyObj := extensions.ThirdPartyResourceData{} err := master.thirdPartyStorage.Get(context.TODO(), key, &thirdPartyObj, false) - if !etcdutil.IsEtcdNotFound(err) { + if !storage.IsNotFound(err) { t.Errorf("expected deletion didn't happen: %v", err) } } diff --git a/pkg/registry/service/allocator/etcd/etcd.go b/pkg/registry/service/allocator/etcd/etcd.go index 3ef4a4fbed4..74487e37a9b 100644 --- a/pkg/registry/service/allocator/etcd/etcd.go +++ b/pkg/registry/service/allocator/etcd/etcd.go @@ -28,7 +28,6 @@ import ( "k8s.io/kubernetes/pkg/registry/service/allocator" "k8s.io/kubernetes/pkg/runtime" "k8s.io/kubernetes/pkg/storage" - etcdutil "k8s.io/kubernetes/pkg/storage/etcd/util" "golang.org/x/net/context" ) @@ -174,7 +173,7 @@ func (e *Etcd) Refresh() (*api.RangeAllocation, error) { existing := &api.RangeAllocation{} if err := e.storage.Get(context.TODO(), e.baseKey, existing, false); err != nil { - if etcdutil.IsEtcdNotFound(err) { + if storage.IsNotFound(err) { return nil, nil } return nil, etcderr.InterpretGetError(err, e.kind, "") diff --git a/pkg/storage/errors.go b/pkg/storage/errors.go new file mode 100644 index 00000000000..8fe1df643d2 --- /dev/null +++ b/pkg/storage/errors.go @@ -0,0 +1,45 @@ +/* +Copyright 2015 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package storage + +import ( + etcdutil "k8s.io/kubernetes/pkg/storage/etcd/util" +) + +// IsNotFound returns true if and only if err is "key" not found error. +func IsNotFound(err error) bool { + // TODO: add alternate storage error here + return etcdutil.IsEtcdNotFound(err) +} + +// IsNodeExist returns true if and only if err is an node already exist error. +func IsNodeExist(err error) bool { + // TODO: add alternate storage error here + return etcdutil.IsEtcdNodeExist(err) +} + +// IsUnreachable returns true if and only if err indicates the server could not be reached. +func IsUnreachable(err error) bool { + // TODO: add alternate storage error here + return etcdutil.IsEtcdUnreachable(err) +} + +// IsTestFailed returns true if and only if err is a write conflict. +func IsTestFailed(err error) bool { + // TODO: add alternate storage error here + return etcdutil.IsEtcdTestFailed(err) +}