fix(executor): P2 — pre-register managed NS before adopt/cleanup (closes AdoptExistingResources race)
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
This commit is contained in:
@@ -347,6 +347,11 @@ func StartExecutor(ctx context.Context, clientGen crd.ClientGeneratorInterface,
|
|||||||
executorTypes[ndm.GetTypeName(ctx)] = ndm
|
executorTypes[ndm.GetTypeName(ctx)] = ndm
|
||||||
executorTypes[cnm.GetTypeName(ctx)] = cnm
|
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"))
|
adoptExistingResources, _ := strconv.ParseBool(os.Getenv("ADOPT_EXISTING_RESOURCES"))
|
||||||
|
|
||||||
wg := &sync.WaitGroup{}
|
wg := &sync.WaitGroup{}
|
||||||
|
|||||||
@@ -65,6 +65,7 @@ import (
|
|||||||
"fmt"
|
"fmt"
|
||||||
|
|
||||||
"go.uber.org/zap"
|
"go.uber.org/zap"
|
||||||
|
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||||
"k8s.io/client-go/kubernetes"
|
"k8s.io/client-go/kubernetes"
|
||||||
|
|
||||||
fv1 "github.com/fission/fission/pkg/apis/core/v1"
|
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))
|
logger.Info("multitenant.NSWatcher: deregistered namespace", zap.String("namespace", ns))
|
||||||
return joinErr
|
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)))
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user