Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Add logging to aid troubleshooting #5501

Merged
merged 3 commits into from
Mar 24, 2021

Conversation

copierrj
Copy link
Contributor

@copierrj copierrj commented Mar 24, 2021

While investigating stability issues we discovered that Argo wasn't retrying various failing k8s api calls. We didn't really understand what was going on until we deployed an experimental Argo build containing additional logging.

Therefore we propose make the root cause of failures more obvious and indicate if an error is considered to be transient or not.

Checklist:

Signed-off-by: Reijer Copier <reijer.copier@kadaster.nl>
@codecov
Copy link

codecov bot commented Mar 24, 2021

Codecov Report

Merging #5501 (e02f13b) into master (3065941) will increase coverage by 0.16%.
The diff coverage is 100.00%.

❗ Current head e02f13b differs from pull request most recent head be01721. Consider uploading reports for the commit be01721 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5501      +/-   ##
==========================================
+ Coverage   46.54%   46.70%   +0.16%     
==========================================
  Files         240      240              
  Lines       15001    14994       -7     
==========================================
+ Hits         6982     7003      +21     
+ Misses       7119     7093      -26     
+ Partials      900      898       -2     
Impacted Files Coverage Δ
util/errors/errors.go 100.00% <100.00%> (ø)
pkg/apiclient/apiclient.go 5.55% <0.00%> (-4.45%) ⬇️
cmd/argo/commands/get.go 56.66% <0.00%> (ø)
pkg/apiclient/http1-client.go 0.00% <0.00%> (ø)
pkg/apiclient/logs-intermediary.go 0.00% <0.00%> (ø)
pkg/apiclient/http1/event-watch-client.go 0.00% <0.00%> (ø)
...lient/error-translating-workflow-service-client.go 0.00% <0.00%> (ø)
...o-kube-cluster-workflow-template-service-client.go 0.00% <0.00%> (ø)
workflow/controller/operator.go 70.47% <0.00%> (+0.21%) ⬆️
cmd/argoexec/commands/emissary.go 50.00% <0.00%> (+1.56%) ⬆️
... and 2 more

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 3065941...be01721. Read the comment docs.

Copy link
Member

@simster7 simster7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This could be very useful

util/errors/errors.go Outdated Show resolved Hide resolved
@copierrj copierrj requested a review from simster7 March 24, 2021 15:00
Co-authored-by: Simon Behar <simbeh7@gmail.com>
Signed-off-by: Reijer Copier <reijer.copier@kadaster.nl>
Copy link
Member

@simster7 simster7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we need to lint, then we should be good to go

Signed-off-by: Reijer Copier <reijer.copier@kadaster.nl>
@simster7 simster7 merged commit beb0f26 into argoproj:master Mar 24, 2021
@copierrj copierrj deleted the is_transient_logging branch March 24, 2021 16:36
This was referenced Mar 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants