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.
This commit is contained in:
Clayton Coleman
2015-02-11 14:35:28 -05:00
parent 3afefc464c
commit 6f85b655cc
2 changed files with 18 additions and 16 deletions

View File

@@ -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) { return r.AtomicUpdate(containerKey, &api.BoundPods{}, func(in runtime.Object) (runtime.Object, error) {
boundPods := in.(*api.BoundPods) boundPods := in.(*api.BoundPods)
for ix := range boundPods.Items { for ix := range boundPods.Items {
if boundPods.Items[ix].Name == pod.Name { item := &boundPods.Items[ix]
boundPods.Items[ix].Spec = pod.Spec if item.Name == pod.Name && item.Namespace == pod.Namespace {
item.Spec = pod.Spec
return boundPods, nil return boundPods, nil
} }
} }
@@ -303,9 +304,9 @@ func (r *Registry) DeletePod(ctx api.Context, podID string) error {
pods := in.(*api.BoundPods) pods := in.(*api.BoundPods)
newPods := make([]api.BoundPod, 0, len(pods.Items)) newPods := make([]api.BoundPod, 0, len(pods.Items))
found := false found := false
for _, pod := range pods.Items { for _, item := range pods.Items {
if pod.Name != podID { if item.Name != pod.Name || item.Namespace != pod.Namespace {
newPods = append(newPods, pod) newPods = append(newPods, item)
} else { } else {
found = true found = true
} }

View File

@@ -467,7 +467,7 @@ func TestEtcdUpdatePodScheduled(t *testing.T) {
key, _ := makePodKey(ctx, "foo") key, _ := makePodKey(ctx, "foo")
fakeClient.Set(key, runtime.EncodeOrDie(latest.Codec, &api.Pod{ 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{ Spec: api.PodSpec{
// Host: "machine", // Host: "machine",
Containers: []api.Container{ Containers: []api.Container{
@@ -485,7 +485,7 @@ func TestEtcdUpdatePodScheduled(t *testing.T) {
fakeClient.Set(contKey, runtime.EncodeOrDie(latest.Codec, &api.BoundPods{ fakeClient.Set(contKey, runtime.EncodeOrDie(latest.Codec, &api.BoundPods{
Items: []api.BoundPod{ Items: []api.BoundPod{
{ {
ObjectMeta: api.ObjectMeta{Name: "foo"}, ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: "other"},
Spec: api.PodSpec{ Spec: api.PodSpec{
Containers: []api.Container{ 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{ Spec: api.PodSpec{
Containers: []api.Container{ Containers: []api.Container{
{ {
@@ -510,6 +510,7 @@ func TestEtcdUpdatePodScheduled(t *testing.T) {
podIn := api.Pod{ podIn := api.Pod{
ObjectMeta: api.ObjectMeta{ ObjectMeta: api.ObjectMeta{
Name: "foo", Name: "foo",
Namespace: api.NamespaceDefault,
ResourceVersion: "1", ResourceVersion: "1",
Labels: map[string]string{ Labels: map[string]string{
"foo": "bar", "foo": "bar",
@@ -553,8 +554,8 @@ func TestEtcdUpdatePodScheduled(t *testing.T) {
t.Fatalf("unexpected error decoding response: %v", err) t.Fatalf("unexpected error decoding response: %v", err)
} }
if len(list.Items) != 2 || !api.Semantic.DeepEqual(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[0].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") key, _ := makePodKey(ctx, "foo")
fakeClient.Set(key, runtime.EncodeOrDie(latest.Codec, &api.Pod{ 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"}, Status: api.PodStatus{Host: "machine"},
}), 0) }), 0)
fakeClient.Set("/registry/nodes/machine/boundpods", runtime.EncodeOrDie(latest.Codec, &api.BoundPods{ fakeClient.Set("/registry/nodes/machine/boundpods", runtime.EncodeOrDie(latest.Codec, &api.BoundPods{
Items: []api.BoundPod{ Items: []api.BoundPod{
{ObjectMeta: api.ObjectMeta{Name: "foo"}}, {ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: api.NamespaceDefault}},
}, },
}), 0) }), 0)
registry := NewTestEtcdRegistry(fakeClient) registry := NewTestEtcdRegistry(fakeClient)
@@ -601,13 +602,13 @@ func TestEtcdDeletePodMultipleContainers(t *testing.T) {
fakeClient.TestIndex = true fakeClient.TestIndex = true
key, _ := makePodKey(ctx, "foo") key, _ := makePodKey(ctx, "foo")
fakeClient.Set(key, runtime.EncodeOrDie(latest.Codec, &api.Pod{ 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"}, Status: api.PodStatus{Host: "machine"},
}), 0) }), 0)
fakeClient.Set("/registry/nodes/machine/boundpods", runtime.EncodeOrDie(latest.Codec, &api.BoundPods{ fakeClient.Set("/registry/nodes/machine/boundpods", runtime.EncodeOrDie(latest.Codec, &api.BoundPods{
Items: []api.BoundPod{ Items: []api.BoundPod{
{ObjectMeta: api.ObjectMeta{Name: "foo"}}, {ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: "other"}},
{ObjectMeta: api.ObjectMeta{Name: "bar"}}, {ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: api.NamespaceDefault}},
}, },
}), 0) }), 0)
registry := NewTestEtcdRegistry(fakeClient) registry := NewTestEtcdRegistry(fakeClient)
@@ -631,7 +632,7 @@ func TestEtcdDeletePodMultipleContainers(t *testing.T) {
if len(boundPods.Items) != 1 { if len(boundPods.Items) != 1 {
t.Fatalf("Unexpected boundPod set: %#v, expected empty", boundPods) 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) t.Errorf("Deleted wrong boundPod: %#v", boundPods)
} }
} }