Merge pull request #10632 from nikhiljindal/revert-etcderrors

pkg/api/errors/errors.go

@@ -372,11 +372,6 @@ func IsServerTimeout(err error) bool {
 	return reasonForError(err) == api.StatusReasonServerTimeout
 }
 
-// IsInternalServerError determines if err is an error which indicates that there was an internal server error.
-func IsInternalServerError(err error) bool {
-	return reasonForError(err) == api.StatusReasonInternalError
-}
-
 // IsUnexpectedServerError returns true if the server response was not in the expected API format,
 // and may be the result of another HTTP actor.
 func IsUnexpectedServerError(err error) bool {
@@ -414,14 +409,6 @@ func SuggestsClientDelay(err error) (int, bool) {
 	return 0, false
 }
 
-func IsAPIStatusError(err error) bool {
-	switch err.(type) {
-	case *StatusError:
-		return true
-	}
-	return false
-}
-
 func reasonForError(err error) api.StatusReason {
 	switch t := err.(type) {
 	case *StatusError:

pkg/api/errors/etcd/etcd.go

@@ -27,10 +27,8 @@ func InterpretGetError(err error, kind, name string) error {
 	switch {
 	case tools.IsEtcdNotFound(err):
 		return errors.NewNotFound(kind, name)
-	case errors.IsAPIStatusError(err):
-		return err
 	default:
-		return errors.NewInternalError(err)
+		return err
 	}
 }
 
@@ -40,10 +38,8 @@ func InterpretCreateError(err error, kind, name string) error {
 	switch {
 	case tools.IsEtcdNodeExist(err):
 		return errors.NewAlreadyExists(kind, name)
-	case errors.IsAPIStatusError(err):
-		return err
 	default:
-		return errors.NewInternalError(err)
+		return err
 	}
 }
 
@@ -53,10 +49,8 @@ func InterpretUpdateError(err error, kind, name string) error {
 	switch {
 	case tools.IsEtcdTestFailed(err), tools.IsEtcdNodeExist(err):
 		return errors.NewConflict(kind, name, err)
-	case errors.IsAPIStatusError(err):
-		return err
 	default:
-		return errors.NewInternalError(err)
+		return err
 	}
 }
 
@@ -66,9 +60,7 @@ func InterpretDeleteError(err error, kind, name string) error {
 	switch {
 	case tools.IsEtcdNotFound(err):
 		return errors.NewNotFound(kind, name)
-	case errors.IsAPIStatusError(err):
-		return err
 	default:
-		return errors.NewInternalError(err)
+		return err
 	}
 }

pkg/registry/etcd/etcd.go

@@ -108,10 +108,7 @@ func (r *Registry) CreateService(ctx api.Context, svc *api.Service) (*api.Servic
 	}
 	out := &api.Service{}
 	err = r.CreateObj(key, svc, out, 0)
-	if err != nil {
-		err = etcderr.InterpretCreateError(err, "Service", svc.Name)
-	}
-	return out, err
+	return out, etcderr.InterpretCreateError(err, "service", svc.Name)
 }
 
 // GetService obtains a Service specified by its name.
@@ -123,7 +120,7 @@ func (r *Registry) GetService(ctx api.Context, name string) (*api.Service, error
 	var svc api.Service
 	err = r.ExtractObj(key, &svc, false)
 	if err != nil {
-		return nil, etcderr.InterpretGetError(err, "Service", name)
+		return nil, etcderr.InterpretGetError(err, "service", name)
 	}
 	return &svc, nil
 }
@@ -136,7 +133,7 @@ func (r *Registry) DeleteService(ctx api.Context, name string) error {
 	}
 	err = r.Delete(key, true)
 	if err != nil {
-		return etcderr.InterpretDeleteError(err, "Service", name)
+		return etcderr.InterpretDeleteError(err, "service", name)
 	}
 
 	// TODO: can leave dangling endpoints, and potentially return incorrect
@@ -156,15 +153,12 @@ func (r *Registry) UpdateService(ctx api.Context, svc *api.Service) (*api.Servic
 	}
 	out := &api.Service{}
 	err = r.SetObj(key, svc, out, 0)
-	if err != nil {
-		err = etcderr.InterpretUpdateError(err, "Service", svc.Name)
-	}
-	return out, err
+	return out, etcderr.InterpretUpdateError(err, "service", svc.Name)
 }
 
 // WatchServices begins watching for new, changed, or deleted service configurations.
 func (r *Registry) WatchServices(ctx api.Context, label labels.Selector, field fields.Selector, resourceVersion string) (watch.Interface, error) {
-	version, err := tools.ParseWatchResourceVersion(resourceVersion, "Service")
+	version, err := tools.ParseWatchResourceVersion(resourceVersion, "service")
 	if err != nil {
 		return nil, err
 	}

pkg/registry/generic/etcd/etcd.go

@@ -191,9 +191,7 @@ func (e *Etcd) CreateWithName(ctx api.Context, name string, obj runtime.Object)
 		return err
 	}
 	err = e.Helper.CreateObj(key, obj, nil, ttl)
-	if err != nil {
-		err = etcderr.InterpretCreateError(err, e.EndpointName, name)
-	}
+	err = etcderr.InterpretCreateError(err, e.EndpointName, name)
 	if err == nil && e.Decorator != nil {
 		err = e.Decorator(obj)
 	}
@@ -252,9 +250,7 @@ func (e *Etcd) UpdateWithName(ctx api.Context, name string, obj runtime.Object)
 		return err
 	}
 	err = e.Helper.SetObj(key, obj, nil, ttl)
