Skip to content

Comments

Don't disable monitors before after delete hook run#675

Merged
ldmonster merged 2 commits intomainfrom
fix/after-delete-hooks
Sep 30, 2025
Merged

Don't disable monitors before after delete hook run#675
ldmonster merged 2 commits intomainfrom
fix/after-delete-hooks

Conversation

@timmilesdw
Copy link
Contributor

Overview

Defer stopping Kubernetes monitors for modules scheduled to be disabled. In converge phase, only disable schedule bindings; keep Kubernetes watchers alive until afterDeleteHelm runs during DeleteModule. Full hook disabling still happens right after afterDeleteHelm.

What this PR does / why we need it

  • Previously, converge disabled all module hooks (including Kubernetes monitors) immediately for modules in ModulesToDisable. As a side effect, afterDeleteHelm hooks received empty snapshots because monitors were already stopped.
  • This PR:
    • Changes converge to disable only schedule bindings for modules to disable.
    • Leaves Kubernetes monitors running so DeleteModule can execute afterDeleteHelm with populated snapshots.
    • Keeps the existing cleanup: DisableModuleHooks is still invoked in DeleteModule right after afterDeleteHelm, stopping both schedules and monitors.

Why needed:

  • Restores documented behavior that afterDeleteHelm hooks receive snapshots of their Kubernetes bindings.
  • Unblocks reliable cleanup logic (e.g., deleting CRs/secrets) during module disable.
  • Minimizes event noise by keeping schedules off while preventing data loss for snapshots.

Signed-off-by: Timur Tuktamyshev <timur.tuktamyshev@flant.com>
Signed-off-by: Timur Tuktamyshev <timur.tuktamyshev@flant.com>
@timmilesdw timmilesdw self-assigned this Sep 25, 2025
@timmilesdw timmilesdw added the enhancement New feature or request label Sep 25, 2025
@ipaqsa ipaqsa self-requested a review September 25, 2025 11:58
@ldmonster ldmonster merged commit a32d65c into main Sep 30, 2025
10 of 12 checks passed
@ldmonster ldmonster deleted the fix/after-delete-hooks branch September 30, 2025 10:20
ldmonster pushed a commit that referenced this pull request Sep 30, 2025
Signed-off-by: Timur Tuktamyshev <timur.tuktamyshev@flant.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants