From c9eff4e906618e9d6f44fd1a7de07046093cfe15 Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Thu, 18 Feb 2021 17:03:58 -0800 Subject: [PATCH] Get filesystem stats for files on Windows --- pkg/kubelet/kubelet.go | 4 +- pkg/kubelet/stats/host_stats_provider.go | 17 ++--- pkg/kubelet/stats/host_stats_provider_test.go | 67 +++++++++++++++++++ pkg/volume/util/fs/fs_windows.go | 6 ++ pkg/volume/util/fs/fs_windows_test.go | 58 ++++++++++++++-- test/e2e/windows/kubelet_stats.go | 5 ++ 6 files changed, 142 insertions(+), 15 deletions(-) create mode 100644 pkg/kubelet/stats/host_stats_provider_test.go diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index a1dd15cb5c7..0c9b029286c 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -664,8 +664,8 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration, klet.runtimeCache = runtimeCache // common provider to get host file system usage associated with a pod managed by kubelet - hostStatsProvider := stats.NewHostStatsProvider(kubecontainer.RealOS{}, func(podUID types.UID) string { - return getEtcHostsPath(klet.getPodDir(podUID)) + hostStatsProvider := stats.NewHostStatsProvider(kubecontainer.RealOS{}, func(podUID types.UID) (string, bool) { + return getEtcHostsPath(klet.getPodDir(podUID)), klet.containerRuntime.SupportsSingleFileMapping() }) if kubeDeps.useLegacyCadvisorStats { klet.StatsProvider = stats.NewCadvisorStatsProvider( diff --git a/pkg/kubelet/stats/host_stats_provider.go b/pkg/kubelet/stats/host_stats_provider.go index 9c03fb542a4..108fbfb8f14 100644 --- a/pkg/kubelet/stats/host_stats_provider.go +++ b/pkg/kubelet/stats/host_stats_provider.go @@ -29,8 +29,8 @@ import ( "k8s.io/kubernetes/pkg/volume" ) -// PodEtcHostsPathFunc is a function to fetch a etc hosts path by pod uid. -type PodEtcHostsPathFunc func(podUID types.UID) string +// PodEtcHostsPathFunc is a function to fetch a etc hosts path by pod uid and whether etc host path is supported by the runtime +type PodEtcHostsPathFunc func(podUID types.UID) (string, bool) // metricsProviderByPath maps a path to its metrics provider type metricsProviderByPath map[string]volume.MetricsProvider @@ -79,7 +79,13 @@ func (h hostStatsProvider) getPodContainerLogStats(podNamespace, podName string, // getPodEtcHostsStats gets status for pod etc hosts usage func (h hostStatsProvider) getPodEtcHostsStats(podUID types.UID, rootFsInfo *cadvisorapiv2.FsInfo) (*statsapi.FsStats, error) { - metrics := h.podEtcHostsMetrics(podUID) + // Runtimes may not support etc hosts file (Windows with docker) + podEtcHostsPath, isEtcHostsSupported := h.podEtcHostsPathFunc(podUID) + if !isEtcHostsSupported { + return nil, nil + } + + metrics := volume.NewMetricsDu(podEtcHostsPath) hostMetrics, err := metrics.GetMetrics() if err != nil { return nil, fmt.Errorf("failed to get stats %v", err) @@ -103,11 +109,6 @@ func (h hostStatsProvider) podContainerLogMetrics(podNamespace, podName string, return h.fileMetricsByDir(podContainerLogsDirectoryPath) } -func (h hostStatsProvider) podEtcHostsMetrics(podUID types.UID) volume.MetricsProvider { - podEtcHostsPath := h.podEtcHostsPathFunc(podUID) - return volume.NewMetricsDu(podEtcHostsPath) -} - // fileMetricsByDir returns metrics by path for each file under specified directory func (h hostStatsProvider) fileMetricsByDir(dirname string) (metricsProviderByPath, error) { files, err := h.osInterface.ReadDir(dirname) diff --git a/pkg/kubelet/stats/host_stats_provider_test.go b/pkg/kubelet/stats/host_stats_provider_test.go new file mode 100644 index 00000000000..72714a1c730 --- /dev/null +++ b/pkg/kubelet/stats/host_stats_provider_test.go @@ -0,0 +1,67 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package stats + +import ( + "reflect" + "testing" + + cadvisorapiv2 "github.com/google/cadvisor/info/v2" + + kubecontainertest "k8s.io/kubernetes/pkg/kubelet/container/testing" + + "k8s.io/apimachinery/pkg/types" + statsapi "k8s.io/kubelet/pkg/apis/stats/v1alpha1" +) + +func Test_hostStatsProvider_getPodEtcHostsStats(t *testing.T) { + tests := []struct { + name string + podEtcHostsPathFunc PodEtcHostsPathFunc + podUID types.UID + rootFsInfo *cadvisorapiv2.FsInfo + want *statsapi.FsStats + wantErr bool + }{ + { + name: "Should return nil for runtimes that do not support etc host file", + podEtcHostsPathFunc: func(podUID types.UID) (string, bool) { + return "", false + }, + podUID: "fake0001", + rootFsInfo: nil, + want: nil, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + h := hostStatsProvider{ + osInterface: &kubecontainertest.FakeOS{}, + podEtcHostsPathFunc: tt.podEtcHostsPathFunc, + } + got, err := h.getPodEtcHostsStats(tt.podUID, tt.rootFsInfo) + if (err != nil) != tt.wantErr { + t.Errorf("getPodEtcHostsStats() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("getPodEtcHostsStats() got = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/volume/util/fs/fs_windows.go b/pkg/volume/util/fs/fs_windows.go index 0fab9bcb6ce..8d16eabcefe 100644 --- a/pkg/volume/util/fs/fs_windows.go +++ b/pkg/volume/util/fs/fs_windows.go @@ -21,6 +21,7 @@ package fs import ( "fmt" "os" + "path/filepath" "syscall" "unsafe" @@ -40,6 +41,11 @@ func Info(path string) (int64, int64, int64, int64, int64, int64, error) { var freeBytesAvailable, totalNumberOfBytes, totalNumberOfFreeBytes int64 var err error + // The equivalent linux call supports calls against files but the syscall for windows + // fails for files with error code: The directory name is invalid. (#99173) + // https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499- + // By always ensuring the directory path we meet all uses cases of this function + path = filepath.Dir(path) ret, _, err := syscall.Syscall6( procGetDiskFreeSpaceEx.Addr(), 4, diff --git a/pkg/volume/util/fs/fs_windows_test.go b/pkg/volume/util/fs/fs_windows_test.go index 1fd47681962..df57d82b548 100644 --- a/pkg/volume/util/fs/fs_windows_test.go +++ b/pkg/volume/util/fs/fs_windows_test.go @@ -21,8 +21,9 @@ import ( "io/ioutil" "os" - "k8s.io/apimachinery/pkg/api/resource" "testing" + + "k8s.io/apimachinery/pkg/api/resource" ) func TestDiskUsage(t *testing.T) { @@ -54,8 +55,8 @@ func TestDiskUsage(t *testing.T) { if err != nil { t.Fatalf("TestDiskUsage failed: %s", err.Error()) } - file1 := dir1 + "/" + "test" - file2 := dir2 + "/" + "test" + file1 := tmpfile1.Name() + file2 := tmpfile2.Name() fileInfo1, err := os.Lstat(file1) if err != nil { t.Fatalf("TestDiskUsage failed: %s", err.Error()) @@ -74,8 +75,55 @@ func TestDiskUsage(t *testing.T) { if err != nil { t.Fatalf("TestDiskUsage failed: %s", err.Error()) } - if size.Cmp(used) != 1 { - t.Fatalf("TestDiskUsage failed: %s", err.Error()) + if size.Cmp(used) != 0 { + t.Fatalf("TestDiskUsage failed: expected 0, got %d", size.Cmp(used)) + } +} + +func TestInfo(t *testing.T) { + dir1, err := ioutil.TempDir("", "dir_1") + if err != nil { + t.Fatalf("TestInfo failed: %s", err.Error()) + } + defer os.RemoveAll(dir1) + + // should pass for folder path + availablebytes, capacity, usage, inodesTotal, inodesFree, inodeUsage, err := Info(dir1) + if err != nil { + t.Errorf("Info() should not error = %v", err) + return + } + validateInfo(t, availablebytes, capacity, usage, inodesTotal, inodeUsage, inodesFree) + + // should pass for file + tmpfile1, err := ioutil.TempFile(dir1, "test") + if _, err = tmpfile1.WriteString("just for testing"); err != nil { + t.Fatalf("TestInfo failed: %s", err.Error()) + } + availablebytes, capacity, usage, inodesTotal, inodesFree, inodeUsage, err = Info(tmpfile1.Name()) + validateInfo(t, availablebytes, capacity, usage, inodesTotal, inodeUsage, inodesFree) +} + +func validateInfo(t *testing.T, availablebytes int64, capacity int64, usage int64, inodesTotal int64, inodeUsage int64, inodesFree int64) { + // All of these should be greater than zero on a real system + if availablebytes <= 0 { + t.Errorf("Info() availablebytes should be greater than 0, got %v", availablebytes) + } + if capacity <= 0 { + t.Errorf("Info() capacity should be greater than 0, got %v", capacity) + } + if usage <= 0 { + t.Errorf("Info() got usage should be greater than 0, got %v", usage) } + // inodes will always be zero for Windows + if inodesTotal != 0 { + t.Errorf("Info() inodesTotal = %v, want %v", inodeUsage, 0) + } + if inodesFree != 0 { + t.Errorf("Info() inodesFree = %v, want %v", inodesFree, 0) + } + if inodeUsage != 0 { + t.Errorf("Info() inodeUsage = %v, want %v", inodeUsage, 0) + } } diff --git a/test/e2e/windows/kubelet_stats.go b/test/e2e/windows/kubelet_stats.go index 238cebfbf5f..0abbc250d47 100644 --- a/test/e2e/windows/kubelet_stats.go +++ b/test/e2e/windows/kubelet_stats.go @@ -79,6 +79,11 @@ var _ = SIGDescribe("[Feature:Windows] Kubelet-Stats [Serial]", func() { framework.ExpectEqual(*podStats.CPU.UsageCoreNanoSeconds > 0, true, "Pod stats should not report 0 cpu usage") framework.ExpectEqual(*podStats.Memory.WorkingSetBytes > 0, true, "Pod stats should not report 0 bytes for memory working set ") + + for _, containerStats := range podStats.Containers { + framework.ExpectEqual(containerStats.Logs != nil, true, "Pod stats should have container log stats") + framework.ExpectEqual(*containerStats.Logs.AvailableBytes > 0, true, "container log stats should not report 0 bytes for AvailableBytes") + } } framework.ExpectEqual(statsChecked, 10, "Should find stats for 10 pods in kubelet stats")