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 to fix interface stats bugs & improve performance #17883

Merged
merged 2 commits into from
Dec 22, 2015
Merged

Bump cadvisor to fix interface stats bugs & improve performance #17883

merged 2 commits into from
Dec 22, 2015

Conversation

jimmidyson
Copy link
Member

Includes necessary godep upgrades for docker & systemd packages as well as
migrating from docker/libcontainer to opencontainers/runc/libcontainer.

Fixes #17725

/cc @dchen1107 @vishh

@k8s-github-robot
Copy link

Labelling this PR as size/XS

@k8s-github-robot k8s-github-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Nov 27, 2015
@k8s-bot
Copy link

k8s-bot commented Nov 27, 2015

GCE e2e build/test failed for commit bf1f1780e94c05e35ededea7c294bd50612f90dc.

@jimmidyson
Copy link
Member Author

Looks like a failure to create the cluster for e2e - can someone rekick please?

@jimmidyson
Copy link
Member Author

@quinton-hoole Could you retest this please now that e2e cluster is back up again?

@smarterclayton
Copy link
Contributor

ok to test

@k8s-bot
Copy link

k8s-bot commented Nov 29, 2015

GCE e2e build/test failed for commit bf1f1780e94c05e35ededea7c294bd50612f90dc.

@k8s-bot
Copy link

k8s-bot commented Nov 30, 2015

GCE e2e build/test failed for commit 01acbddc971d13cb20ca1bf368fb6179511833a4.

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 4, 2015
@jimmidyson jimmidyson force-pushed the cadvisor-bump-0.19.4 branch from 01acbdd to ac15224 Compare December 4, 2015 22:47
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 4, 2015
@k8s-bot
Copy link

k8s-bot commented Dec 4, 2015

GCE e2e test build/test passed for commit ac15224b18c301afc22f4aa45a3c09369dfc3f0d.

@jimmidyson
Copy link
Member Author

Ready for review if you have time, @vishh @dchen1107

@ghost
Copy link

ghost commented Dec 8, 2015

Reassigned to @vishh

@ghost ghost assigned vishh and unassigned ghost Dec 8, 2015
@jimmidyson jimmidyson force-pushed the cadvisor-bump-0.19.4 branch from ac15224 to 1a90819 Compare December 9, 2015 20:02
@jimmidyson
Copy link
Member Author

@piosz Including your latest cadvisor changes.

@k8s-bot
Copy link

k8s-bot commented Dec 9, 2015

GCE e2e test build/test passed for commit 1a908198c68e65cc5052b80bb29a6846cd847b7f.

@jimmidyson
Copy link
Member Author

Please can someone review this pr so we can merge it? I don't know why this is taking so long with no feedback...

@piosz
Copy link
Member

piosz commented Dec 10, 2015

cc @jszczepkowski

@jszczepkowski
Copy link
Contributor

Can we bump cadvisor to a version where google/cadvisor#1012 is merged? We need it for custom metrics.

@vishh vishh closed this Dec 10, 2015
@vishh vishh reopened this Dec 10, 2015
@vishh
Copy link
Contributor

vishh commented Dec 10, 2015

Wonder why the tests are failing.

@vishh vishh added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 10, 2015
@jimmidyson
Copy link
Member Author

Travis failure is a problem with godep rebuild timing out.

Jenkins unit/integration could be a problem with runc migration. Tests pass locally for me with ./hack/test-cmd.sh so not sure what's different though? Do you know what distro the integration tests are running on so I can test like for like?

@k8s-bot
Copy link

k8s-bot commented Dec 21, 2015

GCE e2e test build/test passed for commit 4a933424e4e5f95f417078995b6a4e08a3e485d8.

@k8s-bot
Copy link

k8s-bot commented Dec 21, 2015

GCE e2e test build/test passed for commit a16b493a2485f097c3de27729db47b41d499aab9.

@k8s-bot
Copy link

k8s-bot commented Dec 21, 2015

GCE e2e test build/test passed for commit 3fee78de86fbe7d313a47f8f02890257f72bab02.

@jimmidyson
Bump cadvisor to fix interface stats bugs & improve performance
041ab17 
Includes necessary godep upgrades for docker & systemd packages as well as
migrating from docker/libcontainer to opencontainers/runc/libcontainer.
@jimmidyson
Preload docker dep to work around Docker package refactoring & godep …
62eb82d 
…incompatibility

Fixes kubernetes#18833
@jimmidyson
Copy link
Member Author

Phew fixing godep issues with Docker refactoring their packages was fun...

@vishh @dchen1107 @timstclair Would like to get this merged once green if possible?

@k8s-bot
Copy link

k8s-bot commented Dec 21, 2015

GCE e2e test build/test passed for commit 62eb82d.

@jimmidyson
Copy link
Member Author

Please can someone retest this? Looks like a Jenkins flake:

FAIL k8s.io/kubernetes/pkg/storage/etcd

@vishh vishh closed this Dec 21, 2015
@vishh vishh reopened this Dec 21, 2015
@vishh
Copy link
Contributor

vishh commented Dec 21, 2015

@k8s-bot: unit test this

@yujuhong
Copy link
Contributor

@k8s-bot unit test this

@jimmidyson
Copy link
Member Author

Different flake...

@vishh
Copy link
Contributor

vishh commented Dec 21, 2015

@k8s-bot: unit test this

@jimmidyson
Copy link
Member Author

It's not retesting...

@jimmidyson
Copy link
Member Author

I see integration tests failing for most open PRs so unrelated to this PR. e2e is good so how do you want to proceed? I really want to get this in.

@jimmidyson
Copy link
Member Author

Tests run fine locally... All help appreciated. Off on vacation from tomorrow so want to squeeze this in today if possible.

@yujuhong
Copy link
Contributor

@k8s-bot unit test this

@k8s-github-robot
Copy link

The author of this PR is not in the whitelist for merge, can one of the admins add the 'ok-to-merge' label?

@yujuhong yujuhong added ok-to-merge lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed needs-ok-to-merge labels Dec 22, 2015
@jimmidyson
Copy link
Member Author

Thanks @yujuhong!

brendandburns added a commit that referenced this pull request Dec 22, 2015
@brendandburns
Merge pull request #17883 from jimmidyson/cadvisor-bump-0.19.4
ac5c0aa 
Bump cadvisor to fix interface stats bugs & improve performance
@brendandburns brendandburns merged commit ac5c0aa into kubernetes:master Dec 22, 2015
@yujuhong yujuhong mentioned this pull request Jan 6, 2016
Closed
@luxas
Copy link
Member

luxas commented Jan 11, 2016

Thanks @jimmidyson! This also fixed #17060.

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. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.