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:Image garbage collection is always disabled in kubelet in 1.11 #66051

Merged
merged 1 commit into from
Jul 11, 2018
Merged

Fix:Image garbage collection is always disabled in kubelet in 1.11 #66051

merged 1 commit into from
Jul 11, 2018

Conversation

jiaxuanzhou
Copy link
Contributor

@jiaxuanzhou jiaxuanzhou commented Jul 11, 2018 •
edited by yujuhong
Loading

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #66046

Special notes for your reviewer:
@yujuhong @derekwaynecarr
Release note:

Fix the bug where image garbage collection is disabled by mistake.

@k8s-ci-robot k8s-ci-robot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 11, 2018
@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
  • If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: [email protected]

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Jul 11, 2018
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jul 11, 2018
yujuhong
yujuhong reviewed Jul 11, 2018
View reviewed changes
pkg/kubelet/kubelet.go Outdated
// when the high threshold is set to 100, stub the image GC manager
if kl.kubeletConfiguration.ImageGCHighThresholdPercent == 100 {
glog.V(2).Infof("ImageGCHighThresholdPercent is set 100, Disable image GC")
go func() { stopChan <- struct{}{} }()
go func() { close(stopChan) }()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just returning here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modified

@yujuhong
Copy link
Contributor

Please add a release note.

@yujuhong yujuhong added this to the v1.11 milestone Jul 11, 2018
@yujuhong
Copy link
Contributor

/cc @foxish, the 1.11 patch release manager.
This should be included in 1.11.1

@yujuhong yujuhong added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Jul 11, 2018
@yujuhong
Copy link
Contributor

/ok-to-test

@dashpole, do we have any image gc tests?

@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 11, 2018
@yujuhong yujuhong added sig/node Categorizes an issue or PR as relevant to SIG Node. kind/bug Categorizes issue or PR as related to a bug. status/approved-for-milestone labels Jul 11, 2018
@yujuhong yujuhong self-assigned this Jul 11, 2018
@k8s-github-robot
Copy link

[MILESTONENOTIFIER] Milestone Pull Request: Up-to-date for process

@jiaxuanzhou @yujuhong

Pull Request Labels
  • sig/node: Pull Request will be escalated to these SIGs if needed.
  • priority/critical-urgent: Never automatically move pull request out of a release milestone; continually escalate to contributor and SIG through all available channels.
  • kind/bug: Fixes a bug discovered during the current release.
Help

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jul 11, 2018
@yujuhong
Copy link
Contributor

/lgtm
/retest

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 11, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jiaxuanzhou, yujuhong

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 11, 2018
@foxish
Copy link
Contributor

foxish commented Jul 11, 2018 •
edited
Loading

ack, let's cherrypick this into 1.11.1

@jiaxuanzhou
Copy link
Contributor Author

/test pull-kubernetes-e2e-gce

@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 66051, 66049). If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit 6c6449c into kubernetes:master Jul 11, 2018
@jiaxuanzhou jiaxuanzhou deleted the imageGC branch July 11, 2018 09:11
@yujuhong
Copy link
Contributor

@jianglingxia cold you please create PRs to cherrypick the fix to 1.11? Thanks.

@yujuhong yujuhong mentioned this pull request Jul 11, 2018
Merged
@yujuhong yujuhong added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Jul 11, 2018
k8s-github-robot pushed a commit that referenced this pull request Jul 12, 2018
Merge pull request #66079 from yujuhong/automated-cherry-pick-of-#660…
e3db721 
…51-upstream-release-1.11

Automatic merge from submit-queue.

Automated cherry pick of #66051: fix bug for garbage collection

Cherry pick of #66051 on release-1.11.

#66051: fix bug for garbage collection

```release-note
Fix the bug where image garbage collection is disabled by mistake.
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yujuhong yujuhong yujuhong left review comments

@dchen1107 dchen1107 Awaiting requested review from dchen1107

@mtaufen mtaufen Awaiting requested review from mtaufen

Assignees

@yujuhong yujuhong

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/node Categorizes an issue or PR as relevant to SIG Node. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
v1.11
Development

Successfully merging this pull request may close these issues.

Image garbage collection is always disabled in kubelet in 1.11
5 participants
@jiaxuanzhou @k8s-ci-robot @yujuhong @k8s-github-robot @foxish