doc: add multitenant porting guide for upstream upgrade

This commit is contained in:
“Naeel”
2026-05-15 07:49:47 +04:00
parent 5d52c9ba94
commit 728cc351b5
+313
View File
@@ -0,0 +1,313 @@
# Fission Multi-Tenant Porting Guide
> **Purpose:** This document exists so that any AI agent or engineer can fully
> understand what was changed to add multi-tenancy to this Fission fork, why
> each decision was made, and what needs to be ported when a new upstream
> Fission release arrives.
>
> Fork base: `github.com/fission/fission` tag `v1.22.0` (2025-12-16)
> Our branch: `feature/multitenant`
---
## 1. The Problem We Solved
In stock Fission v1.22.0 all resource namespaces must be listed in the
`FISSION_RESOURCE_NAMESPACES` environment variable **before** the process starts.
Adding a new tenant namespace requires:
1. Patching that env var on executor, router, buildermgr deployments
2. Triggering a rolling restart of all three components (~30 s downtime each)
At scale (hundreds of tenants created continuously) this causes a permanent
rolling-restart loop and cascading failures for existing users.
**Our solution:** hot namespace registration without pod restarts. Any platform
(console, operator, CI/CD) creates a Kubernetes Namespace with label
`fission.io/managed=true` — all three Fission components detect it within
milliseconds via k8s Watch and register it live.
---
## 2. Integration Contract (external platforms)
The entire contract between an external platform and Fission is a single label:
```yaml
apiVersion: v1
kind: Namespace
metadata:
name: tenant-abc123
labels:
fission.io/managed: "true"
```
No other coupling to Fission internals is required.
To remove a tenant namespace: delete the namespace or remove the label.
Current removal strategy is `track-only` (the manager records the event but does
not actively deregister — the executor types stop receiving events for deleted
resources naturally). `dispatch-remove` strategy exists in the model but is not
wired by default (see §8).
---
## 3. Backward Compatibility
`FISSION_RESOURCE_NAMESPACES` continues to work exactly as before. Namespaces
listed there are bootstrapped at startup with source `env` and do not require the
label. The NSWatcher layer adds **on top** of the existing mechanism — nothing
was removed.
---
## 4. File Map
### New files (did not exist in v1.22.0)
| File | Purpose | What breaks if removed |
|------|---------|------------------------|
| `pkg/utils/namespace_manager_model.go` | All types: `NamespaceRecord`, `NamespacePhase`, `NamespaceSource`, `NamespaceEvent`, `NamespaceRemovalStrategy`, `ManagedNamespaceWatcherConfig` | Everything — all other files import these types |
| `pkg/utils/namespace_manager.go` | `NamespaceManager` interface + `inMemoryNamespaceManager` implementation. `RunManagedNamespaceWatcher()` — the single entry point used by all three components. `NewNamespaceWatcherEventHandlers()` — k8s informer callbacks. `EnsureNamespaceSA` helper call site. | All NSWatcher functionality |
| `pkg/utils/namespace_manager_test.go` | Unit + integration tests for NamespaceManager | Tests only |
| `pkg/utils/namespace_manager_model_test.go` | Tests for model helpers | Tests only |
| `pkg/utils/serviceaccount.go` (was modified, `EnsureNamespaceSA` added at bottom) | `EnsureNamespaceSA(ctx, client, logger, ns)` — creates fission-fetcher SA/Role/RoleBinding in a new namespace idempotently | Pool pods in new namespaces fail to start (no SA to run fetcher) |
| `pkg/executor/multitenant/ns_watcher.go` | `StartNSWatcher()` — executor entry point. `registerNamespace()` — calls `AddNamespace` on global resolver + `EnsureNamespaceSA` + all executor types. | Executor never learns about new namespaces |
| `pkg/executor/multitenant/namespace_subscriber.go` | `NewNamespaceSubscriber()` — adapter from `NamespaceSubscriber` interface to executor `registerNamespace()` | Same as above |
| `pkg/executor/multitenant/ns_watcher_test.go` | Tests | Tests only |
| `pkg/executor/multitenant/namespace_subscriber_test.go` | Tests | Tests only |
| `pkg/router/ns_watcher.go` | `StartNSWatcher()` — router entry point, 5 lines | Router never learns about new namespaces |
| `pkg/router/namespace_subscriber.go` | `NewNamespaceSubscriber()` — adapter calling `HTTPTriggerSet.AddNamespace` | Same as above |
| `pkg/router/namespace_subscriber_test.go` | Tests | Tests only |
| `pkg/buildermgr/ns_watcher.go` | `StartNSWatcher()` — buildermgr entry point, 5 lines | Buildermgr never learns about new namespaces |
| `pkg/buildermgr/namespace_subscriber.go` | `NewNamespaceSubscriber()` — adapter calling `envWatcher.AddNamespace` + `pkgWatcher.AddNamespace` | Same as above |
| `pkg/buildermgr/namespace_subscriber_test.go` | Tests | Tests only |
| `deploy/multitenant/rbac.yaml` | ClusterRoles + ClusterRoleBindings for all three components (see §6) | Components crash at startup or fail to watch namespaces |
### Modified files (existed in v1.22.0, we changed them)
| File | What we added | What breaks if reverted |
|------|--------------|-------------------------|
| `pkg/utils/namespace.go` | `ManagedNamespaceLabelKey/Value` constants, `ManagedNamespaceLabelSelector()`, `IsManagedNamespace()`, `AddNamespace()` (thread-safe dedup), `Snapshot()` (sorted slice copy under read-lock), `SnapshotWithOptions()` | All callers of `Snapshot()` break — there are many; label constants used by informer filter |
| `pkg/utils/namespace_test.go` | Tests for new methods | Tests only |
| `pkg/utils/informer.go` | `NewSharedInformerFactoryForNamespaces(namespaces []string)` — creates informer factory filtered to a dynamic list of namespaces | Executor types cannot create per-NS informers for new namespaces |
| `pkg/executor/executor.go` | `StartNSWatcher(...)` call added after executor types are initialized | NSWatcher never starts in executor |
| `pkg/executor/executortype/executortype.go` | `AddNamespace(ctx, ns, mgr)` added to the `ExecutorType` interface | All three executor types must implement this; compilation fails |
| `pkg/executor/executortype/poolmgr/gpm.go` | `AddNamespace()` implementation — creates per-NS informer factory, pod lister, event handlers | poolmgr never picks up functions in new namespaces |
| `pkg/executor/executortype/poolmgr/poolpodcontroller.go` | Uses `Snapshot()` in runtime loop instead of static namespace list | Pool pods not created in new namespaces |
| `pkg/executor/executortype/newdeploy/newdeploymgr.go` | `AddNamespace()` implementation | newdeploy never picks up functions in new namespaces |
| `pkg/executor/executortype/container/containermgr.go` | `AddNamespace()` implementation | container executor never picks up functions in new namespaces |
| `pkg/router/router.go` | `StartNSWatcher(...)` call added | NSWatcher never starts in router |
| `pkg/router/httpTriggers.go` | `AddNamespace(ns string)` on `HTTPTriggerSet` — starts per-NS informers for HTTPTriggers and Functions | Router ignores HTTPTriggers in new namespaces |
| `pkg/router/functionReferenceResolver.go` | Uses `Snapshot()` in runtime loop | Router resolves functions only in statically-configured namespaces |
| `pkg/buildermgr/buildermgr.go` | `StartNSWatcher(...)` call added | NSWatcher never starts in buildermgr |
| `pkg/buildermgr/envwatcher.go` | `AddNamespace(ns string)` — starts per-NS Environment informer | Buildermgr ignores Environments in new namespaces |
| `pkg/buildermgr/pkgwatcher.go` | `AddNamespace(ns string)` — starts per-NS Package informer | Buildermgr ignores Packages in new namespaces |
| `pkg/storagesvc/archivePruner.go` | Uses `Snapshot()` instead of static namespace list | Archive pruner only cleans old namespaces |
| `.gitignore` | Added `*.token` | Minor — token files would be committed accidentally |
| `deploy/multitenant/rbac.yaml` | New file (see above) | — |
---
## 5. Data Flow: from label to HTTP 200
```
kubectl label ns tenant-abc123 fission.io/managed=true
k8s API server emits ADDED event on Namespace stream
▼ (within ~50ms)
utils.RunManagedNamespaceWatcher ← informer AddFunc
│ (all 3 components share this function)
NamespaceManager.DispatchAdd(ctx, "tenant-abc123")
├──► executor subscriber:
│ registerNamespace()
│ 1. DefaultNSResolver().AddNamespace("tenant-abc123")
│ 2. EnsureNamespaceSA(ctx, client, logger, "tenant-abc123")
│ └─ creates fission-fetcher SA + Role + RoleBinding
│ 3. poolmgr.AddNamespace("tenant-abc123")
│ └─ creates per-NS InformerFactory, pod lister, handlers
│ 4. newdeploy.AddNamespace("tenant-abc123")
│ 5. container.AddNamespace("tenant-abc123")
├──► router subscriber:
│ HTTPTriggerSet.AddNamespace("tenant-abc123")
│ └─ starts watching HTTPTriggers + Functions in that NS
└──► buildermgr subscriber:
envWatcher.AddNamespace("tenant-abc123")
pkgWatcher.AddNamespace("tenant-abc123")
└─ starts watching Environments + Packages in that NS
User creates: fission env create --namespace tenant-abc123 ...
fission fn create --namespace tenant-abc123 ...
fission httptrigger create --namespace tenant-abc123 ...
Router picks up HTTPTrigger → resolves Function → cold-starts pod in tenant-abc123
HTTP 200 ← function response
```
---
## 6. RBAC Explained
Three ClusterRoles in `deploy/multitenant/rbac.yaml`:
### `fission-executor-ns-watcher` (also for router: `fission-router-ns-watcher`)
```
namespaces: list, watch
```
Without this: informer fails to start with "Forbidden" — components never learn
about new namespaces.
### `fission-executor-sa-provisioner`
```
serviceaccounts: get, list, watch, create, update, patch
events: create
localsubjectaccessreviews: create
roles: get, list, watch, create, update, patch
rolebindings: get, list, watch, create, update, patch
```
Why `events.create`: Kubernetes forbids creating a Role that grants permissions
the caller does not currently hold. Since `fission-fetcher` gets `events.create`
in the Role we create for it, `fission-executor` must hold `events.create` itself
to be allowed to create that Role. This is a k8s RBAC escalation prevention rule.
Why `localsubjectaccessreviews.create`: `EnsureNamespaceSA` calls
`setupSAAndRoleBindings` which first checks if the SA already has each permission
before creating it — that check uses a `LocalSubjectAccessReview`.
---
## 7. Key Design Decisions and Why
### Decision: pub/sub via `NamespaceManager`, not direct calls
**Why not:** simply call `poolmgr.AddNamespace`, `router.AddNamespace` etc.
directly from a shared goroutine.
**Why pub/sub:** executor, router and buildermgr run as separate processes
(different pod). Each process has its own copy of the watcher. Subscribers are
registered in-process. This pattern makes each component fully self-contained and
testable in isolation. No cross-process coupling.
### Decision: `fission.io/managed=true` label as the only trigger
**Why not:** watch all namespaces, or use a CRD, or use annotations.
**Why label:** labels are the idiomatic k8s way to select resources. A label
selector in the informer factory (`fission.io/managed=true`) means the informer
only receives events for labeled namespaces — zero overhead for the hundreds of
system namespaces.
### Decision: `EnsureNamespaceSA` in executor, not in a separate operator
**Why:** the SA must exist before the first pool pod starts. The executor is
already in the hot path — it processes the NS event and then immediately triggers
pod scheduling. Doing it in a separate controller would introduce a race. Doing it
in the executor keeps the lifecycle coupled correctly.
### Decision: `NamespaceRemovalStrategy = track-only` (not `dispatch-remove`)
**Why:** removing a namespace in k8s is already an irreversible event — all
resources inside are cascade-deleted by k8s. The executor types detect the pod
deletions themselves. Dispatching an explicit "remove" to all subscribers would
require each subscriber to implement a cleanup path — added complexity for zero
operational benefit in our use case. Can be switched per-component via
`ManagedNamespaceWatcherConfig.RemovalStrategy`.
### Decision: `AddNamespace` is idempotent (safe to call multiple times)
**Why:** k8s informers can deliver the same event more than once (resync). Every
`AddNamespace` call is a no-op if the namespace is already registered. No locks
held across the whole function — `NamespaceResolver.AddNamespace` uses a write
lock only for the map write, checks for existence first under the same lock.
### Decision: global `NamespaceResolver` (`DefaultNSResolver()`) updated once in executor
**Why:** `DefaultNSResolver().AddNamespace(ns)` is called exactly once in
`registerNamespace()` — before the executor types are called. Each executor type
does NOT call it themselves. This avoids a subtle race: if executor type A adds
the NS to the global resolver first, and executor type B checks the global resolver
as its dedup mechanism, B would see it as already registered and skip — even
though B has not actually processed it yet. The correct dedup is per executor type.
---
## 8. What Is NOT Done (deliberately deferred)
| Missing feature | Why deferred | File/interface to extend |
|----------------|--------------|--------------------------|
| `dispatch-remove` full wiring | Not needed for current use case | `ManagedNamespaceWatcherConfig.RemovalStrategy` — just switch the constant |
| Buildermgr ClusterRole in rbac.yaml | Buildermgr uses the same SA as executor in our deployment; check your setup | Add a third ClusterRole/Binding to `deploy/multitenant/rbac.yaml` |
| Helm chart integration | We apply rbac.yaml manually. A proper Helm chart would include these RBAC objects | `charts/fission-all/templates/` |
| Layer 2 (tenant isolation: per-NS network policy, resource quotas) | Out of scope for Layer 1 | Not started |
| Layer 3 (per-tenant auth, billing hooks) | Out of scope | Not started |
| `NamespaceManager.DispatchRemove` subscriber wiring | Each subscriber has a `RemoveFunc` stub returning nil | Implement per subscriber |
---
## 9. Porting to a New Upstream Version
When `github.com/fission/fission` releases v1.23 or later, follow this order:
1. **Check the upstream changelog** for any changes to:
- `pkg/utils/namespace.go` — if they renamed or refactored `NamespaceResolver`, our `AddNamespace`/`Snapshot` additions need to be reapplied
- `pkg/executor/executortype/executortype.go` — if they changed the `ExecutorType` interface, our `AddNamespace` method needs to be reapplied
- `pkg/router/httpTriggers.go` — if `HTTPTriggerSet` changed, our `AddNamespace` method on it needs to be reapplied
- `pkg/buildermgr/envwatcher.go`, `pkgwatcher.go` — same
- `pkg/utils/informer.go` — if the informer factory pattern changed
2. **Apply in this order** (each depends on the previous):
1. `pkg/utils/namespace_manager_model.go` — pure types, no deps on other changed files
2. `pkg/utils/namespace.go` additions (`AddNamespace`, `Snapshot`, label constants)
3. `pkg/utils/namespace_manager.go` — depends on model + namespace.go
4. `pkg/utils/serviceaccount.go` — add `EnsureNamespaceSA` at the bottom
5. `pkg/utils/informer.go` — add `NewSharedInformerFactoryForNamespaces`
6. `pkg/executor/executortype/executortype.go` — add `AddNamespace` to interface
7. Executor types: `poolmgr/gpm.go`, `newdeploy/newdeploymgr.go`, `container/containermgr.go` — implement `AddNamespace`
8. `pkg/executor/multitenant/` — copy the whole package as-is
9. `pkg/executor/executor.go` — add `StartNSWatcher` call
10. `pkg/router/httpTriggers.go` — add `AddNamespace` method on `HTTPTriggerSet`
11. `pkg/router/namespace_subscriber.go`, `pkg/router/ns_watcher.go` — copy as-is
12. `pkg/router/router.go` — add `StartNSWatcher` call
13. `pkg/buildermgr/envwatcher.go`, `pkgwatcher.go` — add `AddNamespace` method
14. `pkg/buildermgr/namespace_subscriber.go`, `pkg/buildermgr/ns_watcher.go` — copy as-is
15. `pkg/buildermgr/buildermgr.go` — add `StartNSWatcher` call
16. `pkg/storagesvc/archivePruner.go` — replace static namespace list with `Snapshot()`
17. `deploy/multitenant/rbac.yaml` — apply unchanged
3. **Run tests:**
```bash
go test ./pkg/utils/... ./pkg/executor/... ./pkg/router/... ./pkg/buildermgr/...
```
4. **Build and deploy:**
```bash
# on VM:
docker run --rm -v $PWD:/src -w /src golang:1.26-alpine \
sh -c 'go build -o /src/fission-bundle-bin ./cmd/fission-bundle/'
docker build -f Dockerfile.mt -t naeel/fission-bundle:vNEW_TAG .
docker push naeel/fission-bundle:vNEW_TAG
kubectl set image deployment/executor -n fission executor=naeel/fission-bundle:vNEW_TAG
kubectl set image deployment/router -n fission router=naeel/fission-bundle:vNEW_TAG
kubectl set image deployment/buildermgr -n fission buildermgr=naeel/fission-bundle:vNEW_TAG
```
5. **Run e2e test:**
```bash
bash ~/terra/fission/scripts/test_layer1.sh
# Expected: PASS=5 FAIL=0
```
---
## 10. Test Coverage
| Test file | What it covers |
|-----------|---------------|
| `pkg/utils/namespace_test.go` | `AddNamespace` dedup, `Snapshot` sorted output, `IsManagedNamespace` |
| `pkg/utils/namespace_manager_test.go` | `Bootstrap`, `DispatchAdd`/`Remove`/`Resync`, subscriber dispatch, `TestStartManagedNamespaceWatcherIntegration` — full k8s fake informer → subscriber pipeline |
| `pkg/utils/namespace_manager_model_test.go` | Model helpers, `Clone`, `IsActive`, `IsTerminal` |
| `pkg/executor/multitenant/ns_watcher_test.go` | `registerNamespace` with fake k8s client |
| `pkg/executor/multitenant/namespace_subscriber_test.go` | Subscriber adapter |
| `pkg/router/namespace_subscriber_test.go` | Router subscriber adapter |
| `pkg/buildermgr/namespace_subscriber_test.go` | Buildermgr subscriber adapter |
| `~/terra/fission/scripts/test_layer1.sh` | End-to-end: create labeled NS → executor registers it → create env/fn/trigger → call function → HTTP 200 |