Scheduler shutdown returns while a scheduling pass is still running and still writing

[thc1006]

What happened:

Run shuts the scheduler down like this:

go wait.Until(s.worker, time.Second, ctx.Done())
...
<-ctx.Done()
s.queue.ShutDown()

Both halves work. wait.Until stops handing out new work, and #4916 added the ShutDown that lets worker leave its loop. What neither of them reaches is the scheduling pass that was already running when the context was cancelled. Nothing joins the worker goroutine, and everything below doSchedule takes context.TODO(), so Run returns while that pass is still going and still writing.

On the path from doSchedule the calls that ignore the scheduler's context are:

• Algorithm.Schedule at L600, L641, L827 and L868
• the schedule-result patches at L702 and L929
• the status patches at L1051 and L1102

Algorithm.Schedule is where the accurate estimator's gRPC calls happen, so the window is as long as that request is willing to wait, and the patches sit on the far side of it.

What you expected to happen:

Cancelling the scheduler's context should bound what the scheduler still does. Once shutdown has finished I would expect no further writes from it. Today the last pass keeps its own schedule, and whether it finishes before the pod goes away is a matter of timing.

This matters most during a rolling restart. The replacement scheduler is free to pick up the same binding while the outgoing one is still inside a pass that started before it was told to stop, and the older decision can be the one that lands last. I have only reproduced the local half of that, which is the part below.

How to reproduce it (as minimally and precisely as possible):

Against 1c278577e, drop this in pkg/scheduler/ and run it. It reuses mockAlgorithm, fakeBindingLister and setupScheme from scheduler_test.go.

package scheduler

import (
	"context"
	"testing"
	"time"

	corev1 "k8s.io/api/core/v1"
	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
	"k8s.io/client-go/tools/record"
	"k8s.io/client-go/util/workqueue"

	policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1"
	workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2"
	karmadafake "github.com/karmada-io/karmada/pkg/generated/clientset/versioned/fake"
	"github.com/karmada-io/karmada/pkg/scheduler/core"
)

func TestShutdownWithInFlightSchedule(t *testing.T) {
	rb := &workv1alpha2.ResourceBinding{
		ObjectMeta: metav1.ObjectMeta{Name: "test-binding", Namespace: "default"},
		Spec: workv1alpha2.ResourceBindingSpec{
			Placement: &policyv1alpha1.Placement{
				ClusterAffinity: &policyv1alpha1.ClusterAffinity{ClusterNames: []string{"cluster1"}},
			},
		},
	}

	entered := make(chan struct{})
	release := make(chan struct{})
	handed := make(chan context.Context, 1)

	algo := &mockAlgorithm{
		scheduleFunc: func(ctx context.Context, _ *workv1alpha2.ResourceBindingSpec, _ *workv1alpha2.ResourceBindingStatus, _ *core.ScheduleAlgorithmOption) (core.ScheduleResult, error) {
			handed <- ctx
			close(entered)
			<-release
			return core.ScheduleResult{
				SuggestedClusters: []workv1alpha2.TargetCluster{{Name: "cluster1", Replicas: 1}},
			}, nil
		},
	}

	client := karmadafake.NewClientset(rb)
	eb := record.NewBroadcaster()
	s := &Scheduler{
		KarmadaClient: client,
		queue:         workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[any]()),
		bindingLister: &fakeBindingLister{binding: rb},
		Algorithm:     algo,
		eventRecorder: eb.NewRecorder(setupScheme(), corev1.EventSource{Component: "probe"}),
	}

	ctx, cancel := context.WithCancel(context.Background())

	workerDone := make(chan struct{})
	go func() {
		defer close(workerDone)
		s.worker()
	}()

	// Shut down the way Run does.
	runReturned := make(chan struct{})
	go func() {
		defer close(runReturned)
		<-ctx.Done()
		s.queue.ShutDown()
	}()

	s.queue.Add("default/test-binding")
	<-entered

	cancel()
	<-runReturned

	inFlight := <-handed
	select {
	case <-inFlight.Done():
	case <-time.After(time.Second):
		t.Errorf("shutdown finished with a pass still running and its context still live (%v)", inFlight)
	}

	client.ClearActions()
	close(release)
	<-workerDone

	for _, a := range client.Actions() {
		if a.GetVerb() != "get" && a.GetVerb() != "list" && a.GetVerb() != "watch" {
			t.Errorf("pass issued a %s on %s after shutdown returned", a.GetVerb(), a.GetResource().Resource)
		}
	}
}

--- FAIL: TestShutdownWithInFlightSchedule (1.03s)
    shutdown finished with a pass still running and its context still live (context.TODO)
    pass issued a patch on resourcebindings after shutdown returned
    pass issued a patch on resourcebindings after shutdown returned

The two patches are the schedule result and the status, both written after the shutdown path was done. context.TODO in that message is the value Go printed, not a paraphrase.

The probe blocks in the algorithm because that is the easiest place to hold the pass still. A real one is held there by the estimator, or by the API server, for however long those take.

Anything else we need to know?:

Threading the context from Run down through worker, scheduleNext, doSchedule and into the two schedule paths would close it, and Run waiting for the worker before it returns would make the guarantee observable rather than incidental. Which of the two you want, or both, is a call about how much a shutdown is supposed to promise, so I would rather ask than assume.

One thing worth keeping if the context does get threaded: handleErr and legacyHandleErr only Forget on success or on a terminating namespace, so a pass that ends in context.Canceled goes to backoff and stays retriable. That is the behaviour you want across a restart, and it would be easy to lose by adding cancellation to the Forget condition.

I am happy to put together a patch with tests once you have said which shape you prefer.

establishEstimatorConnections also uses context.TODO() at L990, but that runs at startup rather than on the scheduling path, so I have left it out of the above.

Environment:

• Karmada version: main at 1c278577e
• kubectl-karmada or karmadactl version: not applicable, this reproduces in a package test
• Others: go 1.26.5, go test ./pkg/scheduler/