Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return standard API errors from etcd registry by operation #1192

Merged
merged 1 commit into from
Sep 8, 2014
+121 −58
Merged

Return standard API errors from etcd registry by operation #1192

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions pkg/api/errors/etcd/doc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
Copyright 2014 Google Inc. 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 etcd provides conversion of etcd errors to API errors.
package etcd
66 changes: 66 additions & 0 deletions pkg/api/errors/etcd/etcd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
Copyright 2014 Google Inc. 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 etcd

import (
"github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
"github.com/GoogleCloudPlatform/kubernetes/pkg/tools"
)

// InterpretGetError converts a generic etcd error on a retrieval
// operation into the appropriate API error.
func InterpretGetError(err error, kind, name string) error {
switch {
case tools.IsEtcdNotFound(err):
return errors.NewNotFound(kind, name)
default:
return err
}
}

// InterpretCreateError converts a generic etcd error on a create
// operation into the appropriate API error.
func InterpretCreateError(err error, kind, name string) error {
switch {
case tools.IsEtcdNodeExist(err):
return errors.NewAlreadyExists(kind, name)
default:
return err
}
}

// InterpretUpdateError converts a generic etcd error on a create
// operation into the appropriate API error.
func InterpretUpdateError(err error, kind, name string) error {
switch {
case tools.IsEtcdTestFailed(err), tools.IsEtcdNodeExist(err):
return errors.NewConflict(kind, name, err)
default:
return err
}
}

// InterpretDeleteError converts a generic etcd error on a create
// operation into the appropriate API error.
func InterpretDeleteError(err error, kind, name string) error {
switch {
case tools.IsEtcdNotFound(err):
return errors.NewNotFound(kind, name)
default:
return err
}
}
71 changes: 25 additions & 46 deletions pkg/registry/etcd/etcd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
"fmt"

