From 20eb3c49c5a233a979de62f93274339a8c502ff4 Mon Sep 17 00:00:00 2001 From: James DeFelice Date: Tue, 3 Mar 2015 23:19:17 +0000 Subject: [PATCH] resolves #4103 clarify resource conflict status, rebase to master remove ResourceConflict, replace usage with Conflict --- pkg/registry/pod/etcd/etcd.go | 27 +++++++++++------ pkg/registry/pod/etcd/etcd_test.go | 48 ++++++++++++++++++++++++++++-- 2 files changed, 64 insertions(+), 11 deletions(-) diff --git a/pkg/registry/pod/etcd/etcd.go b/pkg/registry/pod/etcd/etcd.go index 1bfaf50cade..4afa6f096a9 100644 --- a/pkg/registry/pod/etcd/etcd.go +++ b/pkg/registry/pod/etcd/etcd.go @@ -148,15 +148,15 @@ func (r *BindingREST) Create(ctx api.Context, obj runtime.Object) (out runtime.O if len(binding.Target.Name) == 0 { return nil, errors.NewInvalid("binding", binding.Name, errors.ValidationErrorList{errors.NewFieldRequired("to.name", binding.Target.Name)}) } - err = r.assignPod(ctx, binding.Name, binding.Target.Name) - err = etcderr.InterpretCreateError(err, "binding", "") + err = r.assignPod(ctx, binding.Name, binding.Target.Name, binding.Annotations) out = &api.Status{Status: api.StatusSuccess} return } -// setPodHostTo sets the given pod's host to 'machine' iff it was previously 'oldMachine'. +// setPodHostAndAnnotations sets the given pod's host to 'machine' iff it was previously 'oldMachine' and merges +// the provided annotations with those of the pod. // Returns the current state of the pod, or an error. -func (r *BindingREST) setPodHostTo(ctx api.Context, podID, oldMachine, machine string) (finalPod *api.Pod, err error) { +func (r *BindingREST) setPodHostAndAnnotations(ctx api.Context, podID, oldMachine, machine string, annotations map[string]string) (finalPod *api.Pod, err error) { podKey, err := r.store.KeyFunc(ctx, podID) if err != nil { return nil, err @@ -171,6 +171,12 @@ func (r *BindingREST) setPodHostTo(ctx api.Context, podID, oldMachine, machine s } pod.Spec.Host = machine pod.Status.Host = machine + if pod.Annotations == nil { + pod.Annotations = make(map[string]string) + } + for k, v := range annotations { + pod.Annotations[k] = v + } finalPod = pod return pod, nil }) @@ -178,12 +184,15 @@ func (r *BindingREST) setPodHostTo(ctx api.Context, podID, oldMachine, machine s } // assignPod assigns the given pod to the given machine. -func (r *BindingREST) assignPod(ctx api.Context, podID string, machine string) error { - _, err := r.setPodHostTo(ctx, podID, "", machine) - if err != nil { - return err +func (r *BindingREST) assignPod(ctx api.Context, podID string, machine string, annotations map[string]string) (err error) { + if _, err = r.setPodHostAndAnnotations(ctx, podID, "", machine, annotations); err != nil { + err = etcderr.InterpretGetError(err, "pod", podID) + err = etcderr.InterpretUpdateError(err, "pod", podID) + if _, ok := err.(*errors.StatusError); !ok { + err = errors.NewConflict("binding", podID, err) + } } - return err + return } type podLifecycle struct{} diff --git a/pkg/registry/pod/etcd/etcd_test.go b/pkg/registry/pod/etcd/etcd_test.go index 5f8d5dc1cb8..426f98c00cb 100644 --- a/pkg/registry/pod/etcd/etcd_test.go +++ b/pkg/registry/pod/etcd/etcd_test.go @@ -894,8 +894,12 @@ func TestEtcdCreateWithContainersNotFound(t *testing.T) { // Suddenly, a wild scheduler appears: _, err = bindingRegistry.Create(ctx, &api.Binding{ - ObjectMeta: api.ObjectMeta{Namespace: api.NamespaceDefault, Name: "foo"}, - Target: api.ObjectReference{Name: "machine"}, + ObjectMeta: api.ObjectMeta{ + Namespace: api.NamespaceDefault, + Name: "foo", + Annotations: map[string]string{"label1": "value1"}, + }, + Target: api.ObjectReference{Name: "machine"}, }) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -914,6 +918,46 @@ func TestEtcdCreateWithContainersNotFound(t *testing.T) { if pod.Name != "foo" { t.Errorf("Unexpected pod: %#v %s", pod, resp.Node.Value) } + if !(pod.Annotations != nil && pod.Annotations["label1"] == "value1") { + t.Fatalf("Pod annotations don't match the expected: %v", pod.Annotations) + } +} + +func TestEtcdCreateWithConflict(t *testing.T) { + registry, bindingRegistry, _, fakeClient, _ := newStorage(t) + ctx := api.NewDefaultContext() + fakeClient.TestIndex = true + key, _ := registry.store.KeyFunc(ctx, "foo") + fakeClient.Data[key] = tools.EtcdResponseWithError{ + R: &etcd.Response{ + Node: nil, + }, + E: tools.EtcdErrorNotFound, + } + + _, err := registry.Create(ctx, validNewPod()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Suddenly, a wild scheduler appears: + binding := api.Binding{ + ObjectMeta: api.ObjectMeta{ + Namespace: api.NamespaceDefault, + Name: "foo", + Annotations: map[string]string{"label1": "value1"}, + }, + Target: api.ObjectReference{Name: "machine"}, + } + _, err = bindingRegistry.Create(ctx, &binding) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + _, err = bindingRegistry.Create(ctx, &binding) + if err == nil || !errors.IsConflict(err) { + t.Fatalf("expected resource conflict error, not: %v", err) + } } func TestEtcdCreateWithExistingContainers(t *testing.T) {