feat: optional command timeout field in config file that defaults to syncTimeout

[marshallford]

Adds the ability to set an optional timeout duration for all commands that defaults to the sync timeout instead of time.Minute. Attempts to resolve: #3049. Keep in mind that in order to fix the issue described, the sync timeout must be greater than or equal to the sum of the generator command timeouts. I am open to ideas or to scraping this completely. Thanks Flux team!

Example:

version: 1
patchUpdated:
  generators:
  - command: kustomize build .
    timeout: 3m # if field is not provided, sync timeout is used
  patchFile: flux-patch.yaml

Couple of other notes:

1. I looked into parsing timeout from the config yaml as the type time.Duration instead of a string but the use of the ghodss/yaml package made that a bit confusing (for a new golang user anyway). It would have been nice to return an error in ParseConfigFile if an invalid duration was given instead of using the syncTimeout as a default without notice.

Solved by C&P this snippet: https://stackoverflow.com/a/48051946

2. I'm not super happy with adding the field defaultTimeout to the configAware struct. Open to suggestions on how to improve passing through the sync timeout.

TODO:

• Investigate better way of passing around/through sync timeout flag
• Update docs once this method of setting a command timeout per generator is approved

[marshallford]

@stefanprodan could I get a review on this? Would you be open to a schema change like this?

[marshallford]

@stefanprodan ping

[marshallford]

ping @hiddeco

[rdubya16]

@stefanprodan Any movement on this? This is blocking us as well.

[mike-stewart]

@squaremo We're running into this bug as well. We are running a build of this PR in one of our clusters now, and it seems to be resolving the issue. Any chance this PR could be merged?

[squaremo]

Thanks for putting time into this! I appreciate the need, and I'm happy with the idea. Passing the default timeout is a bit awkward, as you mention; I'm fine with it though, at least it's obvious what's happening.
There's some bits in the implementation that need another look, which I've flagged up.

[squaremo]

I've rebased this, and added a commit for using metav1.Duration. Once it passes CI I'll squash those early few commits and merge.

[marshallford]

I've rebased this, and added a commit for using metav1.Duration. Once it passes CI I'll squash those early few commits and merge.

Awesome Thanks!

[squaremo]

@marshallford Can you do me a quick favour? We now require a DCO (as of today!). It's pretty easy to apply it -- instructions are in that link, and you will need to git reset your local branch head first, to pick up my rebase.

[squaremo]

Brill, thank you @marshallford!