-
Notifications
You must be signed in to change notification settings - Fork 38.7k
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
resource consumer - prevent consumeCPU/Mem processes from becoming zombies upon exit #17031
Conversation
Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist") If this message is too spammy, please complain to ixdy. |
1 similar comment
Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist") If this message is too spammy, please complain to ixdy. |
Labelling this PR as size/XS |
ok to test |
it looks like |
GCE e2e build/test failed for commit a556f69d7dd943031ee64813886fcd0a7e98febd. |
Yes, I did not see that. |
GCE e2e test build/test passed for commit ac9484b. |
@piosz - can you please take a look? |
LGTM, thanks for the fix. Did you have a chance to build a new Docker image with your changes and then test it? |
Continuous integration appears to have missed, closing and re-opening to trigger it |
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
GCE e2e test build/test passed for commit ac9484b. |
Automatic merge from submit-queue |
Auto commit by PR queue bot
@piosz Sorry for the late reply. I built the Docker image and pushed it into my local Docker registry, then pointed the HPA e2e tests to use that. I verified that in the running pods, no more consume-cpu zombies remain, so fix is effective. |
Thank you @xelalexv! |
While looking into issue #16425, I noticed that the consumeCPU and consumeMem processes remain as zombies once they exit, and pile up. This is because
Wait()
is not called on the correspondingexec.cmd
s. While I'm sure this is not the cause for issue #16425, I still thought this should be straightened out.