[feature] add beforeDeleteHelm module hook binding#754
Open
Conversation
Signed-off-by: Artem Kuleshov <artem.kuleshov@flant.com>
Signed-off-by: Artem Kuleshov <artem.kuleshov@flant.com>
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Adds a new
beforeDeleteHelmmodule hook binding that fires just beforehelm uninstallinDeleteModule, symmetric tobeforeHelm/helm install. Hook failure aborts deletion:helm uninstallandafterDeleteHelmare not executed and converge retries with backoff. The hook is skipped when the module has no Helm release (call sits inside theif releaseExistsbranch inmodule_manager.go). Snapshots fromkubernetesbindings are populated the same way as for the other Helm-lifecycle bindings.Wired through every hook flavor:
BatchHook.GetBeforeDeleteHelm)addon-operator/sdkregistry pathWhat this PR does / why we need it
This binding is the native, addon-operator-side replacement for chart-level Helm
pre-deletehooks. The hook runs in the operator process with full access to module values, snapshots, the patch collector, and metrics — instead of as a Job inside the cluster. Typical use cases that used to require chart-levelpre-delete: removing finalizers from CRs before the chart drops their CRDs, revoking external integrations (Vault tokens, GitHub webhooks) before the pod with credentials is gone, taking a backup of state, or cleaning up resources the chart does not own.it relates with:
End-to-end test in a Deckhouse cluster
Test module: external
a-k-testv0.0.4 registered through aModuleSource. Single batch hook (compiled viamodule-sdk) declares the new binding plus akubernetesbinding on a chart-rendered marker ConfigMap, and readsforceFailfrom config-values to flip into an error return:Scenario 1 — hook fires before
helm uninstallAfter
dcmd a-k-test(withforceFail: false),binding=beforeDeleteHelmruns inqueue=main, thenhelm uninstallremoves the chart resources. The structuredbindingfield clearly distinguishes the firing fromkubernetes/Synchronization that happens on module enable.Post-condition: marker ConfigMap, Deployment, Service, Ingress are all gone after the hook returned
nil.Scenario 2 — hook failure aborts deletion (abort-on-error)
With
spec.settings.forceFail: truein ModuleConfig,dcmd a-k-testproduces repeated module-delete failures and never callshelm uninstall:Stack trace points at
addon-operator/pkg/task/tasks/module-delete/task.go:103. Throughout the backoff loop (~12 min observed) the chart's resources stay alive:No
helm uninstall/DeleteReleaselog line fora-k-testis ever emitted. FlippingforceFailback tofalselets the next retry succeed and deletion proceeds normally.Scenario 3 — kubernetes-binding snapshot is populated at hook time
The hook subscribes to a chart-rendered marker ConfigMap and logs snapshot contents on each fire. With v0.0.3 (no marker in templates)
hook_countwas0; with v0.0.4 (marker present) it became1and exposed the actual ConfigMap name — proving snapshots are alive atbeforeDeleteHelmtime and the snapshot-injection condition added toRunHooksByBindingcovers the new binding:Scenario 4 — hook is skipped when the module has no Helm release
Verified by code review: in
module_manager.goDeleteModulethe newRunHooksByBinding(ctx, BeforeDeleteHelm, ...)call is placed inside theif releaseExistsbranch, so a module with no chart (or whose release was already gone) skips the hook entirely — symmetric with skippinghelm uninstallitself.This was not exercised in the cluster on its own (would require a separate test module with empty
templates/), since the surrounding logic for the no-release case was already covered byafterDeleteHelmand is unchanged.Scenario 5 — works through module-sdk batch hook (not just shell)
The entire end-to-end test ran on the batch hook path: the test module ships a Go binary compiled with module-sdk, which dumps its config containing
beforeDeleteHelm: 10, addon-operator parses that JSON viaBatchHook.GetBeforeDeleteHelm, registers the binding, and fires it on delete. So scenarios 1–3 are all simultaneous proof that the SDK ↔ addon-operator type/DTO contract works end-to-end (OnBeforeDeleteHelm→ DTObeforeDeleteHelm→ typed accessor → registered binding → fired hook).Scenarios summary
helm uninstallhook_count=1)if releaseExistsbranch)