feat: start timeout for host runs

[lkingland]

• 🎁 adds run options
• 🎁 start timeout for host runs

Host-runs now support a Start Timeout option. The setting is added to the function as f.Run.StartTimeout.

Related: #1760

/kind enhancement

[knative-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lkingland

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [lkingland]

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

[codecov]

Codecov Report

Patch coverage: 82.75% and project coverage change: +1.18 🎉

Comparison is base (f6c11c9) 62.37% compared to head (99bd945) 63.55%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1782      +/-   ##
==========================================
+ Coverage   62.37%   63.55%   +1.18%     
==========================================
  Files         106      106              
  Lines       13500    13553      +53     
==========================================
+ Hits         8420     8614     +194     
+ Misses       4285     4118     -167     
- Partials      795      821      +26     

Flag	Coverage Δ
e2e-test	37.60% <24.13%> (-0.06%)	⬇️
e2e-test-oncluster	33.29% <10.34%> (+0.04%)	⬆️
e2e-test-oncluster-runtime	28.39% <10.34%> (?)
e2e-test-runtime-go	27.27% <10.34%> (?)
e2e-test-runtime-node	28.28% <10.34%> (?)
e2e-test-runtime-python	28.28% <10.34%> (?)
e2e-test-runtime-quarkus	28.41% <10.34%> (?)
e2e-test-runtime-springboot	27.33% <10.34%> (?)
e2e-test-runtime-typescript	28.41% <10.34%> (?)
integration-tests	51.88% <20.68%> (+3.51%)	⬆️
unit-tests-macos-latest	50.06% <82.75%> (+0.21%)	⬆️
unit-tests-ubuntu-latest	50.77% <82.75%> (+0.13%)	⬆️
unit-tests-windows-latest	50.01% <79.31%> (+0.12%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/functions/errors.go	30.76% <ø> (ø)
pkg/functions/function.go	85.80% <ø> (ø)
cmd/run.go	73.52% <72.72%> (-0.80%)	⬇️
pkg/functions/runner.go	78.04% <85.71%> (+2.53%)	⬆️
pkg/functions/client.go	65.08% <86.36%> (+1.17%)	⬆️
pkg/docker/runner.go	66.66% <100.00%> (ø)

... and 6 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[lance]

Some minor feedback (nits, really). Looks good so far. There appear to be modifications from some of your other PRs in here which presumably will make this PR smaller when the others are merged

[lkingland]

Thanks for helping make sure the logic was correct @matejvasek .

@lance do you think the suggestion above will work: allowing this PR through, which implements the start-timeout for host-based runs, and then opening separate issues to track implementing the option for containerized runs and on deployed services?

[aslom]

Could we use feature flag (simple environment variable?) to hide options until they are fully working?

[lance]

Thanks for helping make sure the logic was correct @matejvasek .

@lance do you think the suggestion above will work: allowing this PR through, which implements the start-timeout for host-based runs, and then opening separate issues to track implementing the option for containerized runs and on deployed services?

That seems like a reasonable approach to me

[lkingland]

Could we use feature flag (simple environment variable?) to hide options until they are fully working?

Sure, I set this flag as hidden (if used, it will yield an informative error that it's not supported for containerized runs), and will add a feature flag to the next PR, #1748 (which enables the Host builder) until such time as a few other PRs are merged (~7 pending. you can check out my develop branch for the latest) which should add the minimum necessary base functionality.

[aslom]

@lkingland with feature flag we should have more freedom to experiment without worrying that new options in CLI may confuse users? Also make it easier to test if previous behavior is not affected if the code changing it is behind feature flag? I am ok either way.

[aslom]

I have verified that make test works and did basic func run and deploy and everything looks good. Great to see run timeout addition. Hopefully soon exposed:

../func/func run --start-timeout 10m
Error: the ability to specify the startup timeout for containerized runs is coming soon

/lgtm