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.

Sign up for GitHub

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

Jump to bottom

Fix wc zombie goroutine issue in volume util #39477

Merged
merged 1 commit into from
Jan 10, 2017
Merged

Fix wc zombie goroutine issue in volume util #39477

merged 1 commit into from
Jan 10, 2017

Conversation

dashpole
Copy link
Contributor

@dashpole dashpole commented Jan 5, 2017

See Cadvisor #1558. This should solve problems for those using images that do not support "wc".
cc: @timstclair

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 5, 2017
@k8s-reviewable
Copy link

This change is Reviewable

@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-label-needed labels Jan 5, 2017
@timstclair
Copy link

Can we change the cAdvisor method to a function (it doesn't need the RealFSInfo object), and just call that from here instead of duplicating the logic?

@dashpole
Copy link
Contributor Author

dashpole commented Jan 6, 2017

Sure. My plan is to replace all of this code with cAdvisor function calls once core metrics are established.

@dashpole
Copy link
Contributor Author

dashpole commented Jan 6, 2017

But I can start here, I guess.

@vishh
Copy link
Contributor

vishh commented Jan 9, 2017

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 9, 2017
@k8s-github-robot k8s-github-robot added the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Jan 9, 2017
@dashpole
Copy link
Contributor Author

dashpole commented Jan 9, 2017 •
edited
Loading

@vish, this shouldnt need a release note.

@vishh vishh added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. labels Jan 9, 2017
@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 39486, 37288, 39477, 39455, 39542)

@k8s-github-robot k8s-github-robot merged commit c03ec46 into kubernetes:master Jan 10, 2017
@dashpole dashpole deleted the zombie_wc branch January 11, 2017 16:20
@euank euank mentioned this pull request Jan 16, 2017
Closed
@calebamiles
Copy link
Contributor

@saad-ali, I think this needs to be cherry picked onto 1.5. Can we make that happen?

@saad-ali saad-ali added this to the v1.5 milestone Jan 31, 2017
@saad-ali saad-ali added cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. cherrypick-candidate labels Jan 31, 2017
@saad-ali
Copy link
Member

@saad-ali, I think this needs to be cherry picked onto 1.5. Can we make that happen?

Yes. @dashpole feel free to cherry-pick this and associated PRs. Please assign cherry pick PRs to me.

@zakharovvi
Copy link

Hello!

Could you please cherry-pick this fix to 1.4 branch too?

We had 30k+ zombie 'wc -l' processes on some nodes last time.

@stuszynski stuszynski mentioned this pull request Mar 15, 2017
Closed
@dashpole dashpole mentioned this pull request Apr 11, 2017
Closed
@dashpole dashpole mentioned this pull request Apr 12, 2017
Merged
k8s-github-robot pushed a commit that referenced this pull request Apr 15, 2017
Merge pull request #44413 from dashpole/cherrypick_volume_fix
3b2b6c2 
Automatic merge from submit-queue

[release 1.5] Cherrypick: Fixed wc zombie goroutine issue

cherrypick of #39477.
I forgot about this until now...

cc @saad-ali
@k8s-cherrypick-bot
Copy link

Commit found in the "release-1.5" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@timstclair timstclair

@vishh vishh

@saad-ali saad-ali

Labels
cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.5
Development

Successfully merging this pull request may close these issues.
10 participants
@dashpole @k8s-reviewable @timstclair @vishh @k8s-github-robot @calebamiles @saad-ali @zakharovvi @k8s-cherrypick-bot @k8s-ci-robot