diff --git a/bin/SHA256SUMS b/bin/SHA256SUMS index 69e0ae2..e6a8783 100644 --- a/bin/SHA256SUMS +++ b/bin/SHA256SUMS @@ -1,4 +1,4 @@ -a6d853b6f203593d45e50b77ef674aac534140cb2efb9a33336902769cc0c81e control-api +b5d1ae5c625c1ab12a42b91d0e7dbfe5eb15897fbc46e1b94db21aca5f84e77f control-api 48c9b99fa88be751d9badfba8b7d80f894e326a743a90e85478f1d2251ce4ab6 validator-agent 43fb660b78179b204b57388241a508345881aa16f3531d77b8fd75af60e7f2d8 prober -6dd6a3de14079030bfca1ad0255e580baf11b6c54b51d570f03c66cd9d72a434 admin-dashboard +4e1e550bf026b9f9b4aa60a9bf38e1e409693f7ed8d4fd1853175f882ba349ca admin-dashboard diff --git a/bin/admin-dashboard b/bin/admin-dashboard index bbc3d17..e40d372 100755 Binary files a/bin/admin-dashboard and b/bin/admin-dashboard differ diff --git a/bin/control-api b/bin/control-api index cc55357..8fedd04 100755 Binary files a/bin/control-api and b/bin/control-api differ diff --git a/configs/control-api.example.yaml b/configs/control-api.example.yaml index c6e44ad..b979ce0 100644 --- a/configs/control-api.example.yaml +++ b/configs/control-api.example.yaml @@ -45,6 +45,15 @@ orchestrator: max_retries: 3 lease_ttl_seconds: 180 heartbeat_timeout_seconds: 30 + # Pause (seconds) between FIP association and the start of self-check — + # gives the OpenStack data plane time to start forwarding traffic + # through the newly attached floating IP. 0 = no pause (default). + # This is only the one-time seed value used the first time control-api + # starts against an empty database; after that it's managed at runtime + # via PUT /api/v1/admin/config/orchestrator (or the dashboard's + # /settings page) and this field is ignored. Must satisfy + # fip_settle_seconds + self_check_timeout_seconds < lease_ttl_seconds. + fip_settle_seconds: 0 aggregation: missing_counts_as_fail: true diff --git a/docs/API.md b/docs/API.md index 685766c..a2db31d 100644 --- a/docs/API.md +++ b/docs/API.md @@ -455,6 +455,28 @@ curl -s -X POST "$BASE/api/v1/admin/ips/clear" | PUT | `/api/v1/admin/config/check-types/{name}` | `{"enabled","targets":["group",...]}` | `200` | `400`, если названа несуществующая группа | | DELETE | `/api/v1/admin/config/check-types/{name}` | — | `200` | `404` | +### Настройки оркестратора: `/api/v1/admin/config/orchestrator` + +Единственный на сегодня параметр — `fip_settle_seconds`: пауза между +привязкой Floating IP к валидатору и моментом, когда self-check по этому +адресу становится доступен агенту (`GET +/api/v1/agents/{id}/assignment` до истечения паузы отдаёт `204`, как если +бы валидатору просто нечего было делать — никаких изменений в протоколе +агента). Нужна, чтобы дать data plane OpenStack время реально начать +пропускать трафик через только что привязанный адрес, прежде чем +запускать по нему проверки. `0` — без паузы (поведение по умолчанию, как +до появления этого параметра). + +| Метод | Путь | Тело | Успех | Ошибки | +|---|---|---|---|---| +| GET | `/api/v1/admin/config/orchestrator` | — | `{"fip_settle_seconds":N}` | | +| PUT | `/api/v1/admin/config/orchestrator` | `{"fip_settle_seconds":N}` | `200` | `400`, если `N < 0`, или если `fip_settle_seconds + self_check_timeout_seconds >= lease_ttl_seconds` (пауза не должна съедать весь лизинг адреса — иначе self-check не успеет пройти до истечения `lease_ttl_seconds`, и адрес будет вечно возвращаться в очередь) | + +Как и остальные разделы этой группы, YAML-поле `orchestrator. +fip_settle_seconds` в `control-api.yaml` — только одноразовый bootstrap +для пустой БД; дальше источник истины — сама база, менять значение нужно +через `PUT` выше (или страницу `/settings` в дашборде). + ### Пример: конфигурация целиком через API, без единой строки в YAML ```bash @@ -509,6 +531,13 @@ queued ──(control-api сам, без вызова API)──▶ assigning_fi `orchestrator.poll_interval_seconds`), явного HTTP-метода для их запуска нет — это фоновый цикл (`Tick`), а не запрос/ответ. +Вход в `awaiting_self_check` не означает мгновенную видимость агенту: если +настроена пауза (`fip_settle_seconds`, см. +[«Настройки оркестратора»](#настройки-оркестратора-apiv1adminconfigorchestrator) +выше), `GET /api/v1/agents/{id}/assignment` продолжает отдавать `204` до +истечения паузы, и только потом начинает отдавать assignment — состояние +в БД при этом уже `awaiting_self_check`. + Площадки (`siteN_complete`) — опциональны: сколько их учитывается, целиком определяется текущим списком `sites` (0–3 записи, управляется через `/api/v1/admin/config/sites` — см. diff --git a/docs/DASHBOARD.md b/docs/DASHBOARD.md index c12d4e2..a9821cc 100644 --- a/docs/DASHBOARD.md +++ b/docs/DASHBOARD.md @@ -49,12 +49,13 @@ admin-dashboard -config /etc/cloud-ip-validator/admin-dashboard.yaml | Страница | Назначение | |---|---| | `/overview` | Сводная статистика: счётчики по состояниям, «текущая проверка» (live-снимок всех IP не в терминальном состоянии) и «последние N завершённых» (по умолчанию 20, `overview.last_completed_count`) с разбивкой pass/partial/fail/cancelled. Обновляется каждые `overview.poll_interval_seconds` секунд без перезагрузки страницы. | -| `/ips` | Полная очередь. Форма сверху принимает список адресов (по одному на строке или через запятую) и отправляет их в `POST /api/v1/admin/ips` — **один и тот же вызов** добавляет новые адреса и принудительно перезапускает уже завершённые (см. ниже). У каждого адреса — кнопка «Перепроверить» (для `done`/`failed`) или «Отменить» (для активных состояний), и всегда — «Удалить» (безвозвратно, в отличие от «Отменить», см. ниже). Чекбоксы у строк + кнопка «Удалить выбранные» удаляют список одним вызовом; «Очистить всё» удаляет вообще всё, включая активные проверки — обе операции требуют явного подтверждения. | +| `/ips` | Полная очередь. Форма сверху принимает список адресов (по одному на строке или через запятую) и отправляет их в `POST /api/v1/admin/ips` — **один и тот же вызов** добавляет новые адреса и принудительно перезапускает уже завершённые (см. ниже). У каждого адреса — кнопка «Перепроверить» (для `done`/`failed`) или «Отменить» (для активных состояний), и всегда — «Удалить» (безвозвратно, в отличие от «Отменить», см. ниже). Чекбоксы у строк + кнопка «Удалить выбранные» удаляют список одним вызовом; «Очистить всё» удаляет вообще всё, включая активные проверки — обе операции требуют явного подтверждения. Пока не истекла настроенная на `/settings` пауза (`fip_settle_seconds`), только что привязавший Floating IP адрес показывает отдельный бейдж «прогрев FIP» вместо обычного статуса. | | `/ips/{ip}` | Детали одного адреса: все проверки текущей попытки и вся история событий. | | `/validators` | Список валидаторов + создание/изменение `os_port_id`/удаление. | | `/sites` | Три фиксированных слота площадок (1/2/3) — назначить/сменить/освободить `site_id`. | | `/targets` | Группы целей для egress-проверок — создание/редактирование/удаление. | | `/check-types` | Типы проверок (`https`/`icmp`/`ssh`/...), включение/выключение, привязка к группам целей. | +| `/settings` | Единственная настройка на сегодня — `fip_settle_seconds`, пауза (в секундах) между привязкой Floating IP и началом self-check («прогрев» дата-плейна OpenStack, см. [USAGE.md](USAGE.md#пауза-перед-self-check-fip_settle_seconds)). | ### «Текущая» и «последняя завершённая» проверка diff --git a/docs/PLAN_FIP_SETTLE_DELAY.md b/docs/PLAN_FIP_SETTLE_DELAY.md new file mode 100644 index 0000000..ab57f78 --- /dev/null +++ b/docs/PLAN_FIP_SETTLE_DELAY.md @@ -0,0 +1,323 @@ +# План: пауза перед self-check после привязки Floating IP (`fip_settle_seconds`) + +> Статус: **реализовано**. Актуальное описание — +> [docs/API.md](API.md#настройки-оркестратора-apiv1adminconfigorchestrator), +> [docs/USAGE.md](USAGE.md#пауза-перед-self-check-fip_settle_seconds), +> [docs/DASHBOARD.md](DASHBOARD.md). + +## Context + +Сейчас, как только `orchestrator.associateFIP` успешно привязывает Floating +IP к порту валидатора, адрес немедленно становится виден агенту +(`state=awaiting_self_check`), и агент запускает self-check на ближайшем +опросе `GET /assignment` (по умолчанию раз в 5с, `poll_interval_seconds`). +Никакой паузы нет — а data plane OpenStack может не успеть реально начать +пропускать трафик через только что привязанный FIP к этому моменту, +из-за чего self-check ложно проваливается по причине, не связанной с +самой привязкой. + +Нужна управляемая администратором пауза между привязкой FIP и началом +self-check. Решено: +- без новых состояний `ip_queue` — пауза реализуется тем, что сервер + *придерживает* назначение (agent просто продолжает получать `204` и + поллить дальше — это уже штатное поведение, доработок на стороне + `validator-agent` не требуется); +- значение — единый глобальный параметр, управляемый не только через + YAML-бутстрап, но и **на лету через admin API и dashboard** (по + аналогии с уже существующими `validators`/`sites`/`targets`/ + `check_types`, но это не CRUD над именованными сущностями, а один + скаляр — поэтому отдельная таблица-синглтон, а не переиспользование + существующего паттерна); +- жёсткая валидация: `fip_settle_seconds + self_check_timeout_seconds` + должно быть `< lease_ttl_seconds`, иначе связка «пауза + сам self-check» + не влезает в лизинг, и адрес будет вечно уходить в reclaim + (`sweepExpiredLeases`), так и не успев пройти self-check; +- в dashboard: отдельная страница `/settings` (задел на будущие + orchestrator-wide параметры) + видимость паузы в таблице `/ips` + (отдельный бейдж «прогрев FIP» вместо обычного статуса, пока пауза не + истекла). + +## 1. `internal/db` — схема, модели, запросы + +**Новая миграция** `internal/db/migrations/0003_fip_settle_delay.sql`: +```sql +ALTER TABLE ip_queue ADD COLUMN fip_associated_at TIMESTAMP; + +-- Синглтон-строка под единственный на сегодня скаляр-параметр; не +-- generic key-value — если параметров станет больше, добавлять колонки +-- сюда, а не заводить схему "на вырост". +CREATE TABLE settings ( + id INTEGER PRIMARY KEY CHECK (id = 1), + fip_settle_seconds INTEGER NOT NULL DEFAULT 0, + created_at TIMESTAMP NOT NULL, + updated_at TIMESTAMP NOT NULL +); +``` +`internal/db/db.go`: `//go:embed migrations/0003_fip_settle_delay.sql` → +новая переменная, добавить `{3, ...}` в конец слайса `migrations` (тот же +паттерн, что уже даёт `{1, initSchema}`, `{2, dynamicConfigSchema}`). + +**`internal/db/models.go`**: в `IPQueueItem` — новое поле +`FIPAssociatedAt *time.Time` (после `AssignedAt`, по хронологии: +claim → associate → aggregate → release). `AssignedAt` для этой цели не +подходит — его пишет `ClaimNextQueued` в момент захвата очереди, **до** +привязки FIP, а нужен именно момент привязки. + +**`internal/db/queries_ipqueue.go`**: +- `ipQueueSelect` — добавить колонку `fip_associated_at`. +- `scanIPQueueItem` — добавить сканирование через `sql.NullString` + + `nullStringToTimePtr`, зеркально `AssignedAt`/`AggregatedAt`/ + `FIPReleasedAt` чуть выше в этом же файле. +- `SetFIPAssociated` — писать `fip_associated_at=?` (текущий `now`) + вместе с уже существующими `state`/`fip_id`/`lease_expires_at`/ + `updated_at`. +- `RequeueOrFail` (ветка возврата в `queued`) и `SubmitIPs` (ветка + `done`/`failed → queued`) — добавить `fip_associated_at=NULL` в списки + сброса полей (рядом с уже существующими `assigned_at=NULL` и т.п.) — + новая попытка не должна наследовать таймер паузы от предыдущей. + +**Новый файл `internal/db/queries_settings.go`**: +- `type Settings struct { FIPSettleSeconds int; CreatedAt, UpdatedAt time.Time }`. +- `GetSettings(ctx) (Settings, error)` — `SELECT ... FROM settings WHERE id=1`. +- `SetFIPSettleSeconds(ctx, seconds int) error` — проверяет `seconds >= 0` + (`ErrValidation` иначе), затем `UPDATE settings SET + fip_settle_seconds=?, updated_at=? WHERE id=1`. Кросс-валидацию против + `lease_ttl_seconds`/`self_check_timeout_seconds` здесь не делать — у + этого пакета нет доступа к `config.OrchestratorConfig`, она живёт в + `orchestrator` (см. ниже). + +**`internal/db/bootstrap.go`**: новая `bootstrapSettings(ctx, fipSettleSeconds int) error` +по образцу `bootstrapSites`/`bootstrapCheckTypes` (тот же паттерн: `SELECT +COUNT(*) FROM settings`, если `>0` — no-op, иначе один `INSERT ... VALUES +(1, ?, now, now)`), вызвать из `BootstrapFromConfig` рядом с остальными +четырьмя bootstrap-вызовами, перед `SeedQueue`. + +## 2. `internal/config` + +`internal/config/config.go`, `OrchestratorConfig`: новое поле +`FIPSettleSeconds int` с тегом `` yaml:"fip_settle_seconds" `` — без +дефолта в `LoadControlAPI` (в отличие от остальных полей структуры): `0` — +сам по себе корректный и обратно-совместимый дефолт (без паузы), задавать +что-то другое было бы неверно. Это поле используется только как +одноразовое seed-значение при бутстрапе пустой `settings`-таблицы — +дальше источник истины БД, как и у validators/sites/targets/check_types. + +## 3. `internal/orchestrator` + +**`internal/orchestrator/orchestrator.go`**, `AssignmentForValidator`: +после существующей проверки состояния +(`if item.State != db.IPAwaitingSelfCheck && item.State != db.IPChecking { return nil, nil, nil }`) +добавить проверку паузы, применимую только к `awaiting_self_check` (уже +прошедший self-check `checking` она не касается): + +```go +if item.State == db.IPAwaitingSelfCheck { + settled, err := o.isFIPSettled(ctx, item) + if err != nil { + return nil, nil, err + } + if !settled { + return nil, nil, nil + } +} +``` + +Новый приватный `isFIPSettled(ctx, item *db.IPQueueItem) (bool, error)`: +читает `o.DB.GetSettings(ctx)` заново при каждом вызове (тот же принцип +"читать из БД на каждое использование", что уже применён к +`check_types`/`sites` в этом файле — правки админа применяются мгновенно +даже к уже ожидающему адресу). Возвращает `true`, если +`settings.FIPSettleSeconds <= 0` **или** `item.FIPAssociatedAt == nil` +(это и есть гарантия обратной совместимости — см. ниже), иначе сравнивает +`db.Now()` с `item.FIPAssociatedAt.Add(settle)`. + +Новый экспортируемый `SetFIPSettleSeconds(ctx, seconds int) error` — +именно здесь, а не в `internal/db`, потому что нужен доступ к +`o.Cfg.SelfCheckTimeoutSeconds`/`o.Cfg.LeaseTTLSeconds`: +```go +if seconds < 0 { ... ErrValidation } +if seconds+o.Cfg.SelfCheckTimeoutSeconds >= o.Cfg.LeaseTTLSeconds { + ... ErrValidation ("fip_settle_seconds (%d) + self_check_timeout_seconds (%d) must be < lease_ttl_seconds (%d)") +} +return o.DB.SetFIPSettleSeconds(ctx, seconds) +``` +GET-путь отдельного метода на `Orchestrator` не получает — HTTP-хендлер +читает `s.DB.GetSettings` напрямую, как уже делают read-only +`handleConfigList*` в `handlers_config.go`. + +`associateFIP` (строка со `SetFIPAssociated`) не меняется — новую колонку +проставляет сам DB-слой. + +## 4. `internal/httpapi` + +`dto_admin.go`: `orchestratorSettingsDTO{FIPSettleSeconds int}` с тегом +`` json:"fip_settle_seconds" `` (используется и для GET-ответа, и как тело +PUT-запроса — по образцу однополевых DTO вроде `putSiteRequest`). + +`handlers_config.go`: `handleConfigGetOrchestratorSettings` (GET, зовёт +`s.DB.GetSettings`) и `handleConfigPutOrchestratorSettings` (PUT, зовёт +`s.Orch.SetFIPSettleSeconds`, ошибку — через уже существующий +`writeDBError`, который `db.ErrValidation` уже мапит на `400`). + +`routes.go`: `GET /api/v1/admin/config/orchestrator`, `PUT +/api/v1/admin/config/orchestrator` — рядом с остальным блоком +`/api/v1/admin/config/*`. + +`handlers_agent.go` не меняется: `handleAgentAssignment` уже превращает +`item == nil` в `204 No Content` — как только `AssignmentForValidator` +начнёт возвращать `nil` во время паузы, это автоматически попадёт в уже +существующую ветку. + +## 5. `internal/dashboard` + +`dto.go`: `FIPAssociatedAt *time.Time` в `ipQueueItem` (после +`AssignedAt` — важно совпадение имени с `db.IPQueueItem`, т.к. эти DTO +декодируются по имени поля без JSON-тегов); плюс +`orchestratorSettingsDTO{FIPSettleSeconds int}`. + +`client.go`: `GetOrchestratorSettings(ctx)`/`PutOrchestratorSettings(ctx, +seconds int)` — по образцу существующих методов, бьют в +`/api/v1/admin/config/orchestrator`. + +**Новая страница `/settings`** (не переиспользуем `sites.html`'s +3-слотовый layout — это одна форма с одним числовым полем, ближе по духу +к форме «Добавить/перепроверить» на `/ips`, чем к табличным +CRUD-страницам): +- новый `internal/dashboard/handlers_settings.go`: + `handleSettingsPage` (GET, зовёт `s.CA.GetOrchestratorSettings`, + `ActiveNav="settings"`, рендерит `settings_page`), + `handleSettingsPut` (PUT, `r.ParseForm()` + + `strconv.Atoi(r.PostFormValue("fip_settle_seconds"))` → на ошибку + парсинга `apiErr{400}`, иначе `s.CA.PutOrchestratorSettings`, дальше + переиспользовать `renderFragment` с фрагментом формы, тем же принципом, + что `renderIPsTable`: перечитать текущее значение через GET и + отрендерить независимо от исхода мутации). +- новый `internal/dashboard/templates/settings.html`: `settings_page` / + `settings_content` / `settings_form` (структура head/sidebar/topbar — + один в один с `ips.html`/`sites.html`), внутри — один `
` с `` и кнопкой + «Сохранить», плюс поясняющий `

