container: cover the status that has not caught up with the scale request

Turn the scale-from-zero regression into a table so it covers both the pre-scale read and a status the controller has not written yet, and give the type assertion in the reactor the SAFETY comment the lint gate wants.

pkg/executor/executortype/container/deployment_test.go

@@ -310,61 +310,98 @@ func TestContainerGetResources(t *testing.T) {
 }
 
 // TestContainerScaleFromZeroWaitsForAvailability covers the scale-to-zero race
-// reported in #3183. The first read reports one available replica (the pod the
+// reported in #3183. On the scale-from-zero path the deployment status can
-// idle reaper is tearing down) and every later read reports none, so a caller
+// still describe the pod the idle reaper is tearing down, either because it
-// that trusts the pre-scale status reports success with no serving pod.
+// was read before the scale request or because the controller has not written
+// a status for that request yet. A caller that trusts such a status reports
+// success while the Service has no endpoint and the router forwards into it.
 func TestContainerScaleFromZeroWaitsForAvailability(t *testing.T) {
 	t.Parallel()
 
-	fn := newTestContainerFunction()
+	// Generation of the spec after the scale request.
-	fn.Spec.PodSpec.Containers[0].Ports = []apiv1.ContainerPort{{ContainerPort: 8080}}
+	const generation = 2
-	fn.Spec.InvokeStrategy.ExecutionStrategy.MinScale = 0
 
-	ns := utils.DefaultNSResolver().GetFunctionNS(fn.Namespace)
+	tests := []struct {
-	zero := int32(0)
+		name string
-	scaledIn := &appsv1.Deployment{
+		// status is the deployment status served by the nth read, 1-based.
-		ObjectMeta: metav1.ObjectMeta{
+		status func(read int) appsv1.DeploymentStatus
-			Name:        "ctr-fn",
+	}{
-			Namespace:   ns,
+		{
-			Annotations: map[string]string{fv1.EXECUTOR_INSTANCEID_LABEL: "test-instance"},
+			// The copy createOrGetDeployment read before scaling up.
+			name: "pre-scale read still counts the terminating pod",
+			status: func(read int) appsv1.DeploymentStatus {
+				available := int32(0)
+				if read == 1 {
+					available = 1
+				}
+				return appsv1.DeploymentStatus{ObservedGeneration: generation, AvailableReplicas: available}
+			},
+		},
+		{
+			// Status is written asynchronously by the deployment controller, so
+			// every read up to the deadline can still describe the generation
+			// before the scale request.
+			name: "status has not caught up with the scale request",
+			status: func(int) appsv1.DeploymentStatus {
+				return appsv1.DeploymentStatus{ObservedGeneration: generation - 1, AvailableReplicas: 1}
+			},
 		},
-		Spec:   appsv1.DeploymentSpec{Replicas: &zero},
-		Status: appsv1.DeploymentStatus{AvailableReplicas: 0},
 	}
 
-	kubeClient := fake.NewClientset(scaledIn)
+	for _, tt := range tests {
-	var reads int
+		t.Run(tt.name, func(t *testing.T) {
-	kubeClient.PrependReactor("get", "deployments", func(action k8stesting.Action) (bool, runtime.Object, error) {
+			t.Parallel()
-		depl := scaledIn.DeepCopy()
-		reads++
-		if reads == 1 {
-			depl.Status.AvailableReplicas = 1
-		}
-		return true, depl, nil
-	})
-	// The fake clientset has no built-in scale subresource for deployments.
-	kubeClient.PrependReactor("update", "deployments", func(action k8stesting.Action) (bool, runtime.Object, error) {
-		if action.GetSubresource() != "scale" {
-			return false, nil, nil
-		}
-		return true, action.(k8stesting.UpdateAction).GetObject(), nil
-	})
 
-	cn := &Container{
+			fn := newTestContainerFunction()
-		logger:                 loggerfactory.GetLogger(),
+			fn.Spec.PodSpec.Containers[0].Ports = []apiv1.ContainerPort{{ContainerPort: 8080}}
-		kubernetesClient:       kubeClient,
+			fn.Spec.InvokeStrategy.ExecutionStrategy.MinScale = 0
-		instanceID:             "test-instance",
+
-		nsResolver:             utils.DefaultNSResolver(),
+			ns := utils.DefaultNSResolver().GetFunctionNS(fn.Namespace)
-		runtimeImagePullPolicy: apiv1.PullIfNotPresent,
+			zero := int32(0)
+			scaledIn := &appsv1.Deployment{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        "ctr-fn",
+					Namespace:   ns,
+					Generation:  generation,
+					Annotations: map[string]string{fv1.EXECUTOR_INSTANCEID_LABEL: "test-instance"},
+				},
+				Spec: appsv1.DeploymentSpec{Replicas: &zero},
+			}
+
+			kubeClient := fake.NewClientset(scaledIn)
+			var reads int
+			kubeClient.PrependReactor("get", "deployments", func(k8stesting.Action) (bool, runtime.Object, error) {
+				depl := scaledIn.DeepCopy()
+				reads++
+				depl.Status = tt.status(reads)
+				return true, depl, nil
+			})
+			// The fake clientset has no built-in scale subresource for deployments.
+			kubeClient.PrependReactor("update", "deployments", func(action k8stesting.Action) (bool, runtime.Object, error) {
+				if action.GetSubresource() != "scale" {
+					return false, nil, nil
+				}
+				// SAFETY: this reactor is registered for "update" and returns early unless the subresource is scale, so the action is an UpdateAction.
+				return true, action.(k8stesting.UpdateAction).GetObject(), nil
+			})
+
+			cn := &Container{
+				logger:                 loggerfactory.GetLogger(),
+				kubernetesClient:       kubeClient,
+				instanceID:             "test-instance",
+				nsResolver:             utils.DefaultNSResolver(),
+				runtimeImagePullPolicy: apiv1.PullIfNotPresent,
+			}
+
+			// The deployment never becomes available in the fake cluster, so a
+			// correct implementation runs the readiness wait out to this deadline.
+			ctx, cancel := context.WithTimeout(t.Context(), 2*time.Second)
+			defer cancel()
+
+			_, err := cn.createOrGetDeployment(ctx, fn, "ctr-fn", map[string]string{"app": "ctr"}, nil, ns)
+			require.Error(t, err, "scale-from-zero must not report success while no replica is available")
+			assert.ErrorIs(t, err, context.DeadlineExceeded,
+				"expected the readiness wait to run to its deadline, got: %v", err)
+		})
 	}
-
-	// The deployment never becomes available in the fake cluster, so a correct
-	// implementation runs the readiness wait out to this deadline.
-	ctx, cancel := context.WithTimeout(t.Context(), 2*time.Second)
-	defer cancel()
-
-	_, err := cn.createOrGetDeployment(ctx, fn, "ctr-fn", map[string]string{"app": "ctr"}, nil, ns)
-	require.Error(t, err, "scale-from-zero must not report success while no replica is available")
-	assert.ErrorIs(t, err, context.DeadlineExceeded,
-		"expected the readiness wait to run to its deadline, got: %v", err)
 }