diff --git a/pkg/kubelet/cm/devicemanager/manager_test.go b/pkg/kubelet/cm/devicemanager/manager_test.go index ec03d8b5eae..5d12ea318c4 100644 --- a/pkg/kubelet/cm/devicemanager/manager_test.go +++ b/pkg/kubelet/cm/devicemanager/manager_test.go @@ -81,8 +81,8 @@ func tmpSocketDir() (socketDir, socketName, pluginSocketName string, err error) if err != nil { return } - socketName = socketDir + "/server.sock" - pluginSocketName = socketDir + "/device-plugin.sock" + socketName = filepath.Join(socketDir, "server.sock") + pluginSocketName = filepath.Join(socketDir, "device-plugin.sock") os.MkdirAll(socketDir, 0755) return } diff --git a/pkg/kubelet/pluginmanager/pluginwatcher/plugin_watcher_test.go b/pkg/kubelet/pluginmanager/pluginwatcher/plugin_watcher_test.go index 1fa883cdb99..b9e2c764f61 100644 --- a/pkg/kubelet/pluginmanager/pluginwatcher/plugin_watcher_test.go +++ b/pkg/kubelet/pluginmanager/pluginwatcher/plugin_watcher_test.go @@ -20,6 +20,7 @@ import ( "flag" "fmt" "os" + "path/filepath" "sync" "testing" "time" @@ -32,8 +33,6 @@ import ( ) var ( - socketDir string - supportedVersions = []string{"v1beta1", "v1beta2"} ) @@ -44,18 +43,17 @@ func init() { flag.Set("alsologtostderr", fmt.Sprintf("%t", true)) flag.StringVar(&logLevel, "logLevel", "6", "test") flag.Lookup("v").Value.Set(logLevel) - - d, err := os.MkdirTemp("", "plugin_test") - if err != nil { - panic(fmt.Sprintf("Could not create a temp directory: %s", d)) - } - - socketDir = d } -func cleanup(t *testing.T) { - require.NoError(t, os.RemoveAll(socketDir)) - os.MkdirAll(socketDir, 0755) +func initTempDir(t *testing.T) string { + // Creating a different directory. os.RemoveAll is not atomic enough; + // os.MkdirAll can get into an "Access Denied" error on Windows. + d, err := os.MkdirTemp("", "plugin_test") + if err != nil { + t.Fatalf("Could not create a temp directory %s: %v", d, err) + } + + return d } func waitForRegistration( @@ -106,13 +104,14 @@ func retryWithExponentialBackOff(initialDuration time.Duration, fn wait.Conditio } func TestPluginRegistration(t *testing.T) { - defer cleanup(t) + socketDir := initTempDir(t) + defer os.RemoveAll(socketDir) dsw := cache.NewDesiredStateOfWorld() - newWatcher(t, dsw, wait.NeverStop) + newWatcher(t, socketDir, dsw, wait.NeverStop) for i := 0; i < 10; i++ { - socketPath := fmt.Sprintf("%s/plugin-%d.sock", socketDir, i) + socketPath := filepath.Join(socketDir, fmt.Sprintf("plugin-%d.sock", i)) pluginName := fmt.Sprintf("example-plugin-%d", i) p := NewTestExamplePlugin(pluginName, registerapi.DevicePlugin, socketPath, supportedVersions...) @@ -139,16 +138,17 @@ func TestPluginRegistration(t *testing.T) { } func TestPluginRegistrationSameName(t *testing.T) { - defer cleanup(t) + socketDir := initTempDir(t) + defer os.RemoveAll(socketDir) dsw := cache.NewDesiredStateOfWorld() - newWatcher(t, dsw, wait.NeverStop) + newWatcher(t, socketDir, dsw, wait.NeverStop) // Make 10 plugins with the same name and same type but different socket path; // all 10 should be in desired state of world cache pluginName := "dep-example-plugin" for i := 0; i < 10; i++ { - socketPath := fmt.Sprintf("%s/plugin-%d.sock", socketDir, i) + socketPath := filepath.Join(socketDir, fmt.Sprintf("plugin-%d.sock", i)) p := NewTestExamplePlugin(pluginName, registerapi.DevicePlugin, socketPath, supportedVersions...) require.NoError(t, p.Serve("v1beta1", "v1beta2")) @@ -164,14 +164,15 @@ func TestPluginRegistrationSameName(t *testing.T) { } func TestPluginReRegistration(t *testing.T) { - defer cleanup(t) + socketDir := initTempDir(t) + defer os.RemoveAll(socketDir) dsw := cache.NewDesiredStateOfWorld() - newWatcher(t, dsw, wait.NeverStop) + newWatcher(t, socketDir, dsw, wait.NeverStop) // Create a plugin first, we are then going to remove the plugin, update the plugin with a different name // and recreate it. - socketPath := fmt.Sprintf("%s/plugin-reregistration.sock", socketDir) + socketPath := filepath.Join(socketDir, "plugin-reregistration.sock") pluginName := "reregister-plugin" p := NewTestExamplePlugin(pluginName, registerapi.DevicePlugin, socketPath, supportedVersions...) require.NoError(t, p.Serve("v1beta1", "v1beta2")) @@ -206,12 +207,13 @@ func TestPluginReRegistration(t *testing.T) { } func TestPluginRegistrationAtKubeletStart(t *testing.T) { - defer cleanup(t) + socketDir := initTempDir(t) + defer os.RemoveAll(socketDir) plugins := make([]*examplePlugin, 10) for i := 0; i < len(plugins); i++ { - socketPath := fmt.Sprintf("%s/plugin-%d.sock", socketDir, i) + socketPath := filepath.Join(socketDir, fmt.Sprintf("plugin-%d.sock", i)) pluginName := fmt.Sprintf("example-plugin-%d", i) p := NewTestExamplePlugin(pluginName, registerapi.DevicePlugin, socketPath, supportedVersions...) @@ -224,7 +226,7 @@ func TestPluginRegistrationAtKubeletStart(t *testing.T) { } dsw := cache.NewDesiredStateOfWorld() - newWatcher(t, dsw, wait.NeverStop) + newWatcher(t, socketDir, dsw, wait.NeverStop) var wg sync.WaitGroup for i := 0; i < len(plugins); i++ { @@ -252,7 +254,7 @@ func TestPluginRegistrationAtKubeletStart(t *testing.T) { } } -func newWatcher(t *testing.T, desiredStateOfWorldCache cache.DesiredStateOfWorld, stopCh <-chan struct{}) *Watcher { +func newWatcher(t *testing.T, socketDir string, desiredStateOfWorldCache cache.DesiredStateOfWorld, stopCh <-chan struct{}) *Watcher { w := NewWatcher(socketDir, desiredStateOfWorldCache) require.NoError(t, w.Start(stopCh)) diff --git a/pkg/kubelet/pluginmanager/reconciler/reconciler_test.go b/pkg/kubelet/pluginmanager/reconciler/reconciler_test.go index 56c9c6dd80d..e0c95eb1a1d 100644 --- a/pkg/kubelet/pluginmanager/reconciler/reconciler_test.go +++ b/pkg/kubelet/pluginmanager/reconciler/reconciler_test.go @@ -19,6 +19,7 @@ package reconciler import ( "fmt" "os" + "path/filepath" "testing" "time" @@ -187,7 +188,7 @@ func Test_Run_Positive_Register(t *testing.T) { stopChan := make(chan struct{}) defer close(stopChan) go reconciler.Run(stopChan) - socketPath := fmt.Sprintf("%s/plugin.sock", socketDir) + socketPath := filepath.Join(socketDir, "plugin.sock") pluginName := fmt.Sprintf("example-plugin") p := pluginwatcher.NewTestExamplePlugin(pluginName, registerapi.DevicePlugin, socketPath, supportedVersions...) require.NoError(t, p.Serve("v1beta1", "v1beta2")) @@ -233,7 +234,7 @@ func Test_Run_Positive_RegisterThenUnregister(t *testing.T) { defer close(stopChan) go reconciler.Run(stopChan) - socketPath := fmt.Sprintf("%s/plugin.sock", socketDir) + socketPath := filepath.Join(socketDir, "plugin.sock") pluginName := fmt.Sprintf("example-plugin") p := pluginwatcher.NewTestExamplePlugin(pluginName, registerapi.DevicePlugin, socketPath, supportedVersions...) require.NoError(t, p.Serve("v1beta1", "v1beta2")) @@ -289,7 +290,7 @@ func Test_Run_Positive_ReRegister(t *testing.T) { defer close(stopChan) go reconciler.Run(stopChan) - socketPath := fmt.Sprintf("%s/plugin2.sock", socketDir) + socketPath := filepath.Join(socketDir, "plugin2.sock") pluginName := fmt.Sprintf("example-plugin2") p := pluginwatcher.NewTestExamplePlugin(pluginName, registerapi.DevicePlugin, socketPath, supportedVersions...) require.NoError(t, p.Serve("v1beta1", "v1beta2")) diff --git a/pkg/proxy/apis/config/validation/validation_test.go b/pkg/proxy/apis/config/validation/validation_test.go index f340067bb41..b07dec4cc21 100644 --- a/pkg/proxy/apis/config/validation/validation_test.go +++ b/pkg/proxy/apis/config/validation/validation_test.go @@ -427,6 +427,11 @@ func TestValidateKubeProxyConfiguration(t *testing.T) { } for name, testCase := range testCases { + if runtime.GOOS == "windows" && testCase.config.Mode == kubeproxyconfig.ProxyModeIPVS { + // IPVS is not supported on Windows. + t.Log("Skipping test on Windows: ", name) + continue + } t.Run(name, func(t *testing.T) { errs := Validate(&testCase.config) if len(testCase.expectedErrs) != len(errs) { @@ -694,9 +699,11 @@ func TestValidateKubeProxyConntrackConfiguration(t *testing.T) { func TestValidateProxyMode(t *testing.T) { newPath := field.NewPath("KubeProxyConfiguration") successCases := []kubeproxyconfig.ProxyMode{""} + expectedNonExistentErrorMsg := "must be iptables,ipvs or blank (blank means the best-available proxy [currently iptables])" if runtime.GOOS == "windows" { successCases = append(successCases, kubeproxyconfig.ProxyModeKernelspace) + expectedNonExistentErrorMsg = "must be kernelspace or blank (blank means the most-available proxy [currently kernelspace])" } else { successCases = append(successCases, kubeproxyconfig.ProxyModeIPTables, kubeproxyconfig.ProxyModeIPVS) } @@ -717,7 +724,7 @@ func TestValidateProxyMode(t *testing.T) { }, "invalid mode non-existent": { mode: kubeproxyconfig.ProxyMode("non-existing"), - expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ProxyMode"), "non-existing", "must be iptables,ipvs or blank (blank means the best-available proxy [currently iptables])")}, + expectedErrs: field.ErrorList{field.Invalid(newPath.Child("ProxyMode"), "non-existing", expectedNonExistentErrorMsg)}, }, } for _, testCase := range testCases { diff --git a/pkg/proxy/winkernel/hns_test.go b/pkg/proxy/winkernel/hns_test.go index 749b0c23c2f..47bccc7920b 100644 --- a/pkg/proxy/winkernel/hns_test.go +++ b/pkg/proxy/winkernel/hns_test.go @@ -132,11 +132,11 @@ func TestGetEndpointByIpAddressAndName(t *testing.T) { t.Errorf("%v does not match %v", endpoint.ip, Endpoint.IpConfigurations[0].IpAddress) } - endpoint, err = hns.getEndpointByName(Endpoint.Name) + endpoint2, err := hns.getEndpointByName(Endpoint.Name) if err != nil { t.Error(err) } - diff := cmp.Diff(endpoint, Endpoint) + diff := cmp.Diff(endpoint, endpoint2) if diff != "" { t.Errorf("getEndpointByName(%s) returned a different endpoint. Diff: %s ", Endpoint.Name, diff) } diff --git a/pkg/proxy/winkernel/proxier_test.go b/pkg/proxy/winkernel/proxier_test.go index 29218803d6d..a7ac7b36528 100644 --- a/pkg/proxy/winkernel/proxier_test.go +++ b/pkg/proxy/winkernel/proxier_test.go @@ -138,18 +138,19 @@ func NewFakeProxier(syncPeriod time.Duration, minSyncPeriod time.Duration, clust networkType: networkType, } proxier := &Proxier{ - serviceMap: make(proxy.ServiceMap), - endpointsMap: make(proxy.EndpointsMap), - clusterCIDR: clusterCIDR, - hostname: testHostName, - nodeIP: nodeIP, - serviceHealthServer: healthcheck.NewFakeServiceHealthServer(), - network: *hnsNetworkInfo, - sourceVip: sourceVip, - hostMac: macAddress, - isDSR: false, - hns: newFakeHNS(), - endPointsRefCount: make(endPointsReferenceCountMap), + serviceMap: make(proxy.ServiceMap), + endpointsMap: make(proxy.EndpointsMap), + clusterCIDR: clusterCIDR, + hostname: testHostName, + nodeIP: nodeIP, + serviceHealthServer: healthcheck.NewFakeServiceHealthServer(), + network: *hnsNetworkInfo, + sourceVip: sourceVip, + hostMac: macAddress, + isDSR: false, + hns: newFakeHNS(), + endPointsRefCount: make(endPointsReferenceCountMap), + forwardHealthCheckVip: true, } serviceChanges := proxy.NewServiceChangeTracker(proxier.newServiceInfo, v1.IPv4Protocol, nil, proxier.serviceMapChange) diff --git a/pkg/routes/const_other.go b/pkg/routes/const_other.go new file mode 100644 index 00000000000..fff6446e54f --- /dev/null +++ b/pkg/routes/const_other.go @@ -0,0 +1,26 @@ +//go:build !windows +// +build !windows + +/* +Copyright 2022 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 routes + +import "syscall" + +const ( + fileNameTooLong = syscall.ENAMETOOLONG +) diff --git a/pkg/routes/const_windows.go b/pkg/routes/const_windows.go new file mode 100644 index 00000000000..32662b8ad8c --- /dev/null +++ b/pkg/routes/const_windows.go @@ -0,0 +1,23 @@ +/* +Copyright 2022 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 routes + +import "golang.org/x/sys/windows" + +const ( + fileNameTooLong = windows.ERROR_FILENAME_EXCED_RANGE +) diff --git a/pkg/routes/logs.go b/pkg/routes/logs.go index cc649912518..95635d453d1 100644 --- a/pkg/routes/logs.go +++ b/pkg/routes/logs.go @@ -20,7 +20,6 @@ import ( "net/http" "os" "path" - "syscall" "github.com/emicklei/go-restful/v3" ) @@ -63,7 +62,7 @@ func logFileListHandler(req *restful.Request, resp *restful.Response) { func logFileNameIsTooLong(filePath string) bool { _, err := os.Stat(filePath) if err != nil { - if e, ok := err.(*os.PathError); ok && e.Err == syscall.ENAMETOOLONG { + if e, ok := err.(*os.PathError); ok && e.Err == fileNameTooLong { return true } } diff --git a/pkg/routes/logs_test.go b/pkg/routes/logs_test.go index 0b2ede3023a..6fbbaad7f4c 100644 --- a/pkg/routes/logs_test.go +++ b/pkg/routes/logs_test.go @@ -19,11 +19,13 @@ package routes import ( "fmt" "os" + "path/filepath" "testing" ) func TestPreCheckLogFileNameLength(t *testing.T) { - oversizeFileName := fmt.Sprintf("%0256s", "a") + // In windows, with long file name support enabled, file names can have up to 32,767 characters. + oversizeFileName := fmt.Sprintf("%032768s", "a") normalFileName := fmt.Sprintf("%0255s", "a") // check file with oversize name. @@ -37,11 +39,19 @@ func TestPreCheckLogFileNameLength(t *testing.T) { } // check file with normal name which does exist. - _, err := os.Create(normalFileName) + dir, err := os.MkdirTemp("", "logs") + if err != nil { + t.Fatal("failed to create temp dir") + } + defer os.RemoveAll(dir) + + normalFileName = filepath.Join(dir, normalFileName) + f, err := os.Create(normalFileName) if err != nil { t.Error("failed to create test file") } defer os.Remove(normalFileName) + defer f.Close() if logFileNameIsTooLong(normalFileName) { t.Error("failed to check normal filename") } diff --git a/pkg/scheduler/internal/queue/scheduling_queue_test.go b/pkg/scheduler/internal/queue/scheduling_queue_test.go index 15261b7ecb3..5b0a1832cbe 100644 --- a/pkg/scheduler/internal/queue/scheduling_queue_test.go +++ b/pkg/scheduler/internal/queue/scheduling_queue_test.go @@ -1957,8 +1957,14 @@ func TestMoveAllToActiveOrBackoffQueue_PreEnqueueChecks(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() q := NewTestQueue(ctx, newDefaultQueueSort()) - for _, podInfo := range tt.podInfos { + for i, podInfo := range tt.podInfos { q.AddUnschedulableIfNotPresent(podInfo, q.schedulingCycle) + // NOTE: On Windows, time.Now() is not as precise, 2 consecutive calls may return the same timestamp, + // resulting in 0 time delta / latency. This will cause the pods to be backed off in a random + // order, which would cause this test to fail, since the expectation is for them to be backed off + // in a certain order. + // See: https://github.com/golang/go/issues/8687 + podInfo.Timestamp = podInfo.Timestamp.Add(time.Duration((i - len(tt.podInfos))) * time.Millisecond) } q.MoveAllToActiveOrBackoffQueue(TestEvent, tt.preEnqueueCheck) var got []string diff --git a/pkg/util/removeall/removeall_test.go b/pkg/util/removeall/removeall_test.go index 2b2a641af17..bb5acc1c521 100644 --- a/pkg/util/removeall/removeall_test.go +++ b/pkg/util/removeall/removeall_test.go @@ -20,6 +20,7 @@ import ( "errors" "os" "path" + "path/filepath" "strings" "testing" @@ -33,7 +34,7 @@ type fakeMounter struct { // IsLikelyNotMountPoint overrides mount.FakeMounter.IsLikelyNotMountPoint for our use. func (f *fakeMounter) IsLikelyNotMountPoint(file string) (bool, error) { - name := path.Base(file) + name := filepath.Base(file) if strings.HasPrefix(name, "mount") { return false, nil } diff --git a/pkg/volume/util/fs/fs_windows.go b/pkg/volume/util/fs/fs_windows.go index 57bb4bc2647..6e138514a6a 100644 --- a/pkg/volume/util/fs/fs_windows.go +++ b/pkg/volume/util/fs/fs_windows.go @@ -20,7 +20,6 @@ limitations under the License. package fs import ( - "fmt" "os" "path/filepath" "syscall" @@ -105,7 +104,7 @@ func diskUsage(currPath string, info os.FileInfo) (int64, error) { for _, file := range files { if file.IsDir() { - s, err := diskUsage(fmt.Sprintf("%s/%s", currPath, file.Name()), file) + s, err := diskUsage(filepath.Join(currPath, file.Name()), file) if err != nil { return size, err } diff --git a/pkg/volume/util/fs/fs_windows_test.go b/pkg/volume/util/fs/fs_windows_test.go index c9f69ba6941..a5ded068910 100644 --- a/pkg/volume/util/fs/fs_windows_test.go +++ b/pkg/volume/util/fs/fs_windows_test.go @@ -47,6 +47,12 @@ func TestDiskUsage(t *testing.T) { t.Fatalf("TestDiskUsage failed: %s", err.Error()) } + // File creation is not atomic. If we're calculating the DiskUsage before the data is flushed, + // we'd get zeroes for sizes, and fail with this error: + // TestDiskUsage failed: expected 0, got -1 + tmpfile1.Sync() + tmpfile2.Sync() + dirInfo1, err := os.Lstat(dir1) if err != nil { t.Fatalf("TestDiskUsage failed: %s", err.Error())