Skip to content

[shell-operator] Let an embedding application choose how hooks are run - #942

Merged
yalosev merged 1 commit into
mainfrom
feat/pluggable-hook-runner
Oct 13, 2026
Merged

yalosev merged 1 commit into
mainfrom
feat/pluggable-hook-runner

Conversation

@yalosev

@yalosev yalosev commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

Overview

Adds executor.Runner: the application that links shell-operator as a library can decide how a hook is run for an event. The default stays the same, every hook is started as a child process.

What this PR does / why we need it

Every event starts the hook as a new process. For a Python hook most of that time goes to interpreter start-up and imports. In deckhouse webhook-handler, which runs admission and conversion webhooks through shell-operator, one Python validating hook takes 24-32 ms, and about 2 ms of it is the hook itself (python:3.12-alpine, 1 CPU). A long-lived interpreter that forks per event runs the same hook in 1.9 ms.

This PR adds the extension point for that, without changing anything by default:

  1. executor.Runner with one method, Run(ctx, *exec.Cmd) (*CmdUsage, error). The command carries the hook path, arguments, environment, working directory and the stdout/stderr writers.
  2. executor.ProcessRunner is the existing behaviour moved behind the interface: start, register the PID for the zombie reaper, wait, report rusage.
  3. The runner is set with shell_operator.WithHookRunner(...), or HookRunner in hook.ManagerConfig, and reaches every hook through HookManager.
  4. Log proxying, StderrError and the error returned to the caller stay in Executor.RunAndLogLines and apply to any runner.
  5. The --config call made while loading hooks always starts a process.

Special notes for your reviewer

  • Tests: TestRunAndLogLines_WithRunner covers output and errors through a custom runner and the nil fallback. Test_ManagerConfig_HookRunner_RunsHooks checks that the runner from ManagerConfig reaches Hook.Run and that --config is still fetched by starting a process. The second test fails when the line that wires the runner into hooks is removed.
  • go test ./pkg/... passes.
  • The deckhouse change that uses this will link the PR commit until a release is tagged.

Every event runs a hook by starting it as a child process. For an
interpreted hook most of that time is interpreter start-up and imports:
in deckhouse webhook-handler a Python validating hook takes 24-32 ms, of
which about 2 ms is the hook itself.

executor.Runner lets the application that links shell-operator decide how
a prepared hook command is run, for example by handing it to a long-lived
interpreter that forks per event. It is set with WithHookRunner on the
ShellOperator, or HookRunner in hook.ManagerConfig.

Nothing changes by default: ProcessRunner keeps starting a process, and
the --config call made while loading hooks always starts one. Log proxying,
StderrError and resource usage reporting stay in the executor and apply to
any Runner.

Signed-off-by: Yuriy Losev <yuriy.losev@flant.com>
@yalosev yalosev added the enhancement New feature or request label Oct 11, 2026
@yalosev
yalosev requested a review from fuldaxxx October 12, 2026 14:25
@yalosev
yalosev merged commit d5a9c01 into main Oct 13, 2026
13 of 14 checks passed
@yalosev
yalosev deleted the feat/pluggable-hook-runner branch October 13, 2026 15:33
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.

2 participants