From ec753fcb5588c5ba7beb8d991ded11dea82a709f Mon Sep 17 00:00:00 2001 From: Claudiu Belu Date: Thu, 15 Dec 2022 16:03:37 +0000 Subject: [PATCH] unittests: Fixes unit tests for Windows (part 6) Currently, there are some unit tests that are failing on Windows due to various reasons: - On Windows, consecutive time.Now() calls may return the same timestamp, which would cause the TestFreeSpaceRemoveByLeastRecentlyUsed test to flake. - tests in kuberuntime_container_windows_test.go fail on Nodes that have fewer than 3 CPUs, expecting the CPU max set to be more than 100% of available CPUs, which is not possible. - calls in summary_windows_test.go are missing context. - filterTerminatedContainerInfoAndAssembleByPodCgroupKey will filter and group container information by the Pod cgroup key, if it exists. However, we don't have cgroups on Windows, thus we can't make the same assertions. --- pkg/apis/batch/validation/validation_test.go | 55 ++----------------- pkg/kubelet/images/image_gc_manager_test.go | 8 +++ .../kuberuntime_container_windows_test.go | 5 ++ .../server/stats/summary_windows_test.go | 10 ++-- .../stats/cadvisor_stats_provider_test.go | 16 ++++-- 5 files changed, 33 insertions(+), 61 deletions(-) diff --git a/pkg/apis/batch/validation/validation_test.go b/pkg/apis/batch/validation/validation_test.go index 401950fc8a6..b15f3bdd7b5 100644 --- a/pkg/apis/batch/validation/validation_test.go +++ b/pkg/apis/batch/validation/validation_test.go @@ -19,12 +19,7 @@ package validation import ( _ "time/tzdata" - "archive/zip" "fmt" - "io" - "os" - "path/filepath" - "runtime" "strings" "testing" @@ -44,10 +39,10 @@ var ( timeZoneEmpty = "" timeZoneLocal = "LOCAL" timeZoneUTC = "UTC" - timeZoneCorrect = "Continent/Zone" - timeZoneBadPrefix = " Continent/Zone" - timeZoneBadSuffix = "Continent/Zone " - timeZoneBadName = "Continent/InvalidZone" + timeZoneCorrect = "Europe/Rome" + timeZoneBadPrefix = " Europe/Rome" + timeZoneBadSuffix = "Europe/Rome " + timeZoneBadName = "Europe/InvalidRome" timeZoneEmptySpace = " " ) @@ -1444,12 +1439,6 @@ func TestValidateCronJob(t *testing.T) { validPodTemplateSpec := getValidPodTemplateSpecForGenerated(getValidGeneratedSelector()) validPodTemplateSpec.Labels = map[string]string{} - zoneDir := t.TempDir() - if err := setupFakeTimeZoneDatabase(zoneDir); err != nil { - t.Fatalf("Unexpected error setting up fake timezone database: %v", err) - } - t.Setenv("ZONEINFO", zoneDir) - successCases := map[string]batch.CronJob{ "basic scheduled job": { ObjectMeta: metav1.ObjectMeta{ @@ -1942,42 +1931,6 @@ func TestValidateCronJob(t *testing.T) { } } -// Sets up fake timezone database in a zoneDir directory with a single valid -// time zone called "Continent/Zone" by copying UTC metadata from golang's -// built-in databse. Returns an error in case of problems. -func setupFakeTimeZoneDatabase(zoneDir string) error { - reader, err := zip.OpenReader(runtime.GOROOT() + "/lib/time/zoneinfo.zip") - if err != nil { - return err - } - defer reader.Close() - - if err := os.Mkdir(filepath.Join(zoneDir, "Continent"), os.ModePerm); err != nil { - return err - } - zoneFile, err := os.OpenFile(filepath.Join(zoneDir, "Continent", "Zone"), os.O_RDWR|os.O_TRUNC|os.O_CREATE, 0666) - if err != nil { - return err - } - defer zoneFile.Close() - - for _, file := range reader.File { - if file.Name != "UTC" { - continue - } - rc, err := file.Open() - if err != nil { - return err - } - if _, err := io.Copy(zoneFile, rc); err != nil { - return err - } - rc.Close() - break - } - return nil -} - func TestValidateCronJobSpec(t *testing.T) { validPodTemplateSpec := getValidPodTemplateSpecForGenerated(getValidGeneratedSelector()) validPodTemplateSpec.Labels = map[string]string{} diff --git a/pkg/kubelet/images/image_gc_manager_test.go b/pkg/kubelet/images/image_gc_manager_test.go index 2d2c08be387..a2dbf7c8d7d 100644 --- a/pkg/kubelet/images/image_gc_manager_test.go +++ b/pkg/kubelet/images/image_gc_manager_test.go @@ -19,6 +19,7 @@ package images import ( "context" "fmt" + goruntime "runtime" "testing" "time" @@ -464,6 +465,13 @@ func TestFreeSpaceRemoveByLeastRecentlyUsed(t *testing.T) { }, }}, } + // manager.detectImages uses time.Now() to update the image's lastUsed field. + // On Windows, consecutive time.Now() calls can return the same timestamp, which would mean + // that the second image is NOT newer than the first one. + // time.Sleep will result in the timestamp to be updated as well. + if goruntime.GOOS == "windows" { + time.Sleep(time.Millisecond) + } _, err = manager.detectImages(ctx, time.Now()) require.NoError(t, err) fakeRuntime.AllPodList = []*containertest.FakePod{ diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container_windows_test.go b/pkg/kubelet/kuberuntime/kuberuntime_container_windows_test.go index 7a303e06152..3f50c7ef00e 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container_windows_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container_windows_test.go @@ -86,6 +86,11 @@ func TestApplyPlatformSpecificContainerConfig(t *testing.T) { limit := int64(3000) expectedCpuMax := 10 * limit / int64(winstats.ProcessorCount()) + // Above, we're setting the limit to 3 CPUs. But we can't expect more than 100% of the CPUs + // we have. (e.g.: if we only have 2 CPUs, we can't have 150% CPU max). + if expectedCpuMax > 10000 { + expectedCpuMax = 10000 + } expectedWindowsConfig := &runtimeapi.WindowsContainerConfig{ Resources: &runtimeapi.WindowsContainerResources{ CpuMaximum: expectedCpuMax, diff --git a/pkg/kubelet/server/stats/summary_windows_test.go b/pkg/kubelet/server/stats/summary_windows_test.go index 27fe25c7618..00d2355bc61 100644 --- a/pkg/kubelet/server/stats/summary_windows_test.go +++ b/pkg/kubelet/server/stats/summary_windows_test.go @@ -20,6 +20,7 @@ limitations under the License. package stats import ( + "context" "testing" "time" @@ -36,6 +37,7 @@ import ( func TestSummaryProvider(t *testing.T) { var ( + ctx = context.Background() podStats = []statsapi.PodStats{*getPodStats()} imageFsStats = getFsStats() rootFsStats = getFsStats() @@ -60,9 +62,9 @@ func TestSummaryProvider(t *testing.T) { mockStatsProvider.EXPECT().GetNode().Return(node, nil).AnyTimes() mockStatsProvider.EXPECT().GetNodeConfig().Return(nodeConfig).AnyTimes() mockStatsProvider.EXPECT().GetPodCgroupRoot().Return(cgroupRoot).AnyTimes() - mockStatsProvider.EXPECT().ListPodStats().Return(podStats, nil).AnyTimes() - mockStatsProvider.EXPECT().ListPodStatsAndUpdateCPUNanoCoreUsage().Return(podStats, nil).AnyTimes() - mockStatsProvider.EXPECT().ImageFsStats().Return(imageFsStats, nil).AnyTimes() + mockStatsProvider.EXPECT().ListPodStats(ctx).Return(podStats, nil).AnyTimes() + mockStatsProvider.EXPECT().ListPodStatsAndUpdateCPUNanoCoreUsage(ctx).Return(podStats, nil).AnyTimes() + mockStatsProvider.EXPECT().ImageFsStats(ctx).Return(imageFsStats, nil).AnyTimes() mockStatsProvider.EXPECT().RootFsStats().Return(rootFsStats, nil).AnyTimes() mockStatsProvider.EXPECT().RlimitStats().Return(nil, nil).AnyTimes() mockStatsProvider.EXPECT().GetCgroupStats("/", true).Return(cgroupStatsMap["/"].cs, cgroupStatsMap["/"].ns, nil).AnyTimes() @@ -70,7 +72,7 @@ func TestSummaryProvider(t *testing.T) { kubeletCreationTime := metav1.Now() systemBootTime := metav1.Now() provider := summaryProviderImpl{kubeletCreationTime: kubeletCreationTime, systemBootTime: systemBootTime, provider: mockStatsProvider} - summary, err := provider.Get(true) + summary, err := provider.Get(ctx, true) assert.NoError(err) assert.Equal(summary.Node.NodeName, "test-node") diff --git a/pkg/kubelet/stats/cadvisor_stats_provider_test.go b/pkg/kubelet/stats/cadvisor_stats_provider_test.go index 523cdd036c8..8eab29909cc 100644 --- a/pkg/kubelet/stats/cadvisor_stats_provider_test.go +++ b/pkg/kubelet/stats/cadvisor_stats_provider_test.go @@ -40,11 +40,6 @@ import ( ) func TestFilterTerminatedContainerInfoAndAssembleByPodCgroupKey(t *testing.T) { - // Skip tests that fail on Windows, as discussed during the SIG Testing meeting from January 10, 2023 - if runtime.GOOS == "windows" { - t.Skip("Skipping test that fails on Windows") - } - const ( seedPastPod0Infra = 1000 seedPastPod0Container0 = 2000 @@ -98,7 +93,16 @@ func TestFilterTerminatedContainerInfoAndAssembleByPodCgroupKey(t *testing.T) { t.Errorf("%q is expected to be in the output\n", c) } } - for _, c := range []string{"pod0-i-terminated-1", "pod0-c0-terminated-1", "pod0-i-terminated-2", "pod0-c0-terminated-2", "pod0-i", "pod0-c0", "c1"} { + + expectedInfoKeys := []string{"pod0-i-terminated-1", "pod0-c0-terminated-1", "pod0-i-terminated-2", "pod0-c0-terminated-2", "pod0-i", "pod0-c0"} + // NOTE: on Windows, IsSystemdStyleName will return false, which means that the Container Info will + // not be assembled by cgroup key. + if runtime.GOOS != "windows" { + expectedInfoKeys = append(expectedInfoKeys, "c1") + } else { + expectedInfoKeys = append(expectedInfoKeys, "pod1-c1.slice") + } + for _, c := range expectedInfoKeys { if _, found := allInfos[c]; !found { t.Errorf("%q is expected to be in the output\n", c) }