Repository navigation
Support per-step timeout field in Kptfile pipeline - #4814
SurbhiAgarwal1 wants to merge 6 commits into
Conversation
The container and exec function runtimes accept a timeout, but nothing set it, so every function ran with the default of 5 minutes. Add a --timeout flag to both commands and pass it to the runtimes. A function that times out is now reported with its stderr and a 'Timed out after' line, and kpt no longer waits past the timeout for a child process that keeps the output open. Part of kptdev#4473 Assisted-by: Claude:claude-opus-5-5 Assisted-by: Codex:gpt-6.1-sol Signed-off-by: Nikshay <nikshayyadav90@gmail.com>
Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Nikshay <nikshayyadav90@gmail.com>
Signed-off-by: Nikshay <nikshayyadav90@gmail.com> Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Nikshay <nikshayyadav90@gmail.com> Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Nikshay <nikshayyadav90@gmail.com>
✅ Deploy Preview for kptdocs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| opts := runneroptions.RunnerOptions{} | ||
| opts.InitDefaults(runneroptions.GHCRImagePrefix) | ||
| opts.AllowExec = true | ||
| opts.Timeout = 10 * time.Second |
There was a problem hiding this comment.
this test shows the shorter limit wins, a 100ms step still times out under a 10s flag. the PR description says the flag overrides the step timeout though. I like shorter-wins better, so maybe only the description needs a small change?
| When this flag is not set, there is no limit for the whole pipeline, and each | ||
| function is allowed to run for 5 minutes. |
There was a problem hiding this comment.
this line came from #4798 and isn't right anymore once a step sets its own timeout. maybe something like
| When this flag is not set, there is no limit for the whole pipeline, and each | |
| function is allowed to run for 5 minutes. | |
| When this flag is not set, there is no limit for the whole pipeline, and each | |
| function is allowed to run for 5 minutes, or for its `timeout` from the | |
| Kptfile if it sets one. |
the same sentence is also in the flag help in cmdrender.go and in internal/docs/generated/fndocs/docs.go.
| require.Len(t, fnResults.Items, 1) | ||
| assert.Equal(t, timeoutExitCode, fnResults.Items[0].ExitCode) | ||
| } | ||
|
|
There was a problem hiding this comment.
gofmt wants this trailing blank line gone.
| b.Timeout = &value | ||
| return b | ||
| } | ||
|
|
There was a problem hiding this comment.
same trailing blank line here, gofmt flags this file too.
|
|
||
| // Timeout specifies the maximum execution time for this function step | ||
| // (e.g. "30s", "1m", "5m"). If omitted, the pipeline timeout or default | ||
| // function timeout (5m) is used. |
There was a problem hiding this comment.
small thing, this doesn't apply with --allow-alpha-wasm, since runner.go only passes it to ContainerFn and ExecFn. wasm functions don't get the 5m default on main either, so it's not new. the --timeout docs already say "container and exec based functions only", maybe the same note fits here?
Add support for specifying a timeout field on individual mutator and validator pipeline steps in the Kptfile (e.g. timeout: 30s). - Add Timeout field to Function type in api/kptfile/v1/types.go and generated applyconfigurations. - Validate that the timeout field contains a valid, positive duration. - Parse step timeout and pass to ContainerFn and ExecFn runtimes. - Maintain precedence: the shorter limit wins (pipeline-level --timeout flag limits the whole run, step-level timeout limits the individual function, and step-level timeout overrides the default 5-minute timeout). - Update Kptfile schema definitions (JSON and YAML). - Add unit tests for validation, step execution timeout, and precedence. Part of kptdev#4473 Signed-off-by: SurbhiAgarwal <agarwalsurbhi1807@gmail.com>
8b858f1 to
0d88a10
Compare
|
| // (e.g. "30s", "1m", "5m"). If omitted, the pipeline timeout or default | ||
| // function timeout (5m) is used. Applicable to container and exec based | ||
| // functions only. | ||
| Timeout string `yaml:"timeout,omitempty" json:"timeout,omitempty"` |
There was a problem hiding this comment.
I would propose using metav1.Duration for this as it already has proper unmarshalling, but that would require pulling in apimachinery...
Do we want to maybe have a copy of that metav1.Duration type somewhere so we can use it here (and elsewhere later) @kptdev/maintainers?
| --timeout: | ||
| Maximum time the function is allowed to run, e.g. `30s` or `10m`. | ||
| If the function has not finished by then, the command fails. Default: `5m`. | ||
| Note that this is applicable to container and exec based functions only. |
There was a problem hiding this comment.
We only have container and exec functions
| When this flag is not set, there is no limit for the whole pipeline, and each | ||
| function is allowed to run for 5 minutes, or for its `timeout` from the | ||
| Kptfile if it sets one. | ||
| Note that this is applicable to container and exec based functions only. |
There was a problem hiding this comment.
We only have container and exec functions
| // function timeout (5m) is used. Applicable to container and exec based | ||
| // functions only. |
There was a problem hiding this comment.
We only have container and exec functions
|
|
||
| // RunContext is like Run, but the function is also killed when the time | ||
| // limit of ctx has passed. | ||
| func (f *ExecFn) RunContext(ctx context.Context, r io.Reader, w io.Writer) error { |
There was a problem hiding this comment.
TODO: We should add this to the FunctionRunner interface, more likely a FunctionRunnerV2 to not break BC
| "fmt" | ||
| "time" | ||
|
|
||
| "github.com/kptdev/kpt/pkg/printer" |
There was a problem hiding this comment.
We shouldn't import printer here... and moving these errors to the api module is in the works, maybe put this there already?




Description
This PR adds support for a per-function
timeoutfield on pipeline mutator and validator steps in the Kptfile as requested in issue #4473.Timeoutfield toFunctiontype inapi/kptfile/v1/types.goandapplyconfigurationsapi/kptfile/v1/validation.goensuring positive, valid duration syntax (e.g.30s,1m,5m)ContainerFnandExecFnruntimes inpkg/fn/runtime/runner.go--timeoutflag overrides step-level timeout, and step-level timeout overrides the 5-minute defaultkptfile.yamlandkptfile.json)feat/4473-timeout-flag)Motivation
Currently, both ContainerFn and ExecFn can accept a timeout, but there was no mechanism to configure a timeout per individual function step in the Kptfile pipeline. Adding this allows fast functions to fail quickly and heavy functions to have dedicated time limits in GitOps and CI workflows without relying solely on global CLI flags.
Testing
api/kptfile/v1/validation_test.gofor valid durations, invalid syntax, zero, and negative valuespkg/fn/runtime/runner_test.gopkg/fn/runtime/runner_test.goverifying CLI pipeline timeout precedence and step timeout precedencepkg/lib/kptops/render_executor_test.goPart of #4473