RC/RS: Fix ignoring inactive Pods.

pkg/controller/replicaset/replica_set.go

@@ -553,13 +553,13 @@ func (rsc *ReplicaSetController) syncReplicaSet(key string) error {
 	// list all pods to include the pods that don't match the rs`s selector
 	// anymore but has the stale controller ref.
 	// TODO: Do the List and Filter in a single pass, or use an index.
-	pods, err := rsc.podLister.Pods(rs.Namespace).List(labels.Everything())
+	allPods, err := rsc.podLister.Pods(rs.Namespace).List(labels.Everything())
 	if err != nil {
 		return err
 	}
 	// Ignore inactive pods.
 	var filteredPods []*v1.Pod
-	for _, pod := range pods {
+	for _, pod := range allPods {
 		if controller.IsPodActive(pod) {
 			filteredPods = append(filteredPods, pod)
 		}

pkg/controller/replicaset/replica_set_test.go

@@ -305,10 +305,16 @@ func TestSyncReplicaSetCreates(t *testing.T) {
 	defer close(stopCh)
 	manager, informers := testNewReplicaSetControllerFromClient(client, stopCh, BurstReplicas)
 
-	// A controller with 2 replicas and no pods in the store, 2 creates expected
+	// A controller with 2 replicas and no active pods in the store.
+	// Inactive pods should be ignored. 2 creates expected.
 	labelMap := map[string]string{"foo": "bar"}
 	rs := newReplicaSet(2, labelMap)
 	informers.Extensions().V1beta1().ReplicaSets().Informer().GetIndexer().Add(rs)
+	failedPod := newPod("failed-pod", rs, v1.PodFailed, nil, true)
+	deletedPod := newPod("deleted-pod", rs, v1.PodRunning, nil, true)
+	deletedPod.DeletionTimestamp = &metav1.Time{Time: time.Now()}
+	informers.Core().V1().Pods().Informer().GetIndexer().Add(failedPod)
+	informers.Core().V1().Pods().Informer().GetIndexer().Add(deletedPod)
 
 	fakePodControl := controller.FakePodControl{}
 	manager.podControl = &fakePodControl

pkg/controller/replication/replication_controller.go

@@ -571,13 +571,13 @@ func (rm *ReplicationManager) syncReplicationController(key string) error {
 	// list all pods to include the pods that don't match the rc's selector
 	// anymore but has the stale controller ref.
 	// TODO: Do the List and Filter in a single pass, or use an index.
-	pods, err := rm.podLister.Pods(rc.Namespace).List(labels.Everything())
+	allPods, err := rm.podLister.Pods(rc.Namespace).List(labels.Everything())
 	if err != nil {
 		return err
 	}
 	// Ignore inactive pods.
 	var filteredPods []*v1.Pod
-	for _, pod := range pods {
+	for _, pod := range allPods {
 		if controller.IsPodActive(pod) {
 			filteredPods = append(filteredPods, pod)
 		}
@@ -585,7 +585,7 @@ func (rm *ReplicationManager) syncReplicationController(key string) error {
 	cm := controller.NewPodControllerRefManager(rm.podControl, rc, labels.Set(rc.Spec.Selector).AsSelectorPreValidated(), controllerKind)
 	// NOTE: filteredPods are pointing to objects from cache - if you need to
 	// modify them, you need to copy it first.
-	filteredPods, err = cm.ClaimPods(pods)
+	filteredPods, err = cm.ClaimPods(filteredPods)
 	if err != nil {
 		return err
 	}

pkg/controller/replication/replication_controller_test.go

@@ -259,11 +259,17 @@ func TestDeleteFinalStateUnknown(t *testing.T) {
 
 func TestSyncReplicationControllerCreates(t *testing.T) {
 	c := clientset.NewForConfigOrDie(&restclient.Config{Host: "", ContentConfig: restclient.ContentConfig{GroupVersion: &api.Registry.GroupOrDie(v1.GroupName).GroupVersion}})
-	manager, _, rcInformer := newReplicationManagerFromClient(c, BurstReplicas)
+	manager, podInformer, rcInformer := newReplicationManagerFromClient(c, BurstReplicas)
 
-	// A controller with 2 replicas and no pods in the store, 2 creates expected
+	// A controller with 2 replicas and no active pods in the store.
+	// Inactive pods should be ignored. 2 creates expected.
 	rc := newReplicationController(2)
 	rcInformer.Informer().GetIndexer().Add(rc)
+	failedPod := newPod("failed-pod", rc, v1.PodFailed, nil, true)
+	deletedPod := newPod("deleted-pod", rc, v1.PodRunning, nil, true)
+	deletedPod.DeletionTimestamp = &metav1.Time{Time: time.Now()}
+	podInformer.Informer().GetIndexer().Add(failedPod)
+	podInformer.Informer().GetIndexer().Add(deletedPod)
 
 	fakePodControl := controller.FakePodControl{}
 	manager.podControl = &fakePodControl