Skip to content

Manager startup spins after cancellation before cache readiness #3629

Open

Description

@thomhurst

Manager.Start can spin indefinitely when cancellation arrives before its cache readiness check succeeds. The wait in runnableGroup.Start handles ctx.Done() by recording a non-cancellation error, but never returns. If the cache readiness check returns false, that runnable never sends a readiness signal and the startup queue never empties. The deferred manager stop procedure is never reached.

This is reproducible with a real informer cache and no running API server. It also reproduces with a cache whose WaitForCacheSync explicitly returns false after cancellation. Both cases fail 3/3 with v0.25.0. A local source experiment that returns from the cancellation branch, removes the readiness-channel close, and makes readiness sends cancellation-aware passes both cases 3/3. The latter two changes prevent a late readiness callback from sending to a closed channel or blocking forever after startup returns.

The same cancellation loop is present in v0.25.2. The stack remains inside (*runnableGroup).Start.func1, called by (*controllerManager).Start at its cache startup step. Cancellation should let startup unwind and run the existing shutdown procedure, while preserving deadline and runnable errors.

Minimal reproduction (place in a Go module that requires sigs.k8s.io/controller-runtime v0.25.0 and matching Kubernetes dependencies, then run go test -v):

package cancellation_test

import (
    "context"
    "net/http"
    "testing"
    "time"

    "github.com/go-logr/logr"
    corev1 "k8s.io/api/core/v1"
    "k8s.io/apimachinery/pkg/api/meta"
    "k8s.io/apimachinery/pkg/runtime"
    "k8s.io/apimachinery/pkg/runtime/schema"
    "k8s.io/client-go/rest"
    "sigs.k8s.io/controller-runtime/pkg/cache"
    "sigs.k8s.io/controller-runtime/pkg/manager"
    metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
)

func TestCancellationWithRealCache(t *testing.T) {
    ctx, cancel := context.WithCancel(context.Background())
    defer cancel()
    scheme := runtime.NewScheme()
    if err := corev1.AddToScheme(scheme); err != nil { t.Fatal(err) }
    mapper := meta.NewDefaultRESTMapper([]schema.GroupVersion{corev1.SchemeGroupVersion})
    mapper.Add(corev1.SchemeGroupVersion.WithKind("ConfigMap"), meta.RESTScopeNamespace)
    opts := manager.Options{
        Scheme: scheme,
        LeaderElection: false,
        BaseContext: func() context.Context { return ctx },
        Logger: logr.Discard(),
        Metrics: metricsserver.Options{BindAddress: "0"},
        MapperProvider: func(*rest.Config, *http.Client) (meta.RESTMapper, error) {
            return mapper, nil
        },
    }
    mgr, err := manager.New(&rest.Config{Host: "http://127.0.0.1:1"}, opts)
    if err != nil { t.Fatal(err) }
    _, err = mgr.GetCache().GetInformer(ctx, &corev1.ConfigMap{}, cache.BlockUntilSynced(false))
    if err != nil { t.Fatal(err) }
    cancel()
    done := make(chan error, 1)
    go func() { done <- mgr.Start(ctx) }()
    select {
    case err := <-done:
        if err != nil { t.Fatal(err) }
    case <-time.After(time.Second):
        t.Fatal("Manager.Start did not finish after cancellation with a real informer cache")
    }
}

The one-second bound is only a regression guard. Increasing a manager shutdown deadline cannot resolve the infinite startup wait. No controller shutdown deadline, error filtering, or process termination change is needed to reproduce this.

Following CONTRIBUTING.md, reporting the proposed change here before opening a PR. Is returning from the startup cancellation branch with cancellation-aware readiness delivery the intended fix?

Activity

  1. huong8373tt-beep commented on Oct 31, 2026

    @huong8373tt-beep
    Contributor

    I agree with the proposed fix direction: return from the startup cancellation branch while preserving non-cancellation errors, and make readiness delivery cancellation-aware so a late callback cannot block or send to a closed channel. I’ll prepare a focused regression test and implementation on that basis, without changing shutdown deadlines or error filtering. I’ll wait for maintainer confirmation before opening a PR, per the contribution guidelines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions