Commit f9ccee77 authored by Kubernetes Submit Queue's avatar Kubernetes Submit Queue Committed by GitHub

Merge pull request #42435 from dashpole/timestamps_for_fsstats

Automatic merge from submit-queue (batch tested with PRs 42369, 42375, 42397, 42435, 42455) [Bug Fix]: Avoid evicting more pods than necessary by adding Timestamps for fsstats and ignoring stale stats Continuation of #33121. Credit for most of this goes to @sjenning. I added volume fs timestamps. **why is this a bug** This PR attempts to fix part of https://github.com/kubernetes/kubernetes/issues/31362 which results in multiple pods getting evicted unnecessarily whenever the node runs into resource pressure. This PR reduces the chances of such disruptions by avoiding reacting to old/stale metrics. Without this PR, kubernetes nodes under resource pressure will cause unnecessary disruptions to user workloads. This PR will also help deflake a node e2e test suite. The eviction manager currently avoids evicting pods if metrics are old. However, timestamp data is not available for filesystem data, and this causes lots of extra evictions. See the [inode eviction test flakes](https://k8s-testgrid.appspot.com/google-node#kubelet-flaky-gce-e2e) for examples. This should probably be treated as a bugfix, as it should help mitigate extra evictions. cc: @kubernetes/sig-storage-pr-reviews @kubernetes/sig-node-pr-reviews @vishh @derekwaynecarr @sjenning
parents 51a3d7b6 a90c7951
...@@ -191,6 +191,8 @@ type VolumeStats struct { ...@@ -191,6 +191,8 @@ type VolumeStats struct {
// FsStats contains data about filesystem usage. // FsStats contains data about filesystem usage.
type FsStats struct { type FsStats struct {
// The time at which these stats were updated.
Time metav1.Time `json:"time"`
// AvailableBytes represents the storage space available (bytes) for the filesystem. // AvailableBytes represents the storage space available (bytes) for the filesystem.
// +optional // +optional
AvailableBytes *uint64 `json:"availableBytes,omitempty"` AvailableBytes *uint64 `json:"availableBytes,omitempty"`
......
...@@ -664,14 +664,14 @@ func makeSignalObservations(summaryProvider stats.SummaryProvider, nodeProvider ...@@ -664,14 +664,14 @@ func makeSignalObservations(summaryProvider stats.SummaryProvider, nodeProvider
result[evictionapi.SignalNodeFsAvailable] = signalObservation{ result[evictionapi.SignalNodeFsAvailable] = signalObservation{
available: resource.NewQuantity(int64(*nodeFs.AvailableBytes), resource.BinarySI), available: resource.NewQuantity(int64(*nodeFs.AvailableBytes), resource.BinarySI),
capacity: resource.NewQuantity(int64(*nodeFs.CapacityBytes), resource.BinarySI), capacity: resource.NewQuantity(int64(*nodeFs.CapacityBytes), resource.BinarySI),
// TODO: add timestamp to stat (see memory stat) time: nodeFs.Time,
} }
} }
if nodeFs.InodesFree != nil && nodeFs.Inodes != nil { if nodeFs.InodesFree != nil && nodeFs.Inodes != nil {
result[evictionapi.SignalNodeFsInodesFree] = signalObservation{ result[evictionapi.SignalNodeFsInodesFree] = signalObservation{
available: resource.NewQuantity(int64(*nodeFs.InodesFree), resource.BinarySI), available: resource.NewQuantity(int64(*nodeFs.InodesFree), resource.BinarySI),
capacity: resource.NewQuantity(int64(*nodeFs.Inodes), resource.BinarySI), capacity: resource.NewQuantity(int64(*nodeFs.Inodes), resource.BinarySI),
// TODO: add timestamp to stat (see memory stat) time: nodeFs.Time,
} }
} }
} }
...@@ -681,13 +681,13 @@ func makeSignalObservations(summaryProvider stats.SummaryProvider, nodeProvider ...@@ -681,13 +681,13 @@ func makeSignalObservations(summaryProvider stats.SummaryProvider, nodeProvider
result[evictionapi.SignalImageFsAvailable] = signalObservation{ result[evictionapi.SignalImageFsAvailable] = signalObservation{
available: resource.NewQuantity(int64(*imageFs.AvailableBytes), resource.BinarySI), available: resource.NewQuantity(int64(*imageFs.AvailableBytes), resource.BinarySI),
capacity: resource.NewQuantity(int64(*imageFs.CapacityBytes), resource.BinarySI), capacity: resource.NewQuantity(int64(*imageFs.CapacityBytes), resource.BinarySI),
// TODO: add timestamp to stat (see memory stat) time: imageFs.Time,
} }
if imageFs.InodesFree != nil && imageFs.Inodes != nil { if imageFs.InodesFree != nil && imageFs.Inodes != nil {
result[evictionapi.SignalImageFsInodesFree] = signalObservation{ result[evictionapi.SignalImageFsInodesFree] = signalObservation{
available: resource.NewQuantity(int64(*imageFs.InodesFree), resource.BinarySI), available: resource.NewQuantity(int64(*imageFs.InodesFree), resource.BinarySI),
capacity: resource.NewQuantity(int64(*imageFs.Inodes), resource.BinarySI), capacity: resource.NewQuantity(int64(*imageFs.Inodes), resource.BinarySI),
// TODO: add timestamp to stat (see memory stat) time: imageFs.Time,
} }
} }
} }
......
...@@ -128,12 +128,14 @@ func (sb *summaryBuilder) build() (*stats.Summary, error) { ...@@ -128,12 +128,14 @@ func (sb *summaryBuilder) build() (*stats.Summary, error) {
} }
rootStats := sb.containerInfoV2ToStats("", &rootInfo) rootStats := sb.containerInfoV2ToStats("", &rootInfo)
cStats, _ := sb.latestContainerStats(&rootInfo)
nodeStats := stats.NodeStats{ nodeStats := stats.NodeStats{
NodeName: sb.node.Name, NodeName: sb.node.Name,
CPU: rootStats.CPU, CPU: rootStats.CPU,
Memory: rootStats.Memory, Memory: rootStats.Memory,
Network: sb.containerInfoV2ToNetworkStats("node:"+sb.node.Name, &rootInfo), Network: sb.containerInfoV2ToNetworkStats("node:"+sb.node.Name, &rootInfo),
Fs: &stats.FsStats{ Fs: &stats.FsStats{
Time: metav1.NewTime(cStats.Timestamp),
AvailableBytes: &sb.rootFsInfo.Available, AvailableBytes: &sb.rootFsInfo.Available,
CapacityBytes: &sb.rootFsInfo.Capacity, CapacityBytes: &sb.rootFsInfo.Capacity,
UsedBytes: &sb.rootFsInfo.Usage, UsedBytes: &sb.rootFsInfo.Usage,
...@@ -144,6 +146,7 @@ func (sb *summaryBuilder) build() (*stats.Summary, error) { ...@@ -144,6 +146,7 @@ func (sb *summaryBuilder) build() (*stats.Summary, error) {
StartTime: rootStats.StartTime, StartTime: rootStats.StartTime,
Runtime: &stats.RuntimeStats{ Runtime: &stats.RuntimeStats{
ImageFs: &stats.FsStats{ ImageFs: &stats.FsStats{
Time: metav1.NewTime(cStats.Timestamp),
AvailableBytes: &sb.imageFsInfo.Available, AvailableBytes: &sb.imageFsInfo.Available,
CapacityBytes: &sb.imageFsInfo.Capacity, CapacityBytes: &sb.imageFsInfo.Capacity,
UsedBytes: &sb.imageStats.TotalStorageBytes, UsedBytes: &sb.imageStats.TotalStorageBytes,
...@@ -181,8 +184,14 @@ func (sb *summaryBuilder) containerInfoV2FsStats( ...@@ -181,8 +184,14 @@ func (sb *summaryBuilder) containerInfoV2FsStats(
info *cadvisorapiv2.ContainerInfo, info *cadvisorapiv2.ContainerInfo,
cs *stats.ContainerStats) { cs *stats.ContainerStats) {
lcs, found := sb.latestContainerStats(info)
if !found {
return
}
// The container logs live on the node rootfs device // The container logs live on the node rootfs device
cs.Logs = &stats.FsStats{ cs.Logs = &stats.FsStats{
Time: metav1.NewTime(lcs.Timestamp),
AvailableBytes: &sb.rootFsInfo.Available, AvailableBytes: &sb.rootFsInfo.Available,
CapacityBytes: &sb.rootFsInfo.Capacity, CapacityBytes: &sb.rootFsInfo.Capacity,
InodesFree: sb.rootFsInfo.InodesFree, InodesFree: sb.rootFsInfo.InodesFree,
...@@ -196,15 +205,12 @@ func (sb *summaryBuilder) containerInfoV2FsStats( ...@@ -196,15 +205,12 @@ func (sb *summaryBuilder) containerInfoV2FsStats(
// The container rootFs lives on the imageFs devices (which may not be the node root fs) // The container rootFs lives on the imageFs devices (which may not be the node root fs)
cs.Rootfs = &stats.FsStats{ cs.Rootfs = &stats.FsStats{
Time: metav1.NewTime(lcs.Timestamp),
AvailableBytes: &sb.imageFsInfo.Available, AvailableBytes: &sb.imageFsInfo.Available,
CapacityBytes: &sb.imageFsInfo.Capacity, CapacityBytes: &sb.imageFsInfo.Capacity,
InodesFree: sb.imageFsInfo.InodesFree, InodesFree: sb.imageFsInfo.InodesFree,
Inodes: sb.imageFsInfo.Inodes, Inodes: sb.imageFsInfo.Inodes,
} }
lcs, found := sb.latestContainerStats(info)
if !found {
return
}
cfs := lcs.Filesystem cfs := lcs.Filesystem
if cfs != nil { if cfs != nil {
......
...@@ -120,7 +120,7 @@ func (s *volumeStatCalculator) parsePodVolumeStats(podName string, metric *volum ...@@ -120,7 +120,7 @@ func (s *volumeStatCalculator) parsePodVolumeStats(podName string, metric *volum
inodesUsed := uint64(metric.InodesUsed.Value()) inodesUsed := uint64(metric.InodesUsed.Value())
return stats.VolumeStats{ return stats.VolumeStats{
Name: podName, Name: podName,
FsStats: stats.FsStats{AvailableBytes: &available, CapacityBytes: &capacity, UsedBytes: &used, FsStats: stats.FsStats{Time: metric.Time, AvailableBytes: &available, CapacityBytes: &capacity,
Inodes: &inodes, InodesFree: &inodesFree, InodesUsed: &inodesUsed}, UsedBytes: &used, Inodes: &inodes, InodesFree: &inodesFree, InodesUsed: &inodesUsed},
} }
} }
...@@ -50,7 +50,6 @@ go_test( ...@@ -50,7 +50,6 @@ go_test(
name = "go_default_test", name = "go_default_test",
srcs = [ srcs = [
"metrics_nil_test.go", "metrics_nil_test.go",
"metrics_statfs_test.go",
"plugins_test.go", "plugins_test.go",
"util_test.go", "util_test.go",
], ],
...@@ -66,13 +65,15 @@ go_test( ...@@ -66,13 +65,15 @@ go_test(
"//vendor:k8s.io/apimachinery/pkg/types", "//vendor:k8s.io/apimachinery/pkg/types",
"//vendor:k8s.io/apimachinery/pkg/util/sets", "//vendor:k8s.io/apimachinery/pkg/util/sets",
"//vendor:k8s.io/apimachinery/pkg/watch", "//vendor:k8s.io/apimachinery/pkg/watch",
"//vendor:k8s.io/client-go/util/testing",
], ],
) )
go_test( go_test(
name = "go_default_xtest", name = "go_default_xtest",
srcs = ["metrics_du_test.go"], srcs = [
"metrics_du_test.go",
"metrics_statfs_test.go",
],
tags = ["automanaged"], tags = ["automanaged"],
deps = [ deps = [
"//pkg/volume:go_default_library", "//pkg/volume:go_default_library",
......
...@@ -18,6 +18,7 @@ package volume ...@@ -18,6 +18,7 @@ package volume
import ( import (
"k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/kubernetes/pkg/volume/util" "k8s.io/kubernetes/pkg/volume/util"
) )
...@@ -40,7 +41,7 @@ func NewMetricsDu(path string) MetricsProvider { ...@@ -40,7 +41,7 @@ func NewMetricsDu(path string) MetricsProvider {
// and gathering filesystem info for the Volume path. // and gathering filesystem info for the Volume path.
// See MetricsProvider.GetMetrics // See MetricsProvider.GetMetrics
func (md *metricsDu) GetMetrics() (*Metrics, error) { func (md *metricsDu) GetMetrics() (*Metrics, error) {
metrics := &Metrics{} metrics := &Metrics{Time: metav1.Now()}
if md.path == "" { if md.path == "" {
return metrics, NewNoPathDefinedError() return metrics, NewNoPathDefinedError()
} }
......
...@@ -80,7 +80,7 @@ func TestMetricsDuRequirePath(t *testing.T) { ...@@ -80,7 +80,7 @@ func TestMetricsDuRequirePath(t *testing.T) {
metrics := NewMetricsDu("") metrics := NewMetricsDu("")
actual, err := metrics.GetMetrics() actual, err := metrics.GetMetrics()
expected := &Metrics{} expected := &Metrics{}
if *actual != *expected { if !volumetest.MetricsEqualIgnoreTimestamp(actual, expected) {
t.Errorf("Expected empty Metrics from uninitialized MetricsDu, actual %v", *actual) t.Errorf("Expected empty Metrics from uninitialized MetricsDu, actual %v", *actual)
} }
if err == nil { if err == nil {
...@@ -94,7 +94,7 @@ func TestMetricsDuRequireRealDirectory(t *testing.T) { ...@@ -94,7 +94,7 @@ func TestMetricsDuRequireRealDirectory(t *testing.T) {
metrics := NewMetricsDu("/not/a/real/directory") metrics := NewMetricsDu("/not/a/real/directory")
actual, err := metrics.GetMetrics() actual, err := metrics.GetMetrics()
expected := &Metrics{} expected := &Metrics{}
if *actual != *expected { if !volumetest.MetricsEqualIgnoreTimestamp(actual, expected) {
t.Errorf("Expected empty Metrics from incorrectly initialized MetricsDu, actual %v", *actual) t.Errorf("Expected empty Metrics from incorrectly initialized MetricsDu, actual %v", *actual)
} }
if err == nil { if err == nil {
......
...@@ -18,6 +18,7 @@ package volume ...@@ -18,6 +18,7 @@ package volume
import ( import (
"k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/kubernetes/pkg/volume/util" "k8s.io/kubernetes/pkg/volume/util"
) )
...@@ -39,7 +40,7 @@ func NewMetricsStatFS(path string) MetricsProvider { ...@@ -39,7 +40,7 @@ func NewMetricsStatFS(path string) MetricsProvider {
// GetMetrics calculates the volume usage and device free space by executing "du" // GetMetrics calculates the volume usage and device free space by executing "du"
// and gathering filesystem info for the Volume path. // and gathering filesystem info for the Volume path.
func (md *metricsStatFS) GetMetrics() (*Metrics, error) { func (md *metricsStatFS) GetMetrics() (*Metrics, error) {
metrics := &Metrics{} metrics := &Metrics{Time: metav1.Now()}
if md.path == "" { if md.path == "" {
return metrics, NewNoPathDefinedError() return metrics, NewNoPathDefinedError()
} }
......
...@@ -14,20 +14,22 @@ See the License for the specific language governing permissions and ...@@ -14,20 +14,22 @@ See the License for the specific language governing permissions and
limitations under the License. limitations under the License.
*/ */
package volume package volume_test
import ( import (
"os" "os"
"testing" "testing"
utiltesting "k8s.io/client-go/util/testing" utiltesting "k8s.io/client-go/util/testing"
. "k8s.io/kubernetes/pkg/volume"
volumetest "k8s.io/kubernetes/pkg/volume/testing"
) )
func TestGetMetricsStatFS(t *testing.T) { func TestGetMetricsStatFS(t *testing.T) {
metrics := NewMetricsStatFS("") metrics := NewMetricsStatFS("")
actual, err := metrics.GetMetrics() actual, err := metrics.GetMetrics()
expected := &Metrics{} expected := &Metrics{}
if *actual != *expected { if !volumetest.MetricsEqualIgnoreTimestamp(actual, expected) {
t.Errorf("Expected empty Metrics from uninitialized MetricsStatFS, actual %v", *actual) t.Errorf("Expected empty Metrics from uninitialized MetricsStatFS, actual %v", *actual)
} }
if err == nil { if err == nil {
...@@ -36,7 +38,7 @@ func TestGetMetricsStatFS(t *testing.T) { ...@@ -36,7 +38,7 @@ func TestGetMetricsStatFS(t *testing.T) {
metrics = NewMetricsStatFS("/not/a/real/directory") metrics = NewMetricsStatFS("/not/a/real/directory")
actual, err = metrics.GetMetrics() actual, err = metrics.GetMetrics()
if *actual != *expected { if !volumetest.MetricsEqualIgnoreTimestamp(actual, expected) {
t.Errorf("Expected empty Metrics from incorrectly initialized MetricsStatFS, actual %v", *actual) t.Errorf("Expected empty Metrics from incorrectly initialized MetricsStatFS, actual %v", *actual)
} }
if err == nil { if err == nil {
......
...@@ -754,3 +754,13 @@ func CreateTestPVC(capacity string, accessModes []v1.PersistentVolumeAccessMode) ...@@ -754,3 +754,13 @@ func CreateTestPVC(capacity string, accessModes []v1.PersistentVolumeAccessMode)
} }
return &claim return &claim
} }
func MetricsEqualIgnoreTimestamp(a *Metrics, b *Metrics) bool {
available := a.Available == b.Available
capacity := a.Capacity == b.Capacity
used := a.Used == b.Used
inodes := a.Inodes == b.Inodes
inodesFree := a.InodesFree == b.InodesFree
inodesUsed := a.InodesUsed == b.InodesUsed
return available && capacity && used && inodes && inodesFree && inodesUsed
}
...@@ -26,6 +26,7 @@ import ( ...@@ -26,6 +26,7 @@ import (
"github.com/golang/glog" "github.com/golang/glog"
"k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/types"
"k8s.io/kubernetes/pkg/api/v1" "k8s.io/kubernetes/pkg/api/v1"
) )
...@@ -52,6 +53,9 @@ type MetricsProvider interface { ...@@ -52,6 +53,9 @@ type MetricsProvider interface {
// Metrics represents the used and available bytes of the Volume. // Metrics represents the used and available bytes of the Volume.
type Metrics struct { type Metrics struct {
// The time at which these stats were updated.
Time metav1.Time
// Used represents the total bytes used by the Volume. // Used represents the total bytes used by the Volume.
// Note: For block devices this maybe more than the total size of the files. // Note: For block devices this maybe more than the total size of the files.
Used *resource.Quantity Used *resource.Quantity
......
...@@ -116,6 +116,7 @@ var _ = framework.KubeDescribe("Summary API", func() { ...@@ -116,6 +116,7 @@ var _ = framework.KubeDescribe("Summary API", func() {
"MajorPageFaults": bounded(0, 10), "MajorPageFaults": bounded(0, 10),
}), }),
"Rootfs": ptrMatchAllFields(gstruct.Fields{ "Rootfs": ptrMatchAllFields(gstruct.Fields{
"Time": recent(maxStatsAge),
"AvailableBytes": fsCapacityBounds, "AvailableBytes": fsCapacityBounds,
"CapacityBytes": fsCapacityBounds, "CapacityBytes": fsCapacityBounds,
"UsedBytes": bounded(kb, 10*mb), "UsedBytes": bounded(kb, 10*mb),
...@@ -124,6 +125,7 @@ var _ = framework.KubeDescribe("Summary API", func() { ...@@ -124,6 +125,7 @@ var _ = framework.KubeDescribe("Summary API", func() {
"InodesUsed": bounded(0, 1E8), "InodesUsed": bounded(0, 1E8),
}), }),
"Logs": ptrMatchAllFields(gstruct.Fields{ "Logs": ptrMatchAllFields(gstruct.Fields{
"Time": recent(maxStatsAge),
"AvailableBytes": fsCapacityBounds, "AvailableBytes": fsCapacityBounds,
"CapacityBytes": fsCapacityBounds, "CapacityBytes": fsCapacityBounds,
"UsedBytes": bounded(kb, 10*mb), "UsedBytes": bounded(kb, 10*mb),
...@@ -145,6 +147,7 @@ var _ = framework.KubeDescribe("Summary API", func() { ...@@ -145,6 +147,7 @@ var _ = framework.KubeDescribe("Summary API", func() {
"test-empty-dir": gstruct.MatchAllFields(gstruct.Fields{ "test-empty-dir": gstruct.MatchAllFields(gstruct.Fields{
"Name": Equal("test-empty-dir"), "Name": Equal("test-empty-dir"),
"FsStats": gstruct.MatchAllFields(gstruct.Fields{ "FsStats": gstruct.MatchAllFields(gstruct.Fields{
"Time": recent(maxStatsAge),
"AvailableBytes": fsCapacityBounds, "AvailableBytes": fsCapacityBounds,
"CapacityBytes": fsCapacityBounds, "CapacityBytes": fsCapacityBounds,
"UsedBytes": bounded(kb, 1*mb), "UsedBytes": bounded(kb, 1*mb),
...@@ -183,6 +186,7 @@ var _ = framework.KubeDescribe("Summary API", func() { ...@@ -183,6 +186,7 @@ var _ = framework.KubeDescribe("Summary API", func() {
"TxErrors": bounded(0, 100000), "TxErrors": bounded(0, 100000),
})), })),
"Fs": ptrMatchAllFields(gstruct.Fields{ "Fs": ptrMatchAllFields(gstruct.Fields{
"Time": recent(maxStatsAge),
"AvailableBytes": fsCapacityBounds, "AvailableBytes": fsCapacityBounds,
"CapacityBytes": fsCapacityBounds, "CapacityBytes": fsCapacityBounds,
"UsedBytes": bounded(kb, 10*gb), "UsedBytes": bounded(kb, 10*gb),
...@@ -192,6 +196,7 @@ var _ = framework.KubeDescribe("Summary API", func() { ...@@ -192,6 +196,7 @@ var _ = framework.KubeDescribe("Summary API", func() {
}), }),
"Runtime": ptrMatchAllFields(gstruct.Fields{ "Runtime": ptrMatchAllFields(gstruct.Fields{
"ImageFs": ptrMatchAllFields(gstruct.Fields{ "ImageFs": ptrMatchAllFields(gstruct.Fields{
"Time": recent(maxStatsAge),
"AvailableBytes": fsCapacityBounds, "AvailableBytes": fsCapacityBounds,
"CapacityBytes": fsCapacityBounds, "CapacityBytes": fsCapacityBounds,
"UsedBytes": bounded(kb, 10*gb), "UsedBytes": bounded(kb, 10*gb),
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment