VPA updater: fix unready DaemonSet updates

Signed-off-by: Luke Addison <lukeaddison785@gmail.com>

Author: dippynark

vertical-pod-autoscaler/pkg/updater/restriction/pods_restriction_factory.go

@@ -148,10 +148,10 @@ func (f *PodsRestrictionFactoryImpl) getReplicaCount(creator podReplicaCreator)
 		if !ok {
 			return 0, errors.New("failed to parse DaemonSet")
 		}
-		if ds.Status.NumberReady == 0 {
-			return 0, fmt.Errorf("daemon set %s/%s has no number ready pods", creator.Namespace, creator.Name)
+		if ds.Status.DesiredNumberScheduled == 0 {
+			return 0, fmt.Errorf("daemon set %s/%s has no desired scheduled pods", creator.Namespace, creator.Name)
 		}
-		return int(ds.Status.NumberReady), nil
+		return int(ds.Status.DesiredNumberScheduled), nil
 	}
 	return 0, nil
 }

vertical-pod-autoscaler/pkg/updater/restriction/pods_restriction_factory_test.go

@@ -639,44 +639,65 @@ func TestEvictReplicatedByStatefulSet(t *testing.T) {
 }
 
 func TestEvictReplicatedByDaemonSet(t *testing.T) {
-	livePods := int32(5)
-
-	ds := appsv1.DaemonSet{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "ds",
-			Namespace: "default",
-		},
-		TypeMeta: metav1.TypeMeta{
-			Kind: "DaemonSet",
+	testCases := []struct {
+		name                   string
+		desiredNumberScheduled int32
+		numberReady            int32
+	}{
+		{
+			name:                   "Evict two pods (half of 5).",
+			desiredNumberScheduled: 5,
+			numberReady:            5,
 		},
-		Status: appsv1.DaemonSetStatus{
-			NumberReady: livePods,
+		{
+			name:                   "Evict two pods (half of 5) even if no pods are ready.",
+			desiredNumberScheduled: 5,
+			numberReady:            0,
 		},
 	}
 
-	pods := make([]*corev1.Pod, livePods)
-	for i := range pods {
-		pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&ds.ObjectMeta, &ds.TypeMeta).Get()
-	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			ds := appsv1.DaemonSet{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "ds",
+					Namespace: "default",
+				},
+				TypeMeta: metav1.TypeMeta{
+					Kind: "DaemonSet",
+				},
+				Status: appsv1.DaemonSetStatus{
+					DesiredNumberScheduled: tc.desiredNumberScheduled,
+					NumberReady:            tc.numberReady,
+				},
+			}
 
-	basicVpa := getBasicVpa()
-	factory, err := getRestrictionFactory(nil, nil, nil, &ds, 2, 0.5, nil, nil, nil, false)
-	assert.NoError(t, err)
-	creatorToSingleGroupStatsMap, podToReplicaCreatorMap, err := factory.GetCreatorMaps(pods, basicVpa)
-	assert.NoError(t, err)
-	eviction := factory.NewPodsEvictionRestriction(creatorToSingleGroupStatsMap, podToReplicaCreatorMap)
+			pods := make([]*corev1.Pod, tc.desiredNumberScheduled)
+			for i := range pods {
+				pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&ds.ObjectMeta, &ds.TypeMeta).Get()
+			}
 
-	for _, pod := range pods {
-		assert.True(t, eviction.CanEvict(pod))
-	}
+			basicVpa := getBasicVpa()
+			factory, err := getRestrictionFactory(nil, nil, nil, &ds, 2, 0.5, nil, nil, nil, false)
+			assert.NoError(t, err)
+			creatorToSingleGroupStatsMap, podToReplicaCreatorMap, err := factory.GetCreatorMaps(pods, basicVpa)
+			assert.NoError(t, err)
+			assert.Len(t, creatorToSingleGroupStatsMap, 1, "DaemonSet should not be skipped")
+			eviction := factory.NewPodsEvictionRestriction(creatorToSingleGroupStatsMap, podToReplicaCreatorMap)
 
-	for _, pod := range pods[:2] {
-		err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder())
-		assert.Nil(t, err, "Should evict with no error")
-	}
-	for _, pod := range pods[2:] {
-		err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder())
-		assert.Error(t, err, "Error expected")
+			for _, pod := range pods {
+				assert.True(t, eviction.CanEvict(pod))
+			}
+
+			for _, pod := range pods[:2] {
+				err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder())
+				assert.Nil(t, err, "Should evict with no error")
+			}
+			for _, pod := range pods[2:] {
+				err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder())
+				assert.Error(t, err, "Error expected")
+			}
+		})
 	}
 }