Skip to content

Conversation

@IvanOgurchenok
Copy link

@IvanOgurchenok IvanOgurchenok commented Nov 27, 2025

Description

Adds CONTROLLER_STYLE_GUIDE.md - style guide for writing controllers based on review feedback and controller-runtime best practices.

Changes:

  • New documentation: docs/dev/controllers/CONTROLLER_STYLE_GUIDE.md

No code changes: Documentation only.

Why do we need it, and what problem does it solve?

Establishes consistent standards for controller development to ensure code quality and reduce review back-and-forth. Documents when to use standard reconciler, .For() method, structured logging, and parallel processing patterns.

Based on review feedback: Guidelines derived from actual code review comments.

What is the expected result?

Developers have a reference document for controller development standards. New controllers will follow consistent patterns.

How to use:

  • Reference when implementing new controllers
  • Use as checklist during code reviews

MUST NOT change:

  • Existing controllers remain unchanged (documentation only)

Checklist

  • The code is covered by unit tests.
  • e2e tests passed.
  • Documentation updated according to the changes.
  • Changes were tested in the Kubernetes cluster manually.

@IvanOgurchenok IvanOgurchenok changed the base branch from main to astef-prototype November 27, 2025 13:16
@IvanOgurchenok IvanOgurchenok marked this pull request as draft November 27, 2025 13:39
- Update handlers section: prioritize For() and EnqueueRequestForOwner
- Add EnqueueRequestsFromMapFunc usage guidelines
- Add Ginkgo/Gomega testing framework requirements
- Update examples to reflect new best practices
@IvanOgurchenok
Copy link
Author

@asergunov Fixed it few time for be close to wanned specs, if you have time, check pls

@IvanOgurchenok IvanOgurchenok changed the title Astef prototype controller style guide [controller] [doc] [low priority] Astef prototype controller style guide Nov 28, 2025
@IvanOgurchenok IvanOgurchenok marked this pull request as ready for review November 28, 2025 11:05

**Обоснование:** `.For()` и `EnqueueRequestForOwner` покрывают 99% случаев. `EnqueueRequestsFromMapFunc` используется только когда нет owner reference, но нужен маппинг событий. Остальные handlers добавляют сложность и используются только для оптимизации производительности.

**Пример:**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Плохой пример? Может хороший сделать?

- Функции-конструкторы (`NewReconciler`) - создавайте напрямую в `controller.go`

**✅ Используйте по умолчанию:**
- Минимальная структура с только необходимыми полями: `Cl`, `Log`, `LogAlt`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LogAlt не нужен. И все с маленькой буквы должны быть

- Рекомендуется, если констант несколько (2+)
- Не обязательно, если константа одна или две

4. **`reconciler_test.go`** - unit тесты
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

У гинкго еще suit. Без него тесты не запустятся. Еще пакет у тестов должен кончаться на _test


return fake.NewClientBuilder().
WithScheme(scheme).
WithStatusSubresource(&v1alpha3.SomeResource{}).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Больше одного через запятую


BeforeEach(func() {
cl = newFakeClient()
rec = &mycontroller.Reconciler{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NewReconciler. Потому что поля приватные у него

}
})

It("returns no error when resource does not exist", func(ctx context.Context) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Еще добавь With с общей инициализацией в JustBeforeEach. И несколькими It внутри


---

## Чеклист при создании контроллера
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Нафиг это повторение

- Операции действительно независимы
- Обсуждено с командой

**Пример безопасной параллельной обработки (обсуждаемо):**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это спорно прям. Проще канал завести в контроллер по типу Watch

}
```

**Важно:** При использовании goroutines убедитесь, что:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Спорно надо ли их вообще

- Используется правильная обработка ошибок и контекста
- Это обсуждено с командой перед реализацией

### Тестирование
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это было?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants