fix: allow cli/ui to follow logs

[danielhelfand]

Closes #8947

#8404 introduced a new environment var (ARGOCD_K8S_TCP_TIMEOUT) with a default of 32 seconds. This is causing requests for logs with follow configured to be timed out. Maybe the default should be no timeout and should be explicitly configured by the user if requested.

For any user looking to configure these values, we should consider including these logging considerations in documentation for these env vars. There is an issue open for documenting these vars: #8527.

Additionally, the max param for using env.ParseDurationFromEnv func is being passed as arg math.MaxInt32, which is converted to 2.147483647s as a time.Duration. This then only allows users to configure a value between 0 and 2.147483647s. This pr also updates parse duration funcs using math.MaxInt32 as a max to math.MaxInt32*time.Second to fix this issue.

Note on DCO:

If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.

Checklist:

• Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
• The title of the PR states what changed and the related issues number (used for the release note).
• I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
• I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
• Does this PR require documentation updates?
• I've updated documentation as required by this PR.
• Optional. My organization is added to USERS.md.
• I have signed off all my commits as required by DCO
• I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
• My build is green (troubleshooting builds).

[alexmt]

Thank you! Cherry-picking it into 2.3

[codecov]

Codecov Report

Merging #8987 (3a693a5) into master (a5933db) will decrease coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #8987      +/-   ##
==========================================
- Coverage   45.00%   44.95%   -0.05%     
==========================================
  Files         212      212              
  Lines       25272    25272              
==========================================
- Hits        11373    11362      -11     
- Misses      12292    12306      +14     
+ Partials     1607     1604       -3     

Impacted Files	Coverage Δ
applicationset/services/scm_provider/github.go	63.52% <0.00%> (-17.65%)	⬇️
applicationset/services/scm_provider/utils.go	88.50% <0.00%> (+4.59%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5933db...3a693a5. Read the comment docs.

[crenshaw-dev]

LGTM! @jannfis was there any reason you needed the default to be 32s instead of 0?

[jannfis]

@crenshaw-dev Thanks for pulling me in.

The server side timeout is the timeout passed to Kubernetes API requests. The default used by the K8s API client is 32 seconds, and should imho stay this way. But probably, when set to 0, the API client will use the 32s default anyway. I was just making it explicit.

[jannfis]

I think the reason why the follow logs fails is a different tho.

It's probably this addition, introduced in my change:

argo-cd/pkg/apis/application/v1alpha1/types.go

Line 2346 in a5933db

IdleConnTimeout: K8sTCPIdleConnTimeout,

EDIT: Hm. That's 5 minutes per default. Shouldn't be the 30s culprit then.

[danielhelfand]

@jannfis Thanks for the perspective on this. I tried configuring the various options to see if any of these were the culprit. ARGOCD_K8S_TCP_TIMEOUT was the only change that stopped the timeouts.

Happy to take our time on this and debug a bit further though if you have further questions I can look into.

[danielhelfand]

The default used by the K8s API client is 32 seconds

Any docs on this? Am only seeing 32 seconds as default for the discovery client.

[danielhelfand]

Following up here, I think having no timeout as the default is the way to go.

Looking further into the k8s client set up, I feel pretty confident that no timeout is being set by default and the 32 second timeout is specific to the discovery client.

This would make sense since, prior to #8404, the timeout was not configurable or being set at all.

[crenshaw-dev]

@jannfis is that explanation persuasive / would you be alright with merging this PR?

[crenshaw-dev]

@danielhelfand I'm not super familiar with this area of the code, but your explanation makes sense to me. We can always revert if there's trouble.