-	if err != nil {
-		err = etcderr.InterpretUpdateError(err, e.EndpointName, name)
-	}
+	err = etcderr.InterpretUpdateError(err, e.EndpointName, name)
 	if err == nil && e.Decorator != nil {
 		err = e.Decorator(obj)
 	}

pkg/registry/pod/etcd/etcd.go

@@ -145,13 +145,13 @@ func (r *BindingREST) setPodHostAndAnnotations(ctx api.Context, podID, oldMachin
 	err = r.store.Helper.GuaranteedUpdate(podKey, &api.Pod{}, false, tools.SimpleUpdate(func(obj runtime.Object) (runtime.Object, error) {
 		pod, ok := obj.(*api.Pod)
 		if !ok {
-			return nil, errors.NewInternalError(fmt.Errorf("received object is not of type pod: %#v", obj))
+			return nil, fmt.Errorf("unexpected object: %#v", obj)
 		}
 		if pod.DeletionTimestamp != nil {
-			return nil, errors.NewConflict("pod", podID, fmt.Errorf("pod %s is being deleted, cannot be assigned to a host", pod.Name))
+			return nil, fmt.Errorf("pod %s is being deleted, cannot be assigned to a host", pod.Name)
 		}
 		if pod.Spec.NodeName != oldMachine {
-			return nil, errors.NewConflict("pod", podID, fmt.Errorf("pod %v is already assigned to node %q", pod.Name, pod.Spec.NodeName))
+			return nil, fmt.Errorf("pod %v is already assigned to node %q", pod.Name, pod.Spec.NodeName)
 		}
 		pod.Spec.NodeName = machine
 		if pod.Annotations == nil {
@@ -222,7 +222,7 @@ func (r *LogREST) New() runtime.Object {
 func (r *LogREST) Get(ctx api.Context, name string, opts runtime.Object) (runtime.Object, error) {
 	logOpts, ok := opts.(*api.PodLogOptions)
 	if !ok {
-		return nil, errors.NewInternalError(fmt.Errorf("received object is not of type PodLogOptions: %#v", opts))
+		return nil, fmt.Errorf("Invalid options object: %#v", opts)
 	}
 	location, transport, err := pod.LogLocation(r.store, r.kubeletConn, ctx, name, logOpts)
 	if err != nil {
@@ -270,7 +270,7 @@ func (r *ProxyREST) NewConnectOptions() (runtime.Object, bool, string) {
 func (r *ProxyREST) Connect(ctx api.Context, id string, opts runtime.Object) (rest.ConnectHandler, error) {
 	proxyOpts, ok := opts.(*api.PodProxyOptions)
 	if !ok {
-		return nil, errors.NewInternalError(fmt.Errorf("received object is not of type PodProxyOptions: %#v", opts))
+		return nil, fmt.Errorf("Invalid options object: %#v", opts)
 	}
 	location, _, err := pod.ResourceLocation(r.store, ctx, id)
 	if err != nil {
@@ -301,7 +301,7 @@ func (r *ExecREST) New() runtime.Object {
 func (r *ExecREST) Connect(ctx api.Context, name string, opts runtime.Object) (rest.ConnectHandler, error) {
 	execOpts, ok := opts.(*api.PodExecOptions)
 	if !ok {
-		return nil, errors.NewInternalError(fmt.Errorf("received object is not of type PodExecOptions: %#v", opts))
+		return nil, fmt.Errorf("Invalid options object: %#v", opts)
 	}
 	location, transport, err := pod.ExecLocation(r.store, r.kubeletConn, ctx, name, execOpts)
 	if err != nil {

pkg/registry/pod/etcd/etcd_test.go

@@ -153,7 +153,7 @@ func TestCreateRegistryError(t *testing.T) {
 
 	pod := validNewPod()
 	_, err := storage.Create(api.NewDefaultContext(), pod)
-	if !errors.IsInternalServerError(err) {
+	if err != fakeEtcdClient.Err {
 		t.Fatalf("unexpected error: %v", err)
 	}
 }

pkg/registry/resourcequota/etcd/etcd_test.go

@@ -116,7 +116,7 @@ func TestCreateRegistryError(t *testing.T) {
 
 	resourcequota := validNewResourceQuota()
 	_, err := storage.Create(api.NewDefaultContext(), resourcequota)
-	if !errors.IsInternalServerError(err) {
+	if err != fakeEtcdClient.Err {
 		t.Fatalf("unexpected error: %v", err)
 	}
 }

pkg/registry/service/allocator/etcd/etcd.go

@@ -17,6 +17,7 @@ limitations under the License.
 package etcd
 
 import (
+	"errors"
 	"fmt"
 	"sync"
 
@@ -30,8 +31,7 @@ import (
 )
 
 var (
-	// Placeholder error that should not be surfaced to the user.
-	errorUnableToAllocate = k8serr.NewInternalError(fmt.Errorf("unable to allocate"))
+	errorUnableToAllocate = errors.New("unable to allocate")
 )
 
 // Etcd exposes a service.Allocator that is backed by etcd.
@@ -121,9 +121,6 @@ func (e *Etcd) AllocateNext() (int, bool, error) {
 		}
 		return nil
 	})
-	if err != nil && err.Error() == errorUnableToAllocate.Error() {
-		err = nil
-	}
 	return offset, ok, err
 }
 
@@ -164,10 +161,7 @@ func (e *Etcd) tryUpdate(fn func() error) error {
 			return existing, nil
 		}),
 	)
-	if err != nil {
-		err = etcderr.InterpretUpdateError(err, e.kind, "")
-	}
-	return err
+	return etcderr.InterpretUpdateError(err, e.kind, "")
 }
 
 // Refresh reloads the RangeAllocation from etcd.