From 2a7d6101b1f5e5625e92d8e1f636dc04f3e1f16c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9CNaeel=E2=80=9D?= Date: Mon, 18 May 2026 11:42:48 +0400 Subject: [PATCH] =?UTF-8?q?fix(executor):=20P2=20=E2=80=94=20pre-register?= =?UTF-8?q?=20managed=20NS=20before=20adopt/cleanup=20(closes=20AdoptExist?= =?UTF-8?q?ingResources=20race)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem: executor starts → AdoptExistingResources + CleanupOldExecutorObjects run against utils.DefaultNSResolver().Snapshot() which returns ONLY static NS from FISSION_RESOURCE_NAMESPACES. Managed (labeled) namespaces are registered later, asynchronously, by StartNSWatcher. Result: - Pods from a previous executor in managed NS are never adopted (no instanceID patch) → poolmgr creates new pool pods → cold start for first request after executor restart. - Old executor objects (RS/deployments) in managed NS accumulate without being cleaned up (resource leak). Fix: Add multitenant.PreRegisterManagedNamespaces(ctx, logger, kubernetesClient) called synchronously in executor.go BEFORE the adopt/cleanup goroutines. The function does a single Namespaces.List with label fission.io/managed=true and calls DefaultNSResolver().AddNamespace() for each result. This is idempotent with the later watcher AddFunc calls. Failure is non-fatal: a warning is logged and startup proceeds with static NS only (safe degraded mode). After this call DefaultNSResolver().Snapshot() includes managed NS, so: - AdoptExistingResources patches old pods in managed NS with new instanceID - CleanupOldExecutorObjects removes stale objects from managed NS - GetReaperNamespace() returns the full tenant NS set Files: pkg/executor/multitenant/ns_watcher.go — PreRegisterManagedNamespaces() pkg/executor/executor.go — call before adopt/cleanup --- pkg/executor/executor.go | 5 +++++ pkg/executor/multitenant/ns_watcher.go | 27 ++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/pkg/executor/executor.go b/pkg/executor/executor.go index ea65610b..e076a747 100644 --- a/pkg/executor/executor.go +++ b/pkg/executor/executor.go @@ -347,6 +347,11 @@ func StartExecutor(ctx context.Context, clientGen crd.ClientGeneratorInterface, executorTypes[ndm.GetTypeName(ctx)] = ndm executorTypes[cnm.GetTypeName(ctx)] = cnm + // Pre-populate DefaultNSResolver with managed (labeled) namespaces so that + // AdoptExistingResources and CleanupOldExecutorObjects cover the full tenant NS set. + // Must run before the adopt/cleanup goroutines below. Non-fatal on error. + multitenant.PreRegisterManagedNamespaces(ctx, logger, kubernetesClient) + adoptExistingResources, _ := strconv.ParseBool(os.Getenv("ADOPT_EXISTING_RESOURCES")) wg := &sync.WaitGroup{} diff --git a/pkg/executor/multitenant/ns_watcher.go b/pkg/executor/multitenant/ns_watcher.go index b1544ede..d11a485b 100644 --- a/pkg/executor/multitenant/ns_watcher.go +++ b/pkg/executor/multitenant/ns_watcher.go @@ -65,6 +65,7 @@ import ( "fmt" "go.uber.org/zap" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" fv1 "github.com/fission/fission/pkg/apis/core/v1" @@ -172,3 +173,29 @@ func deregisterNamespace( logger.Info("multitenant.NSWatcher: deregistered namespace", zap.String("namespace", ns)) return joinErr } + +// PreRegisterManagedNamespaces does a one-time synchronous List of all Namespaces +// labeled fission.io/managed=true and adds them to the global DefaultNSResolver. +// +// Called at executor startup BEFORE AdoptExistingResources and CleanupOldExecutorObjects +// so that adopt and cleanup cover managed (dynamic) namespaces, not just static ones +// from FISSION_RESOURCE_NAMESPACES. This closes the P2 race where old executor pods +// in managed NS were never adopted (causing unnecessary cold starts) and never cleaned +// (causing orphaned pod accumulation). +// +// Failure is non-fatal: a warning is logged and the executor proceeds with static NS only. +func PreRegisterManagedNamespaces(ctx context.Context, logger *zap.Logger, client kubernetes.Interface) { + nsList, err := client.CoreV1().Namespaces().List(ctx, metav1.ListOptions{ + LabelSelector: utils.ManagedNamespaceLabelSelector(), + }) + if err != nil { + logger.Warn("PreRegisterManagedNamespaces: failed to list managed namespaces; adopt/cleanup will use static NS only", + zap.Error(err)) + return + } + for i := range nsList.Items { + utils.DefaultNSResolver().AddNamespace(nsList.Items[i].Name) + } + logger.Info("PreRegisterManagedNamespaces: pre-registered managed namespaces", + zap.Int("count", len(nsList.Items))) +}