"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
etcderr "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors/etcd"
"github.com/GoogleCloudPlatform/kubernetes/pkg/constraint"
"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
Expand Down Expand Up @@ -96,7 +96,7 @@ func (r *Registry) WatchPods(resourceVersion uint64, filter func(*api.Pod) bool)
func (r *Registry) GetPod(podID string) (*api.Pod, error) {
var pod api.Pod
if err := r.ExtractObj(makePodKey(podID), &pod, false); err != nil {
return nil, err
return nil, etcderr.InterpretGetError(err, "pod", podID)
}
// TODO: Currently nothing sets CurrentState.Host. We need a feedback loop that sets
// the CurrentState.Host and Status fields. Here we pretend that reality perfectly
Expand All @@ -117,12 +117,13 @@ func (r *Registry) CreatePod(pod *api.Pod) error {
// DesiredState.Host == "" is a signal to the scheduler that this pod needs scheduling.
pod.DesiredState.Status = api.PodRunning
pod.DesiredState.Host = ""
return r.CreateObj(makePodKey(pod.ID), pod)
err := r.CreateObj(makePodKey(pod.ID), pod)
return etcderr.InterpretCreateError(err, "pod", pod.ID)
}

// ApplyBinding implements binding's registry
func (r *Registry) ApplyBinding(binding *api.Binding) error {
return r.assignPod(binding.PodID, binding.Host)
return etcderr.InterpretCreateError(r.assignPod(binding.PodID, binding.Host), "binding", "")
}

// setPodHostTo sets the given pod's host to 'machine' iff it was previously 'oldMachine'.
Expand Down Expand Up @@ -183,20 +184,14 @@ func (r *Registry) DeletePod(podID string) error {
var pod api.Pod
podKey := makePodKey(podID)
err := r.ExtractObj(podKey, &pod, false)
if tools.IsEtcdNotFound(err) {
return errors.NewNotFound("pod", podID)
}
if err != nil {
return err
return etcderr.InterpretDeleteError(err, "pod", podID)
}
// First delete the pod, so a scheduler doesn't notice it getting removed from the
// machine and attempt to put it somewhere.
err = r.Delete(podKey, true)
if tools.IsEtcdNotFound(err) {
return errors.NewNotFound("pod", podID)
}
if err != nil {
return err
return etcderr.InterpretDeleteError(err, "pod", podID)
}
machine := pod.DesiredState.Host
if machine == "" {
Expand Down Expand Up @@ -248,37 +243,29 @@ func (r *Registry) GetController(controllerID string) (*api.ReplicationControlle
var controller api.ReplicationController
key := makeControllerKey(controllerID)
err := r.ExtractObj(key, &controller, false)
if tools.IsEtcdNotFound(err) {
return nil, errors.NewNotFound("replicationController", controllerID)
}
if err != nil {
return nil, err
return nil, etcderr.InterpretGetError(err, "replicationController", controllerID)
}
return &controller, nil
}

// CreateController creates a new ReplicationController.
func (r *Registry) CreateController(controller *api.ReplicationController) error {
err := r.CreateObj(makeControllerKey(controller.ID), controller)
if tools.IsEtcdNodeExist(err) {
return errors.NewAlreadyExists("replicationController", controller.ID)
}
return err
return etcderr.InterpretCreateError(err, "replicationController", controller.ID)
}

// UpdateController replaces an existing ReplicationController.
func (r *Registry) UpdateController(controller *api.ReplicationController) error {
return r.SetObj(makeControllerKey(controller.ID), controller)
err := r.SetObj(makeControllerKey(controller.ID), controller)
return etcderr.InterpretUpdateError(err, "replicationController", controller.ID)
}

// DeleteController deletes a ReplicationController specified by its ID.
func (r *Registry) DeleteController(controllerID string) error {
key := makeControllerKey(controllerID)
err := r.Delete(key, false)
if tools.IsEtcdNotFound(err) {
return errors.NewNotFound("replicationController", controllerID)
}
return err
return etcderr.InterpretDeleteError(err, "replicationController", controllerID)
}

func makeServiceKey(name string) string {
Expand All @@ -295,22 +282,16 @@ func (r *Registry) ListServices() (*api.ServiceList, error) {
// CreateService creates a new Service.
func (r *Registry) CreateService(svc *api.Service) error {
err := r.CreateObj(makeServiceKey(svc.ID), svc)
if tools.IsEtcdNodeExist(err) {
return errors.NewAlreadyExists("service", svc.ID)
}
return err
return etcderr.InterpretCreateError(err, "service", svc.ID)
}

// GetService obtains a Service specified by its name.
func (r *Registry) GetService(name string) (*api.Service, error) {
key := makeServiceKey(name)
var svc api.Service
err := r.ExtractObj(key, &svc, false)
if tools.IsEtcdNotFound(err) {
return nil, errors.NewNotFound("service", name)
}
if err != nil {
return nil, err
return nil, etcderr.InterpretGetError(err, "service", name)
}
return &svc, nil
}
Expand All @@ -320,11 +301,8 @@ func (r *Registry) GetEndpoints(name string) (*api.Endpoints, error) {
key := makeServiceEndpointsKey(name)
var endpoints api.Endpoints
err := r.ExtractObj(key, &endpoints, false)
if tools.IsEtcdNotFound(err) {
return nil, errors.NewNotFound("endpoints", name)
}
if err != nil {
return nil, err
return nil, etcderr.InterpretGetError(err, "endpoints", name)
}
return &endpoints, nil
}
Expand All @@ -337,23 +315,23 @@ func makeServiceEndpointsKey(name string) string {
func (r *Registry) DeleteService(name string) error {
key := makeServiceKey(name)
err := r.Delete(key, true)
if tools.IsEtcdNotFound(err) {
return errors.NewNotFound("service", name)
}
if err != nil {
return err
return etcderr.InterpretDeleteError(err, "service", name)
}

// TODO: can leave dangling endpoints, and potentially return incorrect
// endpoints if a new service is created with the same name
key = makeServiceEndpointsKey(name)
err = r.Delete(key, true)
if !tools.IsEtcdNotFound(err) {
return err
if err := r.Delete(key, true); err != nil && !tools.IsEtcdNotFound(err) {
return etcderr.InterpretDeleteError(err, "endpoints", name)
}
return nil
}

// UpdateService replaces an existing Service.
func (r *Registry) UpdateService(svc *api.Service) error {
return r.SetObj(makeServiceKey(svc.ID), svc)
err := r.SetObj(makeServiceKey(svc.ID), svc)
return etcderr.InterpretUpdateError(err, "service", svc.ID)
}

// WatchServices begins watching for new, changed, or deleted service configurations.
Expand All @@ -380,11 +358,12 @@ func (r *Registry) ListEndpoints() (*api.EndpointsList, error) {
// UpdateEndpoints update Endpoints of a Service.
func (r *Registry) UpdateEndpoints(e *api.Endpoints) error {
// TODO: this is a really bad misuse of AtomicUpdate, need to compute a diff inside the loop.
return r.AtomicUpdate(makeServiceEndpointsKey(e.ID), &api.Endpoints{},
err := r.AtomicUpdate(makeServiceEndpointsKey(e.ID), &api.Endpoints{},
func(input runtime.Object) (runtime.Object, error) {
// TODO: racy - label query is returning different results for two simultaneous updaters
return e, nil
})
return etcderr.InterpretUpdateError(err, "endpoints", e.ID)
}

// WatchEndpoints begins watching for new, changed, or deleted endpoint configurations.
Expand Down
24 changes: 12 additions & 12 deletions pkg/registry/etcd/etcd_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ func TestEtcdGetPodNotFound(t *testing.T) {
}
registry := NewTestEtcdRegistry(fakeClient)
_, err := registry.GetPod("foo")
if err == nil {
t.Errorf("Unexpected non-error.")
if !errors.IsNotFound(err) {
t.Errorf("Unexpected error returned: %#v", err)
}
}

Expand Down Expand Up @@ -144,8 +144,8 @@ func TestEtcdCreatePodAlreadyExisting(t *testing.T) {
ID: "foo",
},
})
if err == nil {
t.Error("Unexpected non-error")
if !errors.IsAlreadyExists(err) {
t.Errorf("Unexpected error returned: %#v", err)
}
}

Expand All @@ -162,7 +162,7 @@ func TestEtcdCreatePodWithContainersError(t *testing.T) {
R: &etcd.Response{
Node: nil,
},
E: tools.EtcdErrorValueRequired,
E: tools.EtcdErrorNodeExist, // validate that ApplyBinding is translating Create errors
}
registry := NewTestEtcdRegistry(fakeClient)
err := registry.CreatePod(&api.Pod{
Expand All @@ -176,16 +176,16 @@ func TestEtcdCreatePodWithContainersError(t *testing.T) {

// Suddenly, a wild scheduler appears:
err = registry.ApplyBinding(&api.Binding{PodID: "foo", Host: "machine"})
if err == nil {
t.Fatalf("Unexpected non error.")
if !errors.IsAlreadyExists(err) {
t.Fatalf("Unexpected error returned: %#v", err)
}

existingPod, err := registry.GetPod("foo")
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
if existingPod.DesiredState.Host == "machine" {
t.Fatal("Pod's host changed in response to an unappliable binding.")
t.Fatal("Pod's host changed in response to an non-apply-able binding.")
}
}

Expand Down Expand Up @@ -568,8 +568,8 @@ func TestEtcdGetControllerNotFound(t *testing.T) {
if ctrl != nil {
t.Errorf("Unexpected non-nil controller: %#v", ctrl)
}
if err == nil {
t.Error("Unexpected non-error.")
if !errors.IsNotFound(err) {
t.Errorf("Unexpected error returned: %#v", err)
}
}

Expand Down Expand Up @@ -745,8 +745,8 @@ func TestEtcdGetServiceNotFound(t *testing.T) {
}
registry := NewTestEtcdRegistry(fakeClient)
_, err := registry.GetService("foo")
if err == nil {
t.Errorf("Unexpected non-error.")
if !errors.IsNotFound(err) {
t.Errorf("Unexpected error returned: %#v", err)
}
}

Expand Down