Make the TaskAction requeue duration configurable

[stantheman0128]

Tracking issue

Closes #7740

Why are the changes needed?

The TaskAction controller requeues running actions with TaskActionDefaultRequeueDuration, a const 10s, referenced directly by eight RequeueAfter: sites. Operators cannot trade responsiveness against reconcile load without rebuilding the executor, which is what #7740 asks for.

What changes were proposed in this pull request?

A new executor config field, requeueDuration, shaped exactly like the existing maxSystemFailures:

Step	maxSystemFailures	requeueDuration
config field with pflag tag	config.go	config.go
validation	setup.go	setup.go
reconciler field	TaskActionReconciler.MaxSystemFailures	TaskActionReconciler.RequeueDuration
zero-value accessor	maxSystemFailures()	requeueDuration()

The default stays 10s, so an executor that does not set the field behaves exactly as before.

The cache reservation heartbeat moves with it. cacheReservationHeartbeatInterval was a const aliased to the requeue duration. It has to keep tracking it, because the reservation is only ever extended on the next reconcile: a lease shorter than the gap to that reconcile lets another owner take a serializable cache reservation while this TaskAction is still waiting.

That said, the lease is a request, not a guarantee, and the code and config docs now say so. cache_service clamps it in Manager.resolvedHeartbeat to its own maxReservationHeartbeat and expires the reservation after heartbeatGracePeriodMultiplier of the clamped value, 10s times 3 by default. Raising requeueDuration past that window without also raising maxReservationHeartbeat still leaves a gap. I chose to document that rather than silently promise a guarantee that does not hold, but if you would rather the executor refuse or warn on that combination, say so and I will add it.

taskaction_condition.go's RequeueAfter: time.Until(deadline) is deliberately untouched. That one wakes up at a timeout deadline rather than polling, so tying it to the poll interval would delay or miss the timeout.

A note on the generated flags file

config_flags.go and config_flags_test.go are generated by pflags, and running the generator available in this tree produces two changes I did not include:

1. It reverts config_flags_test.go from github.com/go-viper/mapstructure/v2 back to github.com/mitchellh/mapstructure. The generator comes from boilerplate/flyte/golang_support_tools, which pins flytestdlib v1.11.0, and that version's template still emits the old import. Every other config_flags_test.go in the tree moved to go-viper in feat(executor): configurable default service account for task pods #7486, so I kept the current import.
2. It emits a maxConcurrentReconciles flag. That field has a pflag tag in config.go but no flag in config_flags.go on main, so the generator is right that something is missing, but it is a pre-existing gap unrelated to this change and I left it alone.

So this diff contains only the generator's requeueDuration output. Happy to split the maxConcurrentReconciles gap into its own PR if that is useful. Worth flagging that check-generate runs make buf and does not cover pflags, so neither of these would be caught by CI.

How was this patch tested?

New unit tests:

• TestRequeueDurationFallsBackToDefault covers unset, negative, and configured values, asserting both requeueDuration() and cacheReservationHeartbeat().
• TestHandleCacheBeforeExecutionReservationHeartbeatTracksRequeueDuration asserts the lease actually handed to the catalog follows a configured requeue duration.

The second one is not tautological. Replacing cacheReservationHeartbeat() with a fixed 10s turns it red:

--- FAIL: TestHandleCacheBeforeExecutionReservationHeartbeatTracksRequeueDuration
    expected: 45s   actual: 10s

The existing assertion in taskaction_cache_test.go was updated to TaskActionDefaultRequeueDuration, since that reconciler leaves RequeueDuration unset and so exercises the fallback.

Full go test ./executor/..., compared against a clean main checkout at the same commit:

                                    this branch      clean main
executor                            ok               ok
executor/pkg/config                 ok               ok
executor/pkg/controller             FAIL             FAIL
executor/pkg/plugin                 ok               ok
executor/pkg/plugin/k8s             ok               ok
executor/pkg/webhook                ok               ok
executor/test/integration           FAIL             FAIL

Both failures are identical on main and are environmental: exec: "\usr\local\kubebuilder\bin\etcd": executable file not found. This machine has no envtest binaries. go build ./executor/... and go vet are clean.

Labels

• added: For new features.

Check all the applicable boxes

• I updated the documentation accordingly.
• All new and existing tests passed.
• All commits are signed-off.

What was not tested

