Proposal for disk accounting #16889

vishh · 2015-11-05T23:48:31Z

@kubernetes/goog-node @bgrant0607 @thockin @smarterclayton

k8s-github-robot · 2015-11-05T23:49:44Z

Labelling this PR as size/XL

k8s-bot · 2015-11-06T00:50:23Z

GCE e2e build/test failed for commit 1957329ee110dcc46c52ef445c7f41b5228a2ac1.

pmorie · 2015-11-06T15:20:45Z

@kubernetes/rh-cluster-infra @kubernetes/rh-storage

k8s-bot · 2015-11-06T20:08:50Z

GCE e2e test build/test passed for commit 07d246add8b62e7b991fbb216afbaeb55d54d5aa.

pmorie · 2015-11-07T07:10:13Z

docs/proposals/disk-accounting.md

+
+3. Container’s logs - when written to stdout/stderr and default logging backend in docker is used.
+
+4. Local volumes - hostPath and emptyDir (TODO: Do git volumes consume local disk?)


The same questions exist for secrets, downward api, etc. I'd argue that hostPath is different from the others because it's necessarily 'outside' kubernetes. Also, what if you do a host mount of another pod's volume dir? The recycler does this, for example -- how do you count hostPath correctly in that scenario?

Also, theoretically you could have an image which has a docker volume. In openshift, we generate volumes for each of docker volume, but unless I'm mistaken the possibility of containers using docker-created volumes exists in kubernetes.

@pmorie: What is the kubernetes equivalent representation for auto-generated volumes in openshift? I'm only concerned about volumes that consume local disk in this proposal. If there are such volumes that I have not included here, let me know and I will update the proposal.

Get rid of TODO here since GitRepo does consume local disk, right?

Done

On Mon, Nov 16, 2015 at 1:28 PM, Dawn Chen notifications@github.com wrote:

In docs/proposals/disk-accounting.md
#16889 (comment)
:

+2. Support for filesystems other than ext4.
+
+### Introduction
+
+Disk accounting in Kubernetes cluster running with docker is complex because of the plethora of ways in which disk gets utilized by a container.
+
+Disk can be consumed for,
+
+1. Container images
+
+2. Container’s writable layer
+
+3. Container’s logs - when written to stdout/stderr and default logging backend in docker is used.
+
+4. Local volumes - hostPath and emptyDir (TODO: Do git volumes consume local disk?)

Get rid of TODO here since GitRepo does consume local disk, right?

—
Reply to this email directly or view it on GitHub
https://github.com/kubernetes/kubernetes/pull/16889/files#r44985742.

pmorie · 2015-11-07T07:22:24Z

@vishh Will there be any API changes for quotas or status?

vishh · 2015-11-08T01:59:13Z

No. New metrics API is being added to kubelet. Accounting will use those
APIs.
Quota is an implementation detail. I'm hoping to reserve a set of gids for
use by the kubelets. This range should not be available for users.
Scheduling might require API changes.

On Fri, Nov 6, 2015 at 11:22 PM, Paul Morie notifications@github.com
wrote:

@vishh https://github.com/vishh Will there be any API changes for
quotas or status?

—
Reply to this email directly or view it on GitHub
#16889 (comment)
.

smarterclayton · 2015-11-08T04:27:35Z

docs/proposals/disk-accounting.md

+
+1. Compatibility with all storage backends. The matrix is pretty large already and the priority is to get disk accounting to on most widely deployed platforms.
+
+2. Support for filesystems other than ext4.


Hrm - we're starting to move customers to XFS in other domains - can you describe why you think this is a non-goal (?

XFS is the default in RHEL 7 https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/7/html/Migration_Planning_Guide/sect-Red_Hat_Enterprise_Linux-Migration_Planning_Guide-File_System_Formats.html (it scales better than ext4).

I think supporting xfs is important. Just now we are discovering that ext4 is having issues with thin devices specially in AWS. Preallocating metadata on large thin devices can take a long time and systemd times out and docker folks are investigating the possibility of switching to xfs by default as container rootfs.

moby/moby#17464
moby/moby#17701

Thanks for the info @smarterclayton @mrunalp and @rhvgoyal. Can someone help me scope out the changes required for xfs? The first step might be to enhance this proposal to include changes required for xfs.

Now docker PR got merged which makes xfs default container fs for devicemapper graph driver.

moby/moby#17903

So far I can't anything special required for xfs. Whatever we were doing to make sure ext4 works, we need to repeat same for xfs.

Thanks for the update. This change will affect the writable layer and the image layers. Since we depend on devicemapper semantics here, this change shouldn't affect this proposal.
I assume that the primary filesystem where volumes and logs will be stored will continue to be ext4? This is important because, the current plan is to use quota for those use cases. Quota configuration is slightly different between ext4 and xfs.

When you say primary file system, are you referring to filesystem on host? I think there also we need to support both ext4 and xfs. RHEL has xfs as default host filesystem. And I am assuming that users will like all this to work on RHEL too.

Ah! ok. That's what I was looking for. So yes, we need to support xfs by default. I plan to get this rolling with ext4 support to start with. I'd appreciate some help with xfs integration once the internal implementation is finalized as part of implementing ext4 support.

vishh · 2015-12-28T20:11:22Z

@pwittrock: Can you review this proposal?

pwittrock · 2016-01-08T19:58:43Z

@vishh yes, taking a look now

pwittrock · 2016-01-08T20:18:18Z