` о смысле паузы и об + ограничении `fip_settle_seconds + self_check_timeout_seconds < + lease_ttl_seconds` (ошибка валидации придёт баннером через уже + существующий `bannerFor`/`error_banner`). +- `internal/dashboard/routes.go`: `GET /settings`, `PUT /settings`. +- `internal/dashboard/templates/layout.html`, блок `sidebar_nav`: новая + `` (по образцу пяти уже существующих) между + пунктом «Типы проверок» и закрывающим ``, с `ActiveNav == + "settings"`. + +**Видимость паузы на `/ips`**: +- `handlers_ips.go`: `ipsPageData` получает поле `FIPSettleSeconds int`; + `handleIPsPage` и `renderIPsTable` дополнительно зовут + `s.CA.GetOrchestratorSettings(r.Context())` (один недорогой лишний + запрос на рендер, тот же принцип, что уже применён на странице обзора) + и прокидывают значение в `ipsPageData`, ошибку — в общий `err`/ + `actionErr` перед `bannerFor`. +- `render.go`, `ipBadge`: расширить сигнатуру до `ipBadge(state, result + string, fipAssociatedAt *time.Time, settleSeconds int) Badge` — новая + ветка `case "awaiting_self_check"`: если `settleSeconds > 0 && + fipAssociatedAt != nil && time.Now().Before(fipAssociatedAt.Add(...))` + → `Badge{"pill-warning", "прогрев FIP"}`, иначе прежний дефолт + `Badge{"pill-info", state}`. `.pill-warning` уже есть в + `static/dashboard.css` — новых CSS-классов не требуется. Время + считается в Go-хелпере, не в шаблоне (тот же принцип, что уже у + `fmtTime`). `funcMap` регистрацию менять не нужно — то же имя, просто + шире сигнатура. +- `templates/ips.html`: `{{$b := ipBadge .State .OverallResult + .FIPAssociatedAt $.FIPSettleSeconds}}` (`$` внутри `{{range .Items}}` + корректно указывает на корневые данные шаблона `ips_table`, то есть + `ipsPageData`). + +## Обратная совместимость + +- `fip_associated_at` — nullable, добавляется через `ALTER TABLE`; все + существующие строки (включая активно проверяемые на момент апгрейда) + получают `NULL`. +- `isFIPSettled` явно трактует `FIPAssociatedAt == nil` как «уже + прогрелся» — так что ни один адрес, начавший цикл до апгрейда, не + будет неожиданно задержан, даже если админ уже успел выставить + ненулевую паузу. Задержке подвержены только адреса, прошедшие + `associateFIP` **после** миграции. +- Дефолт `fip_settle_seconds=0` (если не задан явно ни в YAML, ни через + API) — поведение системы для всех, кто не трогал новую настройку, + не меняется вообще. + +## 6. Тесты + +- **`internal/db`**: новый `queries_settings_test.go` — + `TestBootstrapSettingsSeedsOnceFromConfig` (сид только на пустой + таблице), `TestSetFIPSettleSecondsRejectsNegative`, + `TestGetSetFIPSettleSecondsRoundTrip`. Новый `queries_ipqueue_test.go` — + `TestSetFIPAssociatedStampsTimestamp`, `TestRequeueClearsFIPAssociatedAt`. + Использовать существующий `newTestDB` (`queries_dynconfig_test.go`). +- **`internal/orchestrator`** (`orchestrator_test.go`, существующие + `newTestOrchestrator`/`newTestOrchestratorWithSites`): + `TestFIPSettleDelayWithholdsAssignment` (после `Tick` назначение не + отдаётся, пока не пройдёт настроенная пауза — `time.Sleep`, как уже + делает `TestLeaseReclaim`), `TestFIPSettleDelayZeroIsNoOp`, + `TestFIPSettleDelayIgnoresNilFIPAssociatedAt` (обратная + совместимость), `TestSetFIPSettleSecondsValidation` (табличный тест на + границу с `lease_ttl_seconds`/`self_check_timeout_seconds`). +- **`internal/httpapi`** (`handlers_config_test.go`, + `newConfigTestHarness`/`fakeClient`): `TestOrchestratorSettingsGetPut`, + `TestOrchestratorSettingsPutValidation` (400 на нарушение ограничения); + сквозной тест, что `GET /agents/{id}/assignment` отдаёт `204` во время + паузы и `200` после. +- **`internal/dashboard`** (`dashboard_test.go`'s `fakeControlAPI` — + дополнить GET/PUT `/api/v1/admin/config/orchestrator` и полем + `FIPAssociatedAt` у фейковых IP; `handlers_test.go`): + `TestSettingsGetAndPut`, `TestIPsPageShowsSettleBadge`. +- В конце: `go build ./... && go vet ./... && go test ./...`. + +## 7. Документация + +- `docs/API.md`: новая подсекция `### Настройки оркестратора: + /api/v1/admin/config/orchestrator` рядом с остальными + `/admin/config/*`, с таблицей метод/путь/тело/ошибки и явным описанием + правила валидации; короткая ремарка в разделе «Модель состояний», что + вход в `awaiting_self_check` не означает мгновенную видимость агенту + при ненулевом `fip_settle_seconds`. +- `docs/USAGE.md`: новый короткий раздел про паузу (что это, как + настраивается — `/settings` в дашборде или API, ограничение по + `lease_ttl_seconds`/`self_check_timeout_seconds`, что при нарушении — + жёсткий отказ, не молчаливое обрезание); дополнить существующий пункт + «Адрес не выходит из `awaiting_self_check`» в «Частых проблемах» + ремаркой, что ожидаемая недолгая пауза здесь — не то же самое, что + реальный сетевой сбой, который там уже описан. +- `docs/DASHBOARD.md`: новая строка `/settings` в таблице страниц. +- `configs/control-api.example.yaml`: закомментированное + `fip_settle_seconds: 0` под `orchestrator:` с пояснением (одноразовый + seed, дальше управляется через API/dashboard, ограничение по + `lease_ttl_seconds`). + +## Критичные файлы + +- `internal/db/queries_ipqueue.go`, `internal/db/queries_settings.go` (новый), + `internal/db/bootstrap.go`, `internal/db/db.go`, + `internal/db/migrations/0003_fip_settle_delay.sql` (новый) +- `internal/orchestrator/orchestrator.go` +- `internal/httpapi/handlers_config.go`, `dto_admin.go`, `routes.go` +- `internal/dashboard/handlers_settings.go` (новый), + `internal/dashboard/handlers_ips.go`, `render.go`, `client.go`, `dto.go`, + `routes.go`, `templates/settings.html` (новый), `templates/layout.html`, + `templates/ips.html` + +## Проверка + +1. `go build ./... && go vet ./... && go test ./...`. +2. Ручной прогон: поставить `fip_settle_seconds=5` через + `PUT /api/v1/admin/config/orchestrator` (или страницу `/settings`), + поставить адрес в очередь, убедиться что `GET + /api/v1/agents/{id}/assignment` отдаёт `204` первые ~5с после привязки + FIP (видно в логах `fip_associated`) и `200` после; на `/ips` + убедиться, что строка в этот момент показывает бейдж «прогрев FIP»; + попытаться выставить значение, нарушающее + `fip_settle_seconds + self_check_timeout_seconds < lease_ttl_seconds` + — получить `400`. +3. Пересобрать `bin/control-api` и `bin/admin-dashboard` + (`go build -trimpath -ldflags="-s -w" ...`, обновить + `bin/SHA256SUMS`) — без этого шага повторится та же проблема, что уже + была с 405 при удалении IP (работающий процесс со старым бинарником + не знает новых маршрутов). diff --git a/docs/USAGE.md b/docs/USAGE.md index 9001fc8..6e8f1e0 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -21,6 +21,7 @@ - [Повторная проверка адреса](#повторная-проверка-адреса) - [Принудительная остановка проверки](#принудительная-остановка-проверки) - [Удаление адресов из очереди](#удаление-адресов-из-очереди) +- [Пауза перед self-check (fip_settle_seconds)](#пауза-перед-self-check-fip_settle_seconds) - [Частые проблемы и что с ними делать](#частые-проблемы-и-что-с-ними-делать) ## Как устроена работа с системой @@ -413,6 +414,36 @@ curl -s -X POST http://:8080/api/v1/admin/ips/clear всё» — каждая с подтверждением, явно предупреждающим о необратимости (см. [DASHBOARD.md](DASHBOARD.md)). +## Пауза перед self-check (fip_settle_seconds) + +Как только Floating IP привязывается к валидатору, control-api по +умолчанию сразу же позволяет агенту начать self-check — а data plane +OpenStack может не успеть в этот же момент реально начать пропускать +трафик через только что привязанный адрес, из-за чего self-check ложно +проваливается по причине, не связанной с самой привязкой. Если это +наблюдается на вашем стенде, задайте паузу между привязкой FIP и началом +self-check: + +```bash +curl -s -X PUT http://:8080/api/v1/admin/config/orchestrator \ + -d '{"fip_settle_seconds": 5}' +``` + +То же самое — на странице `/settings` дашборда. `0` (по умолчанию) — без +паузы. Пока пауза не истекла, адрес уже в состоянии +`awaiting_self_check`, но `GET /assignment` агенту продолжает отдавать +`204` (агент просто ждёт следующего опроса, доработок на его стороне не +требуется); в дашборде на `/ips` такой адрес в это время помечен бейджем +«прогрев FIP» вместо обычного статуса. + +Значение обязано оставлять запас внутри лизинга адреса: +`fip_settle_seconds + self_check_timeout_seconds` должно быть **меньше** +`orchestrator.lease_ttl_seconds` — иначе пауза плюс сам self-check не +влезут в лизинг, адрес не успеет пройти self-check до истечения +`lease_ttl_seconds` и будет вечно возвращаться в очередь через +`sweepExpiredLeases`. Попытка задать такое значение отклоняется `400`, не +обрезается молча. + ## Частые проблемы и что с ними делать **Валидатор долго висит в `unreachable`.** @@ -421,7 +452,12 @@ curl -s -X POST http://:8080/api/v1/admin/ips/clear (`systemctl status validator-agent`, `journalctl -u validator-agent`). **Адрес не выходит из `awaiting_self_check` (статус не меняется вообще).** -Self-check запрашивает внешние (вне облака) сервисы из +Если это длится всего несколько секунд и на стенде настроена +[пауза перед self-check](#пауза-перед-self-check-fip_settle_seconds) +(`fip_settle_seconds`) — это ожидаемое поведение, не сбой: адрес +сознательно придерживается, прежде чем агенту разрешат начать проверку. +Проблема — если статус не меняется значительно дольше этой паузы. Тогда +дело обычно в self-check: он запрашивает внешние (вне облака) сервисы из `self_check.ip_echo_urls` в конфиге валидатора (по умолчанию `api.ipify.org`, `ifconfig.me`) — если у ВМ-валидатора нет исходящего доступа в интернет к этим адресам, запрос не проходит вообще, и агент diff --git a/internal/config/config.go b/internal/config/config.go index 9962804..5fa4363 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -70,6 +70,14 @@ type OrchestratorConfig struct { MaxRetries int `yaml:"max_retries"` LeaseTTLSeconds int `yaml:"lease_ttl_seconds"` HeartbeatTimeoutSeconds int `yaml:"heartbeat_timeout_seconds"` + // FIPSettleSeconds is only the one-time seed value used the first time + // control-api starts against an empty database; after that it's + // managed at runtime via PUT /api/v1/admin/config/orchestrator (or the + // dashboard's /settings page) and this field is ignored. Zero (no + // pause) is a valid, backward-compatible default — unlike every other + // field in this struct, it deliberately gets no nonzero default in + // LoadControlAPI below. + FIPSettleSeconds int `yaml:"fip_settle_seconds"` } type AggregationConfig struct { diff --git a/internal/dashboard/client.go b/internal/dashboard/client.go index a8e9d06..e8f9578 100644 --- a/internal/dashboard/client.go +++ b/internal/dashboard/client.go @@ -200,3 +200,16 @@ func (c *client) PutCheckType(ctx context.Context, name string, enabled bool, ta func (c *client) DeleteCheckType(ctx context.Context, name string) error { return c.do(ctx, http.MethodDelete, "/api/v1/admin/config/check-types/"+url.PathEscape(name), nil, nil) } + +func (c *client) GetOrchestratorSettings(ctx context.Context) (orchestratorSettingsDTO, error) { + var out orchestratorSettingsDTO + err := c.do(ctx, http.MethodGet, "/api/v1/admin/config/orchestrator", nil, &out) + return out, err +} + +func (c *client) PutOrchestratorSettings(ctx context.Context, fipSettleSeconds int) (orchestratorSettingsDTO, error) { + var out orchestratorSettingsDTO + err := c.do(ctx, http.MethodPut, "/api/v1/admin/config/orchestrator", + orchestratorSettingsDTO{FIPSettleSeconds: fipSettleSeconds}, &out) + return out, err +} diff --git a/internal/dashboard/dashboard_test.go b/internal/dashboard/dashboard_test.go index 607875c..6e518b5 100644 --- a/internal/dashboard/dashboard_test.go +++ b/internal/dashboard/dashboard_test.go @@ -22,12 +22,13 @@ import ( // re-implementation of control-api's own business rules (that's already // covered by internal/httpapi's own tests). type fakeControlAPI struct { - mu sync.Mutex - ips []ipQueueItem - validators []validatorDTO - sites map[int]string - groups map[string][]string - checkTypes map[string]checkTypeDTO + mu sync.Mutex + ips []ipQueueItem + validators []validatorDTO + sites map[int]string + groups map[string][]string + checkTypes map[string]checkTypeDTO + fipSettleSeconds int } func newFakeControlAPI(t *testing.T) (*fakeControlAPI, string) { @@ -187,6 +188,24 @@ func (f *fakeControlAPI) handler() http.Handler { writeJSON(w, http.StatusOK, resp) }) + mux.HandleFunc("GET /api/v1/admin/config/orchestrator", func(w http.ResponseWriter, r *http.Request) { + f.mu.Lock() + defer f.mu.Unlock() + writeJSON(w, http.StatusOK, orchestratorSettingsDTO{FIPSettleSeconds: f.fipSettleSeconds}) + }) + mux.HandleFunc("PUT /api/v1/admin/config/orchestrator", func(w http.ResponseWriter, r *http.Request) { + f.mu.Lock() + defer f.mu.Unlock() + var req orchestratorSettingsDTO + _ = json.NewDecoder(r.Body).Decode(&req) + if req.FIPSettleSeconds < 0 { + writeAPIErr(w, http.StatusBadRequest, "fip_settle_seconds must be >= 0") + return + } + f.fipSettleSeconds = req.FIPSettleSeconds + writeJSON(w, http.StatusOK, orchestratorSettingsDTO{FIPSettleSeconds: f.fipSettleSeconds}) + }) + mux.HandleFunc("GET /api/v1/admin/config/validators", func(w http.ResponseWriter, r *http.Request) { f.mu.Lock() defer f.mu.Unlock() diff --git a/internal/dashboard/dto.go b/internal/dashboard/dto.go index 197e8fd..ad2af4c 100644 --- a/internal/dashboard/dto.go +++ b/internal/dashboard/dto.go @@ -35,6 +35,7 @@ type ipQueueItem struct { Site3Complete bool OverallResult string AssignedAt *time.Time + FIPAssociatedAt *time.Time AggregatedAt *time.Time FIPReleasedAt *time.Time CreatedAt time.Time @@ -123,3 +124,7 @@ type checkTypeDTO struct { type errorResponse struct { Error string `json:"error"` } + +type orchestratorSettingsDTO struct { + FIPSettleSeconds int `json:"fip_settle_seconds"` +} diff --git a/internal/dashboard/handlers_ips.go b/internal/dashboard/handlers_ips.go index b972bd1..916f4f4 100644 --- a/internal/dashboard/handlers_ips.go +++ b/internal/dashboard/handlers_ips.go @@ -7,7 +7,8 @@ import ( type ipsPageData struct { PageData - Items []ipQueueItem + Items []ipQueueItem + FIPSettleSeconds int } type ipDetailData struct { @@ -17,7 +18,11 @@ type ipDetailData struct { func (s *Server) handleIPsPage(w http.ResponseWriter, r *http.Request) { items, err := s.CA.ListIPs(r.Context()) - data := ipsPageData{Items: items} + settings, settingsErr := s.CA.GetOrchestratorSettings(r.Context()) + if err == nil { + err = settingsErr + } + data := ipsPageData{Items: items, FIPSettleSeconds: settings.FIPSettleSeconds} data.ActiveNav = "ips" data.Banner = bannerFor(err) s.renderPage(w, "ips_page", data) @@ -41,7 +46,11 @@ func (s *Server) renderIPsTable(w http.ResponseWriter, r *http.Request, actionEr if actionErr == nil { actionErr = listErr } - s.renderFragment(w, "ips_table", ipsPageData{Items: items}, actionErr) + settings, settingsErr := s.CA.GetOrchestratorSettings(r.Context()) + if actionErr == nil { + actionErr = settingsErr + } + s.renderFragment(w, "ips_table", ipsPageData{Items: items, FIPSettleSeconds: settings.FIPSettleSeconds}, actionErr) } func (s *Server) handleIPsSubmit(w http.ResponseWriter, r *http.Request) { diff --git a/internal/dashboard/handlers_settings.go b/internal/dashboard/handlers_settings.go new file mode 100644 index 0000000..7a3e883 --- /dev/null +++ b/internal/dashboard/handlers_settings.go @@ -0,0 +1,46 @@ +package dashboard + +import ( + "fmt" + "net/http" + "strconv" +) + +type settingsPageData struct { + PageData + Settings orchestratorSettingsDTO +} + +func (s *Server) handleSettingsPage(w http.ResponseWriter, r *http.Request) { + settings, err := s.CA.GetOrchestratorSettings(r.Context()) + data := settingsPageData{Settings: settings} + data.ActiveNav = "settings" + data.Banner = bannerFor(err) + s.renderPage(w, "settings_page", data) +} + +// renderSettingsForm re-fetches the current settings and renders the +// settings_form fragment, tagging actionErr (if any) on the shared error +// banner — same pattern as renderIPsTable: always reflect true current +// state regardless of whether the mutation itself succeeded. +func (s *Server) renderSettingsForm(w http.ResponseWriter, r *http.Request, actionErr error) { + settings, getErr := s.CA.GetOrchestratorSettings(r.Context()) + if actionErr == nil { + actionErr = getErr + } + s.renderFragment(w, "settings_form", settingsPageData{Settings: settings}, actionErr) +} + +func (s *Server) handleSettingsPut(w http.ResponseWriter, r *http.Request) { + if err := r.ParseForm(); err != nil { + s.renderSettingsForm(w, r, fmt.Errorf("invalid form: %w", err)) + return + } + seconds, err := strconv.Atoi(r.PostFormValue("fip_settle_seconds")) + if err != nil { + s.renderSettingsForm(w, r, &apiErr{Status: http.StatusBadRequest, Message: "пауза должна быть целым числом секунд"}) + return + } + _, err = s.CA.PutOrchestratorSettings(r.Context(), seconds) + s.renderSettingsForm(w, r, err) +} diff --git a/internal/dashboard/handlers_test.go b/internal/dashboard/handlers_test.go index cfb038b..6b27cbb 100644 --- a/internal/dashboard/handlers_test.go +++ b/internal/dashboard/handlers_test.go @@ -175,6 +175,61 @@ func TestIPsClear(t *testing.T) { } } +func TestSettingsGetAndPut(t *testing.T) { + fake, caURL := newFakeControlAPI(t) + ts := newTestServer(t, caURL) + + page := get(t, ts, "/settings") + if !strings.Contains(page, `value="0"`) { + t.Fatalf("expected default 0 in the form, got:\n%s", page) + } + + body := postForm(t, ts, "PUT", "/settings", map[string][]string{"fip_settle_seconds": {"15"}}) + if !strings.Contains(body, `value="15"`) { + t.Fatalf("expected updated value 15 in re-rendered form, got:\n%s", body) + } + if fake.fipSettleSeconds != 15 { + t.Fatalf("expected fake control-api settings updated, got %d", fake.fipSettleSeconds) + } + + // A control-api validation error (negative value here) surfaces via + // the banner, not a crash. + body = postForm(t, ts, "PUT", "/settings", map[string][]string{"fip_settle_seconds": {"-1"}}) + if !strings.Contains(body, "alert-warning") { + t.Fatalf("expected client error banner for invalid value, got:\n%s", body) + } + + // A non-numeric value is caught by the dashboard itself before it ever + // reaches control-api. + body = postForm(t, ts, "PUT", "/settings", map[string][]string{"fip_settle_seconds": {"not-a-number"}}) + if !strings.Contains(body, "alert-warning") { + t.Fatalf("expected client error banner for non-numeric value, got:\n%s", body) + } +} + +func TestIPsPageShowsSettleBadge(t *testing.T) { + fake, caURL := newFakeControlAPI(t) + now := time.Now() + fake.fipSettleSeconds = 60 + fake.ips = []ipQueueItem{ + {IPAddress: "1.1.1.1", State: "awaiting_self_check", FIPAssociatedAt: &now, UpdatedAt: now, CreatedAt: now}, + } + ts := newTestServer(t, caURL) + + body := get(t, ts, "/ips") + if !strings.Contains(body, "прогрев FIP") { + t.Fatalf("expected settle badge for a freshly-associated ip within the pause, got:\n%s", body) + } + + // Once fip_settle_seconds is 0, the same row falls back to the plain + // state pill. + fake.fipSettleSeconds = 0 + body = get(t, ts, "/ips") + if strings.Contains(body, "прогрев FIP") { + t.Fatalf("expected no settle badge once fip_settle_seconds is 0, got:\n%s", body) + } +} + func TestValidatorsCRUD(t *testing.T) { _, caURL := newFakeControlAPI(t) ts := newTestServer(t, caURL) diff --git a/internal/dashboard/render.go b/internal/dashboard/render.go index 91ac3da..79c2655 100644 --- a/internal/dashboard/render.go +++ b/internal/dashboard/render.go @@ -13,7 +13,14 @@ import ( // with "pill" in the template (e.g. `class="pill {{.Class}}"`). type Badge struct{ Class, Label string } -func ipBadge(state, result string) Badge { +// ipBadge picks the status pill for a queue row. fipAssociatedAt/ +// settleSeconds are only consulted for state=="awaiting_self_check": if +// the configured fip_settle_seconds pause (see docs/USAGE.md) hasn't +// elapsed since the floating IP was associated, the row gets a distinct +// "прогрев FIP" badge instead of looking identical to a row just waiting +// on the agent's next poll. The time math happens here, not in the +// template, same as fmtTime's existing precedent. +func ipBadge(state, result string, fipAssociatedAt *time.Time, settleSeconds int) Badge { switch state { case "done", "failed": switch result { @@ -28,6 +35,11 @@ func ipBadge(state, result string) Badge { } case "queued": return Badge{"pill-neutral", "queued"} + case "awaiting_self_check": + if settleSeconds > 0 && fipAssociatedAt != nil && time.Now().Before(fipAssociatedAt.Add(time.Duration(settleSeconds)*time.Second)) { + return Badge{"pill-warning", "прогрев FIP"} + } + return Badge{"pill-info", state} default: return Badge{"pill-info", state} } diff --git a/internal/dashboard/routes.go b/internal/dashboard/routes.go index ac56e2c..f9cb3d1 100644 --- a/internal/dashboard/routes.go +++ b/internal/dashboard/routes.go @@ -36,6 +36,9 @@ func (s *Server) routes(mux *http.ServeMux) { mux.HandleFunc("PUT /check-types/{name}", s.handleCheckTypeUpdate) mux.HandleFunc("DELETE /check-types/{name}", s.handleCheckTypeDelete) + mux.HandleFunc("GET /settings", s.handleSettingsPage) + mux.HandleFunc("PUT /settings", s.handleSettingsPut) + mux.Handle("GET /static/", http.StripPrefix("/static/", http.FileServerFS(staticSubFS()))) } diff --git a/internal/dashboard/templates/ip_detail.html b/internal/dashboard/templates/ip_detail.html index e00b08d..26887d6 100644 --- a/internal/dashboard/templates/ip_detail.html +++ b/internal/dashboard/templates/ip_detail.html @@ -20,7 +20,7 @@ {{define "ip_detail_content"}}

