fix: Surface error during wait timeout for OSS artifact driver API calls

[terrytangyuan]

Signed-off-by: terrytangyuan [email protected]

Checklist:

• My organization is added to USERS.md.

[codecov]

Codecov Report

Merging #5601 (8a4093e) into master (b4ce78b) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #5601      +/-   ##
==========================================
- Coverage   47.07%   47.06%   -0.01%     
==========================================
  Files         240      240              
  Lines       15034    15034              
==========================================
- Hits         7077     7076       -1     
  Misses       7057     7057              
- Partials      900      901       +1     

Impacted Files	Coverage Δ
workflow/artifacts/oss/oss.go	7.89% <0.00%> (ø)
cmd/argo/commands/get.go	55.66% <0.00%> (-0.65%)	⬇️
workflow/controller/operator.go	71.17% <0.00%> (+0.05%)	⬆️

---

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 b4ce78b...8a4093e. Read the comment docs.

[alexec]

can you please attach the outpt of your testing?

[terrytangyuan]

I was getting a timed out waiting for the condition error after the specified retries on transient errors but wasn't able to see the underlying error. Switching to use wait.Backoff will give me something like timed out waiting for the condition: Failed to upload some parts with error: ConnectionTimeoutError: Connect timeout for 60000ms.

[dinever]

This is great! 👍

[alexec]

@terrytangyuan can I confirm you tested each of the artifact drivers with their respective cloud providers? If not, can I suggest we break this into one PR per provider so they can be tested by the community?

[alexec]

testing details requested

[terrytangyuan]

Unfortunately I cannot test for all the providers. I can change this to only modifying OSS artifact driver.

Can I keep the minor refactoring of DefaultRetry in workflow/artifacts/common/common.go in the same PR though as the code would look much cleaner?

[alexec]

Maybe just refactor the OSS code? It is really easy to introduce bugs with retry, so it needs to be tested on actual provider

[terrytangyuan]

Maybe just refactor the OSS code? It is really easy to introduce bugs with retry, so it needs to be tested on actual provider

Yep done!