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

Automated cherry pick of #45923 #46541

Merged
merged 1 commit into from
Jun 12, 2017
Merged

Automated cherry pick of #45923 #46541

merged 1 commit into from
Jun 12, 2017

Conversation

verult
Copy link
Contributor

@verult verult commented May 26, 2017

Cherry pick of #45923 on release-1.6.

#45923: Node status updater now deletes the node entry in attach

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

Hi @verult. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with @k8s-bot ok to test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

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 needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label May 26, 2017
@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. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 26, 2017
@k8s-github-robot
Copy link

The 'parent' PR of a cherry-pick PR must have one of the "release-note" or "release-note-action-required" labels, or this PR must follow the standard/parent release note labeling requirement. (release-note-experimental must be explicit for cherry-picks)

@verult
Copy link
Contributor Author

verult commented May 26, 2017

/release-note-none

@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 May 26, 2017
@enisoc
Copy link
Member

enisoc commented May 30, 2017

@verult The original PR says this fixes "unnecessary node status updates". Can you elaborate on why this needs to be cherrypicked? Are the unnecessary status updates causing harm?

@verult
Copy link
Contributor Author

verult commented May 30, 2017

@enisoc The unnecessary message is logged every 100ms, causing kube-controller-manager.log to get very large very quickly. The relevant issue is here: Issue #42438

@enisoc enisoc added 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. release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. release-note-none Denotes a PR that doesn't merit a release note. labels May 30, 2017
@enisoc enisoc added this to the v1.6 milestone May 30, 2017
@enisoc
Copy link
Member

enisoc commented May 30, 2017

@verult Thanks.

/lgtm

@k8s-bot ok to test

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 30, 2017
@verult
Copy link
Contributor Author

verult commented May 30, 2017

/assign @saad-ali

@verult
Copy link
Contributor Author

verult commented May 30, 2017 •
edited
Loading

@k8s-bot pull-kubernetes-verify test this

1 similar comment
@verult
Copy link
Contributor Author

verult commented Jun 8, 2017

@k8s-bot pull-kubernetes-verify test this

@jsafrane
Copy link
Member

jsafrane commented Jun 9, 2017

@verult, verify tests usually fails from a good reason, this time it's bazel check. And it includes hint what to do:

 I0608 13:29:05.626] Verifying hack/make-rules/../../hack/verify-bazel.sh
I0608 13:29:10.262] 23,25c23,25
I0608 13:29:10.262] <         "//vendor/github.com/golang/glog:go_default_library",
I0608 13:29:10.262] <         "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
I0608 13:29:10.263] <         "//vendor/k8s.io/apimachinery/pkg/util/strategicpatch:go_default_library",
I0608 13:29:10.263] ---
I0608 13:29:10.263] >         "//vendor:github.com/golang/glog",
I0608 13:29:10.263] >         "//vendor:k8s.io/apimachinery/pkg/api/errors",
I0608 13:29:10.263] >         "//vendor:k8s.io/apimachinery/pkg/util/strategicpatch",
W0608 13:29:10.363] DRY-RUN: wrote "pkg/controller/volume/attachdetach/statusupdater/BUILD"
W0608 13:29:10.608] 
W0608 13:29:10.608] 1 BUILD files not up-to-date.
I0608 13:29:10.708] 
I0608 13:29:10.709] Run ./hack/update-bazel.sh

So, run hack/update-bazel.sh, update the PR and that should be it.

@verult verult force-pushed the automated-cherry-pick-of-#45923-upstream-release-1.6 branch from fa88366 to e4ad246 Compare June 9, 2017 20:27
@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 9, 2017
@enisoc enisoc added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 9, 2017
enisoc
enisoc reviewed Jun 9, 2017
View reviewed changes
BUILD.bazel Outdated
@@ -44,6 +44,11 @@ filegroup(
"//examples:all-srcs",
"//federation:all-srcs",
"//hack:all-srcs",
"//kubernetes/cluster:all-srcs",
Copy link
Member

Choose a reason for hiding this comment

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

Hm the hack/update-bazel.sh script should not have added these lines. Is your git tree checked out into a directory structure that ends in */src/k8s.io/kubernetes? For now I think you can just revert this file and the PR should pass, but for the future you should figure out why the script did this in your environment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Turns out I had a kubernetes/ directory under KUBE_ROOT, possibly as build output. Removing that did the trick. Thanks!

@verult
Node status updater now deletes the node entry in attach updates when…
1f42790 
… node is missing in NodeInformer cache. Fixes kubernetes#42438.

- Added RemoveNodeFromAttachUpdates as part of node status updater operations.
@verult verult force-pushed the automated-cherry-pick-of-#45923-upstream-release-1.6 branch from e4ad246 to 1f42790 Compare June 10, 2017 02:56
@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 10, 2017
@jsafrane
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 Jun 12, 2017
@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enisoc, jsafrane, verult

Associated issue: 45923

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

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 3722eaa into kubernetes:release-1.6 Jun 12, 2017
@verult verult deleted the automated-cherry-pick-of-#45923-upstream-release-1.6 branch March 24, 2018 01:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@enisoc enisoc enisoc left review comments

Assignees

@childsb childsb

@jsafrane jsafrane

@enisoc enisoc

@saad-ali saad-ali

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. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.6
Development

Successfully merging this pull request may close these issues.
7 participants
@verult @k8s-ci-robot @k8s-github-robot @enisoc @jsafrane @childsb @saad-ali