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. #29389

Closed
wants to merge 2 commits into from
Closed

Bump cadvisor dependencies to latest head. #29389

wants to merge 2 commits into from

Conversation

Random-Liu
Copy link
Member

@Random-Liu Random-Liu commented Jul 21, 2016 •
edited by k8s-oncall
Loading

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.

This change is Reviewable

@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 21, 2016
@Random-Liu Random-Liu added this to the v1.3 milestone Jul 21, 2016
@ncdc
Copy link
Member

ncdc commented Jul 21, 2016

@Random-Liu we need to find an alternative for Windows terminal resizing

@Random-Liu Random-Liu force-pushed the bumpup-cadvisor-version branch from 3a7eafa to 914509a Compare July 21, 2016 18:38
@Random-Liu
Copy link
Member Author

Random-Liu commented Jul 21, 2016 •
edited
Loading

@ncdc In fact, according to the original PR removing SetWinSize in windows, that function never worked.

HCS interface is mentioned in that PR, but I'm not familiar with that.

@ncdc
Copy link
Member

ncdc commented Jul 21, 2016

@Random-Liu ok let me do a bit more digging.

@k8s-github-robot k8s-github-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 21, 2016
@Random-Liu
Copy link
Member Author

@ncdc Sure, thanks a lot! :)

@ncdc
Copy link
Member

ncdc commented Jul 21, 2016

@Random-Liu ok I can see why it's not needed. It was used indirectly by the docker and rkt container runtimes to set the size of the tty of the container's process (

kubernetes/pkg/kubelet/dockertools/exec.go

Line 66 in 3b21a99

term.SetSize(p.Fd(), size)
and

kubernetes/pkg/kubelet/rkt/rkt.go

Line 2040 in 3b21a99

term.SetSize(p.Fd(), size)
), but because these are both Linux-based and not Windows, it's safe for now to do what you did.

@Random-Liu
Copy link
Member Author

Random-Liu commented Jul 21, 2016 •
edited
Loading

The e2e error is caused by missing ginkgo option flakeAttempts.

The option is added recently onsi/ginkgo#261, but the ginkgo verison in our Godeps is much older - April 9th, onsi/ginkgo@2c2e9bb

@lavalamp

@Random-Liu Random-Liu force-pushed the bumpup-cadvisor-version branch from 914509a to 5ea4608 Compare July 21, 2016 20:12
@Random-Liu
Copy link
Member Author

Updated ginkgo to 7d3240191d7b2311b60090fcbc96e56bffc45ec0, which should have solved the problem #29389 (comment).

@k8s-bot
Copy link

k8s-bot commented Jul 21, 2016

GCE e2e build/test passed for commit 5ea4608.

@lavalamp
Copy link
Member

Thank you for fixing the ginkgo entry, I'm not entirely sure what I did wrong.

@k8s-github-robot
Copy link

@Random-Liu PR needs rebase

@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 23, 2016
@Random-Liu
Copy link
Member Author

Close this one, this one created a upstream branch by mistake.

@Random-Liu Random-Liu closed this Jul 23, 2016
@Random-Liu Random-Liu deleted the bumpup-cadvisor-version branch July 23, 2016 02:53
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
needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. 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
v1.3
Development

Successfully merging this pull request may close these issues.
8 participants
@Random-Liu @ncdc @k8s-bot @lavalamp @k8s-github-robot @googlebot @vishh @pwittrock