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.

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

Handle DeletedFinalStateUnknown in NodeController #34694

Merged

Conversation

gmarek
Copy link
Contributor

@gmarek gmarek commented Oct 13, 2016

Fix #34692

Fix panic in NodeController caused by receiving DeletedFinalStateUnknown object from the cache.

cc @davidopp


This change is Reviewable

Copy link
Member

@wojtek-t wojtek-t left a comment

Choose a reason for hiding this comment

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

We should probably open an issue to go through the code and check we don't have the same problem in other places. @lavalamp - FYI

if !isNode {
deletedState, ok := obj.(cache.DeletedFinalStateUnknown)
if !ok {
glog.Warningf("Received unexpected object: %v", obj)
Copy link
Member

Choose a reason for hiding this comment

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

Errorf (if we don't get neither node nor deletedfinalstateunknown, then we really have problem)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

}
node, ok = deletedState.Obj.(*api.Node)
if !ok {
glog.Warningf("DeletedFinalStateUnknown contained non-Node object: %v", deletedState.Obj)
Copy link
Member

Choose a reason for hiding this comment

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

Errorf

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@gmarek gmarek force-pushed the deleted_final_state_unknown branch from 29599a8 to 8b7e9d3 Compare October 13, 2016 09:31
@wojtek-t
Copy link
Member

@gmarek - this PR probably requires release-note, as well as should probably be cherrrypicked

@gmarek gmarek added release-note Denotes a PR that will be considered when it comes time to generate release notes. cherrypick-candidate labels Oct 13, 2016
@gmarek gmarek added this to the v1.4 milestone Oct 13, 2016
@gmarek
Copy link
Contributor Author

gmarek commented Oct 13, 2016

Agreed

@k8s-github-robot k8s-github-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Oct 13, 2016
@wojtek-t
Copy link
Member

/lgtm

@k8s-github-robot
Copy link

@k8s-bot test this [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 1c65d1d into kubernetes:master Oct 13, 2016
@jessfraz jessfraz added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Oct 13, 2016
@jessfraz
Copy link
Contributor

@wojtek-t @gmarek can you open the cherrypick for this because there are some conflicts you might be better at solving

k8s-github-robot pushed a commit that referenced this pull request Oct 14, 2016
…-upstream-release-1.4

Automatic merge from submit-queue

Automated cherry pick of #34694

Cherry pick of #34694 on release-1.4.

#34694: Handle DeletedFinalStateUnknown in NodeController
@k8s-cherrypick-bot
Copy link

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

@davidopp
Copy link
Member

Oops, sorry I missed this when I did #34786. Thanks for catching it.

shyamjvs pushed a commit to shyamjvs/kubernetes that referenced this pull request Dec 1, 2016
…-of-#34694-upstream-release-1.4

Automatic merge from submit-queue

Automated cherry pick of kubernetes#34694

Cherry pick of kubernetes#34694 on release-1.4.

kubernetes#34694: Handle DeletedFinalStateUnknown in NodeController
@gmarek gmarek deleted the deleted_final_state_unknown branch February 21, 2017 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. 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. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants