Merge pull request #30311 from derekwaynecarr/inode_eviction

docs/proposals/kubelet-eviction.md

@@ -478,9 +478,19 @@ for eviction. Instead `DaemonSet` should ideally include Guaranteed pods only.
 
 ## Known issues
 
+### kubelet may evict more pods than needed
+
 The pod eviction may evict more pods than needed due to stats collection timing gap. This can be mitigated by adding
 the ability to get root container stats on an on-demand basis (https://github.com/google/cadvisor/issues/1247) in the future.
 
+### How kubelet ranks pods for eviction in response to inode exhaustion
+
+At this time, it is not possible to know how many inodes were consumed by a particular container.  If the `kubelet` observes
+inode exhaustion, it will evict pods by ranking them by quality of service.  The following issue has been opened in cadvisor
+to track per container inode consumption (https://github.com/google/cadvisor/issues/1422) which would allow us to rank pods
+by inode consumption.  For example, this would let us identify a container that created large numbers of 0 byte files, and evict
+that pod over others.
+
 <!-- BEGIN MUNGE: GENERATED_ANALYTICS -->
 [![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/docs/proposals/kubelet-eviction.md?pixel)]()
 <!-- END MUNGE: GENERATED_ANALYTICS -->

hack/.linted_packages

@@ -96,6 +96,7 @@ pkg/credentialprovider/aws
 pkg/hyperkube
 pkg/kubelet/api
 pkg/kubelet/container
+pkg/kubelet/eviction
 pkg/kubelet/envvars
 pkg/kubelet/util/format
 pkg/kubelet/util/ioutils

pkg/kubelet/eviction/eviction_manager_test.go

@@ -914,3 +914,228 @@ func TestNodeReclaimFuncs(t *testing.T) {
 		t.Errorf("Manager chose to kill pod: %v when no pod should have been killed", podKiller.pod)
 	}
 }
+
+func TestDiskPressureNodeFsInodes(t *testing.T) {
+	// TODO: we need to know inodes used when cadvisor supports per container stats
+	podMaker := func(name string, requests api.ResourceList, limits api.ResourceList) (*api.Pod, statsapi.PodStats) {
+		pod := newPod(name, []api.Container{
+			newContainer(name, requests, limits),
+		}, nil)
+		podStats := newPodInodeStats(pod)
+		return pod, podStats
+	}
+	summaryStatsMaker := func(rootFsInodesFree, rootFsInodes string, podStats map[*api.Pod]statsapi.PodStats) *statsapi.Summary {
+		rootFsInodesFreeVal := resource.MustParse(rootFsInodesFree)
+		internalRootFsInodesFree := uint64(rootFsInodesFreeVal.Value())
+		rootFsInodesVal := resource.MustParse(rootFsInodes)
+		internalRootFsInodes := uint64(rootFsInodesVal.Value())
+		result := &statsapi.Summary{
+			Node: statsapi.NodeStats{
+				Fs: &statsapi.FsStats{
+					InodesFree: &internalRootFsInodesFree,
+					Inodes:     &internalRootFsInodes,
+				},
+			},
+			Pods: []statsapi.PodStats{},
+		}
+		for _, podStat := range podStats {
+			result.Pods = append(result.Pods, podStat)
+		}
+		return result
+	}
+	// TODO: pass inodes used in future when supported by cadvisor.
+	podsToMake := []struct {
+		name     string
+		requests api.ResourceList
+		limits   api.ResourceList
+	}{
+		{name: "best-effort-high", requests: newResourceList("", ""), limits: newResourceList("", "")},
+		{name: "best-effort-low", requests: newResourceList("", ""), limits: newResourceList("", "")},
+		{name: "burstable-high", requests: newResourceList("100m", "100Mi"), limits: newResourceList("200m", "1Gi")},
+		{name: "burstable-low", requests: newResourceList("100m", "100Mi"), limits: newResourceList("200m", "1Gi")},
+		{name: "guaranteed-high", requests: newResourceList("100m", "1Gi"), limits: newResourceList("100m", "1Gi")},
+		{name: "guaranteed-low", requests: newResourceList("100m", "1Gi"), limits: newResourceList("100m", "1Gi")},
+	}
+	pods := []*api.Pod{}
+	podStats := map[*api.Pod]statsapi.PodStats{}
+	for _, podToMake := range podsToMake {
+		pod, podStat := podMaker(podToMake.name, podToMake.requests, podToMake.limits)
+		pods = append(pods, pod)
+		podStats[pod] = podStat
+	}
+	activePodsFunc := func() []*api.Pod {
+		return pods
+	}
+
+	fakeClock := clock.NewFakeClock(time.Now())
+	podKiller := &mockPodKiller{}
+	diskInfoProvider := &mockDiskInfoProvider{dedicatedImageFs: false}
+	imageGC := &mockImageGC{freed: int64(0), err: nil}
+	nodeRef := &api.ObjectReference{Kind: "Node", Name: "test", UID: types.UID("test"), Namespace: ""}
+
+	config := Config{
+		MaxPodGracePeriodSeconds: 5,
+		PressureTransitionPeriod: time.Minute * 5,
+		Thresholds: []Threshold{
+			{
+				Signal:   SignalNodeFsInodesFree,
+				Operator: OpLessThan,
+				Value: ThresholdValue{
+					Quantity: quantityMustParse("1Mi"),
+				},
+			},
+			{
+				Signal:   SignalNodeFsInodesFree,
+				Operator: OpLessThan,
+				Value: ThresholdValue{
+					Quantity: quantityMustParse("2Mi"),
+				},
+				GracePeriod: time.Minute * 2,
+			},
+		},
+	}
+	summaryProvider := &fakeSummaryProvider{result: summaryStatsMaker("3Mi", "4Mi", podStats)}
+	manager := &managerImpl{
+		clock:           fakeClock,
+		killPodFunc:     podKiller.killPodNow,
+		imageGC:         imageGC,
+		config:          config,
+		recorder:        &record.FakeRecorder{},
+		summaryProvider: summaryProvider,
+		nodeRef:         nodeRef,
+		nodeConditionsLastObservedAt: nodeConditionsObservedAt{},
+		thresholdsFirstObservedAt:    thresholdsObservedAt{},
+	}
+
+	// create a best effort pod to test admission
+	podToAdmit, _ := podMaker("pod-to-admit", newResourceList("", ""), newResourceList("", ""))
+
+	// synchronize
+	manager.synchronize(diskInfoProvider, activePodsFunc)
+
+	// we should not have disk pressure
+	if manager.IsUnderDiskPressure() {
+		t.Errorf("Manager should not report disk pressure")
+	}
+
+	// try to admit our pod (should succeed)
+	if result := manager.Admit(&lifecycle.PodAdmitAttributes{Pod: podToAdmit}); !result.Admit {
+		t.Errorf("Admit pod: %v, expected: %v, actual: %v", podToAdmit, true, result.Admit)
+	}
+
+	// induce soft threshold
+	fakeClock.Step(1 * time.Minute)
+	summaryProvider.result = summaryStatsMaker("1.5Mi", "4Mi", podStats)
+	manager.synchronize(diskInfoProvider, activePodsFunc)
+
+	// we should have disk pressure
+	if !manager.IsUnderDiskPressure() {
+		t.Errorf("Manager should report disk pressure since soft threshold was met")
+	}
+
+	// verify no pod was yet killed because there has not yet been enough time passed.
+	if podKiller.pod != nil {
+		t.Errorf("Manager should not have killed a pod yet, but killed: %v", podKiller.pod)
+	}
+
+	// step forward in time pass the grace period
+	fakeClock.Step(3 * time.Minute)
+	summaryProvider.result = summaryStatsMaker("1.5Mi", "4Mi", podStats)
+	manager.synchronize(diskInfoProvider, activePodsFunc)
+
+	// we should have disk pressure
+	if !manager.IsUnderDiskPressure() {
+		t.Errorf("Manager should report disk pressure since soft threshold was met")
+	}
+
+	// verify the right pod was killed with the right grace period.
+	if podKiller.pod != pods[0] {
+		t.Errorf("Manager chose to kill pod: %v, but should have chosen %v", podKiller.pod, pods[0])
+	}
+	if podKiller.gracePeriodOverride == nil {
+		t.Errorf("Manager chose to kill pod but should have had a grace period override.")
+	}
+	observedGracePeriod := *podKiller.gracePeriodOverride
+	if observedGracePeriod != manager.config.MaxPodGracePeriodSeconds {
+		t.Errorf("Manager chose to kill pod with incorrect grace period.  Expected: %d, actual: %d", manager.config.MaxPodGracePeriodSeconds, observedGracePeriod)
+	}
+	// reset state
+	podKiller.pod = nil
+	podKiller.gracePeriodOverride = nil
+
+	// remove disk pressure
+	fakeClock.Step(20 * time.Minute)
+	summaryProvider.result = summaryStatsMaker("3Mi", "4Mi", podStats)
+	manager.synchronize(diskInfoProvider, activePodsFunc)
+
+	// we should not have disk pressure
+	if manager.IsUnderDiskPressure() {
+		t.Errorf("Manager should not report disk pressure")
+	}
+
+	// induce disk pressure!
+	fakeClock.Step(1 * time.Minute)
+	summaryProvider.result = summaryStatsMaker("0.5Mi", "4Mi", podStats)
+	manager.synchronize(diskInfoProvider, activePodsFunc)
+
+	// we should have disk pressure
+	if !manager.IsUnderDiskPressure() {
+		t.Errorf("Manager should report disk pressure")
+	}
+
+	// check the right pod was killed
+	if podKiller.pod != pods[0] {
+		t.Errorf("Manager chose to kill pod: %v, but should have chosen %v", podKiller.pod, pods[0])
+	}
+	observedGracePeriod = *podKiller.gracePeriodOverride
+	if observedGracePeriod != int64(0) {
+		t.Errorf("Manager chose to kill pod with incorrect grace period.  Expected: %d, actual: %d", 0, observedGracePeriod)
+	}
+
+	// try to admit our pod (should fail)
+	if result := manager.Admit(&lifecycle.PodAdmitAttributes{Pod: podToAdmit}); result.Admit {
+		t.Errorf("Admit pod: %v, expected: %v, actual: %v", podToAdmit, false, result.Admit)
+	}
+
+	// reduce disk pressure
+	fakeClock.Step(1 * time.Minute)
+	summaryProvider.result = summaryStatsMaker("3Mi", "4Mi", podStats)
+	podKiller.pod = nil // reset state
+	manager.synchronize(diskInfoProvider, activePodsFunc)
+
+	// we should have disk pressure (because transition period not yet met)
+	if !manager.IsUnderDiskPressure() {
+		t.Errorf("Manager should report disk pressure")
+	}
+
+	// no pod should have been killed
+	if podKiller.pod != nil {
+		t.Errorf("Manager chose to kill pod: %v when no pod should have been killed", podKiller.pod)
+	}
+
+	// try to admit our pod (should fail)
+	if result := manager.Admit(&lifecycle.PodAdmitAttributes{Pod: podToAdmit}); result.Admit {
+		t.Errorf("Admit pod: %v, expected: %v, actual: %v", podToAdmit, false, result.Admit)
+	}
+
+	// move the clock past transition period to ensure that we stop reporting pressure
+	fakeClock.Step(5 * time.Minute)
+	summaryProvider.result = summaryStatsMaker("3Mi", "4Mi", podStats)
+	podKiller.pod = nil // reset state
+	manager.synchronize(diskInfoProvider, activePodsFunc)
+
+	// we should not have disk pressure (because transition period met)
+	if manager.IsUnderDiskPressure() {
+		t.Errorf("Manager should not report disk pressure")
+	}
+
+	// no pod should have been killed
+	if podKiller.pod != nil {
+		t.Errorf("Manager chose to kill pod: %v when no pod should have been killed", podKiller.pod)
+	}
+
+	// try to admit our pod (should succeed)
+	if result := manager.Admit(&lifecycle.PodAdmitAttributes{Pod: podToAdmit}); !result.Admit {
+		t.Errorf("Admit pod: %v, expected: %v, actual: %v", podToAdmit, true, result.Admit)
+	}
+}

pkg/kubelet/eviction/helpers_test.go

@@ -191,6 +191,49 @@ func TestParseThresholdConfig(t *testing.T) {
 				},
 			},
 		},
+		"inode flag values": {
+			evictionHard:            "imagefs.inodesFree<150Mi,nodefs.inodesFree<100Mi",
+			evictionSoft:            "imagefs.inodesFree<300Mi,nodefs.inodesFree<200Mi",
+			evictionSoftGracePeriod: "imagefs.inodesFree=30s,nodefs.inodesFree=30s",
+			evictionMinReclaim:      "imagefs.inodesFree=2Gi,nodefs.inodesFree=1Gi",
+			expectErr:               false,
+			expectThresholds: []Threshold{
+				{
+					Signal:   SignalImageFsInodesFree,
+					Operator: OpLessThan,
+					Value: ThresholdValue{
+						Quantity: quantityMustParse("150Mi"),
+					},
+					MinReclaim: quantityMustParse("2Gi"),
+				},
+				{
+					Signal:   SignalNodeFsInodesFree,
+					Operator: OpLessThan,
+					Value: ThresholdValue{
+						Quantity: quantityMustParse("100Mi"),
+					},
+					MinReclaim: quantityMustParse("1Gi"),
+				},
+				{
+					Signal:   SignalImageFsInodesFree,
+					Operator: OpLessThan,
+					Value: ThresholdValue{
+						Quantity: quantityMustParse("300Mi"),
+					},
+					GracePeriod: gracePeriod,
+					MinReclaim:  quantityMustParse("2Gi"),
+				},
+				{
+					Signal:   SignalNodeFsInodesFree,
+					Operator: OpLessThan,
+					Value: ThresholdValue{
+						Quantity: quantityMustParse("200Mi"),
+					},
+					GracePeriod: gracePeriod,
+					MinReclaim:  quantityMustParse("1Gi"),
+				},
+			},
+		},
 		"invalid-signal": {
 			evictionHard:            "mem.available<150Mi",
 			evictionSoft:            "",
@@ -400,7 +443,7 @@ func TestOrderedByDisk(t *testing.T) {
 		return result, found
 	}
 	pods := []*api.Pod{pod1, pod2, pod3, pod4, pod5, pod6}
-	orderedBy(disk(statsFn, []fsStatsType{fsStatsRoot, fsStatsLogs, fsStatsLocalVolumeSource})).Sort(pods)
+	orderedBy(disk(statsFn, []fsStatsType{fsStatsRoot, fsStatsLogs, fsStatsLocalVolumeSource}, resourceDisk)).Sort(pods)
 	expected := []*api.Pod{pod6, pod5, pod4, pod3, pod2, pod1}
 	for i := range expected {
 		if pods[i] != expected[i] {
@@ -466,7 +509,7 @@ func TestOrderedByQoSDisk(t *testing.T) {
 		return result, found
 	}
 	pods := []*api.Pod{pod1, pod2, pod3, pod4, pod5, pod6}
-	orderedBy(qosComparator, disk(statsFn, []fsStatsType{fsStatsRoot, fsStatsLogs, fsStatsLocalVolumeSource})).Sort(pods)
+	orderedBy(qosComparator, disk(statsFn, []fsStatsType{fsStatsRoot, fsStatsLogs, fsStatsLocalVolumeSource}, resourceDisk)).Sort(pods)
 	expected := []*api.Pod{pod2, pod1, pod4, pod3, pod6, pod5}
 	for i := range expected {
 		if pods[i] != expected[i] {
@@ -608,6 +651,10 @@ func TestMakeSignalObservations(t *testing.T) {
 	imageFsCapacityBytes := uint64(1024 * 1024 * 2)
 	nodeFsAvailableBytes := uint64(1024)
 	nodeFsCapacityBytes := uint64(1024 * 2)
+	imageFsInodesFree := uint64(1024)
+	imageFsInodes := uint64(1024 * 1024)
+	nodeFsInodesFree := uint64(1024)
+	nodeFsInodes := uint64(1024 * 1024)
 	fakeStats := &statsapi.Summary{
 		Node: statsapi.NodeStats{
 			Memory: &statsapi.MemoryStats{
@@ -618,11 +665,15 @@ func TestMakeSignalObservations(t *testing.T) {
 				ImageFs: &statsapi.FsStats{
 					AvailableBytes: &imageFsAvailableBytes,
 					CapacityBytes:  &imageFsCapacityBytes,
+					InodesFree:     &imageFsInodesFree,
+					Inodes:         &imageFsInodes,
 				},
 			},
 			Fs: &statsapi.FsStats{
 				AvailableBytes: &nodeFsAvailableBytes,
 				CapacityBytes:  &nodeFsCapacityBytes,
+				InodesFree:     &nodeFsInodesFree,
+				Inodes:         &nodeFsInodes,
 			},
 		},
 		Pods: []statsapi.PodStats{},
@@ -664,6 +715,16 @@ func TestMakeSignalObservations(t *testing.T) {
 	if expectedBytes := int64(nodeFsCapacityBytes); nodeFsQuantity.capacity.Value() != expectedBytes {
 		t.Errorf("Expected %v, actual: %v", expectedBytes, nodeFsQuantity.capacity.Value())
 	}
+	nodeFsInodesQuantity, found := actualObservations[SignalNodeFsInodesFree]
+	if !found {
+		t.Errorf("Expected inodes free nodefs observation: %v", err)
+	}
+	if expected := int64(nodeFsInodesFree); nodeFsInodesQuantity.available.Value() != expected {
+		t.Errorf("Expected %v, actual: %v", expected, nodeFsInodesQuantity.available.Value())
+	}
+	if expected := int64(nodeFsInodes); nodeFsInodesQuantity.capacity.Value() != expected {
+		t.Errorf("Expected %v, actual: %v", expected, nodeFsInodesQuantity.capacity.Value())
+	}
 	imageFsQuantity, found := actualObservations[SignalImageFsAvailable]
 	if !found {
 		t.Errorf("Expected available imagefs observation: %v", err)
@@ -674,6 +735,16 @@ func TestMakeSignalObservations(t *testing.T) {
 	if expectedBytes := int64(imageFsCapacityBytes); imageFsQuantity.capacity.Value() != expectedBytes {
 		t.Errorf("Expected %v, actual: %v", expectedBytes, imageFsQuantity.capacity.Value())
 	}
+	imageFsInodesQuantity, found := actualObservations[SignalImageFsInodesFree]
+	if !found {
+		t.Errorf("Expected inodes free imagefs observation: %v", err)
+	}
+	if expected := int64(imageFsInodesFree); imageFsInodesQuantity.available.Value() != expected {
+		t.Errorf("Expected %v, actual: %v", expected, imageFsInodesQuantity.available.Value())
+	}
+	if expected := int64(imageFsInodes); imageFsInodesQuantity.capacity.Value() != expected {
+		t.Errorf("Expected %v, actual: %v", expected, imageFsInodesQuantity.capacity.Value())
+	}
 	for _, pod := range pods {
 		podStats, found := statsFunc(pod)
 		if !found {
@@ -1204,6 +1275,22 @@ func testCompareThresholdValue(t *testing.T) {
 	}
 }
 
+// newPodInodeStats returns stats with specified usage amounts.
+// TODO: in future, this should take a value for inodesUsed per container.
+func newPodInodeStats(pod *api.Pod) statsapi.PodStats {
+	result := statsapi.PodStats{
+		PodRef: statsapi.PodReference{
+			Name: pod.Name, Namespace: pod.Namespace, UID: string(pod.UID),
+		},
+	}
+	for range pod.Spec.Containers {
+		result.Containers = append(result.Containers, statsapi.ContainerStats{
+			Rootfs: &statsapi.FsStats{},
+		})
+	}
+	return result
+}
+
 // newPodDiskStats returns stats with specified usage amounts.
 func newPodDiskStats(pod *api.Pod, rootFsUsed, logsUsed, perLocalVolumeUsed resource.Quantity) statsapi.PodStats {
 	result := statsapi.PodStats{

pkg/kubelet/eviction/types.go

@@ -32,8 +32,12 @@ const (
 	SignalMemoryAvailable Signal = "memory.available"
 	// SignalNodeFsAvailable is amount of storage available on filesystem that kubelet uses for volumes, daemon logs, etc.
 	SignalNodeFsAvailable Signal = "nodefs.available"
+	// SignalNodeFsInodesFree is amount of inodes available on filesystem that kubelet uses for volumes, daemon logs, etc.
+	SignalNodeFsInodesFree Signal = "nodefs.inodesFree"
 	// SignalImageFsAvailable is amount of storage available on filesystem that container runtime uses for storing images and container writable layers.
 	SignalImageFsAvailable Signal = "imagefs.available"
+	// SignalImageFsInodesFree is amount of inodes available on filesystem that container runtime uses for storing images and container writeable layers.
+	SignalImageFsInodesFree Signal = "imagefs.inodesFree"
 )
 
 // fsStatsType defines the types of filesystem stats to collect.