-
Notifications
You must be signed in to change notification settings - Fork 83
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
Expand retry conditions for K8 logs #818
Conversation
625a5d2
to
5eb8c0f
Compare
46cabf7
to
2df4f86
Compare
2df4f86
to
978f1d6
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
pkg/workceptor/kubernetes.go
Outdated
@@ -275,7 +255,7 @@ func (kw *kubeUnit) kubeLoggingWithReconnect(streamWait *sync.WaitGroup, stdout | |||
break | |||
} | |||
} | |||
if err != nil { | |||
if err != nil && remainingRetries > 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the additional conditional && remainingRetries > 0
is needed here
I think the only way that the code reach this point is if we exhausted the retries?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fosterseth am i right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed. I don't think this conditional will ever hit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed, i think Seth is right, now with the changes this condition will never be hit, the first err != nil
will be triggered and this will never be called.
I have removed the whole check.
9591f7e
to
b6c9e6c
Compare
pkg/workceptor/kubernetes.go
Outdated
*stdoutErr = err | ||
kw.Error("Error reading from pod %s/%s: %s", podNamespace, podName, err) | ||
|
||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these code will still need to be executed when the retries is exhausted
9c83f22
to
4295e36
Compare
4295e36
to
a931ac4
Compare
Fixes: ansible/awx#14293
Expanded retry conditions for K8 logs to retry at least 5 times regardless of the error returned. This is due to K8 api timings where the GOAWAY error may not be sent or sent already before log streams get attached.