doc: add detailed namespace rewrite logic

2026-04-26 09:50:06 +03:00
parent 87477d4529
commit 1f53bc1fb7
1 changed files with 589 additions and 0 deletions
@@ -0,0 +1,589 @@
 # 2026-04-26 — Layer 1 namespace rewrite: подробная логика правок
 ## Зачем этот документ
 Нужен не просто список коммитов, а объяснение инженерной логики:
 - что именно было не так в коде;
 - почему исправление выбрано именно таким;
 - почему изменения разбиты на маленькие шаги;
 - какие инварианты я старался сохранить;
 - что уже исправлено, а что еще нет.
 Этот документ описывает серию маленьких безопасных шагов в ветке
 `rewrite/layer1-namespace-manager-step1`.
 Основной принцип серии:
 1. Не делать большой взрывной rewrite.
 2. Сначала сузить race-surface и разъединить старую статическую модель от новой динамической.
 3. Исправлять реальные дефекты отдельно от mechanical refactor.
 4. После каждого шага отдельно проверять соответствующий пакет тестами.
 ---
 ## Исходная архитектурная проблема
 Переделанный Layer 1 жил в гибридном состоянии.
 Старая модель Fission:
 - список resource namespaces задается один раз на старте;
 - компоненты считают этот список immutable;
 - informer factories строятся из startup configuration.
 Новая multi-tenant модель:
 - namespace появляется позже, уже после старта процесса;
 - watcher видит label `fission.io/managed=true`;
 - компоненты должны подключить новый namespace на лету.
 Из-за этого в коде образовался разрыв между двумя мирами:
 1. Часть кода уже работает как dynamic system.
 2. Часть кода все еще читает глобальную map namespace-ов напрямую, как будто она immutable.
 3. В некоторых компонентах startup-path и dynamic-path оказались несимметричными.
 4. В некоторых местах общий global dedup конфликтует с локальной логикой конкретного компонента.
 Это и есть корневой дефект всей подсистемы: не один конкретный баг, а отсутствие единого namespace lifecycle contract.
 ---
 ## Что было решено не делать сразу
 Я сознательно не пошел в большой rewrite в один коммит.
 Почему:
 1. Слишком много точек входа: executor, router, buildermgr, storagesvc, utils.
 2. Если переписать все сразу, невозможно будет локализовать регрессию.
 3. Уже были реальные functional дефекты в нескольких местах, их удобнее чинить изолированно.
 4. Пользователь отдельно попросил идти последовательно и проверять после каждого изменения.
 Поэтому выбран bounded rewrite: сначала вычищать старые опасные предположения, затем исправлять функциональные несовпадения, и только потом идти к более крупному NamespaceManager.
 ---
 ## Инварианты серии
 Во всех шагах я старался держать одинаковые правила.
 ### 1. Не ломать действующий onboarding contract
 Если namespace приходит через label watcher, компоненты должны продолжать подключать его без рестарта. Нельзя было ради рефактора возвращаться к статической модели.
 ### 2. Не менять лишние контракты одновременно
 Если шаг про snapshot API, он не должен заодно переписывать cleanup semantics.
 ### 3. Сначала механические и безопасные сдвиги, потом functional fixes
 Это нужно, чтобы понимать, баг возник из-за новой логики или уже существовал ранее.
 ### 4. Каждый шаг должен быть проверяем локально
 После каждого шага запускались тесты по затронутому пакету, а не абстрактное «кажется, всё нормально».
 ---
 ## Step 1 — Snapshot API для namespace resolver
 Коммит: `c987fa0`
 ### Что было не так
 `NamespaceResolver` уже имел mutex для записи через `AddNamespace`, но многие потребители читали `FissionResourceNS` напрямую.
 Это означало следующее:
 1. Запись в map уже динамическая.
 2. Чтение в части мест по-прежнему не thread-safe.
 3. Код внешне выглядел как безопасный, потому что mutex в структуре есть, но контракт чтения не был централизован.
 То есть защита существовала только наполовину.
 ### Что я сделал
 В `pkg/utils/namespace.go` добавлены:
 - `Snapshot()`
 - `SnapshotWithOptions()`
 Их логика:
 1. Под read lock взять текущее состояние.
 2. Скопировать его в detached slice.
 3. Отсортировать, чтобы получить стабильный детерминированный порядок.
 Почему именно slice snapshot, а не снова map:
 1. Читателям в основном нужен именно проход по namespace-ам.
 2. Slice удобнее для безопасной итерации.
 3. Сортировка убирает дрожание порядка и делает поведение более предсказуемым в тестах и логике startup factory generation.
 ### Почему это был правильный первый шаг
 Этот шаг почти не меняет бизнес-логику. Он не трогает watchers, RBAC, cleanup, lifecycle events. Он вводит базовый безопасный API, на который потом можно переводить потребителей.
 ### Что было переведено сразу
 Чтобы snapshot API не оставался мертвым кодом, на него были переведены:
 - `pkg/utils/informer.go`
 - startup factory creation в `pkg/executor/executor.go`
 Логика этого выбора:
 1. Это общие helper path.
 2. Они касаются большого числа компонентов.
 3. Но при этом change поверхностный: вместо прямой итерации по map берется snapshot.
 ### Отдельный мелкий дефект, найденный на шаге 1
 Новые тесты создали локальный `NamespaceResolver` без logger. Выяснилось, что часть методов предполагает ненулевой logger. Это нехорошо само по себе: utility object не должен падать только потому, что его используют вне global singleton.
 Поэтому были добавлены nil checks вокруг debug/info логов в resolver.
 ### Проверка шага
 Проверялось:
 - `go test ./pkg/utils/...`
 - `go test ./pkg/executor/...`
 Смысл проверки:
 1. Убедиться, что snapshot API корректен как utility layer.
 2. Убедиться, что startup path executor не поменял поведение.
 ---
 ## Step 2 — Исправление namespace routing в serviceaccount checker
 Коммит: `9ce9829`
 ### Что было не так
 В `pkg/utils/serviceaccount.go` был более тонкий дефект, чем просто прямое чтение map.
 В `runSACheck()` одна и та же переменная `ns` переиспользовалась внутри цикла по permission groups.
 Смысл проблемы:
 1. Есть исходный base namespace.
 2. Для fetcher нужен путь через `GetFunctionNS(baseNS)`.
 3. Для builder нужен путь через `GetBuilderNS(baseNS)`.
 4. Но код мутировал саму переменную `ns` по мере обхода permission sets.
 Это опасно, потому что builder resolution начинает зависеть от предыдущего шага цикла, а не от исходного namespace.
 Если `FunctionNamespace` и `BuilderNamespace` различаются, route builder SA может поехать.
 ### Что я сделал
 Изменение было разбито на две части:
 1. Итерироваться не по `FissionResourceNS` напрямую, а по `Snapshot()`.
 2. Явно вычислять `targetNS` из `baseNS` через отдельный метод `resolveSANamespace(baseNS, saName)`.
 Почему выделен отдельный метод:
 1. Логика namespace routing становится читаемой как отдельный контракт.
 2. Её можно тестировать отдельно.
 3. В коде исчезает скрытая мутация переменной цикла.
 ### Почему я не переписывал весь serviceaccount.go сразу
 В файле еще остаются спорные места:
 - глобальные `fetcherCheck` / `builderCheck`;
 - мутация `permission.exists`;
 - runtime provisioning через `LocalSubjectAccessReview`.
 Но если решать всё сразу, шаг становится слишком широким. На этом этапе была цель исправить именно namespace routing bug и убрать прямую итерацию по общей map.
 ### Какой тест был добавлен
 Добавлен unit test на `resolveSANamespace()`:
 - fetcher на default namespace должен идти в function namespace;
 - builder на default namespace должен идти в builder namespace;
 - tenant namespace должен сохраняться как tenant namespace.
 Тест важен не из-за синтаксиса, а потому что он фиксирует смысловую развязку между двумя namespace path.
 ### Проверка шага
 Проверялось:
 - `go test ./pkg/utils/...`
 - `go test ./pkg/executor/...`
 ---
 ## Step 3 — Перевод runtime loops на snapshot API
 Коммит: `6102b27`
 ### Что было не так
 Даже после появления snapshot API ещё оставались runtime loops, которые напрямую читали общую map namespace-ов в горячих путях:
 - adopt existing resources;
 - idle object reaper;
 - orphan archive pruning.
 Это плохо не только из-за race. Это также концептуально закрепляет старую модель «список namespace-ов — это просто глобальная map, в которую можно смотреть отовсюду».
 ### Что я сделал
 Перевёл на `Snapshot()` следующие места:
 - `pkg/executor/executortype/container/containermgr.go`
 - `pkg/executor/executortype/newdeploy/newdeploymgr.go`
 - `pkg/executor/executortype/poolmgr/gpm.go`
 - `pkg/storagesvc/archivePruner.go`
 ### Почему именно эти места были хорошим кандидатом
 Потому что это mechanical refactor:
 1. Логика списков не меняется.
 2. Namespace source меняется с raw map на stable snapshot.
 3. Поведение должно оставаться тем же, кроме устранения unsafe read.
 ### Что это дало
 1. Уменьшило площадь прямого доступа к глобальному mutable состоянию.
 2. Подготовило код к следующему этапу, когда namespace registry станет ещё более централизованным.
 3. Сделало background loops более предсказуемыми при одновременном dynamic onboarding.
 ### Проверка шага
 Проверялось:
 - `go test ./pkg/executor/... ./pkg/storagesvc/...`
 ---
 ## Step 4 — Исправление buildermgr dedup bug
 Коммит: `56a499a`
 ### Это уже не mechanical refactor, а реальный functional fix
 ### Что было не так
 `buildermgr.StartNSWatcher()` при появлении нового namespace делал:
 1. `envw.AddNamespace()`
 2. `pkgw.AddNamespace()`
 Но оба watcher-а использовали один и тот же глобальный dedup через `nsResolver.AddNamespace()`.
 Фактический эффект:
 1. Первый вызов успешно добавляет namespace в global resolver.
 2. Второй вызов видит, что namespace уже «есть».
 3. И просто выходит.
 То есть в buildermgr динамический namespace мог получить только часть подписок.
 Это уже не theoretical risk, а реальный дефект логики.
 ### Почему проблема архитектурная
 Здесь смешались два уровня ответственности:
 1. Global registry должен знать, что namespace существует.
 2. Конкретный компонент должен знать, подписался ли он уже на этот namespace.
 Это разные виды dedup.
 Один глобальный dedup не может корректно заменить локальный dedup для двух разных subcomponents.
 ### Что я сделал
 Логику развёл по уровням:
 1. В `pkg/buildermgr/ns_watcher.go` global resolver обновляется один раз.
 2. `environmentWatcher` dedup делает по своей map `envWatchInformer`.
 3. `packageWatcher` dedup делает по своей map `pkgInformer`.
 ### Почему это правильнее
 Теперь структура похожа на executor path:
 1. Глобальный реестр говорит: namespace известен системе.
 2. Каждый компонент сам решает: свои informers он уже поднял или нет.
 Именно так должен выглядеть multi-component dynamic onboarding.
 ### Что я сознательно не делал
 Не добавлял remove/cleanup и не переделывал buildermgr lifecycle целиком. На шаге требовалось только убрать ошибку дедупликации.
 ### Проверка шага
 Проверялось:
 - `go test ./pkg/buildermgr/...`
 Тестов в пакете немного, но для этого шага важно было хотя бы подтвердить, что wiring собирается и не поломан compile-time.
 ---
 ## Step 5 — Исправление parity gap в newdeploy
 Коммит: `94f26b6`
 ### Что было не так
 `MakeNewDeploy()` на старте процесса регистрировал оба типа handler-ов:
 - `FunctionEventHandlers()`
 - `EnvEventHandlers()`
 Но `AddNamespace()` для динамически появившегося namespace регистрировал только `FunctionEventHandlers()`.
 Это значит, что два namespace-а с одинаковым содержимым вели себя по-разному только из-за времени появления:
 1. startup namespace обслуживается полным code path;
 2. dynamic namespace обслуживается урезанным code path.
 Это очень плохое свойство для Layer 1, потому что поведение перестаёт зависеть только от данных и начинает зависеть от истории запуска процесса.
 ### Что я сделал
 В `newdeploy.AddNamespace()` добавил регистрацию `EnvEventHandlers()` рядом с `FunctionEventHandlers()`.
 ### Почему fix именно такой
 Потому что это минимальное исправление семантической несимметрии.
 Я не придумывал новую абстракцию, а привёл dynamic path к уже существующему startup contract.
 ### Инженерный смысл шага
 Это важный принцип всей серии: если startup-path и late onboarding-path делают похожую работу, они должны проходить через один и тот же контракт, а не через два слегка разных набора side effects.
 ### Проверка шага
 Проверялось:
 - `go test ./pkg/executor/executortype/newdeploy`
 ---
 ## Step 6 — Защита router informer maps от гонок
 Коммит: `87477d4`
 ### Что было не так
 В router динамический namespace добавляет новые informer-ы в две map:
 - `triggerInformer`
 - `funcInformer`
 Параллельно `updateRouter()` итерируется по тем же map, собирая триггеры и функции для rebuild router-а.
 Плюс `functionReferenceResolver` получает `funcInformer` и тоже читает его напрямую.
 Это создаёт классическую проблему:
 1. одна goroutine пишет в map;
 2. другая одновременно по ней итерируется;
 3. третья читает её через resolver.
 Результат может быть от паники `concurrent map iteration and map write` до тихого чтения неполного состояния.
 ### Почему шаг стал чуть шире
 Простой mutex только вокруг `HTTPTriggerSet.AddNamespace()` не решал бы проблему полностью, потому что `functionReferenceResolver` держал свою ссылку на ту же mutable структуру.
 Поэтому понадобилось сделать две вещи одновременно:
 1. Защитить maps в `HTTPTriggerSet` через `RWMutex` и snapshot helpers.
 2. Дать `functionReferenceResolver` собственный thread-safe путь доступа к informer registry.
 ### Что я сделал
 В `HTTPTriggerSet`:
 - добавлен `RWMutex`;
 - добавлены `snapshotTriggerInformers()`;
 - добавлены `snapshotFuncInformers()`;
 - `updateRouter()` и setup handlers теперь работают по snapshot-спискам.
 В `functionReferenceResolver`:
 - добавлен `RWMutex`;
 - чтение informer-а по namespace теперь под read lock;
 - добавлен `addInformer()` для безопасного добавления нового namespace.
 В `router.AddNamespace()`:
 - запись в `triggerInformer` и `funcInformer` идёт под lock;
 - resolver получает новый informer через собственный безопасный метод.
 ### Почему именно snapshot-helpers, а не держать lock во время всей итерации
 Потому что rebuild router-а и чтение store-ов могут быть относительно дорогими. Держать глобальный lock на всё это время было бы лишним. Нам нужен был не coarse lock на длинный процесс, а короткий lock на получение стабильного снимка ссылок на informer-ы.
 То есть стратегия такая:
 1. Быстро снять snapshot ссылок.
 2. Отпустить lock.
 3. Работать со snapshot уже без блокировки записи.
 Это лучше и по безопасности, и по latency.
 ### Проверка шага
 Проверялось:
 - `go test ./pkg/router/...`
 ---
 ## Почему шаги документировались отдельно
 Я сохранял отдельный thinking-файл на каждый шаг не ради бюрократии, а ради трассируемости.
 Когда изменения маленькие, отдельные документы позволяют понять:
 1. какой дефект исправлял именно этот коммит;
 2. что было осознанно оставлено за рамками;
 3. какой тест подтверждал именно этот шаг;
 4. где functional fix, а где только mechanical safety refactor.
 Именно это позволяет потом анализировать regressions не по памяти, а по истории.
 ---
 ## Что осталось нерешённым после step 6
 Несмотря на шесть шагов, это ещё не финальный NamespaceManager rewrite.
 Остаются важные вопросы.
 ### 1. Нет remove/cleanup semantics
 Система умеет add, но почти не умеет delete/relabel cleanup.
 Что это значит practically:
 - informer-ы и локальные registry entries живут вечно;
 - once onboarded, always onboarded;
 - короткоживущие tenant namespace-ы будут оставлять мусор.
 ### 2. `serviceaccount.go` всё ещё не идеален
 Текущий `serviceaccount.go` уже лучше, чем до step 2, но файл всё ещё сложный:
 - глобальные `fetcherCheck` / `builderCheck` живут как process-wide mutable objects;
 - `permission.exists` мутируется в runtime;
 - provisioning и permission-check тесно сцеплены.
 Это отдельный кандидат на следующий bounded refactor, но уже не маленький mechanical шаг.
 ### 3. Глобальный resolver всё ещё остаётся transitional abstraction
 `NamespaceResolver` теперь безопаснее для чтения, но это пока ещё не полноценный NamespaceManager с событиями, remove lifecycle и подписками.
 Он всё ещё ближе к thread-safe registry, чем к полной orchestration layer.
 ### 4. Cleanup/restart/backfill lifecycle ещё не централизован
 Часть компонентов уже ближе к единообразию, но по-прежнему нет одного центрального orchestration contract вида:
 - add existing namespaces on startup;
 - reconcile on relabel;
 - remove on delete;
 - rebuild after restart;
 - re-register late component safely.
 ---
 ## Почему я не стал сразу делать remove/cleanup
 Потому что это уже следующая категория сложности.
 До step 6 изменения укладывались в схему:
 - локальный и понятный дефект;
 - ограниченный blast radius;
 - тестируемый пакет;
 - отдельный маленький commit.
 Remove/cleanup меняет уже жизненный цикл системы и затрагивает много мест одновременно:
 - watcher behavior;
 - manager lifecycle;
 - informer shutdown semantics;
 - cache invalidation;
 - resolver state.
 Это не тот шаг, который разумно смешивать с небольшими safety fixes.
 ---
 ## Почему такая стратегия лучше, чем «переписать всё сразу»
 Потому что сейчас уже есть видимый результат с низким риском:
 1. Уменьшено число прямых доступов к общей mutable map.
 2. Исправлен реальный functional bug в buildermgr.
 3. Исправлена реальная логическая ошибка в serviceaccount namespace routing.
 4. Исправлена несимметрия в newdeploy dynamic path.
 5. Закрыта явная router race-surface.
 И всё это не одним большим коммитом, а серией шагов с локальной верификацией.
 Для инфраструктурного кода это важнее, чем «красивый большой rewrite», который сложно раскладывать при регрессиях.
 ---
 ## Какие проверки были прогнаны по ходу серии
 После шагов запускались:
 - `go test ./pkg/utils/...`
 - `go test ./pkg/executor/...`
 - `go test ./pkg/storagesvc/...`
 - `go test ./pkg/buildermgr/...`
 - `go test ./pkg/router/...`
 Логика была такая:
 1. Не гонять каждый раз всю репу, если шаг локальный.
 2. Но обязательно проверять затронутый пакет и соседний пакет, если change касается shared utility layer.
 ---
 ## Текущее состояние после серии
 Серия шагов 1-6 не завершает rewrite, но заметно улучшает базу для следующего этапа.
 Что теперь стало лучше:
 1. Namespace reads стали заметно более дисциплинированными.
 2. Dynamic namespace onboarding стал логически ровнее между компонентами.
 3. В router исчезла наиболее явная race-surface на informer maps.
 4. Buildermgr больше не теряет часть подписок на новый namespace из-за неправильного dedup.
 Что остаётся следующим осмысленным этапом:
 1. Вынесение уже полноценного NamespaceManager как orchestration layer.
 2. Remove/cleanup lifecycle.
 3. Разделение discovery, registry и provisioning.
 4. Дополнительные тесты на restart/relabel/delete/burst onboarding.
 ---
 ## Отдельная заметка про `serviceaccount.go`
 На момент написания этого документа файл `pkg/utils/serviceaccount.go` был заново перечитан по текущему содержимому. Документ описывает актуальную логику файла в его текущем состоянии, а не только то состояние, которое было в момент коммита step 2.
 Это важно, потому что именно в этом файле пользовательский контекст отдельно предупредил о возможных дополнительных изменениях между сообщениями.