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

Bump cadvisor dependencies to latest head. #29492

Merged
merged 2 commits into from
Jul 28, 2016
Merged

Bump cadvisor dependencies to latest head. #29492

merged 2 commits into from
Jul 28, 2016

Conversation

Random-Liu
Copy link
Member

Fixes #28619
Fixes #28997

This is another try of #29153.

To update cadvisor godeps, we did:

  • Bump up docker version to v1.11.2 for both cadvisor [https://github.com/Update docker to v1.11.2 google/cadvisor#1388] and k8s.
  • Bump up cadvisor go-systemd version to be the same with k8s [https://github.com/Bump up go-systemd to 4484981625c1a6a2ecb40a390fcb6a9bcfee76e3. google/cadvisor#1390]. Or else, a package github.com/coreos/pkg/dlopen will be removed by Godep, because it is used by new go-systemd in k8s, but not used by old go-systemd in cadvisor.
  • Bump up runc version to be the same with docker v1.11.2 just in case.
  • Add github.com/Azure/go-ansiterm dependency which is needed by docker v1.11.2.
  • Change pkg/util/term/, because SetWinsSize is removed from windows platform in docker v1.11.2. [The first commit]

@vishh
/cc @ncdc for the pkg/util/term change.

@Random-Liu Random-Liu added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. cherrypick-candidate labels Jul 23, 2016
@Random-Liu Random-Liu added this to the v1.3 milestone Jul 23, 2016
@k8s-github-robot k8s-github-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 23, 2016
@ncdc
Copy link
Member

ncdc commented Jul 23, 2016

SetWinsize change LGTM

@Random-Liu
Copy link
Member Author

Kindly ping @vishh

@Random-Liu
Copy link
Member Author

@ncdc Thanks for reviewing! :)

@dchen1107 dchen1107 mentioned this pull request Jul 25, 2016
Closed
@dchen1107 dchen1107 removed this from the v1.3 milestone Jul 25, 2016
@Random-Liu Random-Liu mentioned this pull request Jul 25, 2016
Merged
k8s-github-robot pushed a commit that referenced this pull request Jul 26, 2016
Merge pull request #29581 from Random-Liu/panic-if-cadvisor-not-started
ed3a29b 
Automatic merge from submit-queue

Kubelet: Fail kubelet if cadvisor is not started.

Fixes #28997.

We started cadvisor in `sync.Do()`, which only run once no matter cadvisor successfully starts or not.

Once it fails, kubelet will be stuck in a bad state. Kubelet could never start sync loop because there is an internal error, but kubelet would never retry starting cadvisor again.

This PR just fails kubelet when cadvisor start fails, and then relies on the babysitter to restart kubelet.
In the future, we may want to add backoff logic in the babysitter to protect the system.

On the other hand, #29492 will fix cadvisor side to prevent cadvisor failing because of these kind of transient error.

Mark P1 to match the original issue.

@dchen1107 @vishh
@gmarek
Copy link
Contributor

gmarek commented Jul 26, 2016

@vish - PTAL, the problem this PR is solving causes a number of failures of tests and blockages of the merge queue.

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 27, 2016
@vishh
Copy link
Contributor

vishh commented Jul 27, 2016

@Random-Liu This PR LGTM. Given that you are updating ginkgo, just keep a tab on post-submit e2es once this PR is merged.

@vishh vishh added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 27, 2016
@vishh
Copy link
Contributor

vishh commented Jul 27, 2016

@gmarek Apologies for the delay! Thanks for pinging me on this PR!

@Random-Liu
Copy link
Member Author

Random-Liu commented Jul 27, 2016 •
edited
Loading

@vishh Sure. And I'll rebase and reapply LGTM.

@Random-Liu Random-Liu force-pushed the bumpup-cadvisor-version branch from 99c4419 to 54f04ee Compare July 28, 2016 00:26
@k8s-github-robot k8s-github-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 28, 2016
@gmarek
Copy link
Contributor

gmarek commented Jul 28, 2016

@Random-Liu: FAILED hack/make-rules/../../hack/verify-godeps.sh 323s

@Random-Liu
Copy link
Member Author

@gmarek Yeah, dependencies order is massed up during rebase. Will fix and update the PR soon. :)

@Random-Liu Random-Liu force-pushed the bumpup-cadvisor-version branch from 54f04ee to cb1b3c8 Compare July 28, 2016 01:29
@Random-Liu Random-Liu added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 28, 2016
@Random-Liu
Copy link
Member Author

Random-Liu commented Jul 28, 2016 •
edited
Loading

Just did a rebase. Apply LGTM based on #29492 (comment).

@Random-Liu
Copy link
Member Author

@k8s-bot test this issue #29451.

@k8s-bot
Copy link

k8s-bot commented Jul 28, 2016

GCE e2e build/test passed for commit cb1b3c8.

@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. retest-not-required-docs-only and removed retest-not-required-docs-only labels Jul 28, 2016
@Random-Liu Random-Liu removed the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Jul 28, 2016
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Jul 28, 2016

GCE e2e build/test passed for commit cb1b3c8.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 685fc92 into kubernetes:master Jul 28, 2016
@Random-Liu Random-Liu deleted the bumpup-cadvisor-version branch July 28, 2016 08:19
@dubstack
Copy link

dubstack commented Jul 28, 2016 •
edited
Loading

@Random-Liu This PR reverted some updates to Libcontainer that I made.
It doesn't point to libcontainer HEAD.
Can we fix that?

@dubstack dubstack mentioned this pull request Jul 28, 2016
Closed
@Random-Liu
Copy link
Member Author

@dubstack If so, you may need to send a PR to bump up runc version again. :(
Sorry.

@dubstack
Copy link

Oh No worries. I will send out a PR. I just wanted to confirm if runc was reverted for a specific reason and if bumping runc to the latest head would cause problems to this PR.

@dubstack dubstack mentioned this pull request Aug 2, 2016
Merged
k8s-github-robot pushed a commit that referenced this pull request Aug 4, 2016
Merge pull request #29928 from dubstack/bump-libcontainer
d10e47b 
Automatic merge from submit-queue

Bump Libcontainer to latest head

@Random-Liu or @yujuhong Can any one of you please do a quick review.

I updated libcontainer in a previous PR but  #29492 reverted those changes. This is needed for #27204. 

Signed-off-by: Buddha Prakash <buddhap@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@vishh vishh

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@Random-Liu @ncdc @gmarek @vishh @k8s-bot @k8s-github-robot @dubstack @googlebot @dchen1107