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
Problem
-------
The namespace reconciler (RunReconciler, added previously) retries namespaces
in NamespacePhaseFailed every 30s by calling DispatchResync. But the phase
could never actually reach NamespacePhaseFailed for the executor component
because the executor's NamespaceSubscriber always returned nil — swallowing
any SA-provisioning or informer-init errors. The reconciler was dead code for
the executor path.
Root cause chain
----------------
1. setupSAAndRoleBindings() — void, errors only logged internally.
2. EnsureNamespaceSA() — void, just called setupSAAndRoleBindings.
3. registerNamespace() — void, errors from both functions lost.
4. Executor AddFunc/ResyncFunc — always returned nil to dispatch().
5. dispatch() marks parts Active unconditionally → NamespacePhaseFailed
is never triggered for executor → RunReconciler never fires for executor.
Consequence: if EnsureNamespaceSA failed (transient k8s 503, RBAC webhook
timeout, etc.) the namespace appeared Active in the manager but the fetcher
ServiceAccount was missing. Pool pods would CrashLoopBackOff on every call
to that namespace until a full process restart.
Changes
-------
pkg/utils/serviceaccount.go
- setupSAAndRoleBindings: void → error. Returns the first k8s API error
so callers can decide whether to retry.
- runSACheck: ignores the error with _ = (same behaviour as before, it's
a periodic background loop that already logs internally).
- EnsureNamespaceSA: void → error, propagates setupSAAndRoleBindings.
Updated godoc to explain the retry contract.
pkg/executor/multitenant/ns_watcher.go
- registerNamespace: void → error.
* EnsureNamespaceSA error → wrapped as 'EnsureNamespaceSA: ...' and returned.
* registerExecutorTypes error → wrapped as 'registerExecutorTypes: ...' and returned.
* Success log line only emitted when both succeed.
- Added 'fmt' import for error wrapping.
pkg/executor/multitenant/namespace_subscriber.go
- AddFunc: return registerNamespace(...) instead of ignoring its error.
- ResyncFunc: same — plus a comment explaining why it is safe to call
registerNamespace again (SA creation is idempotent, executor-type
AddNamespace guards against duplicate informer creation).
pkg/utils/namespace_manager.go
- RunReconciler interface signature: added *zap.Logger parameter.
Callers pass the component logger so retries are visible in prod logs.
- RunReconciler implementation:
* Accepts logger; falls back to zap.NewNop() if nil.
* Skips the tick entirely when no failed namespaces are found (no log spam).
* Logs 'retrying failed namespaces' with count + list when found.
* Logs per-namespace 'dispatching resync'.
* Logs 'resync succeeded' or 'resync still failing, will retry' with error.
- RunManagedNamespaceWatcher: passes logger to RunReconciler.
End-to-end flow after this fix
-------------------------------
1. EnsureNamespaceSA fails (k8s 503).
2. registerNamespace returns error.
3. Executor AddFunc returns error.
4. dispatch() calls MarkPartFailed("executor") → deriveNamespacePhase →
NamespacePhaseFailed.
5. RunReconciler tick (30s) finds the namespace → DispatchResync →
registerNamespace called again → EnsureNamespaceSA (idempotent) →
if API recovered: success → MarkPartActive → NamespacePhaseActive.
6. Log line 'namespace reconciler: resync succeeded' confirms recovery.
Backward compatibility
----------------------
- NamespaceManager interface: RunReconciler gained a *zap.Logger param.
There is exactly one implementation (inMemoryNamespaceManager) and one
call site (RunManagedNamespaceWatcher). No external mocks.
- EnsureNamespaceSA: callers outside this codebase (if any) that ignore
the error will still compile (Go allows ignoring return values).
- All 26 affected tests pass: go test ./pkg/utils/... ./pkg/executor/...
./pkg/buildermgr/... ./pkg/router/...
- 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
* Update Go version to 1.24
* Update golangci-lint version
* Add envtest to tool
* Add dashboard linter as a tool
* Uset t.Cleanup
---------
Signed-off-by: Sanket Sudake <sanketsudake@gmail.com>
The bug breaks the poolmgr service which stops the deletion and creation of new environments.
Signed-off-by: Md Soharab Ansari <soharab.ansari@infracloud.io>
* Add DISABLE_OWNER_REFERENCES env variable to executor and buildermgr deployment.
Use this env var to decide adding ownerReferences to K8s resources created by fission CRD.
* Resolve review comments
* Fix lint failure
---------
Signed-off-by: Md Soharab Ansari <soharab.ansari@infracloud.io>
* Poolmanager deployment is created based on environment.
Set environment as owner to poolmanager deployment.
Signed-off-by: Md Soharab Ansari <soharab.ansari@infracloud.io>
* Set OwnerReferences to K8s resources created by fission resources.
```
Set OwnerReferences to deployment, service and HPA created by newdeploy function.
Set OwnerReferences to builderManager deployment and service created by environment.
Set OwnerReferences to deployment, service and HPA created by container function.
```
Signed-off-by: Md Soharab Ansari <soharab.ansari@infracloud.io>
* Use ControllerRef
Signed-off-by: Md Soharab Ansari <soharab.ansari@infracloud.io>
---------
Signed-off-by: Md Soharab Ansari <soharab.ansari@infracloud.io>
Making typed common cache so that we don't use wrong types
across set/get methods and more higher-level methods can be
defined for cache.
Currently, we are not able to operate over all keys of the cache
due to generic types.
I also removed code comments around the cache.
Signed-off-by: Sanket Sudake <sanketsudake@gmail.com>
* used manager's Add function in more places
* exit when ctx.Done is received in archivePruner go routines
* fix manager tests
* fix data race
* added more gpm function in manager and removed manager from a util function
* closed unused channel and stopped ticker after context is done
* added log statements
* used context.Done inside function instead of stopper channel
- added manager to wait for all go routines to end before exit
- code refactor
- renamed Manafer to Interface and GoRoutineManager to GroupManager
- replaced some go routine calls with manager Add func
- added unit tests for manager
“Naeel”