✨ Add pre-start hook support to manager by leontappe · Pull Request #3429 · kubernetes-sigs/controller-runtime

leontappe · 2026-01-23T16:47:37Z

I have a use case similar to the one described in a previous PR and this issue.

My controller needs to consume job status files on start, that would normally be worked on by a custom runnable during runtime. With vanilla controller-runtime this is currently not possible as any code that is run before reconciliation, has no access to a running manager.

This PR provides a way to register runnables as hooks that you can supply to your manager.

This PR is based on a fork of https://github.com/terinjokes/controller-runtime/tree/prestart-hook which is in turn a fork of this repo. I rebased the current master onto the latest development state of the original PR and implemented some of the proposals made by @JoelSpeed

When implementing a controller that uses leader election, there maybe be work that needs to be done after winning the election but before processing enqueued requests. For example, a controller may need to build up an internal mapping of the current state of the cluster before it can begin reconciling. This changeset adds support for adding prestart hooks to controller-runtime's manager implementation. This hook runs after the manager has been elected leader, immediately before the leader election controllers are started. Related kubernetes-sigs#607

…into prestart-hook

feat(manager): add prestart hook support See merge request asylum/upstream/controller-runtime!1

k8s-ci-robot · 2026-01-23T16:47:44Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: leontappe
Once this PR has been reviewed and has the lgtm label, please assign fillzpp for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

linux-foundation-easycla · 2026-01-23T16:47:46Z

The committers listed above are authorized under a signed CLA.

✅ login: leontappe / name: Leon Tappe (60a2b1e, 24887b2, 489ced4, 9c4c8f0)
✅ login: maxihafer / name: Maximilian Hafer (f7cb56b)

k8s-ci-robot · 2026-01-23T16:47:46Z

Welcome @leontappe!

It looks like this is your first PR to kubernetes-sigs/controller-runtime 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/controller-runtime has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2026-01-23T16:47:47Z

Hi @leontappe. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

troy0820 · 2026-01-23T17:55:01Z

/ok-to-test

📖 fix typo in hook type validation error

k8s-ci-robot · 2026-01-24T01:43:44Z

@leontappe: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-controller-runtime-apidiff	`60a2b1e`	link	false	`/test pull-controller-runtime-apidiff`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

maxihafer · 2026-01-24T01:46:49Z

pkg/manager/manager.go


+	// HookTimeout is the duration given to each hook to return successfully.
+	// To use hooks without timeout, set to a negative duration, e.g. time.Duration(-1)
+	HookTimeout *time.Duration


When @leontappe and I evaluated the proposed change in the old PR we decided to keep the -1 -> no-timeout in order to conform with previously decided behavior for GracefulShutdownTimeout.
Other projects also use -1 as a value to the denote absence of any timeout/ratelimiting (rest.Config.QPS)

Given that a 0 value is not logically valid, we could reset to the default here, this is also done in GracefulShotdownTimeout

I think the comparisons are not great, GracefulShutdownTimeout has an additional state of disable this which is indicated by 0 and restConfig.QPS is not a pointer.

I would probably build this as a value <= 0 means no timeout

I'd prefer an explicit value meaning no timeout that we document personally, so I'd say if this is 0, it's no timeout

nil: no opinion, use something default

explicitly 0: no timeout

explicitly >0: use this as the timeout

explicitly <0: error

What other timeout fields do we have in CR as prior art? Or do we only have GracefulShutdownTimeout? (just to end up with something somewhat consistent, it makes sense to make different decisions if the cases are different but we should minimize variations if possible)

alvaroaleman

/cc @JoelSpeed

alvaroaleman · 2026-01-24T17:42:13Z

pkg/manager/manager.go

 	AddReadyzCheck(name string, check healthz.Checker) error

+	// Hook allows to add Runnables as hooks to modify the behavior.
+	Hook(hook HookType, runnable Runnable) error


I know @vincepri had suggested this name in the other PR, but I am not a huge fan because IMHO:

