diff --git a/docs/drafts/ansible-review.md b/docs/drafts/ansible-review.md new file mode 100644 index 0000000..296363a --- /dev/null +++ b/docs/drafts/ansible-review.md @@ -0,0 +1,150 @@ +# Ревью плейбуков: best practices и конвенции Ansible + +Дата: 2026-05-25. Статус: черновик (заметки по итогам ревью, не план работ). + +Проанализированы инвентарь, `ansible.cfg`, роли (`owner`, `eget`, `secrets`) и +репрезентативная выборка плейбуков: `gitea`, `memos`, `wanderer`, `backups`, +`system`, `caddyproxy`, `authelia`, `netdata`, `docker`, `eget`, `all-*`. +Находки отсортированы по влиянию. + +## Договорённость по структуре (важно для контекста) + +Изначальная рекомендация «вынести общий деплой в одну generic-роль `docker_app`» +**отклонена осознанно** и не должна предлагаться снова: + +- приложения реально разные, мелкие отличия больно загонять в единую абстракцию; +- catch-all роль обрастает флагами `when:` и читается хуже, чем N честных плейбуков; +- per-playbook дублирование даёт locality of behavior и возможность обкатать новый + подход на одном сервисе, затем раскатать на остальные. + +Правильное направление — **набор маленьких composable-ролей на инвариантных швах** +(как уже сделано с `owner`), а не одна роль на всё. Per-app конфиг остаётся локально +в плейбуке сервиса. + +## 1. Extraction только на чистых швах (не мега-роль) + +Per-app конфиг (каталоги, шаблоны, env, порты, особенности compose) — оставляем в +плейбуке сервиса. Выносим лишь то, что реально инвариантно и повторилось многократно: + +- **Бэкап** — самый чистый шов: `gobackup.yml` + `backup.sh` + `backup-targets` + + интеграция с restic. Механизм одинаков у всех, различается только список целей. + Роль `backup` с параметром «список targets» не трогает индивидуальность сервиса. +- `owner` уже сделан как отдельная composable-роль — это правильный размер абстракции. + +## 2. `vars_files` в каждом плейбуке → `group_vars/all/` + +В каждом плейбуке повторяется: + +```yaml +vars_files: + - vars/secrets.yml + - vars/vars.yml +``` + +Ansible автоматически подхватывает `group_vars/all.yml` и `group_vars/all/secrets.yml` +(vault) для группы `all`. Перенос `vars/vars.yml` → `group_vars/all/main.yml` и +`vars/secrets.yml` → `group_vars/all/vault.yml` убирает boilerplate из всех плейбуков. +Адаптируется по одному плейбуку за раз. + +## 3. Нет handlers — `state: restarted` безусловный + +Ни в одном плейбуке нет `handlers:`. Вместо этого: + +- `playbook-caddyproxy.yml:106`, `playbook-netdata.yml:143`, `playbook-authelia.yml:92` — + задача `state: restarted` выполняется **всегда**, рестартит контейнер на каждом + прогоне даже без изменений (не идемпотентно, лишний downtime); +- `playbook-gitea.yml` — рестарта нет вовсе (несогласованность). + +Канонический паттерн: шаблон конфига `notify`-ит handler, который рестартит только при +реальном изменении. + +```yaml +- name: "Copy docker compose file" + ansible.builtin.template: { ... } + notify: Restart app + +handlers: + - name: Restart app + community.docker.docker_compose_v2: + project_src: "{{ base_dir }}" + state: restarted +``` + +Связанное: в `playbook-memos.yml:76` результат шаблона регистрируется в +`docker_compose_file_result`, но нигде не используется — задумывалось под `when`/`notify`, +не доведено. + +Внедряется инкрементально, по одному сервису. + +## 4. Идемпотентность и `changed_when` + +- **`playbook-netdata.yml:118-125`** — `changed_when: netdata_docker_group_output.rc != 0` + для read-only запроса лишено смысла (помечает «changed» только при ошибке). Должно быть + `changed_when: false`. Лучше заменить `shell: grep docker /etc/group | cut ...` на модуль: + + ```yaml + - ansible.builtin.getent: + database: group + key: docker + # далее: getent_group['docker'][1] + ``` + + Уйдёт и `set -o pipefail`, и хрупкий парсинг. + +- **`playbook-eget.yml:23-78`** — восемь `command` помечены `changed_when: false`, хотя + реально ставят/обновляют бинарники. Прогон всегда рапортует «ok» — теряется честность + `--diff`. Сама роль `eget` делает корректную проверку версии; те же инсталляции через + неё или через проверку версии были бы идемпотентны по-настоящему. + +- **`playbook-memos.yml:57-67`** (и аналоги) — сборка `backup-targets` через `lineinfile` + в цикле не удаляет устаревшие строки при изменении списка, а `mode: "0750"` на + файле-списке выглядит как copy-paste. Чище — `template`/`copy: content` со всем списком. + +## 5. Роль `owner` — несогласованность с ролью `eget` + +- **`roles/owner/tasks/main.yml:2-10`** — валидация аргументов через `fail` + `when`, + причём две задачи с **идентичным именем**. Роль `eget` для того же делает `assert` + (`roles/eget/tasks/main.yml:15`). Привести к одному стилю — `assert` либо современный + `meta/argument_specs.yml` (декларативная валидация). +- **`roles/owner/tasks/main.yml:32,53`** — `with_items`/`with_dict` устарели; конвенция — + `loop`: `loop: "{{ owner_ssh_keys }}"`, `loop: "{{ owner_env_dict | dict2items }}"`. +- У `owner` нет `meta/main.yml` и README, тогда как у `eget` и `secrets` они есть. +- Имена задач в `owner` с точкой на конце (`"Prepare env variables."`), в остальных без — + ansible-lint в строгом профиле это ловит. + +## 6. Инвентарь и `become` + +- **`production.yml` и `timeweb.yml`** оба объявляют хост с именем `server` под ключом + `ungrouped:`. Хост-специфичные данные (`application_dir`, `mount_external_storage`, + `ansible_host`, `ansible_user`) вписаны инлайн. Конвенциональнее — `host_vars/server.yml`, + хосты в именованной группе. Два инвентаря с одинаковым именем хоста + `hosts: all` = + ошибка `-i` молча уедет не туда. +- `ansible_become: true` глобально в инвентаре — всё бежит под root. Для личного сервера + прагматично; точечный `become`/`become_user` ближе к наименьшим привилегиям. Низкий приоритет. + +## 7. Конкретный баг + +- **`playbook-wanderer.yml:2`** — `name: "Configure gramps application"`, хотя + `app_name: "wanderer"`. Копипаст из gramps, поправить имя play. + +## 8. Мелочи стиля и конфигурации + +- **sudoers**: `playbook-backups.yml:52-59` правит `/etc/sudoers` через `lineinfile`. + Конвенция — отдельный файл в `/etc/sudoers.d/` (через `copy`/`template` с + `validate: visudo -cf %s`), а не модификация центрального файла. +- **`.ansible-lint.yml`** содержит только `exclude_paths` — профиль не задан явно. + AGENTS.md утверждает «профиль production»; либо прописать `profile: production`, либо + поправить документацию. +- **`ansible.cfg`** минимален. Стоит добавить `stdout_callback = yaml`, + `interpreter_python = auto_silent`, `force_handlers = true`. +- Несогласованные кавычки и пути: `'directory'` vs `"directory"`, `src: "./files/..."` vs + `src: "files/..."`, одинарные кавычки в `playbook-all-setup.yml` против двойных в остальных. +- `playbook-system.yml:24` — `apt` без `cache_valid_time`, обновляет кэш каждый прогон. + +## Приоритеты + +1. **#3 handlers** — убирает безусловный рестарт; внедряется по одному сервису. +2. **#1 роль `backup`** — самый чистый шов для extraction; обкатать на одном сервисе. +3. **#4, #7** — быстрые точечные фиксы без структурных изменений. +4. **#2 group_vars** — убирает boilerplate; низкий риск. +5. **#5, #6, #8** — фоновая зачистка стиля и структуры.