fix(namespace): executor/router/buildermgr RemoveNamespace + per-NS informer lifecycle
- Add RemoveNamespace(ctx, ns) to executortype.ExecutorType interface - Implement RemoveNamespace in poolmgr, newdeploy, container executor types - Add per-namespace context cancellation (nsCancels map) in all three types so informer factories are stopped when namespace is removed (fixes goroutine leak) - Add PoolPodController.RemoveNamespace to clear envLister/podLister maps - Add deregisterNamespace() in executor multitenant subscriber - Switch executor/router/buildermgr watcher strategy from TrackOnly to DispatchRemove so RemoveFunc is called when fission.io/managed label is removed - Add RemoveFunc to executor/router/buildermgr namespace subscribers - Add RemoveNamespace to environmentWatcher and packageWatcher with per-NS cancel - Add RemoveNamespace to HTTPTriggerSet: cancels informers, removes from maps, calls syncTriggers - Fix ns_watcher_test.go fakeExecutorType to implement new RemoveNamespace method Fixes: - Executor dedup gap: re-added namespace was silently skipped (envLister/deplLister still present) - Goroutine/FD leak: old informer factories ran forever after namespace removal - Router stale routes: HTTPTriggers for removed namespace stayed in routing table
This commit is contained in:
@@ -90,6 +90,10 @@ type (
|
||||
objectReaperIntervalSecond time.Duration
|
||||
|
||||
enableOwnerReferences bool
|
||||
|
||||
// nsCancels holds per-namespace context cancel functions so informer
|
||||
// factories started in AddNamespace can be stopped on RemoveNamespace.
|
||||
nsCancels map[string]context.CancelFunc
|
||||
}
|
||||
)
|
||||
|
||||
@@ -132,8 +136,7 @@ func MakeContainer(
|
||||
deplLister: make(map[string]appslisters.DeploymentLister),
|
||||
deplListerSynced: make(map[string]k8sCache.InformerSynced),
|
||||
svcLister: make(map[string]corelisters.ServiceLister),
|
||||
svcListerSynced: make(map[string]k8sCache.InformerSynced),
|
||||
|
||||
svcListerSynced: make(map[string]k8sCache.InformerSynced), nsCancels: make(map[string]context.CancelFunc),
|
||||
enableOwnerReferences: utils.IsOwnerReferencesEnabled(),
|
||||
}
|
||||
|
||||
@@ -833,9 +836,36 @@ func (caaf *Container) AddNamespace(ctx context.Context, ns string, mgr manager.
|
||||
return fmt.Errorf("AddNamespace %s (container): add function handler: %w", ns, err)
|
||||
}
|
||||
|
||||
finformer.Start(ctx.Done())
|
||||
cnmInformer.Start(ctx.Done())
|
||||
// Create a per-namespace cancellable context so RemoveNamespace can stop
|
||||
// these specific informer factories without affecting the whole process.
|
||||
nsCtx, nsCancel := context.WithCancel(ctx)
|
||||
caaf.nsCancels[ns] = nsCancel
|
||||
|
||||
finformer.Start(nsCtx.Done())
|
||||
cnmInformer.Start(nsCtx.Done())
|
||||
|
||||
caaf.logger.Info("AddNamespace: done (container)", zap.String("namespace", ns))
|
||||
return nil
|
||||
}
|
||||
|
||||
// RemoveNamespace deregisters a namespace from the container executor.
|
||||
// Cancels the per-namespace informer context and clears all lister maps so that
|
||||
// a subsequent AddNamespace call will re-register the namespace correctly.
|
||||
func (caaf *Container) RemoveNamespace(ctx context.Context, ns string) error {
|
||||
if ns == "" {
|
||||
return nil
|
||||
}
|
||||
caaf.logger.Info("RemoveNamespace: cleaning up namespace (container)", zap.String("namespace", ns))
|
||||
|
||||
if cancel, ok := caaf.nsCancels[ns]; ok {
|
||||
cancel()
|
||||
delete(caaf.nsCancels, ns)
|
||||
}
|
||||
|
||||
delete(caaf.deplLister, ns)
|
||||
delete(caaf.deplListerSynced, ns)
|
||||
delete(caaf.svcLister, ns)
|
||||
delete(caaf.svcListerSynced, ns)
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -74,4 +74,9 @@ type ExecutorType interface {
|
||||
// starts watching Fission CRDs and K8s resources in it without a pod restart.
|
||||
// Called when a Namespace with label fission.io/managed=true appears.
|
||||
AddNamespace(ctx context.Context, ns string, mgr manager.Interface) error
|
||||
|
||||
// RemoveNamespace deregisters a namespace from the executor, cancelling its
|
||||
// informer goroutines and clearing dedup state so that a re-add works correctly.
|
||||
// Called when a Namespace with label fission.io/managed=true is removed.
|
||||
RemoveNamespace(ctx context.Context, ns string) error
|
||||
}
|
||||
|
||||
@@ -94,6 +94,10 @@ type (
|
||||
objectReaperIntervalSecond time.Duration
|
||||
|
||||
enableOwnerReferences bool
|
||||
|
||||
// nsCancels holds per-namespace context cancel functions so informer
|
||||
// factories started in AddNamespace can be stopped on RemoveNamespace.
|
||||
nsCancels map[string]context.CancelFunc
|
||||
}
|
||||
)
|
||||
|
||||
@@ -140,8 +144,7 @@ func MakeNewDeploy(
|
||||
deplLister: make(map[string]appslisters.DeploymentLister),
|
||||
deplListerSynced: make(map[string]k8sCache.InformerSynced),
|
||||
svcLister: make(map[string]corelisters.ServiceLister),
|
||||
svcListerSynced: make(map[string]k8sCache.InformerSynced),
|
||||
|
||||
svcListerSynced: make(map[string]k8sCache.InformerSynced), nsCancels: make(map[string]context.CancelFunc),
|
||||
enableOwnerReferences: utils.IsOwnerReferencesEnabled(),
|
||||
}
|
||||
|
||||
@@ -949,9 +952,36 @@ func (deploy *NewDeploy) AddNamespace(ctx context.Context, ns string, mgr manage
|
||||
return fmt.Errorf("AddNamespace %s (newdeploy): add environment handler: %w", ns, err)
|
||||
}
|
||||
|
||||
finformer.Start(ctx.Done())
|
||||
ndmInformer.Start(ctx.Done())
|
||||
// Create a per-namespace cancellable context so RemoveNamespace can stop
|
||||
// these specific informer factories without affecting the whole process.
|
||||
nsCtx, nsCancel := context.WithCancel(ctx)
|
||||
deploy.nsCancels[ns] = nsCancel
|
||||
|
||||
finformer.Start(nsCtx.Done())
|
||||
ndmInformer.Start(nsCtx.Done())
|
||||
|
||||
deploy.logger.Info("AddNamespace: done (newdeploy)", zap.String("namespace", ns))
|
||||
return nil
|
||||
}
|
||||
|
||||
// RemoveNamespace deregisters a namespace from the newdeploy executor.
|
||||
// Cancels the per-namespace informer context and clears all lister maps so that
|
||||
// a subsequent AddNamespace call will re-register the namespace correctly.
|
||||
func (deploy *NewDeploy) RemoveNamespace(ctx context.Context, ns string) error {
|
||||
if ns == "" {
|
||||
return nil
|
||||
}
|
||||
deploy.logger.Info("RemoveNamespace: cleaning up namespace (newdeploy)", zap.String("namespace", ns))
|
||||
|
||||
if cancel, ok := deploy.nsCancels[ns]; ok {
|
||||
cancel()
|
||||
delete(deploy.nsCancels, ns)
|
||||
}
|
||||
|
||||
delete(deploy.deplLister, ns)
|
||||
delete(deploy.deplListerSynced, ns)
|
||||
delete(deploy.svcLister, ns)
|
||||
delete(deploy.svcListerSynced, ns)
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -98,6 +98,10 @@ type (
|
||||
|
||||
podSpecPatch *apiv1.PodSpec
|
||||
objectReaperIntervalSecond time.Duration
|
||||
|
||||
// nsCancels holds per-namespace context cancel functions so informer
|
||||
// factories started in AddNamespace can be stopped on RemoveNamespace.
|
||||
nsCancels map[string]context.CancelFunc
|
||||
}
|
||||
request struct {
|
||||
requestType
|
||||
@@ -159,6 +163,7 @@ func MakeGenericPoolManager(ctx context.Context,
|
||||
objectReaperIntervalSecond: time.Duration(executorUtils.GetObjectReaperInterval(logger, fv1.ExecutorTypePoolmgr, 5)) * time.Second,
|
||||
podLister: make(map[string]corelisters.PodLister),
|
||||
podListerSynced: make(map[string]k8sCache.InformerSynced),
|
||||
nsCancels: make(map[string]context.CancelFunc),
|
||||
}
|
||||
for ns, informerFactory := range gpmInformerFactory {
|
||||
gpm.podLister[ns] = informerFactory.Core().V1().Pods().Lister()
|
||||
@@ -833,10 +838,40 @@ func (gpm *GenericPoolManager) AddNamespace(ctx context.Context, ns string, mgr
|
||||
return fmt.Errorf("AddNamespace %s: register informers: %w", ns, err)
|
||||
}
|
||||
|
||||
// Create a per-namespace cancellable context so RemoveNamespace can stop
|
||||
// these specific informer factories without affecting the whole process.
|
||||
nsCtx, nsCancel := context.WithCancel(ctx)
|
||||
gpm.nsCancels[ns] = nsCancel
|
||||
|
||||
// Start the factories — they will begin syncing immediately.
|
||||
finformer.Start(ctx.Done())
|
||||
gpmInformer.Start(ctx.Done())
|
||||
finformer.Start(nsCtx.Done())
|
||||
gpmInformer.Start(nsCtx.Done())
|
||||
|
||||
gpm.logger.Info("AddNamespace: informers started for namespace", zap.String("namespace", ns))
|
||||
return nil
|
||||
}
|
||||
|
||||
// RemoveNamespace deregisters a namespace from the poolmgr executor.
|
||||
// Cancels the per-namespace informer context and clears all lister maps so that
|
||||
// a subsequent AddNamespace call will re-register the namespace correctly.
|
||||
func (gpm *GenericPoolManager) RemoveNamespace(ctx context.Context, ns string) error {
|
||||
if ns == "" {
|
||||
return nil
|
||||
}
|
||||
gpm.logger.Info("RemoveNamespace: cleaning up namespace (poolmgr)", zap.String("namespace", ns))
|
||||
|
||||
// Stop informer factories for this namespace.
|
||||
if cancel, ok := gpm.nsCancels[ns]; ok {
|
||||
cancel()
|
||||
delete(gpm.nsCancels, ns)
|
||||
}
|
||||
|
||||
// Clear gpm-level lister maps so dedup passes on next AddNamespace.
|
||||
delete(gpm.podLister, ns)
|
||||
delete(gpm.podListerSynced, ns)
|
||||
|
||||
// Clear PoolPodController lister maps.
|
||||
gpm.poolPodC.RemoveNamespace(ns)
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -528,3 +528,14 @@ func (p *PoolPodController) AddNamespaceInformers(
|
||||
p.logger.Info("AddNamespaceInformers: registered informers for namespace", zap.String("namespace", ns))
|
||||
return nil
|
||||
}
|
||||
|
||||
// RemoveNamespace clears all per-namespace lister state in the PoolPodController.
|
||||
// Called from GenericPoolManager.RemoveNamespace so the dedup check in AddNamespace
|
||||
// will pass if the namespace is re-added later.
|
||||
func (p *PoolPodController) RemoveNamespace(ns string) {
|
||||
delete(p.envLister, ns)
|
||||
delete(p.envListerSynced, ns)
|
||||
delete(p.podLister, ns)
|
||||
delete(p.podListerSynced, ns)
|
||||
p.logger.Info("PoolPodController.RemoveNamespace: cleared lister state", zap.String("namespace", ns))
|
||||
}
|
||||
|
||||
@@ -24,6 +24,9 @@ func NewNamespaceSubscriber(
|
||||
registerNamespace(ctx, logger, kubernetesClient, record.Name, executorTypes, mgr)
|
||||
return nil
|
||||
},
|
||||
RemoveFunc: func(ctx context.Context, record utils.NamespaceRecord) error {
|
||||
return deregisterNamespace(ctx, logger, record.Name, executorTypes)
|
||||
},
|
||||
ResyncFunc: func(ctx context.Context, record utils.NamespaceRecord) error {
|
||||
registerNamespace(ctx, logger, kubernetesClient, record.Name, executorTypes, mgr)
|
||||
return nil
|
||||
|
||||
@@ -87,7 +87,9 @@ func StartNSWatcher(
|
||||
executorTypes map[fv1.ExecutorType]executortype.ExecutorType,
|
||||
mgr manager.Interface,
|
||||
) {
|
||||
_, err := utils.RunManagedNamespaceWatcher(ctx, logger, kubernetesClient, mgr, utils.NewDefaultManagedNamespaceWatcherConfig("multitenant.NSWatcher", NewNamespaceSubscriber(logger, kubernetesClient, executorTypes, mgr)))
|
||||
config := utils.NewDefaultManagedNamespaceWatcherConfig("multitenant.NSWatcher", NewNamespaceSubscriber(logger, kubernetesClient, executorTypes, mgr))
|
||||
config.RemovalStrategy = utils.NamespaceRemovalStrategyDispatchRemove
|
||||
_, err := utils.RunManagedNamespaceWatcher(ctx, logger, kubernetesClient, mgr, config)
|
||||
if err != nil {
|
||||
logger.Error("multitenant.NSWatcher: BootstrapAndDispatch failed", zap.Error(err))
|
||||
}
|
||||
@@ -135,3 +137,26 @@ func registerExecutorTypes(
|
||||
}
|
||||
return joinErr
|
||||
}
|
||||
|
||||
// deregisterNamespace calls RemoveNamespace on every executor type for the given namespace.
|
||||
// Called when a Namespace with label fission.io/managed=true is removed.
|
||||
func deregisterNamespace(
|
||||
ctx context.Context,
|
||||
logger *zap.Logger,
|
||||
ns string,
|
||||
executorTypes map[fv1.ExecutorType]executortype.ExecutorType,
|
||||
) error {
|
||||
var joinErr error
|
||||
|
||||
for _, et := range executorTypes {
|
||||
if err := et.RemoveNamespace(ctx, ns); err != nil {
|
||||
logger.Error("multitenant.NSWatcher: RemoveNamespace failed",
|
||||
zap.String("namespace", ns),
|
||||
zap.Error(err),
|
||||
)
|
||||
joinErr = errors.Join(joinErr, err)
|
||||
}
|
||||
}
|
||||
logger.Info("multitenant.NSWatcher: deregistered namespace", zap.String("namespace", ns))
|
||||
return joinErr
|
||||
}
|
||||
|
||||
@@ -46,6 +46,7 @@ func (f *fakeExecutorType) AddNamespace(ctx context.Context, ns string, mgr mana
|
||||
f.lastNamespace = ns
|
||||
return f.addErr
|
||||
}
|
||||
func (f *fakeExecutorType) RemoveNamespace(ctx context.Context, ns string) error { return nil }
|
||||
|
||||
var _ executortype.ExecutorType = (*fakeExecutorType)(nil)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user