[ws-daemon] Prevent divide by zero error #12315
Conversation
github-actions
bot
added
team: webapp
|
👋 @Furisto hey there, as this already has an issue, we can 1) relate the PR to the issue and 2) relate the issue to the project (I just did those things). Thanks for the quick turnaround! |
There was a problem hiding this comment.
@Furisto can you share how to test? It looks like the code path will exit here now, instead of trying to do the division here. 👍
I ask because I'm not sure what scenario would have caused os.ReadFile(path) to return an error in the first place.
|
@kylos101 I am not sure how exactly this happened either, but returning an error is better than a runtime panic and the error message could help us find out why it is happening. |
|
started the job as gitpod-build-furisto-ws-daemon-panics-with-integer-12295.1 because the annotations in the pull request description changed |
|
started the job as gitpod-build-furisto-ws-daemon-panics-with-integer-12295.3 because the annotations in the pull request description changed |
utam0k
force-pushed
the
furisto/ws-daemon-panics-with-integer-12295
branch
from
2dff4a4 to
e15d433
Compare
|
started the job as gitpod-build-furisto-ws-daemon-panics-with-integer-12295.5 because the annotations in the pull request description changed |
|
@Furisto I put a commit into your branch for more accurate value handling |
Signed-off-by: utam0k <k0ma@utam0k.jp>
utam0k
force-pushed
the
furisto/ws-daemon-panics-with-integer-12295
branch
from
e15d433 to
d3d9ce6
Compare
|
started the job as gitpod-build-furisto-ws-daemon-panics-with-integer-12295.7 because the annotations in the pull request description changed |
roboquat
added
the
deployed: workspace
| return nil, err | ||
| } | ||
|
|
||
| // if no cpu limit has been specified, use the number of cores | ||
| var limit uint64 | ||
| if quota == math.MaxUint64 { | ||
| // TODO(toru): we have to check a parent cgroup instead of a host resources |
There was a problem hiding this comment.
@utam0k this logic is wrong. What we should do here is to set the lower limit of the class, and if that value is not set, use the cpuLimits value from ws-daemon.
@utam0k please revert this commit and implement what I am suggesting. This change breaks the CPU limiting when we cannot read cpu.Max. Please also include details about this change is being tested.
There was a problem hiding this comment.
@aledbf Thanks for your review. I have a question. this method is used for gp top, right? I think this method doesn't relate to the CPU limiting. Please let me know if I am wrong.
There was a problem hiding this comment.
This method is not involved in cpu limiting, it is only used for the endpoint that gp top/supervisor is calling to get resource information.
Description
Prevent divide by zero error
Related Issue(s)
Fixes #12295
Release Notes
Werft options: