From cc16043b8af1e7522d19bee4b7c7cfd3e41fc90c Mon Sep 17 00:00:00 2001 From: Artem Kuleshov Date: Mon, 27 Apr 2026 13:44:23 +0300 Subject: [PATCH 1/2] [feature] add beforeDeleteHelm module hook binding Signed-off-by: Artem Kuleshov --- docs/src/HOOKS.md | 24 +++++-- docs/src/LIFECYCLE-STEPS.md | 7 ++ docs/src/LIFECYCLE.md | 4 +- go.mod | 2 +- go.sum | 4 +- pkg/hook/types/bindings.go | 11 +-- pkg/module_manager/go_hook/go_hook.go | 22 +++--- .../models/hooks/kind/batch_hook.go | 70 ++++++++++++++++--- .../models/hooks/kind/batch_hook_test.go | 26 ++++--- .../models/hooks/kind/binding_warn.go | 59 ++++++++++++++++ .../models/hooks/kind/gohook.go | 8 +++ .../models/hooks/kind/shellhook.go | 46 +++++++++--- .../models/hooks/kind/shellhook_test.go | 48 +++++++------ .../models/hooks/module_hook.go | 5 ++ .../models/hooks/module_hook_config.go | 32 ++++++--- pkg/module_manager/models/modules/basic.go | 3 +- pkg/module_manager/module_manager.go | 14 +++- 17 files changed, 302 insertions(+), 83 deletions(-) create mode 100644 pkg/module_manager/models/hooks/kind/binding_warn.go diff --git a/docs/src/HOOKS.md b/docs/src/HOOKS.md index 0cb22055e..c2f9fa351 100644 --- a/docs/src/HOOKS.md +++ b/docs/src/HOOKS.md @@ -30,9 +30,10 @@ During execution, a module hook receives global values and module values. Module | [onStartup](#onstartup)↗ | – | ✓ | On Addon-operator startup or module enablement | | [beforeAll](#beforeall)↗ | ✓ | – | Before any modules are executed | | [afterAll](#afterall)↗ | ✓ | – | After all modules are executed | -| [beforeHelm](#beforehelm)↗ | – | ✓ | Before executing `helm install` | -| [afterHelm](#afterhelm)↗ | – | ✓ | After executing `helm install` | -| [afterDeleteHelm](#afterdeletehelm)↗ | – | ✓ | After executing `helm delete` | +| [beforeHelm](#beforehelm)↗ | – | ✓ | Before executing `helm install` | +| [afterHelm](#afterhelm)↗ | – | ✓ | After executing `helm install` | +| [beforeDeleteHelm](#beforedeletehelm)↗ | – | ✓ | Before executing `helm delete` | +| [afterDeleteHelm](#afterdeletehelm)↗ | – | ✓ | After executing `helm delete` | | [schedule](#schedule)↗ | ✓ | ✓ | Run on schedule | | [kubernetes](#kubernetes)↗ | ✓ | ✓ | Run on event from Kubernetes | @@ -101,6 +102,21 @@ Parameters: - `ORDER` — an integer value that specifies an execution order. When added to the "main" queue, the hooks will be sorted by this value and then alphabetically by file name. +### beforeDeleteHelm + +Example: + +```yaml +configVersion: v1 +beforeDeleteHelm: ORDER +``` + +Parameters: + +- `ORDER` — an integer value that specifies an execution order. When added to the "main" queue, the hooks will be sorted by this value and then alphabetically by file name. + +The `beforeDeleteHelm` hook runs **just before `helm uninstall`** for the module's release. It is a native, addon-operator-side replacement for a Helm chart-level `pre-delete` hook: the hook runs as part of the addon-operator process (with full access to the module's values, snapshots, patch collector and metrics) instead of inside a Job in the cluster. If the hook fails, `helm uninstall` and `afterDeleteHelm` are not executed and the module is not removed; the converge loop will retry with backoff. If the module has no Helm release at delete time, the hook is skipped (symmetric with skipping `helm uninstall`). + ### afterDeleteHelm Example: @@ -143,7 +159,7 @@ The `$BINDING_CONTEXT_PATH` environment variable contains the path to a file wit The binding context for `schedule` and `kubernetes` hooks contains additional fields, described in Shell-operator [documentation][shell-operator-binding-context]. -`beforeAll` and `afterAll` global hooks and `beforeHelm`, `afterHelm`, and `afterDeleteHelm` module hooks are executed with the binding context that includes a `snapshots` field, which contains all Kubernetes objects that match hook's `kubernetes` bindings configurations. +`beforeAll` and `afterAll` global hooks and `beforeHelm`, `afterHelm`, `beforeDeleteHelm`, and `afterDeleteHelm` module hooks are executed with the binding context that includes a `snapshots` field, which contains all Kubernetes objects that match hook's `kubernetes` bindings configurations. For example, a global hook with `kubernetes` and `beforeAll` bindings may have this configuration: diff --git a/docs/src/LIFECYCLE-STEPS.md b/docs/src/LIFECYCLE-STEPS.md index 70788cb9b..96e81a245 100644 --- a/docs/src/LIFECYCLE-STEPS.md +++ b/docs/src/LIFECYCLE-STEPS.md @@ -231,6 +231,13 @@ Startup steps: - if module values are changed, restart 'module run' 6. 'module delete' for each disabled module + - if a Helm release exists, execute module hooks with 'beforeDeleteHelm' binding ordered by the ORDER value (see [beforeDeleteHelm](HOOKS.md#beforedeletehelm)) + - input + - binding context ($BINDING_CONTEXT_PATH temporary file) + - `{"binding":"beforeDeleteHelm"}` + - extra field `"snapshots"` contains existed objects from all 'kubernetes' bindings of this hook + - config and values are layered the same way as for the other module-lifecycle hooks (see 'beforeHelm' / 'afterHelm' above) + - if any 'beforeDeleteHelm' hook fails, `helm delete --purge` and 'afterDeleteHelm' are NOT executed and the deletion is retried with backoff - run `helm delete --purge` - execute module hooks with 'afterDeleteHelm' binding ordered by the ORDER value (see [afterDeleteHelm](HOOKS.md#afterdeletehelm)) - input diff --git a/docs/src/LIFECYCLE.md b/docs/src/LIFECYCLE.md index 22889a943..19e8f6fa0 100644 --- a/docs/src/LIFECYCLE.md +++ b/docs/src/LIFECYCLE.md @@ -47,6 +47,8 @@ All other actions are handled in a single "main" queue: - `beforeHelm` - execution of `helm` commands - `afterHelm` + - `beforeDeleteHelm` (on deactivation, before `helm delete`) + - `afterDeleteHelm` (on deactivation, after `helm delete`) This document mainly describes modules. To get more information on hooks, see [HOOKS](HOOKS.md) document. To get a full view of how hooks, modules, [values](VALUES.md), binding contexts, and queues are interlinked, see [LIFECYCLE-STEPS](LIFECYCLE-STEPS.md) document. @@ -61,7 +63,7 @@ After the launch the module would start responding to two types of events: - `schedule` — events that are generated by the crontab scheduler built in the addon-operator; - `kubernetes` — events within the cluster that API server announces to the Addon-operator. -When the module is deactivated, the Addon-operator launches command `helm delete --purge` and after the release deletion, the `afterDeleteHelm` hooks are executed. +When the module is deactivated, the Addon-operator runs `beforeDeleteHelm` hooks (only when a Helm release exists), launches `helm delete --purge`, and then runs `afterDeleteHelm` hooks. If any `beforeDeleteHelm` hook returns an error, `helm delete` and `afterDeleteHelm` are skipped and the deletion is retried with backoff. All necessary hooks will be restarted if there are errors during the module activation or deactivation. For example, if an error occurred in the hook with `afterHelm` binding during the first module execution, then after a 5 seconds delay the `onStartup` and `beforeHelm` hooks are executed, the Helm chart is installed and then `afterHelm` hooks are executed. diff --git a/go.mod b/go.mod index f95fff721..b6b4b6a15 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc github.com/deckhouse/deckhouse/pkg/log v0.2.0 github.com/deckhouse/deckhouse/pkg/metrics-storage v0.3.0 - github.com/deckhouse/module-sdk v0.10.4 + github.com/deckhouse/module-sdk v0.10.8-0.20260427102426-3fb05940aa17 github.com/dominikbraun/graph v0.23.0 github.com/ettle/strcase v0.2.0 github.com/flant/kube-client v1.6.0 diff --git a/go.sum b/go.sum index 6ede8dc14..2cef2236e 100644 --- a/go.sum +++ b/go.sum @@ -104,8 +104,8 @@ github.com/deckhouse/deckhouse/pkg/log v0.2.0 h1:6tmZQLwNb1o/hP1gzJQBjcwfA/bubbg github.com/deckhouse/deckhouse/pkg/log v0.2.0/go.mod h1:pbAxTSDcPmwyl3wwKDcEB3qdxHnRxqTV+J0K+sha8bw= github.com/deckhouse/deckhouse/pkg/metrics-storage v0.3.0 h1:xZvbKuexrSQGEw6CB4n3UC7XbOb9QNLbm8UhcGZ2R1I= github.com/deckhouse/deckhouse/pkg/metrics-storage v0.3.0/go.mod h1:Rz++SzCLkFW03WGgftnn91TimGU2shiKb5S/YuxcBuE= -github.com/deckhouse/module-sdk v0.10.4 h1:1C61nXevgQiUnmv5Is+XP9Octu05XWKgjFi6gmL7miM= -github.com/deckhouse/module-sdk v0.10.4/go.mod h1:wpEKjpMUHZ4D5JGPfHChKGzBcOVyZnUN4sP0yTEaHdU= +github.com/deckhouse/module-sdk v0.10.8-0.20260427102426-3fb05940aa17 h1:tPvr/8bkgRLjx98jgiLykj9QTB2k4udjrdugHmgF9e4= +github.com/deckhouse/module-sdk v0.10.8-0.20260427102426-3fb05940aa17/go.mod h1:wpEKjpMUHZ4D5JGPfHChKGzBcOVyZnUN4sP0yTEaHdU= github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f h1:lO4WD4F/rVNCu3HqELle0jiPLLBs70cWOduZpkS1E78= github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f/go.mod h1:cuUVRXasLTGF7a8hSLbxyZXjz+1KgoB3wDUb6vlszIc= github.com/disintegration/imaging v1.6.2 h1:w1LecBlG2Lnp8B3jk5zSuNqd7b4DXhcjwek1ei82L+c= diff --git a/pkg/hook/types/bindings.go b/pkg/hook/types/bindings.go index 52576a5e4..b6e805b20 100644 --- a/pkg/hook/types/bindings.go +++ b/pkg/hook/types/bindings.go @@ -7,9 +7,10 @@ import ( // Additional binding types, specific to addon-operator const ( - BeforeHelm BindingType = "beforeHelm" - AfterHelm BindingType = "afterHelm" - AfterDeleteHelm BindingType = "afterDeleteHelm" - BeforeAll BindingType = "beforeAll" - AfterAll BindingType = "afterAll" + BeforeHelm BindingType = "beforeHelm" + AfterHelm BindingType = "afterHelm" + BeforeDeleteHelm BindingType = "beforeDeleteHelm" + AfterDeleteHelm BindingType = "afterDeleteHelm" + BeforeAll BindingType = "beforeAll" + AfterAll BindingType = "afterAll" ) diff --git a/pkg/module_manager/go_hook/go_hook.go b/pkg/module_manager/go_hook/go_hook.go index bae349655..befd846c8 100644 --- a/pkg/module_manager/go_hook/go_hook.go +++ b/pkg/module_manager/go_hook/go_hook.go @@ -22,6 +22,7 @@ type HookConfigLoader interface { GetOnStartup() *float64 GetBeforeAll() *float64 GetAfterAll() *float64 + GetBeforeDeleteHelm() *float64 GetAfterDeleteHelm() *float64 } @@ -65,16 +66,17 @@ type HookConfig struct { // OnStartup runs hook on module/global startup // Attention! During the startup you don't have snapshots available // use native KubeClient to fetch resources - OnStartup *OrderedConfig - OnBeforeHelm *OrderedConfig - OnAfterHelm *OrderedConfig - OnAfterDeleteHelm *OrderedConfig - OnBeforeAll *OrderedConfig - OnAfterAll *OrderedConfig - AllowFailure bool - Queue string - Settings *HookConfigSettings - Logger *log.Logger + OnStartup *OrderedConfig + OnBeforeHelm *OrderedConfig + OnAfterHelm *OrderedConfig + OnBeforeDeleteHelm *OrderedConfig + OnAfterDeleteHelm *OrderedConfig + OnBeforeAll *OrderedConfig + OnAfterAll *OrderedConfig + AllowFailure bool + Queue string + Settings *HookConfigSettings + Logger *log.Logger } type HookConfigSettings struct { diff --git a/pkg/module_manager/models/hooks/kind/batch_hook.go b/pkg/module_manager/models/hooks/kind/batch_hook.go index f80561cc8..6f3c849e3 100644 --- a/pkg/module_manager/models/hooks/kind/batch_hook.go +++ b/pkg/module_manager/models/hooks/kind/batch_hook.go @@ -220,10 +220,52 @@ func (h *BatchHook) Execute(ctx context.Context, configVersion string, bContext } func (h *BatchHook) getConfig() (*BatchHookConfig, error) { - return GetBatchHookConfig(h.moduleName, h.Path) + cfg, raw, err := getBatchHookConfigWithRaw(h.moduleName, h.Path) + if err != nil { + return nil, err + } + // Forward-compat: scan the raw JSON for top-level keys we don't know, + // so that hooks built with newer module-sdk against older addon-operator + // don't silently lose bindings. + h.warnUnknownKeysFromBatchRaw(raw) + return cfg, nil +} + +// warnUnknownKeysFromBatchRaw decodes the batch-hook config JSON loosely and +// emits warnings for unknown top-level keys per individual hook config. +func (h *BatchHook) warnUnknownKeysFromBatchRaw(o []byte) { + if h.Hook.Logger == nil || len(o) == 0 { + return + } + // New format: {version, hooks: [...]} + var env struct { + Hooks []map[string]interface{} `json:"hooks"` + Readiness map[string]interface{} `json:"readiness"` + } + if err := json.Unmarshal(o, &env); err == nil && (len(env.Hooks) > 0 || env.Readiness != nil) { + for i, raw := range env.Hooks { + warnUnknownHookKeys(h.Hook.Logger, raw, fmt.Sprintf("%s[%d]", h.Path, i)) + } + if env.Readiness != nil { + warnUnknownHookKeys(h.Hook.Logger, env.Readiness, h.Path+"[readiness]") + } + return + } + // Legacy format: [...] + var arr []map[string]interface{} + if err := json.Unmarshal(o, &arr); err == nil { + for i, raw := range arr { + warnUnknownHookKeys(h.Hook.Logger, raw, fmt.Sprintf("%s[%d]", h.Path, i)) + } + } } func GetBatchHookConfig(moduleName, hookPath string) (*BatchHookConfig, error) { + cfg, _, err := getBatchHookConfigWithRaw(moduleName, hookPath) + return cfg, err +} + +func getBatchHookConfigWithRaw(moduleName, hookPath string) (*BatchHookConfig, []byte, error) { args := []string{"hook", "config"} environ := os.Environ() @@ -240,7 +282,7 @@ func GetBatchHookConfig(moduleName, hookPath string) (*BatchHookConfig, error) { o, err := cmd.Output() if err != nil { - return nil, fmt.Errorf("exec file '%s': %w", hookPath, err) + return nil, nil, fmt.Errorf("exec file '%s': %w", hookPath, err) } // Deprecated: old batch hook config format @@ -252,7 +294,7 @@ func GetBatchHookConfig(moduleName, hookPath string) (*BatchHookConfig, error) { buf := bytes.NewReader(o) err = json.NewDecoder(buf).Decode(&hooks) if err != nil { - return nil, fmt.Errorf("decode: %w", err) + return nil, nil, fmt.Errorf("decode: %w", err) } cfg := &BatchHookConfig{} @@ -262,7 +304,7 @@ func GetBatchHookConfig(moduleName, hookPath string) (*BatchHookConfig, error) { cfg.Hooks[strconv.Itoa(idx)] = &h } - return cfg, nil + return cfg, o, nil } cfgs := &sdkhook.BatchHookConfig{} @@ -270,7 +312,7 @@ func GetBatchHookConfig(moduleName, hookPath string) (*BatchHookConfig, error) { buf := bytes.NewReader(o) err = json.NewDecoder(buf).Decode(&cfgs) if err != nil { - return nil, fmt.Errorf("decode: %w", err) + return nil, nil, fmt.Errorf("decode: %w", err) } cfg, err := remapSDKConfigToConfig(cfgs) @@ -280,17 +322,17 @@ func GetBatchHookConfig(moduleName, hookPath string) (*BatchHookConfig, error) { buf := bytes.NewReader(o) err = json.NewDecoder(buf).Decode(&cfgs) if err != nil { - return nil, fmt.Errorf("decode: %w", err) + return nil, nil, fmt.Errorf("decode: %w", err) } if outputLog.Level != "" { - return nil, fmt.Errorf("got log except config: %s", string(o)) + return nil, nil, fmt.Errorf("got log except config: %s", string(o)) } - return nil, fmt.Errorf("remapSDKConfigToConfig: %w", err) + return nil, nil, fmt.Errorf("remapSDKConfigToConfig: %w", err) } - return cfg, nil + return cfg, o, nil } type BatchHookLog struct { @@ -389,6 +431,16 @@ func (h *BatchHook) GetAfterAll() *float64 { return &res } +func (h *BatchHook) GetBeforeDeleteHelm() *float64 { + if h.config == nil || h.config.OnBeforeDeleteHelm == nil { + return nil + } + + res := float64(*h.config.OnBeforeDeleteHelm) + + return &res +} + func (h *BatchHook) GetAfterDeleteHelm() *float64 { if h.config == nil || h.config.OnAfterDeleteHelm == nil { return nil diff --git a/pkg/module_manager/models/hooks/kind/batch_hook_test.go b/pkg/module_manager/models/hooks/kind/batch_hook_test.go index 5f502382f..72a1abb39 100644 --- a/pkg/module_manager/models/hooks/kind/batch_hook_test.go +++ b/pkg/module_manager/models/hooks/kind/batch_hook_test.go @@ -25,6 +25,7 @@ echo '[{"configVersion":"v1", "onStartup":10, "beforeHelm":5, "afterHelm":15, +"beforeDeleteHelm":20, "afterDeleteHelm":25, "metadata":{ "name":"main", @@ -63,6 +64,7 @@ fi assert.Equal(t, ptr.To(uint(10)), hook.OnStartup) assert.Equal(t, ptr.To(uint(5)), hook.OnBeforeHelm) assert.Equal(t, ptr.To(uint(15)), hook.OnAfterHelm) + assert.Equal(t, ptr.To(uint(20)), hook.OnBeforeDeleteHelm) assert.Equal(t, ptr.To(uint(25)), hook.OnAfterDeleteHelm) } @@ -81,11 +83,12 @@ func Test_BatchHook_Config(t *testing.T) { } type wants struct { - err error - onStartup *float64 - beforeHelm *float64 - afterHelm *float64 - afterDeleteHelm *float64 + err error + onStartup *float64 + beforeHelm *float64 + afterHelm *float64 + beforeDeleteHelm *float64 + afterDeleteHelm *float64 } tests := []struct { @@ -105,6 +108,7 @@ func Test_BatchHook_Config(t *testing.T) { "onStartup":10, "beforeHelm":5, "afterHelm":15, + "beforeDeleteHelm":20, "afterDeleteHelm":25, "metadata":{ "name":"main", @@ -122,11 +126,12 @@ func Test_BatchHook_Config(t *testing.T) { "jqFilter":".metadata.name"}]}]`, }, wants: wants{ - err: nil, - onStartup: ptr.To(10.0), - beforeHelm: ptr.To(5.0), - afterHelm: ptr.To(15.0), - afterDeleteHelm: ptr.To(25.0), + err: nil, + onStartup: ptr.To(10.0), + beforeHelm: ptr.To(5.0), + afterHelm: ptr.To(15.0), + beforeDeleteHelm: ptr.To(20.0), + afterDeleteHelm: ptr.To(25.0), }, }, } @@ -170,6 +175,7 @@ fi assert.Equal(t, tt.wants.onStartup, bHook.GetOnStartup()) assert.Equal(t, tt.wants.beforeHelm, bHook.GetBeforeAll()) assert.Equal(t, tt.wants.afterHelm, bHook.GetAfterAll()) + assert.Equal(t, tt.wants.beforeDeleteHelm, bHook.GetBeforeDeleteHelm()) assert.Equal(t, tt.wants.afterDeleteHelm, bHook.GetAfterDeleteHelm()) }) } diff --git a/pkg/module_manager/models/hooks/kind/binding_warn.go b/pkg/module_manager/models/hooks/kind/binding_warn.go new file mode 100644 index 000000000..fedb1b67c --- /dev/null +++ b/pkg/module_manager/models/hooks/kind/binding_warn.go @@ -0,0 +1,59 @@ +package kind + +import ( + "github.com/deckhouse/deckhouse/pkg/log" +) + +// knownModuleHookKeys lists all top-level keys recognized by the addon-operator +// in a module hook configuration. Unknown keys are silently dropped during +// typed decoding (json/yaml unmarshal ignores unknown fields by default) — so +// without an explicit warning hook authors would not learn that a newly-added +// binding is being ignored at runtime by an older addon-operator. +// +// Forward-compat: when a future module-sdk introduces a new binding, an old +// addon-operator missing this set will emit a warning instead of silently +// ignoring the binding, so operators can spot the version mismatch in logs. +var knownModuleHookKeys = map[string]struct{}{ + // shell-operator core + "configVersion": {}, + "onStartup": {}, + "kubernetes": {}, + "schedule": {}, + "settings": {}, + "allowFailure": {}, + "queue": {}, + "logLevel": {}, + // addon-operator module lifecycle + "beforeHelm": {}, + "afterHelm": {}, + "beforeDeleteHelm": {}, + "afterDeleteHelm": {}, + // addon-operator global lifecycle + "beforeAll": {}, + "afterAll": {}, + // batch-hook envelope + "metadata": {}, + "name": {}, + "version": {}, + "hooks": {}, + "readiness": {}, + "has_settings_check": {}, +} + +// warnUnknownHookKeys emits a warning for each top-level key in raw that is +// not recognized by this version of addon-operator. raw is expected to be the +// JSON/YAML-decoded form of a single hook's config (top-level fields). +func warnUnknownHookKeys(logger *log.Logger, raw map[string]interface{}, hookName string) { + if logger == nil || len(raw) == 0 { + return + } + for key := range raw { + if _, ok := knownModuleHookKeys[key]; ok { + continue + } + logger.Warn("unknown key in hook config; addon-operator will ignore it — check addon-operator/module-sdk version compatibility", + "hook", hookName, + "key", key, + ) + } +} diff --git a/pkg/module_manager/models/hooks/kind/gohook.go b/pkg/module_manager/models/hooks/kind/gohook.go index 36f1843c5..2a2565863 100644 --- a/pkg/module_manager/models/hooks/kind/gohook.go +++ b/pkg/module_manager/models/hooks/kind/gohook.go @@ -247,6 +247,14 @@ func (h *GoHook) GetAfterAll() *float64 { return nil } +func (h *GoHook) GetBeforeDeleteHelm() *float64 { + if h.config == nil || h.config.OnBeforeDeleteHelm == nil { + return nil + } + + return &h.config.OnBeforeDeleteHelm.Order +} + func (h *GoHook) GetAfterDeleteHelm() *float64 { if h.config == nil || h.config.OnAfterDeleteHelm == nil { return nil diff --git a/pkg/module_manager/models/hooks/kind/shellhook.go b/pkg/module_manager/models/hooks/kind/shellhook.go index 6a16e3dd6..4826652a4 100644 --- a/pkg/module_manager/models/hooks/kind/shellhook.go +++ b/pkg/module_manager/models/hooks/kind/shellhook.go @@ -325,6 +325,14 @@ func (sh *ShellHook) GetConfigForModule(moduleKind string) (*config.HookConfig, return nil, fmt.Errorf("unmarshal schedule yaml hook config: %s", err) } + // Forward-compat: warn about top-level keys we don't recognize so that + // hooks built with newer module-sdk against older addon-operator don't + // fail silently when they declare unknown bindings. + rawTop := map[string]interface{}{} + if err := yaml.Unmarshal(cfgData, &rawTop); err == nil { + warnUnknownHookKeys(sh.Hook.Logger, rawTop, sh.Name) + } + return cfg, nil } @@ -366,6 +374,15 @@ func (sh *ShellHook) GetAfterAll() *float64 { return nil } +func (sh *ShellHook) GetBeforeDeleteHelm() *float64 { + res := ConvertFloatForBinding(sh.ScheduleConfig.BeforeDeleteHelm) + if res != nil { + return res + } + + return nil +} + func (sh *ShellHook) GetAfterDeleteHelm() *float64 { res := ConvertFloatForBinding(sh.ScheduleConfig.AfterDeleteHelm) if res != nil { @@ -380,9 +397,10 @@ type HookScheduleConfig struct { BeforeAll *uint `json:"beforeAll" yaml:"beforeAll"` AfterAll *uint `json:"afterAll" yaml:"afterAll"` // embedded module - BeforeHelm *uint `json:"beforeHelm" yaml:"beforeHelm"` - AfterHelm *uint `json:"afterHelm" yaml:"afterHelm"` - AfterDeleteHelm *uint `json:"afterDeleteHelm" yaml:"afterDeleteHelm"` + BeforeHelm *uint `json:"beforeHelm" yaml:"beforeHelm"` + AfterHelm *uint `json:"afterHelm" yaml:"afterHelm"` + BeforeDeleteHelm *uint `json:"beforeDeleteHelm" yaml:"beforeDeleteHelm"` + AfterDeleteHelm *uint `json:"afterDeleteHelm" yaml:"afterDeleteHelm"` } func ConvertFloatForBinding(value *uint) *float64 { @@ -433,30 +451,36 @@ func getModuleHookConfigSchema(version string) *spec.Schema { schema := config.Schemas[version] switch version { case "v1": - // add beforeHelm, afterHelm and afterDeleteHelm properties + // add beforeHelm, afterHelm, beforeDeleteHelm and afterDeleteHelm properties schema += ` beforeHelm: type: integer - example: 10 + example: 10 afterHelm: type: integer - example: 10 + example: 10 + beforeDeleteHelm: + type: integer + example: 10 afterDeleteHelm: type: integer - example: 10 + example: 10 ` case "v0": - // add beforeHelm, afterHelm and afterDeleteHelm properties + // add beforeHelm, afterHelm, beforeDeleteHelm and afterDeleteHelm properties schema += ` beforeHelm: type: integer - example: 10 + example: 10 afterHelm: type: integer - example: 10 + example: 10 + beforeDeleteHelm: + type: integer + example: 10 afterDeleteHelm: type: integer - example: 10 + example: 10 ` } config.Schemas[globalHookVersion] = schema diff --git a/pkg/module_manager/models/hooks/kind/shellhook_test.go b/pkg/module_manager/models/hooks/kind/shellhook_test.go index 05c7e638e..0db00d010 100644 --- a/pkg/module_manager/models/hooks/kind/shellhook_test.go +++ b/pkg/module_manager/models/hooks/kind/shellhook_test.go @@ -146,11 +146,12 @@ func Test_ShellHook_Embedded_Config_v0_v1(t *testing.T) { } type wants struct { - err error - onStartup *float64 - beforeHelm *float64 - afterHelm *float64 - afterDeleteHelm *float64 + err error + onStartup *float64 + beforeHelm *float64 + afterHelm *float64 + beforeDeleteHelm *float64 + afterDeleteHelm *float64 } tests := []struct { @@ -169,15 +170,17 @@ func Test_ShellHook_Embedded_Config_v0_v1(t *testing.T) { "schedule":[{"crontab":"*/5 * * * * *"}], "beforeHelm": 5, "afterHelm": 15, + "beforeDeleteHelm": 20, "afterDeleteHelm": 25 }`, }, wants: wants{ - err: nil, - onStartup: ptr.To(10.0), - beforeHelm: ptr.To(5.0), - afterHelm: ptr.To(15.0), - afterDeleteHelm: ptr.To(25.0), + err: nil, + onStartup: ptr.To(10.0), + beforeHelm: ptr.To(5.0), + afterHelm: ptr.To(15.0), + beforeDeleteHelm: ptr.To(20.0), + afterDeleteHelm: ptr.To(25.0), }, // func() { // g.Expect(err).ShouldNot(HaveOccurred()) @@ -222,14 +225,16 @@ func Test_ShellHook_Embedded_Config_v0_v1(t *testing.T) { "kubernetes":[{"kind":"Pod", "watchEvent":["Added"]}], "beforeHelm": 98, "afterHelm": 58, + "beforeDeleteHelm": 33, "afterDeleteHelm": 18}`, }, wants: wants{ - err: nil, - onStartup: ptr.To(10.0), - beforeHelm: ptr.To(98.0), - afterHelm: ptr.To(58.0), - afterDeleteHelm: ptr.To(18.0), + err: nil, + onStartup: ptr.To(10.0), + beforeHelm: ptr.To(98.0), + afterHelm: ptr.To(58.0), + beforeDeleteHelm: ptr.To(33.0), + afterDeleteHelm: ptr.To(18.0), }, // func() { // g.Expect(err).ShouldNot(HaveOccurred()) @@ -260,15 +265,17 @@ kubernetes: watchEvent: ["Added"] beforeHelm: 98 afterHelm: 58 +beforeDeleteHelm: 33 afterDeleteHelm: 18 `, }, wants: wants{ - err: nil, - onStartup: ptr.To(10.0), - beforeHelm: ptr.To(98.0), - afterHelm: ptr.To(58.0), - afterDeleteHelm: ptr.To(18.0), + err: nil, + onStartup: ptr.To(10.0), + beforeHelm: ptr.To(98.0), + afterHelm: ptr.To(58.0), + beforeDeleteHelm: ptr.To(33.0), + afterDeleteHelm: ptr.To(18.0), }, // func() { // g.Expect(err).ShouldNot(HaveOccurred()) @@ -339,6 +346,7 @@ fi assert.Equal(t, tt.wants.onStartup, shHook.GetOnStartup()) assert.Equal(t, tt.wants.beforeHelm, shHook.GetBeforeAll()) assert.Equal(t, tt.wants.afterHelm, shHook.GetAfterAll()) + assert.Equal(t, tt.wants.beforeDeleteHelm, shHook.GetBeforeDeleteHelm()) assert.Equal(t, tt.wants.afterDeleteHelm, shHook.GetAfterDeleteHelm()) }) } diff --git a/pkg/module_manager/models/hooks/module_hook.go b/pkg/module_manager/models/hooks/module_hook.go index b5de31908..342d229c6 100644 --- a/pkg/module_manager/models/hooks/module_hook.go +++ b/pkg/module_manager/models/hooks/module_hook.go @@ -51,6 +51,8 @@ func (mh *ModuleHook) Order(binding shell_op_types.BindingType) float64 { return mh.config.BeforeHelm.Order case addon_op_types.AfterHelm: return mh.config.AfterHelm.Order + case addon_op_types.BeforeDeleteHelm: + return mh.config.BeforeDeleteHelm.Order case addon_op_types.AfterDeleteHelm: return mh.config.AfterDeleteHelm.Order } @@ -141,6 +143,9 @@ func (mh *ModuleHook) GetConfigDescription() string { if mh.config.AfterHelm != nil { bd = append(bd, fmt.Sprintf("afterHelm:%d", int64(mh.config.AfterHelm.Order))) } + if mh.config.BeforeDeleteHelm != nil { + bd = append(bd, fmt.Sprintf("beforeDeleteHelm:%d", int64(mh.config.BeforeDeleteHelm.Order))) + } if mh.config.AfterDeleteHelm != nil { bd = append(bd, fmt.Sprintf("afterDeleteHelm:%d", int64(mh.config.AfterDeleteHelm.Order))) } diff --git a/pkg/module_manager/models/hooks/module_hook_config.go b/pkg/module_manager/models/hooks/module_hook_config.go index bca7ddaf2..0dd1254f2 100644 --- a/pkg/module_manager/models/hooks/module_hook_config.go +++ b/pkg/module_manager/models/hooks/module_hook_config.go @@ -18,9 +18,10 @@ type ModuleHookConfig struct { ModuleV1 *ModuleHookConfigV0 // effective config values - BeforeHelm *BeforeHelmConfig - AfterHelm *AfterHelmConfig - AfterDeleteHelm *AfterDeleteHelmConfig + BeforeHelm *BeforeHelmConfig + AfterHelm *AfterHelmConfig + BeforeDeleteHelm *BeforeDeleteHelmConfig + AfterDeleteHelm *AfterDeleteHelmConfig } type BeforeHelmConfig struct { @@ -33,21 +34,27 @@ type AfterHelmConfig struct { Order float64 } +type BeforeDeleteHelmConfig struct { + CommonBindingConfig + Order float64 +} + type AfterDeleteHelmConfig struct { CommonBindingConfig Order float64 } type ModuleHookConfigV0 struct { - BeforeHelm interface{} `json:"beforeHelm"` - AfterHelm interface{} `json:"afterHelm"` - AfterDeleteHelm interface{} `json:"afterDeleteHelm"` + BeforeHelm interface{} `json:"beforeHelm"` + AfterHelm interface{} `json:"afterHelm"` + BeforeDeleteHelm interface{} `json:"beforeDeleteHelm"` + AfterDeleteHelm interface{} `json:"afterDeleteHelm"` } func (c *ModuleHookConfig) Bindings() []BindingType { res := make([]BindingType, 0) - for _, binding := range []BindingType{OnStartup, Schedule, OnKubernetesEvent, BeforeHelm, AfterHelm, AfterDeleteHelm} { + for _, binding := range []BindingType{OnStartup, Schedule, OnKubernetesEvent, BeforeHelm, AfterHelm, BeforeDeleteHelm, AfterDeleteHelm} { if c.HasBinding(binding) { res = append(res, binding) } @@ -66,6 +73,8 @@ func (c *ModuleHookConfig) HasBinding(binding BindingType) bool { return c.BeforeHelm != nil case AfterHelm: return c.AfterHelm != nil + case BeforeDeleteHelm: + return c.BeforeDeleteHelm != nil case AfterDeleteHelm: return c.AfterDeleteHelm != nil default: @@ -76,7 +85,7 @@ func (c *ModuleHookConfig) HasBinding(binding BindingType) bool { func (c *ModuleHookConfig) BindingsCount() int { res := 0 - for _, binding := range []BindingType{OnStartup, BeforeHelm, AfterHelm, AfterDeleteHelm} { + for _, binding := range []BindingType{OnStartup, BeforeHelm, AfterHelm, BeforeDeleteHelm, AfterDeleteHelm} { if c.HasBinding(binding) { res++ } @@ -121,6 +130,13 @@ func (c *ModuleHookConfig) LoadHookConfig(configLoader gohook.HookConfigLoader) c.AfterHelm.Order = *afterAll } + beforeDelete := configLoader.GetBeforeDeleteHelm() + if beforeDelete != nil { + c.BeforeDeleteHelm = &BeforeDeleteHelmConfig{} + c.BeforeDeleteHelm.BindingName = string(BeforeDeleteHelm) + c.BeforeDeleteHelm.Order = *beforeDelete + } + afterDelete := configLoader.GetAfterDeleteHelm() if afterDelete != nil { c.AfterDeleteHelm = &AfterDeleteHelmConfig{} diff --git a/pkg/module_manager/models/modules/basic.go b/pkg/module_manager/models/modules/basic.go index 8b99900e0..681492bdc 100644 --- a/pkg/module_manager/models/modules/basic.go +++ b/pkg/module_manager/models/modules/basic.go @@ -662,7 +662,8 @@ func (bm *BasicModule) RunHooksByBinding(ctx context.Context, binding sh_op_type Binding: string(binding), } // Update kubernetes snapshots just before execute a hook - if binding == types.BeforeHelm || binding == types.AfterHelm || binding == types.AfterDeleteHelm { + if binding == types.BeforeHelm || binding == types.AfterHelm || + binding == types.BeforeDeleteHelm || binding == types.AfterDeleteHelm { bc.Snapshots = moduleHook.GetHookController().KubernetesSnapshots() bc.Metadata.IncludeAllSnapshots = true } diff --git a/pkg/module_manager/module_manager.go b/pkg/module_manager/module_manager.go index 33f4b4d84..e2336427f 100644 --- a/pkg/module_manager/module_manager.go +++ b/pkg/module_manager/module_manager.go @@ -653,6 +653,10 @@ func (mm *ModuleManager) GetGlobalHooksInOrder(bindingType BindingType) []string return names } +// DeleteModule runs beforeDeleteHelm hooks (only when a Helm release actually exists), +// helm uninstall, and afterDeleteHelm hooks. If beforeDeleteHelm fails, helm uninstall and +// afterDeleteHelm are skipped — converge retries with backoff. Symmetric to RunModule's +// beforeHelm / helm install / afterHelm flow. func (mm *ModuleManager) DeleteModule(ctx context.Context, moduleName string, logLabels map[string]string) error { ctx, span := otel.Tracer(moduleManagerServiceName).Start(ctx, "DeleteModule") defer span.End() @@ -660,7 +664,8 @@ func (mm *ModuleManager) DeleteModule(ctx context.Context, moduleName string, lo ml := mm.GetModule(moduleName) // Note: keep kubernetes monitors alive until afterDeleteHelm runs, - // so hooks can access snapshots. We'll disable hooks after running them. + // so beforeDeleteHelm and afterDeleteHelm hooks can access snapshots. + // We'll disable hooks after running them. // DELETE { @@ -702,6 +707,13 @@ func (mm *ModuleManager) DeleteModule(ctx context.Context, moduleName string, lo slog.String(pkg.LogKeyModule, ml.GetName())) } } else { + // Run beforeDeleteHelm hooks just before helm uninstall. + // On hook failure: helm uninstall and afterDeleteHelm are NOT executed, + // kubernetes informers stay alive, converge will retry with backoff. + if err := ml.RunHooksByBinding(ctx, BeforeDeleteHelm, deleteLogLabels); err != nil { + return fmt.Errorf("run beforeDeleteHelm hooks: %w", err) + } + helmClientOptions := []helm.ClientOption{ helm.WithLogLabels(deleteLogLabels), } From 1044e1102c97749debeb4baf1c6412f73c7461e6 Mon Sep 17 00:00:00 2001 From: Artem Kuleshov Date: Mon, 27 Apr 2026 14:52:01 +0300 Subject: [PATCH 2/2] [feature] add beforeDeleteHelm module hook binding Signed-off-by: Artem Kuleshov --- .../models/hooks/kind/batch_hook.go | 60 +++---------------- .../models/hooks/kind/binding_warn.go | 59 ------------------ .../models/hooks/kind/shellhook.go | 8 --- 3 files changed, 9 insertions(+), 118 deletions(-) delete mode 100644 pkg/module_manager/models/hooks/kind/binding_warn.go diff --git a/pkg/module_manager/models/hooks/kind/batch_hook.go b/pkg/module_manager/models/hooks/kind/batch_hook.go index 6f3c849e3..83f5e88f4 100644 --- a/pkg/module_manager/models/hooks/kind/batch_hook.go +++ b/pkg/module_manager/models/hooks/kind/batch_hook.go @@ -220,52 +220,10 @@ func (h *BatchHook) Execute(ctx context.Context, configVersion string, bContext } func (h *BatchHook) getConfig() (*BatchHookConfig, error) { - cfg, raw, err := getBatchHookConfigWithRaw(h.moduleName, h.Path) - if err != nil { - return nil, err - } - // Forward-compat: scan the raw JSON for top-level keys we don't know, - // so that hooks built with newer module-sdk against older addon-operator - // don't silently lose bindings. - h.warnUnknownKeysFromBatchRaw(raw) - return cfg, nil -} - -// warnUnknownKeysFromBatchRaw decodes the batch-hook config JSON loosely and -// emits warnings for unknown top-level keys per individual hook config. -func (h *BatchHook) warnUnknownKeysFromBatchRaw(o []byte) { - if h.Hook.Logger == nil || len(o) == 0 { - return - } - // New format: {version, hooks: [...]} - var env struct { - Hooks []map[string]interface{} `json:"hooks"` - Readiness map[string]interface{} `json:"readiness"` - } - if err := json.Unmarshal(o, &env); err == nil && (len(env.Hooks) > 0 || env.Readiness != nil) { - for i, raw := range env.Hooks { - warnUnknownHookKeys(h.Hook.Logger, raw, fmt.Sprintf("%s[%d]", h.Path, i)) - } - if env.Readiness != nil { - warnUnknownHookKeys(h.Hook.Logger, env.Readiness, h.Path+"[readiness]") - } - return - } - // Legacy format: [...] - var arr []map[string]interface{} - if err := json.Unmarshal(o, &arr); err == nil { - for i, raw := range arr { - warnUnknownHookKeys(h.Hook.Logger, raw, fmt.Sprintf("%s[%d]", h.Path, i)) - } - } + return GetBatchHookConfig(h.moduleName, h.Path) } func GetBatchHookConfig(moduleName, hookPath string) (*BatchHookConfig, error) { - cfg, _, err := getBatchHookConfigWithRaw(moduleName, hookPath) - return cfg, err -} - -func getBatchHookConfigWithRaw(moduleName, hookPath string) (*BatchHookConfig, []byte, error) { args := []string{"hook", "config"} environ := os.Environ() @@ -282,7 +240,7 @@ func getBatchHookConfigWithRaw(moduleName, hookPath string) (*BatchHookConfig, [ o, err := cmd.Output() if err != nil { - return nil, nil, fmt.Errorf("exec file '%s': %w", hookPath, err) + return nil, fmt.Errorf("exec file '%s': %w", hookPath, err) } // Deprecated: old batch hook config format @@ -294,7 +252,7 @@ func getBatchHookConfigWithRaw(moduleName, hookPath string) (*BatchHookConfig, [ buf := bytes.NewReader(o) err = json.NewDecoder(buf).Decode(&hooks) if err != nil { - return nil, nil, fmt.Errorf("decode: %w", err) + return nil, fmt.Errorf("decode: %w", err) } cfg := &BatchHookConfig{} @@ -304,7 +262,7 @@ func getBatchHookConfigWithRaw(moduleName, hookPath string) (*BatchHookConfig, [ cfg.Hooks[strconv.Itoa(idx)] = &h } - return cfg, o, nil + return cfg, nil } cfgs := &sdkhook.BatchHookConfig{} @@ -312,7 +270,7 @@ func getBatchHookConfigWithRaw(moduleName, hookPath string) (*BatchHookConfig, [ buf := bytes.NewReader(o) err = json.NewDecoder(buf).Decode(&cfgs) if err != nil { - return nil, nil, fmt.Errorf("decode: %w", err) + return nil, fmt.Errorf("decode: %w", err) } cfg, err := remapSDKConfigToConfig(cfgs) @@ -322,17 +280,17 @@ func getBatchHookConfigWithRaw(moduleName, hookPath string) (*BatchHookConfig, [ buf := bytes.NewReader(o) err = json.NewDecoder(buf).Decode(&cfgs) if err != nil { - return nil, nil, fmt.Errorf("decode: %w", err) + return nil, fmt.Errorf("decode: %w", err) } if outputLog.Level != "" { - return nil, nil, fmt.Errorf("got log except config: %s", string(o)) + return nil, fmt.Errorf("got log except config: %s", string(o)) } - return nil, nil, fmt.Errorf("remapSDKConfigToConfig: %w", err) + return nil, fmt.Errorf("remapSDKConfigToConfig: %w", err) } - return cfg, o, nil + return cfg, nil } type BatchHookLog struct { diff --git a/pkg/module_manager/models/hooks/kind/binding_warn.go b/pkg/module_manager/models/hooks/kind/binding_warn.go deleted file mode 100644 index fedb1b67c..000000000 --- a/pkg/module_manager/models/hooks/kind/binding_warn.go +++ /dev/null @@ -1,59 +0,0 @@ -package kind - -import ( - "github.com/deckhouse/deckhouse/pkg/log" -) - -// knownModuleHookKeys lists all top-level keys recognized by the addon-operator -// in a module hook configuration. Unknown keys are silently dropped during -// typed decoding (json/yaml unmarshal ignores unknown fields by default) — so -// without an explicit warning hook authors would not learn that a newly-added -// binding is being ignored at runtime by an older addon-operator. -// -// Forward-compat: when a future module-sdk introduces a new binding, an old -// addon-operator missing this set will emit a warning instead of silently -// ignoring the binding, so operators can spot the version mismatch in logs. -var knownModuleHookKeys = map[string]struct{}{ - // shell-operator core - "configVersion": {}, - "onStartup": {}, - "kubernetes": {}, - "schedule": {}, - "settings": {}, - "allowFailure": {}, - "queue": {}, - "logLevel": {}, - // addon-operator module lifecycle - "beforeHelm": {}, - "afterHelm": {}, - "beforeDeleteHelm": {}, - "afterDeleteHelm": {}, - // addon-operator global lifecycle - "beforeAll": {}, - "afterAll": {}, - // batch-hook envelope - "metadata": {}, - "name": {}, - "version": {}, - "hooks": {}, - "readiness": {}, - "has_settings_check": {}, -} - -// warnUnknownHookKeys emits a warning for each top-level key in raw that is -// not recognized by this version of addon-operator. raw is expected to be the -// JSON/YAML-decoded form of a single hook's config (top-level fields). -func warnUnknownHookKeys(logger *log.Logger, raw map[string]interface{}, hookName string) { - if logger == nil || len(raw) == 0 { - return - } - for key := range raw { - if _, ok := knownModuleHookKeys[key]; ok { - continue - } - logger.Warn("unknown key in hook config; addon-operator will ignore it — check addon-operator/module-sdk version compatibility", - "hook", hookName, - "key", key, - ) - } -} diff --git a/pkg/module_manager/models/hooks/kind/shellhook.go b/pkg/module_manager/models/hooks/kind/shellhook.go index 4826652a4..7cebf4f60 100644 --- a/pkg/module_manager/models/hooks/kind/shellhook.go +++ b/pkg/module_manager/models/hooks/kind/shellhook.go @@ -325,14 +325,6 @@ func (sh *ShellHook) GetConfigForModule(moduleKind string) (*config.HookConfig, return nil, fmt.Errorf("unmarshal schedule yaml hook config: %s", err) } - // Forward-compat: warn about top-level keys we don't recognize so that - // hooks built with newer module-sdk against older addon-operator don't - // fail silently when they declare unknown bindings. - rawTop := map[string]interface{}{} - if err := yaml.Unmarshal(cfgData, &rawTop); err == nil { - warnUnknownHookKeys(sh.Hook.Logger, rawTop, sh.Name) - } - return cfg, nil }