← К очереди

-{{$b := ipBadge .Detail.IP.State .Detail.IP.OverallResult}} +{{$b := ipBadge .Detail.IP.State .Detail.IP.OverallResult .Detail.IP.FIPAssociatedAt 0}}

{{.Detail.IP.IPAddress}} {{$b.Label}}

diff --git a/internal/dashboard/templates/ips.html b/internal/dashboard/templates/ips.html index 18591c2..ac84699 100644 --- a/internal/dashboard/templates/ips.html +++ b/internal/dashboard/templates/ips.html @@ -54,7 +54,7 @@ АдресСостояниеВалидаторПопыткаОбновлено {{range .Items}} -{{$b := ipBadge .State .OverallResult}} +{{$b := ipBadge .State .OverallResult .FIPAssociatedAt $.FIPSettleSeconds}} {{$terminal := or (eq .State "done") (eq .State "failed")}} diff --git a/internal/dashboard/templates/layout.html b/internal/dashboard/templates/layout.html index f0a186e..e8dfabe 100644 --- a/internal/dashboard/templates/layout.html +++ b/internal/dashboard/templates/layout.html @@ -52,6 +52,10 @@ Типы проверок + + +Настройки +
diff --git a/internal/dashboard/templates/overview_fragment.html b/internal/dashboard/templates/overview_fragment.html index 431e4fe..42c9c12 100644 --- a/internal/dashboard/templates/overview_fragment.html +++ b/internal/dashboard/templates/overview_fragment.html @@ -15,7 +15,7 @@ АдресСостояниеВалидаторПопыткаНазначено {{range .CurrentItems}} -{{$b := ipBadge .State .OverallResult}} +{{$b := ipBadge .State .OverallResult .FIPAssociatedAt 0}} {{.IPAddress}} {{$b.Label}} @@ -46,7 +46,7 @@ АдресИтогЗавершено {{range .LastCompleted}} -{{$b := ipBadge .State .OverallResult}} +{{$b := ipBadge .State .OverallResult .FIPAssociatedAt 0}} {{.IPAddress}} {{$b.Label}} diff --git a/internal/dashboard/templates/settings.html b/internal/dashboard/templates/settings.html new file mode 100644 index 0000000..9ffcf67 --- /dev/null +++ b/internal/dashboard/templates/settings.html @@ -0,0 +1,48 @@ +{{define "settings_page"}} + + +{{template "html_head" .}} + + +
+{{template "sidebar_nav" .}} +
+{{template "topbar_mobile" .}} +
+
{{template "banner_inner" .Banner}}
+{{template "settings_content" .}} +
+
+
+ + +{{end}} + +{{define "settings_content"}} +

