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

Enable finalizers independent of GC enablement #51469

Merged
merged 1 commit into from
Aug 29, 2017
Merged

Enable finalizers independent of GC enablement #51469

merged 1 commit into from
Aug 29, 2017

Conversation

ironcladlou
Copy link
Contributor

Decouple finalizer processing from garbage collection configuration.
Finalizers should be effective even when garbage collection is disabled
for a given store.

Fixes #50528.

Backport of #51148.

Finalizers are now honored on custom resources, and on other resources even when garbage collection is disabled via the apiserver flag `--enable-garbage-collector=false`

/cc @kubernetes/sig-api-machinery-bugs
/cc @caesarxuchao @liggitt @sttts @pmorie

@ironcladlou
Enable finalizers independent of GC enablement
6a64a13 
Decouple finalizer processing from garbage collection configuration.
Finalizers should be effective even when garbage collection is disabled
for a given store.

Fixes kubernetes#50528.

Backport of kubernetes#51148.
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. kind/bug Categorizes issue or PR as related to a bug. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Aug 28, 2017
@ironcladlou ironcladlou mentioned this pull request Aug 28, 2017
Merged
@k8s-github-robot k8s-github-robot added do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Aug 28, 2017
@wojtek-t wojtek-t assigned liggitt and caesarxuchao and unassigned dims and mikedanese Aug 28, 2017
@wojtek-t
Copy link
Member

@caesarxuchao @liggitt - can one of you lgtm?

@wojtek-t wojtek-t added this to the v1.7 milestone Aug 28, 2017
@wojtek-t wojtek-t added cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. and removed do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. labels Aug 28, 2017
@liggitt
Copy link
Member

liggitt commented Aug 28, 2017

backport of that commit LGTM. @caesarxuchao @ironcladlou were there any prior changes in 1.8 related to custom resource GC that would have intersected with this?

@ironcladlou
Copy link
Contributor Author

@liggitt

backport of that commit LGTM. @caesarxuchao @ironcladlou were there any prior changes in 1.8 related to custom resource GC that would have intersected with this?

The other significant change to the store related to GC for 1.8 came with 312fb1e, which I don't think is required for this fix.

@caesarxuchao
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 28, 2017
@caesarxuchao
Copy link
Member

/retest

1 similar comment
@wojtek-t
Copy link
Member

/retest

@wojtek-t
Copy link
Member

/approve no-issue

@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: caesarxuchao, ironcladlou, wojtek-t

Associated issue: 50528

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

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-github-robot k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 29, 2017
@wojtek-t
Copy link
Member

/retest

1 similar comment
@wojtek-t
Copy link
Member

/retest

@k8s-github-robot
Copy link

/test all [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 209ba5d into kubernetes:release-1.7 Aug 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@caesarxuchao caesarxuchao Awaiting requested review from caesarxuchao

@liggitt liggitt Awaiting requested review from liggitt

@pmorie pmorie Awaiting requested review from pmorie

@sttts sttts Awaiting requested review from sttts

Assignees

@liggitt liggitt

@caesarxuchao caesarxuchao

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. 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. 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. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.7
Development

Successfully merging this pull request may close these issues.
8 participants
@ironcladlou @wojtek-t @liggitt @caesarxuchao @k8s-github-robot @dims @mikedanese @k8s-ci-robot