The envtest reconcile loop, for the reason above, so there is no end-to-end run of the controller with a raised requeueDuration. The cross-service clamp described earlier is derived from reading Manager.resolvedHeartbeat and the cache_service defaults, not from running two executors against one serializable cache key.

---

This change was developed with AI assistance from Claude. I reviewed and verified it before submitting.

[Copilot]

Pull request overview

This PR makes the TaskAction controller’s requeue interval configurable via executor config, replacing direct uses of the hard-coded TaskActionDefaultRequeueDuration (10s) with a reconciler-level accessor and plumbing that value through cache reservation heartbeats.

Changes:

• Add requeueDuration to executor config/flags and validate it in executor/setup.go (reject negative values).
• Plumb RequeueDuration into TaskActionReconciler and use r.requeueDuration() for requeue behavior across the controller.
• Make the catalog reservation heartbeat track the requeue duration, with updated/added unit tests.

Reviewed changes

Copilot reviewed 5 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
executor/setup.go	Validates requeueDuration and wires it into the TaskAction reconciler.
executor/pkg/controller/taskaction_controller.go	Adds RequeueDuration and requeueDuration() accessor; switches requeue sites to use it.
executor/pkg/controller/taskaction_cache.go	Replaces fixed cache reservation heartbeat with one derived from the reconciler’s requeue duration and documents clamp behavior.
executor/pkg/controller/taskaction_cache_test.go	Updates existing heartbeat assertion and adds tests covering heartbeat tracking and default fallback behavior.
executor/pkg/config/config.go	Adds RequeueDuration to config with defaults and documentation.
executor/pkg/config/config_flags.go	Adds the requeueDuration flag wiring.
executor/pkg/config/config_flags_test.go	Adds generated-style flag decode test for requeueDuration.

Files not reviewed (2)

• executor/pkg/config/config_flags.go: Generated file
• executor/pkg/config/config_flags_test.go: Generated file

Suppressed comments (1)

executor/pkg/controller/taskaction_controller.go:157

• The docstring for requeueDuration() says the cache reservation is held "at least until the next reconcile", but cache_service can clamp the requested heartbeat and expire the reservation after a grace-multiplier window. For larger requeue durations this guarantee does not hold, and the comment is misleading (the more detailed explanation in cacheReservationHeartbeat() is accurate).

// requeueDuration is how long to wait before reconciling a running TaskAction
// again. It also bounds the cache reservation heartbeat, so a serializable
// reservation is held at least until the next reconcile.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[stantheman0128]

Fixed in 5ef08ee. Both comments now say non-positive rather than zero.

The accessor comment on line 155 also still carried a stale claim about the reservation lease holding until the next reconcile. It now points at cacheReservationHeartbeat, which documents that cache_service clamps the requested heartbeat to its own maxReservationHeartbeat.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 7 changed files in this pull request and generated no new comments.

Files not reviewed (2)

• executor/pkg/config/config_flags.go: Generated file
• executor/pkg/config/config_flags_test.go: Generated file

Suppressed comments (3)

executor/pkg/controller/taskaction_controller.go:107

• The RequeueDuration field comment says it only affects reconciling a running TaskAction, but this value is also used for retryable error paths (e.g. failing to build the task execution context) and other non-terminal retries. Updating the comment avoids a doc/behavior mismatch for operators and future maintainers.

	// RequeueDuration overrides how long to wait before reconciling a running
	// TaskAction again. Any non-positive value means TaskActionDefaultRequeueDuration.
	RequeueDuration time.Duration

executor/pkg/config/config.go:107

• RequeueDuration is documented as only affecting "running" TaskActions, but the reconciler uses it for other retryable/non-terminal requeues as well (e.g. transient/system errors). Consider adjusting the doc comment and the pflag help string so they describe the actual behavior.

	// RequeueDuration is how long the controller waits before reconciling a running
	// TaskAction again. Lowering it makes task actions more responsive at the cost of
	// more reconciles and more load on the plugin backends. 0 or unset means the
	// built-in default of 10s.

executor/pkg/config/config_flags_test.go:329

• This generated flag test sets requeueDuration to the default value, so it doesn’t actually exercise parsing/decoding a non-default duration. Using a non-default (but valid) value here would better validate the new flag wiring.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 7 changed files in this pull request and generated no new comments.

Files not reviewed (2)

• executor/pkg/config/config_flags.go: Generated file
• executor/pkg/config/config_flags_test.go: Generated file