unittests: Fixes unit tests for Windows (part 3)

Currently, there are some unit tests that are failing on Windows due to
various reasons:

- paths not properly joined (filepath.Join should be used).
- Proxy Mode IPVS not supported on Windows.
- DeadlineExceeded can occur when trying to read data from an UDP
  socket. This can be used to detect whether the port was closed or not.
- In Windows, with long file name support enabled, file names can have
  up to 32,767 characters. In this case, the error
  windows.ERROR_FILENAME_EXCED_RANGE will be encountered instead.
- files not closed, which means that they cannot be removed / renamed.
- time.Now() is not as precise on Windows, which means that 2
  consecutive calls may return the same timestamp.
- path.Base() will return the same path. filepath.Base() should be used
  instead.
- path.Join() will always join the paths with a / instead of the OS
  specific separator. filepath.Join() should be used instead.
This commit is contained in:
Claudiu Belu 2022-06-06 10:51:42 +03:00
parent 83415e5c9e
commit 9f95b7b18c
14 changed files with 134 additions and 53 deletions

View File

@ -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
}

View File

@ -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))

View File

@ -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"))

View File

@ -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 {

View File

@ -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)
}

View File

@ -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)

26
pkg/routes/const_other.go Normal file
View File

@ -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
)

View File

@ -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
)

View File

@ -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
}
}

View File

@ -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")
}

View File

@ -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

View File

@ -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
}

View File

@ -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
}

View File

@ -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())