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

Cadvisor root path configuration #35136

Merged
merged 3 commits into from
Oct 27, 2016
Merged

Cadvisor root path configuration #35136

merged 3 commits into from
Oct 27, 2016

Conversation

dashpole
Copy link
Contributor

@dashpole dashpole commented Oct 19, 2016 •
edited
Loading

This solves #33444 and reverts PR #33520. This takes the root directory and passes it to cadvisor, which sets up the "nodefs" based on the provided path, rather than using "/" as before.

This PR is pending on changes in cadvisor, and will not pass tests until those changes are merged, and cadvisor godeps are updated.

kubelet summary rootfs now refers to the filesystem that contains the Kubelet RootDirectory (var/lib/kubelet) instead of cadvisor's rootfs ( / ), since they may be different filesystems.

This change is Reviewable

@dashpole dashpole mentioned this pull request Oct 19, 2016
Merged
@k8s-ci-robot
Copy link
Contributor

Jenkins GCE etcd3 e2e failed for commit f2ca5fe2b1af02600c7216db146132fa5f95c92c. Full PR test history.

The magic incantation to run this job again is @k8s-bot gce etcd3 e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCI GCE e2e failed for commit f2ca5fe2b1af02600c7216db146132fa5f95c92c. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins Kubemark GCE e2e failed for commit f2ca5fe2b1af02600c7216db146132fa5f95c92c. Full PR test history.

The magic incantation to run this job again is @k8s-bot kubemark e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCE e2e failed for commit f2ca5fe2b1af02600c7216db146132fa5f95c92c. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins GKE smoke e2e failed for commit f2ca5fe2b1af02600c7216db146132fa5f95c92c. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-github-robot k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note-label-needed labels Oct 19, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins unit/integration failed for commit d2d52a2f8ae13c48cfaa92f260355c2924fa27cf. Full PR test history.

The magic incantation to run this job again is @k8s-bot unit test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCI GKE smoke e2e failed for commit d2d52a2f8ae13c48cfaa92f260355c2924fa27cf. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@dashpole dashpole force-pushed the cadvisor_root_path_configuration branch from d2d52a2 to 1fed915 Compare October 22, 2016 00:30
@k8s-github-robot k8s-github-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 22, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins GCE Node e2e failed for commit 33a6dfa. Full PR test history.

The magic incantation to run this job again is @k8s-bot node e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 22, 2016
@dashpole dashpole force-pushed the cadvisor_root_path_configuration branch 3 times, most recently from c5e9ee9 to d313658 Compare October 22, 2016 21:33
@k8s-ci-robot
Copy link
Contributor

Jenkins verification failed for commit d313658778608b877cd571b151dd5206d169a7bd. Full PR test history.

The magic incantation to run this job again is @k8s-bot verify test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@dashpole dashpole force-pushed the cadvisor_root_path_configuration branch from d313658 to a417ce3 Compare October 22, 2016 23:07
@dashpole
Copy link
Contributor Author

@vishh This should be all good to go.

@dashpole
Copy link
Contributor Author

Note: I tested this on memory, inode, and disk node e2e eviction tests. These tests have been run on ubuntu, coreos, and gci images.

@vishh
Copy link
Contributor

vishh commented Oct 24, 2016

This LGTM. Thanks a tonne for this fix @dashpole!

I generally prefer updating cAdvisor deps to point to a tag rather than an arbitrary commit. If @timstclair has no concerns with creating an additional tag on cAdvisor for this bump, then I'd recommend doing that.

@timstclair
Copy link

I generally prefer updating cAdvisor deps to point to a tag rather than an arbitrary commit. If @timstclair has no concerns with creating an additional tag on cAdvisor for this bump, then I'd recommend doing that.

I don't think it makes sense to create a tag for each arbitrary sync point in the mid-release development cycle. I think it's unclear what the tags mean in that case. I'd prefer to stick to the model we used in 1.4 (don't create tags for each update, only an alpha tag when we cut the k8s release branch, and a full cAdvisor release corresponding to the k8s release).

@dashpole dashpole force-pushed the cadvisor_root_path_configuration branch from a417ce3 to eb19713 Compare October 27, 2016 15:14
@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 Oct 27, 2016
@mtaufen mtaufen added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. 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 Oct 27, 2016
@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 Oct 27, 2016
@mtaufen mtaufen added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 27, 2016
@k8s-github-robot
Copy link

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

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 793b000 into kubernetes:master Oct 27, 2016
@vishh vishh added this to the v1.4 milestone Oct 27, 2016
@jessfraz jessfraz mentioned this pull request Oct 27, 2016
Closed
@jessfraz jessfraz added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Oct 28, 2016
This was referenced Oct 28, 2016
Merged
Closed
@jessfraz jessfraz mentioned this pull request Oct 28, 2016
Merged
jessfraz added a commit that referenced this pull request Oct 28, 2016
@jessfraz
Merge pull request #35824 from jessfraz/automated-cherry-pick-of-#35136-
7964708 
#35817-origin-release-1.4

Automated cherry pick of #35136 #35817
@k8s-cherrypick-bot
Copy link

Commit found in the "release-1.4" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.

k8s-github-robot pushed a commit that referenced this pull request Oct 29, 2016
Merge pull request #35817 from luxas/fix_crossbuild
5eefdb9 
Automatic merge from submit-queue

Fix cadvisor_unsupported and the crossbuild

Resolves a bug in the `cadvisor_unsupported.go` code.
Fixes #35735

Introduced by: #35136
We should consider to cherrypick this as #35136 also was cherrypicked

cc @kubernetes/sig-testing @vishh @dashpole @jessfraz

```release-note
Fix cadvisor_unsupported and the crossbuild
```
k8s-github-robot pushed a commit that referenced this pull request Nov 1, 2016
Merge pull request #35137 from dashpole/per_container_inode_eviction
2244bfe 
Automatic merge from submit-queue

Eviction manager evicts based on inode consumption

Fixes: #32526 Integrate Cadvisor per-container inode stats into the summary api.  Make the eviction manager act based on inode consumption to evict pods using the most inodes.

This PR is pending on a cadvisor godeps update which will be included in PR #35136
jessfraz referenced this pull request in jessfraz/kubernetes Nov 1, 2016
@jessfraz
Revert "Merge pull request kubernetes#35824 from jessfraz/automated-c…
76a9324 
…herry-pick-of-#35136-kubernetes#35817-origin-release-1.4"

This reverts commit 7964708, reversing
changes made to b7ad318.
@dashpole dashpole deleted the cadvisor_root_path_configuration branch November 7, 2016 17:59
shyamjvs pushed a commit to shyamjvs/kubernetes that referenced this pull request Dec 1, 2016
@jessfraz
Merge pull request kubernetes#35824 from jessfraz/automated-cherry-pi…
b31f1f8 
…ck-of-#35136-kubernetes#35817-origin-release-1.4

Automated cherry pick of kubernetes#35136 kubernetes#35817
@chentao1596 chentao1596 mentioned this pull request Dec 5, 2016
Closed
@yue9944882 yue9944882 mentioned this pull request Feb 28, 2018
Closed
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
cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. 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
v1.4
Development

Successfully merging this pull request may close these issues.
9 participants
@dashpole @k8s-ci-robot @vishh @timstclair @k8s-github-robot @k8s-cherrypick-bot @mtaufen @jessfraz @googlebot