Ephemeral storage monitoring via filesystem quotas by RobertKrawitz · Pull Request #66928 · kubernetes/kubernetes

RobertKrawitz · 2018-08-02T19:00:40Z

Use XFS-style quotas to monitor ephemeral storage consumption where possible. Reference https://github.com/kubernetes/enhancements/blob/master/keps/sig-node/0030-20180906-quotas-for-ephemeral-storage.md

kubelet now allows use of XFS quotas (on XFS and suitably configured ext4fs filesystems) to monitor storage consumption for ephemeral storage (currently for emptydir volumes only).  This method of monitoring consumption is faster and more accurate than the old method of walking the filesystem tree.  It does not enforce limits, only monitors consumption.  To utilize this functionality, you must set the feature  gate `LocalStorageCapacityIsolationFSQuotaMonitoring=true`. For ext4fs filesystems, you must create the
filesystem with `mkfs.ext4 -O project <block_device>` and run `tune2fs -Q prjquota `block device`; XFS
filesystems need no additional preparation.  The filesystem must be mounted with option `project` in
`/etc/fstab`.  If your primary partition is the root filesystem, you must also add `rootflags=pquota` to your
GRUB config file.

RobertKrawitz · 2019-05-29T18:27:18Z

/cc @dashpole

Could you look at this and approve as appropriate? Thanks!

…xtension.

Positive test is skipped if quotas not available.

…mptydir limit (if set)

RobertKrawitz · 2019-05-29T19:23:41Z

@dashpole, could you /lgtm, please?

dashpole · 2019-05-29T19:29:41Z

/lgtm

RobertKrawitz · 2019-05-29T20:12:01Z

/test pull-kubernetes-e2e-gce

RobertKrawitz · 2019-05-29T22:23:01Z

/retest

RobertKrawitz · 2019-05-30T15:35:51Z

ping @jingxu97

dashpole · 2019-05-30T16:34:33Z

I spoke with @jingxu97 offline, who took another pass and said it looked good.

RobertKrawitz · 2019-05-30T17:07:10Z

Thanks @dashpole

ping @derekwaynecarr

derekwaynecarr · 2019-05-30T17:11:43Z

@dashpole thanks.

/hold cancel
/approve

RobertKrawitz · 2019-05-30T17:20:19Z

/assign @childsb @luxas

@childsb @luxas can you look at this -- we're trying to get this into 1.15. Thanks!

dashpole · 2019-05-30T17:32:08Z

/assign @msau42
for approvals in /pkg/volume and /test/utils

saad-ali · 2019-05-30T17:48:32Z

/assign

saad-ali

/approve
/hold to give you a chance to address any feedback

saad-ali · 2019-05-30T18:15:49Z

pkg/kubelet/volumemanager/cache/desired_state_of_world.go

+				if volumeSpec.Volume.EmptyDir != nil &&
+					volumeSpec.Volume.EmptyDir.SizeLimit != nil &&
+					volumeSpec.Volume.EmptyDir.SizeLimit.Value() > 0 &&
+					volumeSpec.Volume.EmptyDir.SizeLimit.Value() < sizeLimit.Value() {


Do we not have SizeLimits for other "ephermeral volume types", e.g. for SecretVolume, ConfigMapVolume, etc.?

This project only applies to EmptyDir volumes (and potentially anything else covered by local ephemeral storage, but there are no plans to cover other volume types under that umbrella).

Local ephemeral storage is defined to be EmptyDir volumes, writable layers, and logs. Writable layers and logs aren't volumes and would have to be covered through cadvisor. Secrets and configmaps are not defined to be local ephemeral storage.

saad-ali · 2019-05-30T18:21:26Z

pkg/volume/emptydir/empty_dir.go

+	// enforcement.
 	if err == nil {
 		volumeutil.SetReady(ed.getMetaDir())
+		if mounterArgs.DesiredSize != nil {


Something to keep in mind, if this will be supported for other ephemeral volume types (e.g. SecretVolumes, ConfigMapVolumes, etc.) in the future, those volume types do periodic "remounting" to ensure the data is fresh. Might be worth thinking through what happens in that case.

saad-ali · 2019-05-30T18:25:46Z

pkg/volume/emptydir/empty_dir.go

Do we have to worry about accumulating quota IDs in these error scenarios? Meaning if this happens can the accumulation cause other parts of the system to stop functioning?

saad-ali · 2019-05-30T20:03:52Z

pkg/volume/util/operationexecutor/operation_generator.go

+		mountErr := volumeMounter.SetUp(volume.MounterArgs{
+			FsGroup:     fsGroup,
+			DesiredSize: volumeToMount.DesiredSizeLimit,
+			PodUID:      string(volumeToMount.Pod.UID),


You don't have to add this argument here. It should be available via the emptyDir data structure in the SetUp method

If you want, I can remove it; I agree it's not really needed here.

As for accumulating quotas, it's not impossible that something could go wrong with the teardown and leave the quota in place applying to nothing. It is possible to remove quotas later if need be. We leave records in /etc/projects and /etc/projid with clearly defined names to make it possible to trace back to Kubernetes.

If you want, I can remove it; I agree it's not really needed here.

If you have time go for it, happy to reapply lgtm/approval. If not, no big deal.

saad-ali · 2019-05-30T20:05:55Z

pkg/volume/util/quota/common/quota_linux_common.go

@@ -0,0 +1,105 @@
+// +build linux
+


nit: pkg/volume/util/quota/ seems like a very generic name. Worth being more specific?

It could potentially be fsquota or the like, if you feel strongly about it.

Slight concern about confusing readers. But don't feel strongly. We could rename in the future if there is a collision.

k8s-ci-robot · 2019-05-30T20:13:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: derekwaynecarr, RobertKrawitz, saad-ali

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~pkg/features/OWNERS~~ [derekwaynecarr,saad-ali]
~~pkg/kubelet/OWNERS~~ [derekwaynecarr]
~~pkg/volume/OWNERS~~ [saad-ali]
~~test/e2e_node/OWNERS~~ [derekwaynecarr,saad-ali]
~~test/utils/image/OWNERS~~ [saad-ali]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

saad-ali · 2019-05-30T20:14:41Z

/hold
To give you a chance to address any feedback

RobertKrawitz · 2019-05-30T21:20:35Z

@saad-ali please let me know whether you deem any of the changes you discussed to be critical here.

RobertKrawitz · 2019-05-30T21:51:57Z

/hold cancel

On further thought, /approve means that it's OK, and this was just to let me reply.

k8s-ci-robot · 2019-05-31T00:08:37Z

@RobertKrawitz: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-kubernetes-local-e2e-containerized	31edcd999489109e15cefdaa3551d4016e62eb4e	link	`/test pull-kubernetes-local-e2e-containerized`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

RobertKrawitz · 2019-05-31T00:13:27Z

/test pull-kubernetes-bazel-test

jingxu97 · 2019-08-12T20:34:13Z

pkg/volume/util/operationexecutor/operation_executor.go

+
+	// DesiredSizeLimit indicates the desired upper bound on the size of the volume
+	// (if so implemented)
+	DesiredSizeLimit *resource.Quantity


sorry to comment the late. But where this DesiredSizeLimit is used?

It will be used when we implement quota enforcement.

shouldn't we hold from introducing this until we have a plan to use it? It seems like we introduced this field and almost an year later nobody is using it.

k8s-ci-robot requested review from chakri-nelluri and cofyc August 2, 2018 19:00

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Aug 7, 2018

RobertKrawitz force-pushed the ephemeral-storage-quota-exp branch from bebd4e6 to ac80c5f Compare August 7, 2018 21:49

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Aug 7, 2018

RobertKrawitz force-pushed the ephemeral-storage-quota-exp branch 6 times, most recently from 12644a2 to 50aaa4e Compare August 8, 2018 18:21

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Aug 10, 2018

RobertKrawitz force-pushed the ephemeral-storage-quota-exp branch 6 times, most recently from dddaee9 to cfa9fbc Compare August 14, 2018 18:55

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 15, 2018

RobertKrawitz force-pushed the ephemeral-storage-quota-exp branch from b0fb1ae to 82e0eb9 Compare August 15, 2018 20:15

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 15, 2018

RobertKrawitz force-pushed the ephemeral-storage-quota-exp branch from 82376ef to ab5f1f7 Compare August 16, 2018 20:26

RobertKrawitz and others added 6 commits May 29, 2019 15:12

Change fsGroup to mounterArgs in volume.SetUp() to allow for future e…

5b97b28

…xtension.

Apply quotas via syscalls using cgo.

448e0c4

Use xfs_quota command to apply quotas

f8661d6

Add e2e test for quota-based eviction.

46b3c75

Positive test is skipped if quotas not available.

Log message rather than error if setting quota fails

9f4a4d1

Set sizeLimit to the smaller of the ephemeral storage limit and the e…

38753ef

…mptydir limit (if set)

RobertKrawitz mentioned this pull request May 30, 2019

Quotas for Ephemeral Storage kubernetes/enhancements#1029

Open

22 tasks

saad-ali reviewed May 30, 2019

View reviewed changes

RobertKrawitz mentioned this pull request May 31, 2019

Ephemeral storage quota cleanup #78593

Merged

RobertKrawitz mentioned this pull request Jun 20, 2019

Update vendored dependencies to a released version instead of random SHA(s) #79234

Closed

12 tasks

jingxu97 reviewed Aug 12, 2019

View reviewed changes

pacoxu mentioned this pull request May 13, 2021

promote ephemeral-storage-quotas to beta in 1.25 kubernetes/enhancements#2697

Merged

8 tasks

Conversation

RobertKrawitz commented Aug 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RobertKrawitz commented May 29, 2019

Uh oh!

RobertKrawitz commented May 29, 2019

Uh oh!

dashpole commented May 29, 2019

Uh oh!

RobertKrawitz commented May 29, 2019

Uh oh!

RobertKrawitz commented May 29, 2019

Uh oh!

RobertKrawitz commented May 30, 2019

Uh oh!

dashpole commented May 30, 2019

Uh oh!

RobertKrawitz commented May 30, 2019

Uh oh!

derekwaynecarr commented May 30, 2019

Uh oh!

RobertKrawitz commented May 30, 2019

Uh oh!

dashpole commented May 30, 2019

Uh oh!

saad-ali commented May 30, 2019

Uh oh!

saad-ali left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RobertKrawitz May 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented May 30, 2019

Uh oh!

saad-ali commented May 30, 2019

Uh oh!

RobertKrawitz commented May 30, 2019

Uh oh!

RobertKrawitz commented May 30, 2019

Uh oh!

k8s-ci-robot commented May 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RobertKrawitz commented May 31, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

RobertKrawitz commented Aug 2, 2018 •

edited

Loading

RobertKrawitz May 30, 2019 •

edited

Loading

k8s-ci-robot commented May 31, 2019 •

edited

Loading