docs/proposals/disk-accounting.md

+
+1. Account for disk usage on the nodes.
+
+2. Compatibility with the most important docker storage backends - devicemapper, aufs and overlayfs


nit: consider important -> common

pwittrock · 2016-01-08T22:17:54Z

@vishh Looks good. Just a couple of minor comments.

Signed-off-by: Vishnu Kannan <vishnuk@google.com>

vishh · 2016-01-08T23:08:48Z

@pwittrock: Thanks for the review. Fixed your comments. I might need another LGTM since I updated the patch.

k8s-github-robot · 2016-01-08T23:13:33Z

PR changed after LGTM, removing LGTM.

k8s-bot · 2016-01-08T23:46:44Z

GCE e2e test build/test passed for commit 0b43037.

pwittrock · 2016-01-20T00:41:12Z

@k8s-bot test this please

k8s-bot · 2016-01-20T01:44:27Z

GCE e2e test build/test passed for commit 0b43037.

k8s-github-robot · 2016-01-20T03:14:26Z

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

k8s-bot · 2016-01-20T05:19:05Z

GCE e2e test build/test passed for commit 0b43037.

k8s-github-robot · 2016-01-20T10:35:13Z

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

k8s-bot · 2016-01-20T11:11:13Z

GCE e2e test build/test passed for commit 0b43037.

alex-mohr · 2016-01-21T16:53:41Z

@k8s-bot unit test this

k8s-github-robot · 2016-01-22T11:16:50Z

@k8s-bot test this

Tests are more than 48 hours old. Re-running tests.

k8s-bot · 2016-01-22T11:47:51Z

GCE e2e test build/test passed for commit 0b43037.

Proposal for disk accounting

binarybana · 2016-02-15T05:40:28Z

docs/proposals/disk-accounting.md

+
+    * `chown -R :9000 /var/lib/docker/**container**/b8cc9fae3851f9bcefe922952b7bca0eb33aa31e68e9203ce0639fc9d3f3c61b/*`
+
+##### Testing titbits


s/titbits/tidbits

Proposal for disk accounting

googlebot added the cla: yes label Nov 5, 2015

vishh added the sig/node Categorizes an issue or PR as relevant to SIG Node. label Nov 5, 2015

k8s-github-robot assigned brendandburns Nov 5, 2015

k8s-github-robot added kind/design Categorizes issue or PR as related to design. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 5, 2015

dchen1107 assigned dchen1107 and unassigned brendandburns Nov 6, 2015

vishh force-pushed the disk-accounting branch from 1957329 to 07d246a Compare November 6, 2015 19:22

pmorie reviewed Nov 7, 2015
View reviewed changes

smarterclayton reviewed Nov 8, 2015
View reviewed changes

vishh assigned pwittrock Dec 28, 2015

pwittrock reviewed Jan 8, 2016
View reviewed changes

pwittrock added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 8, 2016

Proposal for disk accounting.

0b43037

Signed-off-by: Vishnu Kannan <vishnuk@google.com>

vishh force-pushed the disk-accounting branch from a75693a to 0b43037 Compare January 8, 2016 23:08

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 8, 2016

pwittrock added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 12, 2016

zmerlynn added a commit that referenced this pull request Jan 22, 2016

Merge pull request #16889 from vishh/disk-accounting

a5f4f3b

Proposal for disk accounting

zmerlynn merged commit a5f4f3b into kubernetes:master Jan 22, 2016

binarybana reviewed Feb 15, 2016
View reviewed changes

xingzhou pushed a commit to xingzhou/kubernetes that referenced this pull request Dec 15, 2016

Merge pull request kubernetes#16889 from vishh/disk-accounting

8f093ad

Proposal for disk accounting


		3. Container’s logs - when written to stdout/stderr and default logging backend in docker is used.

		4. Local volumes - hostPath and emptyDir (TODO: Do git volumes consume local disk?)


		1. Compatibility with all storage backends. The matrix is pretty large already and the priority is to get disk accounting to on most widely deployed platforms.

		2. Support for filesystems other than ext4.


		1. Account for disk usage on the nodes.

		2. Compatibility with the most important docker storage backends - devicemapper, aufs and overlayfs


		* `chown -R :9000 /var/lib/docker/container/b8cc9fae3851f9bcefe922952b7bca0eb33aa31e68e9203ce0639fc9d3f3c61b/*`

		##### Testing titbits

Proposal for disk accounting #16889

Proposal for disk accounting #16889

Conversation

vishh commented Nov 5, 2015

k8s-github-robot commented Nov 5, 2015

k8s-bot commented Nov 6, 2015

pmorie commented Nov 6, 2015

k8s-bot commented Nov 6, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pmorie commented Nov 7, 2015

vishh commented Nov 8, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vishh commented Dec 28, 2015

pwittrock commented Jan 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pwittrock commented Jan 8, 2016

vishh commented Jan 8, 2016

k8s-github-robot commented Jan 8, 2016

k8s-bot commented Jan 8, 2016

pwittrock commented Jan 20, 2016

k8s-bot commented Jan 20, 2016

k8s-github-robot commented Jan 20, 2016

k8s-bot commented Jan 20, 2016

k8s-github-robot commented Jan 20, 2016

k8s-bot commented Jan 20, 2016

alex-mohr commented Jan 21, 2016

k8s-github-robot commented Jan 22, 2016

k8s-bot commented Jan 22, 2016

Choose a reason for hiding this comment