From 6d0b8ea7a7541f3330d05ad1dd5f1710b92069ca Mon Sep 17 00:00:00 2001
From: Brendan Burns <bburns@google.com>
Date: Thu, 5 Feb 2015 09:37:29 -0800
Subject: [PATCH] Fix a regression where we never cleared out failed nodes.

---
 pkg/controller/replication_controller.go |  3 ++-
 pkg/master/pod_cache.go                  | 10 ++++++++
 pkg/master/pod_cache_test.go             | 29 ++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/pkg/controller/replication_controller.go b/pkg/controller/replication_controller.go
index 2f9c03221b5..04212fafc7d 100644
--- a/pkg/controller/replication_controller.go
+++ b/pkg/controller/replication_controller.go
@@ -166,7 +166,8 @@ func FilterActivePods(pods []api.Pod) []api.Pod {
 	var result []api.Pod
 	for _, value := range pods {
 		if api.PodSucceeded != value.Status.Phase &&
-			api.PodFailed != value.Status.Phase {
+			api.PodFailed != value.Status.Phase &&
+			api.PodUnknown != value.Status.Phase {
 			result = append(result, value)
 		}
 	}
diff --git a/pkg/master/pod_cache.go b/pkg/master/pod_cache.go
index d9d1ed3a614..70cf92da24c 100644
--- a/pkg/master/pod_cache.go
+++ b/pkg/master/pod_cache.go
@@ -135,6 +135,12 @@ func (p *PodCache) getNodeStatus(name string) (*api.NodeStatus, error) {
 	return &node.Status, nil
 }
 
+func (p *PodCache) clearNodeStatus() {
+	p.lock.Lock()
+	defer p.lock.Unlock()
+	p.currentNodes = map[objKey]api.NodeStatus{}
+}
+
 // TODO: once Host gets moved to spec, this can take a podSpec + metadata instead of an
 // entire pod?
 func (p *PodCache) updatePodStatus(pod *api.Pod) error {
@@ -221,6 +227,10 @@ func (p *PodCache) GarbageCollectPodStatus() {
 // calling again, or risk having new info getting clobbered by delayed
 // old info.
 func (p *PodCache) UpdateAllContainers() {
+	// TODO: this is silly, we should pro-actively update the pod status when
+	// the API server makes changes.
+	p.clearNodeStatus()
+
 	ctx := api.NewContext()
 	pods, err := p.pods.ListPods(ctx, labels.Everything())
 	if err != nil {
diff --git a/pkg/master/pod_cache_test.go b/pkg/master/pod_cache_test.go
index b87c985fe30..302f44393d0 100644
--- a/pkg/master/pod_cache_test.go
+++ b/pkg/master/pod_cache_test.go
@@ -254,6 +254,35 @@ func makeUnhealthyNode(name string) *api.Node {
 	}
 }
 
+func TestPodUpdateAllContainersClearsNodeStatus(t *testing.T) {
+	node := makeHealthyNode("machine", "1.2.3.5")
+	pod1 := makePod(api.NamespaceDefault, "foo", "machine", "bar")
+	pod2 := makePod(api.NamespaceDefault, "baz", "machine", "qux")
+	config := podCacheTestConfig{
+		kubeletContainerInfo: api.PodStatus{
+			Info: api.PodInfo{"bar": api.ContainerStatus{}}},
+		nodes: []api.Node{*node},
+		pods:  []api.Pod{*pod1, *pod2},
+	}
+	cache := config.Construct()
+
+	if len(cache.currentNodes) != 0 {
+		t.Errorf("unexpected node cache: %v", cache.currentNodes)
+	}
+	key := objKey{"", "machine"}
+	cache.currentNodes[key] = makeUnhealthyNode("machine").Status
+
+	cache.UpdateAllContainers()
+
+	if len(cache.currentNodes) != 1 {
+		t.Errorf("unexpected empty node cache: %v", cache.currentNodes)
+	}
+
+	if !reflect.DeepEqual(cache.currentNodes[key], node.Status) {
+		t.Errorf("unexpected status:\n%#v\nexpected:\n%#v\n", cache.currentNodes[key], node.Status)
+	}
+}
+
 func TestPodUpdateAllContainers(t *testing.T) {
 	pod1 := makePod(api.NamespaceDefault, "foo", "machine", "bar")
 	pod2 := makePod(api.NamespaceDefault, "baz", "machine", "qux")