-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Fix check served logs logic #41272
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 check served logs logic #41272
Conversation
… logs apache#39177 introduced a bug where, if the task was in deferred state, served logs would not be checked.
The PV was only mounted to the worker and not on the webserver, hence the need for the the served logs for old attempts to be fetched from the served logs instead from the webserver. This could have been fixed on our end by adding a mount for the PV to the webserver, but instead trying to fix the bug introduced in upstream was chosen. |
kahlstrm
left a comment
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.
LGTM
|
I apologize if i might be spamming in this PRs regarding this issue, but i am still confusing as this is a issue that is suppose to be fixed. I apologize for my lack of understanding. |
Background:
In #31101, I added logic to check for served logs when we did not find either local or remote logs.
In #32561, contributor @Khrol observed that for a task with multiple tries, if the user was looking at the logs for a non-running try, the UI would show an erroneous and potentially confusing 404 error. @Khrol attempted a fix that would suppress this error message.
In #39177, contributor @kahlstrm reported a bug introduced by #32562 and attempted to fix it.
The bug was reportedly that “non-running task try logs weren’t shown in the UI for running tasks”. This I think refers to when you are looking at the logs for a failed attempt. The contributor added, “This is due to us storing the logs on the worker with a Persistent Volume”. I assume this means that we did not check served logs in that case.
One question: why couldn’t the webserver access the PV? In #39496 same user added more conditions.
Problem:
We can't see logs served from triggerer when task deferred.
This PR:
This PR essentially restores the behavior to what it was prior to #32561. So we undo the enhancement in #32561, the first attempted fix #39177, and the second attempted fix #39496.
This means that while a task is in running state, if you look at logs for prior failed attempts, you may see a "checked served logs and did not find any" message. This can be mildly confusing but it's more important to restore access to logs.