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

On AttachDetachController node status update, do not retry when node … #50806

Merged
merged 1 commit into from
Aug 23, 2017
Merged

On AttachDetachController node status update, do not retry when node … #50806

merged 1 commit into from
Aug 23, 2017

Conversation

verult
Copy link
Contributor

@verult verult commented Aug 16, 2017 •
edited by enisoc
Loading

…doesn't exist but keep the node entry in cache.

What this PR does / why we need it: An alternative fix for #42438 which also fixes #50721.

Instead of removing the node entry entirely from the node status update cache (which prevents the node from ever being updated even when it recovers), here the node status updater does nothing, so that there won't be an update retry until the node is re-added, where the cache entry is set to true.

Will cherry pick to prior versions after this is merged.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #50721

Release Note:

Fix for Pod stuck in ContainerCreating with error "Volume is not yet attached according to node".

/assign @jingxu97
/cc @saad-ali
/sig storage
/release-note

@verult
On AttachDetachController node status update, do not retry when node …
1234d2f 
…doesn't exist but keep the node entry in cache
@k8s-ci-robot k8s-ci-robot added sig/storage Categorizes an issue or PR as relevant to SIG Storage. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Aug 16, 2017
@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels Aug 16, 2017
@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Aug 16, 2017
@verult
Copy link
Contributor Author

verult commented Aug 17, 2017

/retest

@jingxu97
Copy link
Contributor

/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 22, 2017
@jingxu97
Copy link
Contributor

/approve

@verult
Copy link
Contributor Author

verult commented Aug 22, 2017

/retest pull-kubernetes-e2e-kops-aws

@verult
Copy link
Contributor Author

verult commented Aug 22, 2017

/assign @saad-ali

saad-ali
saad-ali approved these changes Aug 22, 2017
View reviewed changes
Copy link
Member

@saad-ali saad-ali left a comment

Choose a reason for hiding this comment

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

Removing code == best kind of fix!

/lgtm
/approve

Make sure to cherry pick as appropriate

@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jingxu97, saad-ali, verult

Associated issue: 42438

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 22, 2017
@saad-ali saad-ali added this to the v1.7 milestone Aug 22, 2017
@saad-ali
Copy link
Member

Modify the first comment to include a user friendly release note in the correct release note format.

@saad-ali saad-ali mentioned this pull request Aug 22, 2017
Closed
@verult
Copy link
Contributor Author

verult commented Aug 22, 2017

/test pull-kubernetes-e2e-kops-aws

@verult
Copy link
Contributor Author

verult commented Aug 22, 2017

/release-note

@k8s-ci-robot k8s-ci-robot 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 Aug 22, 2017
@k8s-github-robot
Copy link

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

@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Aug 23, 2017 •
edited
Loading

@verult: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-e2e-gce-etcd3 1234d2f link /test pull-kubernetes-e2e-gce-etcd3

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 50806, 48789, 49922, 49935, 50438)

@k8s-github-robot k8s-github-robot merged commit 7063227 into kubernetes:master Aug 23, 2017
@wojtek-t wojtek-t mentioned this pull request Aug 23, 2017
Merged
@wojtek-t
Copy link
Member

Cherrypick approved and created in #51177

@wojtek-t wojtek-t added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Aug 23, 2017
@verult verult mentioned this pull request Aug 30, 2017
Merged
wojtek-t added a commit that referenced this pull request Aug 30, 2017
@wojtek-t
Merge pull request #51177 from wojtek-t/automated-cherry-pick-of-#508…
b85878b 
…06-upstream-release-1.7

Automated cherry pick of #50806 upstream release 1.7
@k8s-cherrypick-bot
Copy link

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

k8s-github-robot pushed a commit that referenced this pull request Sep 1, 2017
Merge pull request #51572 from verult/automated-cherry-pick-of-#50806…
a250e58 
…-upstream-release-1.6

Automatic merge from submit-queue

Automated cherry pick of #50806

Cherry pick of #50806 on release-1.6.

#50806: On AttachDetachController node status update, do not retry
@verult verult deleted the VolumeNotYetAttached branch March 24, 2018 01:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@saad-ali saad-ali saad-ali approved these changes

Assignees

@saad-ali saad-ali

@jingxu97 jingxu97

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. 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/storage Categorizes an issue or PR as relevant to SIG Storage. 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.

Pod stuck in ContainerCreating with log message "Volume is not yet attached according to node"
7 participants
@verult @jingxu97 @k8s-github-robot @saad-ali @k8s-ci-robot @wojtek-t @k8s-cherrypick-bot