It is different from Add in that its name is what is being added rather than whats being done/the operation

Having a HookType argument when there is only one HookType seems needlessly complicated to me and requires us to spread out godocs explaining what this does to this method and the HookType rather than having one place where we can explain it

I would suggest AddPrestartHook since that follows the existing naming pattern and is easy to discover. If we ever add another hook, we can then name that Add${OtherType}Hook.

But lets wait for a second opinion from @sbueringer before you end up changing this back and forth

Agree, AddPrestartHook sounds good.

Is there some prior art from other components (kube-controller-manager, other controller frameworks) that might give us an idea how likely it is that we'll have other types of hooks in the future and what these might be?

(but if we don't have more data let's go with AddPrestartHook)

Hm. Is "Prestart" precise enough to describe "before leader election runnables"? No strong opinion, but it's pretty generic.

+ "to modify the behavior" sounds strange. Let's try to explain this a bit better

alvaroaleman · 2026-01-24T17:43:51Z

pkg/manager/manager.go

 	// +optional
 	Controller config.Controller

+	// HookTimeout is the duration given to each hook to return successfully.


WDYT about the idea of having a per-hook timeout rather than a global one for all hooks? Different hook may need different timeouts

alvaroaleman · 2026-01-24T17:47:20Z

pkg/manager/internal.go


 func (cm *controllerManager) startLeaderElectionRunnables() error {
+	cm.logger.Info("Running prestart hooks")
+	for _, hook := range cm.prestartHooks {


Other than that being what you found in the original PR, what is the reason for starting the hooks sequentially and not in parallel? Couldn't the sequential starting be achieved if the user instead submitted one hook that does everything sequentlally?

Probably need to think about how we would collect errors from pre-start hooks on startup and use that to bail out if there are any errors, but that sounds like it would be fairly straightforward with a waitgroup

Agree. I think we should start them in parallel

alvaroaleman · 2026-01-24T17:49:32Z

pkg/manager/internal.go

+	}
+
+	// All the prestart hooks have ben run, clear the slice to free the underlying resources.
+	cm.prestartHooks = nil


Maybe add a log here finished running hooks or such so that if this hangs, there is at least some chance to figure out it is because of the hooks?

alvaroaleman · 2026-01-24T17:54:41Z

pkg/manager/manager.go


+	// HookTimeout is the duration given to each hook to return successfully.
+	// To use hooks without timeout, set to a negative duration, e.g. time.Duration(-1)
+	HookTimeout *time.Duration


I think the comparisons are not great, GracefulShutdownTimeout has an additional state of disable this which is indicated by 0 and restConfig.QPS is not a pointer.

I would probably build this as a value <= 0 means no timeout

sbueringer · 2026-02-02T12:45:52Z

pkg/manager/internal.go

 	defaultRenewDeadline          = 10 * time.Second
 	defaultRetryPeriod            = 2 * time.Second
 	defaultGracefulShutdownPeriod = 30 * time.Second
+	defaultHookPeriod             = 15 * time.Second


Suggested change

defaultHookPeriod = 15 * time.Second

defaultHookTimeoutPeriod = 15 * time.Second

nit

sbueringer · 2026-02-02T12:48:54Z

pkg/manager/internal.go

+	}
+
+	switch hook {
+	case HookPrestartType:


Should this be HookTypePrestart?

sbueringer · 2026-02-02T12:50:50Z

pkg/manager/internal.go


 func (cm *controllerManager) startLeaderElectionRunnables() error {
+	cm.logger.Info("Running prestart hooks")
+	for _, hook := range cm.prestartHooks {


Agree. I think we should start them in parallel

sbueringer · 2026-02-02T12:51:20Z

pkg/manager/internal.go

@@ -648,6 +674,27 @@ func (cm *controllerManager) initLeaderElector() (*leaderelection.LeaderElector,
 }

 func (cm *controllerManager) startLeaderElectionRunnables() error {


Let's rename the func to startPreStartHooksAndLeaderElectionRunnables

sbueringer · 2026-02-02T12:56:30Z

pkg/manager/manager.go

 	AddReadyzCheck(name string, check healthz.Checker) error

+	// Hook allows to add Runnables as hooks to modify the behavior.
+	Hook(hook HookType, runnable Runnable) error


Agree, AddPrestartHook sounds good.

Is there some prior art from other components (kube-controller-manager, other controller frameworks) that might give us an idea how likely it is that we'll have other types of hooks in the future and what these might be?

(but if we don't have more data let's go with AddPrestartHook)

Hm. Is "Prestart" precise enough to describe "before leader election runnables"? No strong opinion, but it's pretty generic.

sbueringer · 2026-02-02T13:01:46Z

pkg/manager/manager.go


+	// HookTimeout is the duration given to each hook to return successfully.
+	// To use hooks without timeout, set to a negative duration, e.g. time.Duration(-1)
+	HookTimeout *time.Duration


What other timeout fields do we have in CR as prior art? Or do we only have GracefulShutdownTimeout? (just to end up with something somewhat consistent, it makes sense to make different decisions if the cases are different but we should minimize variations if possible)

sbueringer · 2026-02-02T13:02:48Z

pkg/manager/manager.go

 	AddReadyzCheck(name string, check healthz.Checker) error

+	// Hook allows to add Runnables as hooks to modify the behavior.
+	Hook(hook HookType, runnable Runnable) error


+ "to modify the behavior" sounds strange. Let's try to explain this a bit better

sbueringer · 2026-02-02T13:03:43Z

pkg/manager/manager.go


+	if options.HookTimeout == nil {
+		hookTimeout := defaultHookPeriod
+		options.HookTimeout = &hookTimeout


nit: if we keep this let's use ptr.To()

terinjokes and others added 4 commits August 25, 2024 15:47

Merge branch 'main' of github.com:kubernetes-sigs/controller-runtime …

9c4c8f0

…into prestart-hook

Merge branch 'prestart-hook' into 'mw'

24887b2

feat(manager): add prestart hook support See merge request asylum/upstream/controller-runtime!1

implement changes from PR comments on upstream

489ced4

k8s-ci-robot requested a review from alvaroaleman January 23, 2026 16:47

k8s-ci-robot requested a review from troy0820 January 23, 2026 16:47

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jan 23, 2026

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 23, 2026

maxihafer and others added 2 commits January 24, 2026 01:18

fix typo in hook type validation error

f7cb56b

Merge pull request #2 from mittwald/typo/hook-type-validation

60a2b1e

📖 fix typo in hook type validation error

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. and removed cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 24, 2026

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jan 24, 2026

maxihafer reviewed Jan 24, 2026

View reviewed changes

alvaroaleman reviewed Jan 24, 2026

View reviewed changes

k8s-ci-robot requested a review from JoelSpeed January 24, 2026 17:55

alvaroaleman added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Jan 24, 2026

sbueringer reviewed Feb 2, 2026

View reviewed changes

	defaultHookPeriod = 15 * time.Second
	defaultHookTimeoutPeriod = 15 * time.Second

		@@ -648,6 +674,27 @@ func (cm controllerManager) initLeaderElector() (leaderelection.LeaderElector,
		}

		func (cm *controllerManager) startLeaderElectionRunnables() error {

Conversation

leontappe commented Jan 23, 2026

Uh oh!

k8s-ci-robot commented Jan 23, 2026

Uh oh!

linux-foundation-easycla bot commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Jan 23, 2026

Uh oh!

k8s-ci-robot commented Jan 23, 2026

Uh oh!

troy0820 commented Jan 23, 2026

Uh oh!

k8s-ci-robot commented Jan 24, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alvaroaleman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbueringer Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbueringer Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

linux-foundation-easycla bot commented Jan 23, 2026 •

edited

Loading

sbueringer Feb 2, 2026 •

edited

Loading

sbueringer Feb 2, 2026 •

edited

Loading