Настройки

+ +
+{{template "settings_form" .}} +
+{{end}} + +{{define "settings_form"}} +
+
+

Пауза между привязкой Floating IP к валидатору и началом self-check — +даёт data plane OpenStack время начать пропускать трафик через только что привязанный адрес, прежде чем +проверять его. 0 — без паузы (поведение по умолчанию). Значение должно оставлять запас внутри лизинга +адреса: fip_settle_seconds + self_check_timeout_seconds < lease_ttl_seconds — иначе адрес не успеет +пройти self-check до истечения лизинга и будет возвращён в очередь.

+ +
+
+ + +
+ +
+ +
+
+{{end}} diff --git a/internal/db/bootstrap.go b/internal/db/bootstrap.go index d15cac9..4e3dc02 100644 --- a/internal/db/bootstrap.go +++ b/internal/db/bootstrap.go @@ -33,6 +33,9 @@ func (d *DB) BootstrapFromConfig(ctx context.Context, cfg *config.ControlAPI) er if err := d.bootstrapCheckTypes(ctx, cfg.CheckTypes); err != nil { return fmt.Errorf("bootstrap check types: %w", err) } + if err := d.bootstrapSettings(ctx, cfg.Orchestrator.FIPSettleSeconds); err != nil { + return fmt.Errorf("bootstrap settings: %w", err) + } if err := d.SeedQueue(ctx, cfg.IPAddresses); err != nil { return fmt.Errorf("seed ip queue: %w", err) } @@ -102,3 +105,18 @@ func (d *DB) bootstrapCheckTypes(ctx context.Context, checkTypes []config.CheckT } return nil } + +func (d *DB) bootstrapSettings(ctx context.Context, fipSettleSeconds int) error { + var count int + if err := d.QueryRowContext(ctx, `SELECT COUNT(*) FROM settings`).Scan(&count); err != nil { + return err + } + if count > 0 { + return nil + } + now := timeToDB(Now()) + _, err := d.ExecContext(ctx, ` + INSERT INTO settings (id, fip_settle_seconds, created_at, updated_at) VALUES (1, ?, ?, ?) + `, fipSettleSeconds, now, now) + return err +} diff --git a/internal/db/db.go b/internal/db/db.go index 163390c..afdc453 100644 --- a/internal/db/db.go +++ b/internal/db/db.go @@ -19,6 +19,9 @@ var initSchema string //go:embed migrations/0002_dynamic_config.sql var dynamicConfigSchema string +//go:embed migrations/0003_fip_settle_delay.sql +var fipSettleDelaySchema string + // migrations is the ordered list of schema versions. Each entry's SQL is // applied, in order, for any version greater than the database's current // PRAGMA user_version — so a fresh database walks the whole list and an @@ -29,6 +32,7 @@ var migrations = []struct { }{ {1, initSchema}, {2, dynamicConfigSchema}, + {3, fipSettleDelaySchema}, } type DB struct { diff --git a/internal/db/migrations/0003_fip_settle_delay.sql b/internal/db/migrations/0003_fip_settle_delay.sql new file mode 100644 index 0000000..6fef717 --- /dev/null +++ b/internal/db/migrations/0003_fip_settle_delay.sql @@ -0,0 +1,15 @@ +-- Support for a configurable pause between FIP association and the start +-- of self-check (see docs/USAGE.md). + +ALTER TABLE ip_queue ADD COLUMN fip_associated_at TIMESTAMP; + +-- Singleton row for the one dynamic scalar setting we have so far. +-- Deliberately a typed single-row table, not a generic key-value settings +-- table: overengineering for exactly one field. If more scalar knobs show +-- up, add columns here first. +CREATE TABLE settings ( + id INTEGER PRIMARY KEY CHECK (id = 1), + fip_settle_seconds INTEGER NOT NULL DEFAULT 0, + created_at TIMESTAMP NOT NULL, + updated_at TIMESTAMP NOT NULL +); diff --git a/internal/db/models.go b/internal/db/models.go index 5486ec8..ad60176 100644 --- a/internal/db/models.go +++ b/internal/db/models.go @@ -84,6 +84,7 @@ type IPQueueItem struct { Site3Complete bool OverallResult string AssignedAt *time.Time + FIPAssociatedAt *time.Time AggregatedAt *time.Time FIPReleasedAt *time.Time CreatedAt time.Time @@ -162,3 +163,11 @@ type DeleteIPsResult struct { Deleted []string NotFound []string } + +// Settings is the singleton row of orchestrator-wide scalar knobs that are +// admin-configurable at runtime (see queries_settings.go). +type Settings struct { + FIPSettleSeconds int + CreatedAt time.Time + UpdatedAt time.Time +} diff --git a/internal/db/queries_ipqueue.go b/internal/db/queries_ipqueue.go index de2062c..8c74304 100644 --- a/internal/db/queries_ipqueue.go +++ b/internal/db/queries_ipqueue.go @@ -110,9 +110,9 @@ func (d *DB) ClaimNextQueued(ctx context.Context, validatorID string, leaseTTL t func (d *DB) SetFIPAssociated(ctx context.Context, ipID int64, fipID string, leaseTTL time.Duration) error { now := Now() _, err := d.ExecContext(ctx, ` - UPDATE ip_queue SET state=?, fip_id=?, lease_expires_at=?, updated_at=? + UPDATE ip_queue SET state=?, fip_id=?, fip_associated_at=?, lease_expires_at=?, updated_at=? WHERE id=? - `, IPAwaitingSelfCheck, fipID, timeToDB(now.Add(leaseTTL)), timeToDB(now), ipID) + `, IPAwaitingSelfCheck, fipID, timeToDB(now), timeToDB(now.Add(leaseTTL)), timeToDB(now), ipID) return err } @@ -197,7 +197,7 @@ func (d *DB) RequeueOrFail(ctx context.Context, ipID int64, validatorID string, UPDATE ip_queue SET state=?, owner_validator_id=NULL, fip_id='', retry_count=?, attempt_number=attempt_number+1, lease_expires_at=NULL, egress_complete=0, site1_complete=0, site2_complete=0, site3_complete=0, - overall_result='', assigned_at=NULL, updated_at=? + overall_result='', assigned_at=NULL, fip_associated_at=NULL, updated_at=? WHERE id=? `, nextState, retryCount, now, ipID) } else { @@ -283,7 +283,7 @@ func (d *DB) SubmitIPs(ctx context.Context, addresses []string) (SubmitIPsResult state=?, sequence=?, owner_validator_id=NULL, fip_id='', retry_count=0, attempt_number=attempt_number+1, lease_expires_at=NULL, egress_complete=0, site1_complete=0, site2_complete=0, site3_complete=0, overall_result='', - assigned_at=NULL, aggregated_at=NULL, fip_released_at=NULL, updated_at=? + assigned_at=NULL, fip_associated_at=NULL, aggregated_at=NULL, fip_released_at=NULL, updated_at=? WHERE ip_address=? `, IPQueued, seq, now, addr); err != nil { return result, fmt.Errorf("requeue %s: %w", addr, err) @@ -480,7 +480,7 @@ func (d *DB) ListExpiredLeases(ctx context.Context, now time.Time) ([]IPQueueIte const ipQueueSelect = ` SELECT id, ip_address, sequence, state, owner_validator_id, fip_id, attempt_number, retry_count, lease_expires_at, egress_complete, site1_complete, site2_complete, site3_complete, overall_result, - assigned_at, aggregated_at, fip_released_at, created_at, updated_at + assigned_at, fip_associated_at, aggregated_at, fip_released_at, created_at, updated_at FROM ip_queue ` @@ -499,13 +499,13 @@ func scanIPQueueItems(rows *sql.Rows) ([]IPQueueItem, error) { func scanIPQueueItem(row rowScanner) (*IPQueueItem, error) { var item IPQueueItem var owner sql.NullString - var leaseExpires, assignedAt, aggregatedAt, fipReleasedAt sql.NullString + var leaseExpires, assignedAt, fipAssociatedAt, aggregatedAt, fipReleasedAt sql.NullString var createdAt, updatedAt string if err := row.Scan( &item.ID, &item.IPAddress, &item.Sequence, &item.State, &owner, &item.FIPID, &item.AttemptNumber, &item.RetryCount, &leaseExpires, &item.EgressComplete, &item.Site1Complete, &item.Site2Complete, &item.Site3Complete, - &item.OverallResult, &assignedAt, &aggregatedAt, &fipReleasedAt, &createdAt, &updatedAt, + &item.OverallResult, &assignedAt, &fipAssociatedAt, &aggregatedAt, &fipReleasedAt, &createdAt, &updatedAt, ); err != nil { return nil, err } @@ -519,6 +519,9 @@ func scanIPQueueItem(row rowScanner) (*IPQueueItem, error) { if item.AssignedAt, err = nullStringToTimePtr(assignedAt); err != nil { return nil, err } + if item.FIPAssociatedAt, err = nullStringToTimePtr(fipAssociatedAt); err != nil { + return nil, err + } if item.AggregatedAt, err = nullStringToTimePtr(aggregatedAt); err != nil { return nil, err } diff --git a/internal/db/queries_ipqueue_test.go b/internal/db/queries_ipqueue_test.go new file mode 100644 index 0000000..13cae25 --- /dev/null +++ b/internal/db/queries_ipqueue_test.go @@ -0,0 +1,71 @@ +package db + +import ( + "testing" + "time" +) + +func TestSetFIPAssociatedStampsTimestamp(t *testing.T) { + d, ctx := newTestDB(t) + + if err := d.SeedQueue(ctx, []string{"1.2.3.4"}); err != nil { + t.Fatalf("seed queue: %v", err) + } + if err := d.AdminCreateValidator(ctx, "validator-1", "port-1"); err != nil { + t.Fatalf("create validator: %v", err) + } + claimed, err := d.ClaimNextQueued(ctx, "validator-1", time.Minute) + if err != nil || claimed == nil { + t.Fatalf("claim: item=%+v err=%v", claimed, err) + } + if claimed.FIPAssociatedAt != nil { + t.Fatalf("expected FIPAssociatedAt nil before association, got %v", claimed.FIPAssociatedAt) + } + + before := Now() + if err := d.SetFIPAssociated(ctx, claimed.ID, "fip-1", time.Minute); err != nil { + t.Fatalf("set fip associated: %v", err) + } + item, err := d.GetIP(ctx, claimed.ID) + if err != nil { + t.Fatalf("get ip: %v", err) + } + if item.FIPAssociatedAt == nil { + t.Fatalf("expected FIPAssociatedAt to be set") + } + if item.FIPAssociatedAt.Before(before) { + t.Fatalf("expected FIPAssociatedAt >= %v, got %v", before, *item.FIPAssociatedAt) + } +} + +func TestRequeueClearsFIPAssociatedAt(t *testing.T) { + d, ctx := newTestDB(t) + + if err := d.SeedQueue(ctx, []string{"1.2.3.4"}); err != nil { + t.Fatalf("seed queue: %v", err) + } + if err := d.AdminCreateValidator(ctx, "validator-1", "port-1"); err != nil { + t.Fatalf("create validator: %v", err) + } + claimed, err := d.ClaimNextQueued(ctx, "validator-1", time.Minute) + if err != nil || claimed == nil { + t.Fatalf("claim: item=%+v err=%v", claimed, err) + } + if err := d.SetFIPAssociated(ctx, claimed.ID, "fip-1", time.Minute); err != nil { + t.Fatalf("set fip associated: %v", err) + } + + if err := d.RequeueOrFail(ctx, claimed.ID, "validator-1", 3); err != nil { + t.Fatalf("requeue or fail: %v", err) + } + item, err := d.GetIP(ctx, claimed.ID) + if err != nil { + t.Fatalf("get ip after requeue: %v", err) + } + if item.State != IPQueued { + t.Fatalf("expected requeued, got state=%s", item.State) + } + if item.FIPAssociatedAt != nil { + t.Fatalf("expected FIPAssociatedAt cleared on requeue, got %v", item.FIPAssociatedAt) + } +} diff --git a/internal/db/queries_settings.go b/internal/db/queries_settings.go new file mode 100644 index 0000000..44eaff0 --- /dev/null +++ b/internal/db/queries_settings.go @@ -0,0 +1,44 @@ +package db + +import ( + "context" + "fmt" +) + +// GetSettings returns the singleton settings row. BootstrapFromConfig +// guarantees it exists before any other code path can observe it, so +// sql.ErrNoRows here would indicate a bootstrap bug, not a normal +// condition. +func (d *DB) GetSettings(ctx context.Context) (Settings, error) { + var s Settings + var createdAt, updatedAt string + err := d.QueryRowContext(ctx, ` + SELECT fip_settle_seconds, created_at, updated_at FROM settings WHERE id=1 + `).Scan(&s.FIPSettleSeconds, &createdAt, &updatedAt) + if err != nil { + return Settings{}, err + } + if s.CreatedAt, err = dbToTime(createdAt); err != nil { + return Settings{}, err + } + if s.UpdatedAt, err = dbToTime(updatedAt); err != nil { + return Settings{}, err + } + return s, nil +} + +// SetFIPSettleSeconds persists a new fip_settle_seconds value. Only +// enforces seconds >= 0 — the cross-field rule against +// lease_ttl_seconds/self_check_timeout_seconds lives in +// orchestrator.Orchestrator.SetFIPSettleSeconds, which has access to the +// static orchestrator config this package doesn't. +func (d *DB) SetFIPSettleSeconds(ctx context.Context, seconds int) error { + if seconds < 0 { + return fmt.Errorf("fip_settle_seconds must be >= 0: %w", ErrValidation) + } + now := timeToDB(Now()) + _, err := d.ExecContext(ctx, ` + UPDATE settings SET fip_settle_seconds=?, updated_at=? WHERE id=1 + `, seconds, now) + return err +} diff --git a/internal/db/queries_settings_test.go b/internal/db/queries_settings_test.go new file mode 100644 index 0000000..86514db --- /dev/null +++ b/internal/db/queries_settings_test.go @@ -0,0 +1,77 @@ +package db + +import ( + "errors" + "testing" + + "cloudipvalidator/internal/config" +) + +func TestBootstrapSettingsSeedsOnceFromConfig(t *testing.T) { + d, ctx := newTestDB(t) + + cfg := &config.ControlAPI{Orchestrator: config.OrchestratorConfig{FIPSettleSeconds: 30}} + if err := d.BootstrapFromConfig(ctx, cfg); err != nil { + t.Fatalf("first bootstrap: %v", err) + } + settings, err := d.GetSettings(ctx) + if err != nil { + t.Fatalf("get settings: %v", err) + } + if settings.FIPSettleSeconds != 30 { + t.Fatalf("expected seeded value 30, got %d", settings.FIPSettleSeconds) + } + + // A second bootstrap with a different YAML value must not overwrite the + // now-non-empty settings table — same "DB is source of truth once + // seeded" semantics as validators/sites/targets/check_types. + cfg2 := &config.ControlAPI{Orchestrator: config.OrchestratorConfig{FIPSettleSeconds: 99}} + if err := d.BootstrapFromConfig(ctx, cfg2); err != nil { + t.Fatalf("second bootstrap: %v", err) + } + settings, err = d.GetSettings(ctx) + if err != nil { + t.Fatalf("get settings after second bootstrap: %v", err) + } + if settings.FIPSettleSeconds != 30 { + t.Fatalf("expected YAML to be ignored on non-empty settings table, got %d", settings.FIPSettleSeconds) + } +} + +func TestSetFIPSettleSecondsRejectsNegative(t *testing.T) { + d, ctx := newTestDB(t) + if err := d.BootstrapFromConfig(ctx, &config.ControlAPI{}); err != nil { + t.Fatalf("bootstrap: %v", err) + } + if err := d.SetFIPSettleSeconds(ctx, -1); !errors.Is(err, ErrValidation) { + t.Fatalf("expected ErrValidation for negative seconds, got %v", err) + } +} + +func TestGetSetFIPSettleSecondsRoundTrip(t *testing.T) { + d, ctx := newTestDB(t) + if err := d.BootstrapFromConfig(ctx, &config.ControlAPI{}); err != nil { + t.Fatalf("bootstrap: %v", err) + } + before, err := d.GetSettings(ctx) + if err != nil { + t.Fatalf("get settings: %v", err) + } + if before.FIPSettleSeconds != 0 { + t.Fatalf("expected default 0, got %d", before.FIPSettleSeconds) + } + + if err := d.SetFIPSettleSeconds(ctx, 45); err != nil { + t.Fatalf("set fip settle seconds: %v", err) + } + after, err := d.GetSettings(ctx) + if err != nil { + t.Fatalf("get settings after set: %v", err) + } + if after.FIPSettleSeconds != 45 { + t.Fatalf("expected 45, got %d", after.FIPSettleSeconds) + } + if after.UpdatedAt.Before(before.UpdatedAt) { + t.Fatalf("expected updated_at not to go backwards, before=%v after=%v", before.UpdatedAt, after.UpdatedAt) + } +} diff --git a/internal/httpapi/dto_admin.go b/internal/httpapi/dto_admin.go index 9602eb7..3f8209b 100644 --- a/internal/httpapi/dto_admin.go +++ b/internal/httpapi/dto_admin.go @@ -73,3 +73,10 @@ type putCheckTypeRequest struct { Enabled bool `json:"enabled"` Targets []string `json:"targets"` } + +// orchestratorSettingsDTO doubles as both the GET response and the PUT +// request body for /api/v1/admin/config/orchestrator — a single-field DTO, +// same shape both ways, like putSiteRequest/siteDTO. +type orchestratorSettingsDTO struct { + FIPSettleSeconds int `json:"fip_settle_seconds"` +} diff --git a/internal/httpapi/handlers_config.go b/internal/httpapi/handlers_config.go index 4352d73..df513c1 100644 --- a/internal/httpapi/handlers_config.go +++ b/internal/httpapi/handlers_config.go @@ -181,3 +181,31 @@ func (s *Server) handleConfigDeleteCheckType(w http.ResponseWriter, r *http.Requ } writeJSON(w, http.StatusOK, okResponse{OK: true}) } + +// --- orchestrator settings --- + +func (s *Server) handleConfigGetOrchestratorSettings(w http.ResponseWriter, r *http.Request) { + settings, err := s.DB.GetSettings(r.Context()) + if err != nil { + writeDBError(w, err) + return + } + writeJSON(w, http.StatusOK, orchestratorSettingsDTO{FIPSettleSeconds: settings.FIPSettleSeconds}) +} + +// handleConfigPutOrchestratorSettings goes through the orchestrator (not a +// direct DB call, unlike the read above) because setting fip_settle_seconds +// needs cross-field validation against the static lease_ttl_seconds/ +// self_check_timeout_seconds config, which only the orchestrator has. +func (s *Server) handleConfigPutOrchestratorSettings(w http.ResponseWriter, r *http.Request) { + var req orchestratorSettingsDTO + if err := readJSON(r, &req); err != nil { + writeError(w, http.StatusBadRequest, "invalid body: "+err.Error()) + return + } + if err := s.Orch.SetFIPSettleSeconds(r.Context(), req.FIPSettleSeconds); err != nil { + writeDBError(w, err) + return + } + writeJSON(w, http.StatusOK, orchestratorSettingsDTO{FIPSettleSeconds: req.FIPSettleSeconds}) +} diff --git a/internal/httpapi/handlers_config_test.go b/internal/httpapi/handlers_config_test.go index acf7353..13ac30c 100644 --- a/internal/httpapi/handlers_config_test.go +++ b/internal/httpapi/handlers_config_test.go @@ -410,3 +410,93 @@ func TestDeleteIPsAndClearQueue(t *testing.T) { t.Fatalf("expected validator freed after clear, got state=%s current_ip=%v", v.State, v.CurrentIPID) } } + +// TestOrchestratorSettingsGetPut proves the settle-delay setting round-trips +// through GET/PUT /api/v1/admin/config/orchestrator. +func TestOrchestratorSettingsGetPut(t *testing.T) { + fc, _, _, _ := newConfigTestHarness(t) + + resp, body := fc.do(http.MethodGet, "/api/v1/admin/config/orchestrator", nil) + if resp.StatusCode != http.StatusOK { + t.Fatalf("get settings: status=%d body=%s", resp.StatusCode, body) + } + var got orchestratorSettingsDTO + if err := json.Unmarshal(body, &got); err != nil { + t.Fatalf("unmarshal get response: %v", err) + } + if got.FIPSettleSeconds != 0 { + t.Fatalf("expected default 0, got %+v", got) + } + + resp, body = fc.do(http.MethodPut, "/api/v1/admin/config/orchestrator", orchestratorSettingsDTO{FIPSettleSeconds: 20}) + if resp.StatusCode != http.StatusOK { + t.Fatalf("put settings: status=%d body=%s", resp.StatusCode, body) + } + if err := json.Unmarshal(body, &got); err != nil { + t.Fatalf("unmarshal put response: %v", err) + } + if got.FIPSettleSeconds != 20 { + t.Fatalf("expected 20, got %+v", got) + } + + resp, body = fc.do(http.MethodGet, "/api/v1/admin/config/orchestrator", nil) + if resp.StatusCode != http.StatusOK { + t.Fatalf("get settings after put: status=%d body=%s", resp.StatusCode, body) + } + if err := json.Unmarshal(body, &got); err != nil { + t.Fatalf("unmarshal get-after-put response: %v", err) + } + if got.FIPSettleSeconds != 20 { + t.Fatalf("expected 20 to persist, got %+v", got) + } +} + +// TestOrchestratorSettingsPutValidation proves a value that would leave no +// room for self-check inside the claim lease is rejected with 400. +func TestOrchestratorSettingsPutValidation(t *testing.T) { + fc, _, _, _ := newConfigTestHarness(t) + // newConfigTestHarness: LeaseTTLSeconds=180, SelfCheckTimeoutSeconds=10. + + resp, body := fc.do(http.MethodPut, "/api/v1/admin/config/orchestrator", orchestratorSettingsDTO{FIPSettleSeconds: 175}) + if resp.StatusCode != http.StatusBadRequest { + t.Fatalf("expected 400 for settle seconds too close to lease ttl, status=%d body=%s", resp.StatusCode, body) + } + + resp, body = fc.do(http.MethodPut, "/api/v1/admin/config/orchestrator", orchestratorSettingsDTO{FIPSettleSeconds: -1}) + if resp.StatusCode != http.StatusBadRequest { + t.Fatalf("expected 400 for negative settle seconds, status=%d body=%s", resp.StatusCode, body) + } +} + +// TestFIPSettleDelayGatesAssignmentEndpoint proves GET +// /api/v1/agents/{id}/assignment returns 204 while the settle window is +// open and 200 once it has elapsed — the end-to-end proof of the whole +// feature over the real HTTP wire contract. +func TestFIPSettleDelayGatesAssignmentEndpoint(t *testing.T) { + fc, _, orch, mock := newConfigTestHarness(t) + ctx := context.Background() + mock.Seed("fip-1", "9.9.9.9", "svc-project") + + resp, body := fc.do(http.MethodPut, "/api/v1/admin/config/orchestrator", orchestratorSettingsDTO{FIPSettleSeconds: 1}) + if resp.StatusCode != http.StatusOK { + t.Fatalf("put settings: status=%d body=%s", resp.StatusCode, body) + } + + fc.do(http.MethodPost, "/api/v1/admin/config/validators", createValidatorRequest{ValidatorID: "validator-1", OSPortID: "port-1"}) + fc.do(http.MethodPost, "/api/v1/agents/register", registerAgentRequest{ValidatorID: "validator-1"}) + fc.do(http.MethodPost, "/api/v1/admin/ips", submitIPsRequest{Addresses: []string{"9.9.9.9"}}) + + orch.Tick(ctx) // claim + associate -> awaiting_self_check, fip attached + + resp, body = fc.do(http.MethodGet, "/api/v1/agents/validator-1/assignment", nil) + if resp.StatusCode != http.StatusNoContent { + t.Fatalf("expected 204 during settle window, status=%d body=%s", resp.StatusCode, body) + } + + time.Sleep(1100 * time.Millisecond) + + resp, body = fc.do(http.MethodGet, "/api/v1/agents/validator-1/assignment", nil) + if resp.StatusCode != http.StatusOK { + t.Fatalf("expected 200 after settle window elapsed, status=%d body=%s", resp.StatusCode, body) + } +} diff --git a/internal/httpapi/routes.go b/internal/httpapi/routes.go index 419609e..9e02761 100644 --- a/internal/httpapi/routes.go +++ b/internal/httpapi/routes.go @@ -43,4 +43,7 @@ func (s *Server) routes(mux *http.ServeMux) { mux.HandleFunc("GET /api/v1/admin/config/check-types", s.handleConfigListCheckTypes) mux.HandleFunc("PUT /api/v1/admin/config/check-types/{name}", s.handleConfigPutCheckType) mux.HandleFunc("DELETE /api/v1/admin/config/check-types/{name}", s.handleConfigDeleteCheckType) + + mux.HandleFunc("GET /api/v1/admin/config/orchestrator", s.handleConfigGetOrchestratorSettings) + mux.HandleFunc("PUT /api/v1/admin/config/orchestrator", s.handleConfigPutOrchestratorSettings) } diff --git a/internal/orchestrator/orchestrator.go b/internal/orchestrator/orchestrator.go index 4e4b1a8..415016f 100644 --- a/internal/orchestrator/orchestrator.go +++ b/internal/orchestrator/orchestrator.go @@ -170,6 +170,15 @@ func (o *Orchestrator) AssignmentForValidator(ctx context.Context, validatorID s if item.State != db.IPAwaitingSelfCheck && item.State != db.IPChecking { return nil, nil, nil } + if item.State == db.IPAwaitingSelfCheck { + settled, err := o.isFIPSettled(ctx, item) + if err != nil { + return nil, nil, err + } + if !settled { + return nil, nil, nil + } + } resolved, err := o.DB.ListResolvedCheckTypes(ctx) if err != nil { return nil, nil, err @@ -181,6 +190,47 @@ func (o *Orchestrator) AssignmentForValidator(ctx context.Context, validatorID s return item, checks, nil } +// isFIPSettled reports whether an address currently awaiting_self_check +// has cleared the configured fip_settle_seconds pause since its floating +// IP was associated — withholding the assignment until then is how the +// pause is enforced, with zero changes needed to the agent's poll loop or +// the assignment endpoint's wire contract (it just keeps seeing 204s). +// Settings are read fresh on every call, same as check_types/sites +// elsewhere in this file, so an admin change applies immediately even to +// an address already mid-wait. A nil FIPAssociatedAt (an in-flight row +// from before this feature's migration) is always treated as settled — +// upgrading control-api must never newly strand an address that was +// already awaiting self-check. +func (o *Orchestrator) isFIPSettled(ctx context.Context, item *db.IPQueueItem) (bool, error) { + settings, err := o.DB.GetSettings(ctx) + if err != nil { + return false, err + } + if settings.FIPSettleSeconds <= 0 || item.FIPAssociatedAt == nil { + return true, nil + } + deadline := item.FIPAssociatedAt.Add(time.Duration(settings.FIPSettleSeconds) * time.Second) + return !db.Now().Before(deadline), nil +} + +// SetFIPSettleSeconds validates and persists a new fip_settle_seconds +// value. Lives here rather than internal/db because the cross-field rule +// below needs o.Cfg, which the db package has no access to: the pause plus +// self-check's own timeout must leave room inside the claim lease, or the +// lease sweep would reclaim the address before self-check ever gets a +// chance to run, producing a perpetual requeue loop. +func (o *Orchestrator) SetFIPSettleSeconds(ctx context.Context, seconds int) error { + if seconds < 0 { + return fmt.Errorf("fip_settle_seconds must be >= 0: %w", db.ErrValidation) + } + if seconds+o.Cfg.SelfCheckTimeoutSeconds >= o.Cfg.LeaseTTLSeconds { + return fmt.Errorf( + "fip_settle_seconds (%d) + self_check_timeout_seconds (%d) must be < lease_ttl_seconds (%d): %w", + seconds, o.Cfg.SelfCheckTimeoutSeconds, o.Cfg.LeaseTTLSeconds, db.ErrValidation) + } + return o.DB.SetFIPSettleSeconds(ctx, seconds) +} + // SiteIndexForID resolves a configured site_id to its 1/2/3 index, or // (0, nil) if unconfigured. func (o *Orchestrator) SiteIndexForID(ctx context.Context, siteID string) (int, error) { diff --git a/internal/orchestrator/orchestrator_test.go b/internal/orchestrator/orchestrator_test.go index 0c6505a..70543db 100644 --- a/internal/orchestrator/orchestrator_test.go +++ b/internal/orchestrator/orchestrator_test.go @@ -409,6 +409,124 @@ func TestClearQueueDisassociatesAllFIPs(t *testing.T) { } } +// TestFIPSettleDelayWithholdsAssignment proves a nonzero fip_settle_seconds +// keeps AssignmentForValidator returning nil (agent keeps polling and +// getting nothing) until the configured pause has elapsed since the +// floating IP was associated. +func TestFIPSettleDelayWithholdsAssignment(t *testing.T) { + ctx := context.Background() + o, d, mock := newTestOrchestrator(t, 180) + if err := o.SetFIPSettleSeconds(ctx, 1); err != nil { + t.Fatalf("set fip settle seconds: %v", err) + } + mock.Seed("fip-1", "1.2.3.4", "svc-project") + _ = d.RegisterValidator(ctx, "validator-1", "host-1", "port-1", "v0.1") + _ = d.SeedQueue(ctx, []string{"1.2.3.4"}) + + o.Tick(ctx) // claim + associate -> awaiting_self_check, fip attached + + item, checks, err := o.AssignmentForValidator(ctx, "validator-1") + if err != nil { + t.Fatalf("assignment for validator: %v", err) + } + if item != nil || checks != nil { + t.Fatalf("expected no assignment during settle window, got item=%+v checks=%+v", item, checks) + } + + time.Sleep(1100 * time.Millisecond) + + item, checks, err = o.AssignmentForValidator(ctx, "validator-1") + if err != nil { + t.Fatalf("assignment for validator after settle: %v", err) + } + if item == nil { + t.Fatalf("expected assignment to be available once settle window elapsed") + } + if len(checks) == 0 { + t.Fatalf("expected check config to be returned alongside the item") + } +} + +// TestFIPSettleDelayZeroIsNoOp proves the default fip_settle_seconds=0 +// never withholds an assignment — no behavior change for upgraded-but- +// unconfigured installs. +func TestFIPSettleDelayZeroIsNoOp(t *testing.T) { + ctx := context.Background() + o, d, mock := newTestOrchestrator(t, 180) + mock.Seed("fip-1", "1.2.3.4", "svc-project") + _ = d.RegisterValidator(ctx, "validator-1", "host-1", "port-1", "v0.1") + _ = d.SeedQueue(ctx, []string{"1.2.3.4"}) + + o.Tick(ctx) + + item, _, err := o.AssignmentForValidator(ctx, "validator-1") + if err != nil { + t.Fatalf("assignment for validator: %v", err) + } + if item == nil { + t.Fatalf("expected assignment immediately available with fip_settle_seconds=0") + } +} + +// TestFIPSettleDelayIgnoresNilFIPAssociatedAt proves an in-flight row from +// before this feature's migration (fip_associated_at NULL) is never newly +// blocked, even with a nonzero configured pause — upgrading control-api +// must not strand an address already awaiting self-check. +func TestFIPSettleDelayIgnoresNilFIPAssociatedAt(t *testing.T) { + ctx := context.Background() + o, d, mock := newTestOrchestrator(t, 180) + if err := o.SetFIPSettleSeconds(ctx, 60); err != nil { + t.Fatalf("set fip settle seconds: %v", err) + } + mock.Seed("fip-1", "1.2.3.4", "svc-project") + _ = d.RegisterValidator(ctx, "validator-1", "host-1", "port-1", "v0.1") + _ = d.SeedQueue(ctx, []string{"1.2.3.4"}) + + o.Tick(ctx) + + item, err := d.GetIPByAddress(ctx, "1.2.3.4") + if err != nil { + t.Fatalf("get ip: %v", err) + } + if _, err := d.ExecContext(ctx, `UPDATE ip_queue SET fip_associated_at=NULL WHERE id=?`, item.ID); err != nil { + t.Fatalf("clear fip_associated_at: %v", err) + } + + assigned, _, err := o.AssignmentForValidator(ctx, "validator-1") + if err != nil { + t.Fatalf("assignment for validator: %v", err) + } + if assigned == nil { + t.Fatalf("expected assignment available for a row with nil FIPAssociatedAt despite nonzero settle delay") + } +} + +// TestSetFIPSettleSecondsValidation proves the cross-field rule against +// lease_ttl_seconds/self_check_timeout_seconds is enforced. +func TestSetFIPSettleSecondsValidation(t *testing.T) { + ctx := context.Background() + o, _, _ := newTestOrchestrator(t, 180) // SelfCheckTimeoutSeconds: 10 (see newTestOrchestratorWithSites) + + cases := []struct { + seconds int + wantErr bool + }{ + {-1, true}, + {170, true}, // 170+10 >= 180 + {169, false}, + {0, false}, + } + for _, c := range cases { + err := o.SetFIPSettleSeconds(ctx, c.seconds) + if c.wantErr && !errors.Is(err, db.ErrValidation) { + t.Errorf("seconds=%d: expected ErrValidation, got %v", c.seconds, err) + } + if !c.wantErr && err != nil { + t.Errorf("seconds=%d: expected success, got %v", c.seconds, err) + } + } +} + // TestInboundChecksPartialSites confirms a partially-configured sites list // (fewer than 3 slots assigned) only waits on the sites actually // configured — the two unassigned slots are never expected.