From 6f85b655cc87f2ca7f96c959aed8688c97a14fb4 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Wed, 11 Feb 2015 14:35:28 -0500 Subject: [PATCH] Bindings were not correctly removed across namespaces on pod update and delete BoundPods must be checked for Name and Namespace equality, not just name equality. In the future, we should also check for UID equality. --- pkg/registry/etcd/etcd.go | 11 ++++++----- pkg/registry/etcd/etcd_test.go | 23 ++++++++++++----------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/pkg/registry/etcd/etcd.go b/pkg/registry/etcd/etcd.go index 0cb49c0246b..d074929ae8a 100644 --- a/pkg/registry/etcd/etcd.go +++ b/pkg/registry/etcd/etcd.go @@ -264,8 +264,9 @@ func (r *Registry) UpdatePod(ctx api.Context, pod *api.Pod) error { return r.AtomicUpdate(containerKey, &api.BoundPods{}, func(in runtime.Object) (runtime.Object, error) { boundPods := in.(*api.BoundPods) for ix := range boundPods.Items { - if boundPods.Items[ix].Name == pod.Name { - boundPods.Items[ix].Spec = pod.Spec + item := &boundPods.Items[ix] + if item.Name == pod.Name && item.Namespace == pod.Namespace { + item.Spec = pod.Spec return boundPods, nil } } @@ -303,9 +304,9 @@ func (r *Registry) DeletePod(ctx api.Context, podID string) error { pods := in.(*api.BoundPods) newPods := make([]api.BoundPod, 0, len(pods.Items)) found := false - for _, pod := range pods.Items { - if pod.Name != podID { - newPods = append(newPods, pod) + for _, item := range pods.Items { + if item.Name != pod.Name || item.Namespace != pod.Namespace { + newPods = append(newPods, item) } else { found = true } diff --git a/pkg/registry/etcd/etcd_test.go b/pkg/registry/etcd/etcd_test.go index ef13c2dfd13..16d077c8caf 100644 --- a/pkg/registry/etcd/etcd_test.go +++ b/pkg/registry/etcd/etcd_test.go @@ -467,7 +467,7 @@ func TestEtcdUpdatePodScheduled(t *testing.T) { key, _ := makePodKey(ctx, "foo") fakeClient.Set(key, runtime.EncodeOrDie(latest.Codec, &api.Pod{ - ObjectMeta: api.ObjectMeta{Name: "foo"}, + ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: api.NamespaceDefault}, Spec: api.PodSpec{ // Host: "machine", Containers: []api.Container{ @@ -485,7 +485,7 @@ func TestEtcdUpdatePodScheduled(t *testing.T) { fakeClient.Set(contKey, runtime.EncodeOrDie(latest.Codec, &api.BoundPods{ Items: []api.BoundPod{ { - ObjectMeta: api.ObjectMeta{Name: "foo"}, + ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: "other"}, Spec: api.PodSpec{ Containers: []api.Container{ { @@ -494,7 +494,7 @@ func TestEtcdUpdatePodScheduled(t *testing.T) { }, }, }, { - ObjectMeta: api.ObjectMeta{Name: "bar"}, + ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: api.NamespaceDefault}, Spec: api.PodSpec{ Containers: []api.Container{ { @@ -510,6 +510,7 @@ func TestEtcdUpdatePodScheduled(t *testing.T) { podIn := api.Pod{ ObjectMeta: api.ObjectMeta{ Name: "foo", + Namespace: api.NamespaceDefault, ResourceVersion: "1", Labels: map[string]string{ "foo": "bar", @@ -553,8 +554,8 @@ func TestEtcdUpdatePodScheduled(t *testing.T) { t.Fatalf("unexpected error decoding response: %v", err) } - if len(list.Items) != 2 || !api.Semantic.DeepEqual(list.Items[0].Spec, podIn.Spec) { - t.Errorf("unexpected container list: %d\n items[0] - %#v\n podin.spec - %#v\n", len(list.Items), list.Items[0].Spec, podIn.Spec) + if len(list.Items) != 2 || !api.Semantic.DeepEqual(list.Items[1].Spec, podIn.Spec) { + t.Errorf("unexpected container list: %d\n items[0] - %#v\n podin.spec - %#v\n", len(list.Items), list.Items[1].Spec, podIn.Spec) } } @@ -565,12 +566,12 @@ func TestEtcdDeletePod(t *testing.T) { key, _ := makePodKey(ctx, "foo") fakeClient.Set(key, runtime.EncodeOrDie(latest.Codec, &api.Pod{ - ObjectMeta: api.ObjectMeta{Name: "foo"}, + ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: api.NamespaceDefault}, Status: api.PodStatus{Host: "machine"}, }), 0) fakeClient.Set("/registry/nodes/machine/boundpods", runtime.EncodeOrDie(latest.Codec, &api.BoundPods{ Items: []api.BoundPod{ - {ObjectMeta: api.ObjectMeta{Name: "foo"}}, + {ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: api.NamespaceDefault}}, }, }), 0) registry := NewTestEtcdRegistry(fakeClient) @@ -601,13 +602,13 @@ func TestEtcdDeletePodMultipleContainers(t *testing.T) { fakeClient.TestIndex = true key, _ := makePodKey(ctx, "foo") fakeClient.Set(key, runtime.EncodeOrDie(latest.Codec, &api.Pod{ - ObjectMeta: api.ObjectMeta{Name: "foo"}, + ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: api.NamespaceDefault}, Status: api.PodStatus{Host: "machine"}, }), 0) fakeClient.Set("/registry/nodes/machine/boundpods", runtime.EncodeOrDie(latest.Codec, &api.BoundPods{ Items: []api.BoundPod{ - {ObjectMeta: api.ObjectMeta{Name: "foo"}}, - {ObjectMeta: api.ObjectMeta{Name: "bar"}}, + {ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: "other"}}, + {ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: api.NamespaceDefault}}, }, }), 0) registry := NewTestEtcdRegistry(fakeClient) @@ -631,7 +632,7 @@ func TestEtcdDeletePodMultipleContainers(t *testing.T) { if len(boundPods.Items) != 1 { t.Fatalf("Unexpected boundPod set: %#v, expected empty", boundPods) } - if boundPods.Items[0].Name != "bar" { + if boundPods.Items[0].Namespace != "other" { t.Errorf("Deleted wrong boundPod: %#v", boundPods) } }