From 22d672c57e79086501f65250142c3cb5760ce8f3 Mon Sep 17 00:00:00 2001 From: DualFroz Date: Fri, 11 Sep 2026 23:07:10 +0200 Subject: [PATCH] container: wait for the replica after scaling a function in from zero (#3183) createOrGetDeployment read the Deployment once and then reused that copy's Status.AvailableReplicas to decide whether to wait for readiness. On the scale-from-zero path the read happens while the idle reaper's pod is still terminating, so the pre-scale status can still report an available replica and the readiness wait is skipped. The executor then returns a Service whose endpoints are empty and the router's forward fails with a 500. Track whether we issued the scale-up and always wait for the deployment in that case; WaitForDeployment re-reads from the API server, so an already available deployment still returns on its first poll. --- .../executortype/container/deployment.go | 6 +- .../executortype/container/deployment_test.go | 62 +++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/pkg/executor/executortype/container/deployment.go b/pkg/executor/executortype/container/deployment.go index a4307b0..92feeb4 100644 --- a/pkg/executor/executortype/container/deployment.go +++ b/pkg/executor/executortype/container/deployment.go @@ -87,14 +87,18 @@ func (cn *Container) createOrGetDeployment(ctx context.Context, fn *fv1.Function return existingDepl, nil } + // existingDepl was read before the scale request, and on the scale-from-zero path + // its status can still count the pod the idle reaper is tearing down. + scaledUp := false if *existingDepl.Spec.Replicas < minScale { err = util.ScaleDeployment(ctx, cn.kubernetesClient, cn.logger, existingDepl.Namespace, existingDepl.Name, minScale) if err != nil { logger.Error(err, "error scaling up function deployment", "function", fn.Name) return nil, err } + scaledUp = true } - if existingDepl.Status.AvailableReplicas < minScale { + if scaledUp || existingDepl.Status.AvailableReplicas < minScale { existingDepl, err = util.WaitForDeployment(ctx, cn.kubernetesClient, cn.logger, existingDepl, minScale, specializationTimeout) } diff --git a/pkg/executor/executortype/container/deployment_test.go b/pkg/executor/executortype/container/deployment_test.go index d298c95..4435d5b 100644 --- a/pkg/executor/executortype/container/deployment_test.go +++ b/pkg/executor/executortype/container/deployment_test.go @@ -14,7 +14,9 @@ import ( appsv1 "k8s.io/api/apps/v1" apiv1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/fake" + k8stesting "k8s.io/client-go/testing" fv1 "github.com/fission/fission/pkg/apis/core/v1" ferror "github.com/fission/fission/pkg/error" @@ -306,3 +308,63 @@ func TestContainerGetResources(t *testing.T) { assert.NotNil(t, res.Requests) assert.NotNil(t, res.Limits) } + +// TestContainerScaleFromZeroWaitsForAvailability covers the scale-to-zero race +// reported in #3183. The first read reports one available replica (the pod the +// idle reaper is tearing down) and every later read reports none, so a caller +// that trusts the pre-scale status reports success with no serving pod. +func TestContainerScaleFromZeroWaitsForAvailability(t *testing.T) { + t.Parallel() + + fn := newTestContainerFunction() + fn.Spec.PodSpec.Containers[0].Ports = []apiv1.ContainerPort{{ContainerPort: 8080}} + fn.Spec.InvokeStrategy.ExecutionStrategy.MinScale = 0 + + ns := utils.DefaultNSResolver().GetFunctionNS(fn.Namespace) + zero := int32(0) + scaledIn := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ctr-fn", + Namespace: ns, + Annotations: map[string]string{fv1.EXECUTOR_INSTANCEID_LABEL: "test-instance"}, + }, + Spec: appsv1.DeploymentSpec{Replicas: &zero}, + Status: appsv1.DeploymentStatus{AvailableReplicas: 0}, + } + + kubeClient := fake.NewClientset(scaledIn) + var reads int + kubeClient.PrependReactor("get", "deployments", func(action k8stesting.Action) (bool, runtime.Object, error) { + 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{ + 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) +}