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 (and dependencies) godeps version #25914

Merged
merged 1 commit into from
May 26, 2016
Merged

Bump cAdvisor (and dependencies) godeps version #25914

merged 1 commit into from
May 26, 2016

Conversation

timstclair
Copy link

Includes important bug fixes for v1.3, as well as support for rktnetes.

Bump cAdvisor to v0.23.2 - See [changelog](https://github.com/google/cadvisor/blob/master/CHANGELOG.md) for details

@timstclair timstclair added area/cadvisor release-note Denotes a PR that will be considered when it comes time to generate release notes. labels May 19, 2016
@timstclair timstclair added this to the v1.3 milestone May 19, 2016
@timstclair
Copy link
Author

/cc @yifan-gu @sjpotter @kubernetes/sig-node

@k8s-github-robot k8s-github-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label May 19, 2016
@timstclair timstclair assigned dchen1107 and unassigned lavalamp May 19, 2016
@timstclair
Copy link
Author

Oops, looks like it wasn't supposed to remove ginko - I'll fix that.

@timstclair
Copy link
Author

Added back gingko & codecgen

vishh
vishh reviewed May 19, 2016
View reviewed changes
Godeps/Godeps.json
@@ -1,7 +1,7 @@
{
"ImportPath": "k8s.io/kubernetes",
"GoVersion": "go1.6",
"GodepVersion": "v63",
"GodepVersion": "v69",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why update godep version?

Copy link
Author

Choose a reason for hiding this comment

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

It looks like we're not pinning versions anymore, so I figured I should use the latest.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh. Thats right - https://github.com/kubernetes/kubernetes/blob/master/hack/verify-godeps.sh#L80

@vishh
Copy link
Contributor

vishh commented May 19, 2016

Just one nit. Otherwise LGTM.

@dchen1107
Copy link
Member

LGTM. Please feel free to apply lgtm once the tests passed.

@dchen1107 dchen1107 added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 19, 2016
@timstclair
Copy link
Author

Hmm, that failure looks bad. Investigating.

@timstclair
Copy link
Author

I got bit by go's nil-interface handling (stretchr/testify#311). The partial failure handling in cAdvisor needs to be fixed.

@timstclair
Copy link
Author

Updated to include google/cadvisor#1294

@k8s-github-robot k8s-github-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 20, 2016
@timstclair timstclair mentioned this pull request May 20, 2016
Merged
@timstclair
Copy link
Author

For some reason rerunning hack/update-godep-licenses.sh generated different output, but it seems like maybe it's stable now...

@k8s-github-robot k8s-github-robot removed the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label May 20, 2016
@timstclair
Copy link
Author

Rebased.

@timstclair timstclair added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 20, 2016
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 20, 2016
@timstclair
Copy link
Author

Reverted collateral change to vendor/github.com/appc/spec/schema/types/isolator_resources.go

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 20, 2016
@timstclair timstclair added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 20, 2016
@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 22, 2016
k8s-github-robot pushed a commit that referenced this pull request May 22, 2016
Merge pull request #25933 from timstclair/subcontainers
f1e528e 
Automatic merge from submit-queue

Handle cAdvisor partial failures

Kubernetes side of google/cadvisor#1286

Partially fixes #25131

(Depends on cAdvisor Godeps bump #25914)

/cc @kubernetes/sig-node
@timstclair
Copy link
Author

Rebased

@timstclair timstclair added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels May 23, 2016
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 23, 2016
@timstclair
Copy link
Author

@k8s-bot node test this: #25993

@timstclair
Copy link
Author

@k8s-bot test node e2e #25993

@k8s-github-robot
Copy link

@timstclair
You must link to the test flake issue which caused you to request this manual re-test.
Re-test requests should be in the form of: k8s-bot test this issue: #<number>
Here is the list of open test flakes.

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 23, 2016
@timstclair
Copy link
Author

@k8s-bot node e2e test this: #25993

@timstclair timstclair added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 24, 2016
@timstclair
Copy link
Author

Reapplying LGTM, removed because of merge bot flake.

@liggitt
Copy link
Member

liggitt commented May 26, 2016

were all the golang.org/x/sys/unix files supposed to be removed by this?

This was referenced May 26, 2016
Closed
Merged
@timstclair
Copy link
Author

@liggitt They weren't. Apparently we were vendoring 2 copies of them, 1 through the github.com/fsouza/go-dockerclient which was intentionally removed (k8s & cadvisor moved to the official docker client). The vendor/golang.org/x/sys/unix/* files should still be present though.

@alex-mohr
Copy link
Contributor

Because submit queue is blocked, I'm going to kick off re-tests of the top 5 PRs in the queue, then merge them if they pass.
@k8s-bot test this please: issue #IGNORE

@k8s-bot
Copy link

k8s-bot commented May 26, 2016

GCE e2e build/test passed for commit 237f90d.

@alex-mohr alex-mohr merged commit 87f3cf1 into kubernetes:master May 26, 2016
@jsravn jsravn mentioned this pull request Aug 8, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@dchen1107 dchen1107

Labels
area/cadvisor 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/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
v1.3
Development

Successfully merging this pull request may close these issues.