From b9bf3e5c49d8cc26c0d1fc67a3b603fdda2c62d5 Mon Sep 17 00:00:00 2001 From: Claudiu Belu Date: Wed, 15 Jun 2022 15:17:24 +0300 Subject: [PATCH] Replaces path.Operation with filepath.Operation (kubelet) The path module has a few different functions: Clean, Split, Join, Ext, Dir, Base, IsAbs. These functions do not take into account the OS-specific path separator, meaning that they won't behave as intended on Windows. For example, Dir is supposed to return all but the last element of the path. For the path "C:\some\dir\somewhere", it is supposed to return "C:\some\dir\", however, it returns ".". Instead of these functions, the ones in filepath should be used instead. --- pkg/kubelet/cm/cpumanager/state/state_checkpoint.go | 4 ++-- pkg/kubelet/cm/devicemanager/endpoint_test.go | 10 +++++----- pkg/kubelet/cm/devicemanager/plugin/v1beta1/stub.go | 4 ++-- .../cm/memorymanager/state/state_checkpoint.go | 4 ++-- pkg/kubelet/kubelet.go | 3 +-- pkg/kubelet/kubelet_pods.go | 5 ++--- pkg/kubelet/kuberuntime/legacy.go | 4 ++-- pkg/kubelet/kuberuntime/legacy_test.go | 6 +++--- pkg/kubelet/stats/cadvisor_stats_provider.go | 4 ++-- pkg/kubelet/stats/cri_stats_provider.go | 4 ++-- .../volumemanager/reconciler/reconciler_test.go | 13 ++++++------- .../volumemanager/reconciler/reconstruct_common.go | 11 +++++------ .../reconciler/reconstruct_new_test.go | 11 +++++------ 13 files changed, 39 insertions(+), 44 deletions(-) diff --git a/pkg/kubelet/cm/cpumanager/state/state_checkpoint.go b/pkg/kubelet/cm/cpumanager/state/state_checkpoint.go index c9aacd19051..9297f23757d 100644 --- a/pkg/kubelet/cm/cpumanager/state/state_checkpoint.go +++ b/pkg/kubelet/cm/cpumanager/state/state_checkpoint.go @@ -18,7 +18,7 @@ package state import ( "fmt" - "path" + "path/filepath" "sync" "k8s.io/klog/v2" @@ -56,7 +56,7 @@ func NewCheckpointState(stateDir, checkpointName, policyName string, initialCont if err := stateCheckpoint.restoreState(); err != nil { //nolint:staticcheck // ST1005 user-facing error message return nil, fmt.Errorf("could not restore state from checkpoint: %v, please drain this node and delete the CPU manager checkpoint file %q before restarting Kubelet", - err, path.Join(stateDir, checkpointName)) + err, filepath.Join(stateDir, checkpointName)) } return stateCheckpoint, nil diff --git a/pkg/kubelet/cm/devicemanager/endpoint_test.go b/pkg/kubelet/cm/devicemanager/endpoint_test.go index d9da8bfbeb5..dbfd3de6342 100644 --- a/pkg/kubelet/cm/devicemanager/endpoint_test.go +++ b/pkg/kubelet/cm/devicemanager/endpoint_test.go @@ -19,7 +19,7 @@ package devicemanager import ( "fmt" "os" - "path" + "path/filepath" "sync" "testing" "time" @@ -72,7 +72,7 @@ func esocketName() string { } func TestNewEndpoint(t *testing.T) { - socket := path.Join(os.TempDir(), esocketName()) + socket := filepath.Join(os.TempDir(), esocketName()) devs := []*pluginapi.Device{ {ID: "ADeviceId", Health: pluginapi.Healthy}, @@ -83,7 +83,7 @@ func TestNewEndpoint(t *testing.T) { } func TestRun(t *testing.T) { - socket := path.Join(os.TempDir(), esocketName()) + socket := filepath.Join(os.TempDir(), esocketName()) devs := []*pluginapi.Device{ {ID: "ADeviceId", Health: pluginapi.Healthy}, @@ -148,7 +148,7 @@ func TestRun(t *testing.T) { } func TestAllocate(t *testing.T) { - socket := path.Join(os.TempDir(), esocketName()) + socket := filepath.Join(os.TempDir(), esocketName()) devs := []*pluginapi.Device{ {ID: "ADeviceId", Health: pluginapi.Healthy}, } @@ -201,7 +201,7 @@ func TestAllocate(t *testing.T) { } func TestGetPreferredAllocation(t *testing.T) { - socket := path.Join(os.TempDir(), esocketName()) + socket := filepath.Join(os.TempDir(), esocketName()) callbackCount := 0 callbackChan := make(chan int) p, e := esetup(t, []*pluginapi.Device{}, socket, "mock", func(n string, d []pluginapi.Device) { diff --git a/pkg/kubelet/cm/devicemanager/plugin/v1beta1/stub.go b/pkg/kubelet/cm/devicemanager/plugin/v1beta1/stub.go index 95ff4c7c0aa..1d226f72a85 100644 --- a/pkg/kubelet/cm/devicemanager/plugin/v1beta1/stub.go +++ b/pkg/kubelet/cm/devicemanager/plugin/v1beta1/stub.go @@ -20,7 +20,7 @@ import ( "context" "net" "os" - "path" + "path/filepath" "sync" "time" @@ -205,7 +205,7 @@ func (m *Stub) Register(kubeletEndpoint, resourceName string, pluginSockDir stri client := pluginapi.NewRegistrationClient(conn) reqt := &pluginapi.RegisterRequest{ Version: pluginapi.Version, - Endpoint: path.Base(m.socket), + Endpoint: filepath.Base(m.socket), ResourceName: resourceName, Options: &pluginapi.DevicePluginOptions{ PreStartRequired: m.preStartContainerFlag, diff --git a/pkg/kubelet/cm/memorymanager/state/state_checkpoint.go b/pkg/kubelet/cm/memorymanager/state/state_checkpoint.go index 286b9fda819..05e32b751a6 100644 --- a/pkg/kubelet/cm/memorymanager/state/state_checkpoint.go +++ b/pkg/kubelet/cm/memorymanager/state/state_checkpoint.go @@ -18,7 +18,7 @@ package state import ( "fmt" - "path" + "path/filepath" "sync" "k8s.io/klog/v2" @@ -52,7 +52,7 @@ func NewCheckpointState(stateDir, checkpointName, policyName string) (State, err if err := stateCheckpoint.restoreState(); err != nil { //nolint:staticcheck // ST1005 user-facing error message return nil, fmt.Errorf("could not restore state from checkpoint: %v, please drain this node and delete the memory manager checkpoint file %q before restarting Kubelet", - err, path.Join(stateDir, checkpointName)) + err, filepath.Join(stateDir, checkpointName)) } return stateCheckpoint, nil diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 870223a575d..0ca42ad0776 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -24,7 +24,6 @@ import ( "net" "net/http" "os" - "path" "path/filepath" sysruntime "runtime" "sort" @@ -1256,7 +1255,7 @@ func (kl *Kubelet) RlimitStats() (*statsapi.RlimitStats, error) { // 4. the pod-resources directory // 5. the checkpoint directory func (kl *Kubelet) setupDataDirs() error { - kl.rootDirectory = path.Clean(kl.rootDirectory) + kl.rootDirectory = filepath.Clean(kl.rootDirectory) pluginRegistrationDir := kl.getPluginsRegistrationDir() pluginsDir := kl.getPluginsDir() if err := os.MkdirAll(kl.getRootDir(), 0750); err != nil { diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index 53ec75b87be..d9041c466fd 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -24,7 +24,6 @@ import ( "net/http" "net/url" "os" - "path" "path/filepath" "runtime" "sort" @@ -123,7 +122,7 @@ func (kl *Kubelet) makeBlockVolumes(pod *v1.Pod, container *v1.Container, podVol } // Get a symbolic link associated to a block device under pod device path dirPath, volName := vol.BlockVolumeMapper.GetPodDeviceMapPath() - symlinkPath := path.Join(dirPath, volName) + symlinkPath := filepath.Join(dirPath, volName) if islinkExist, checkErr := blkutil.IsSymlinkExist(symlinkPath); checkErr != nil { return nil, checkErr } else if islinkExist { @@ -303,7 +302,7 @@ func translateMountPropagation(mountMode *v1.MountPropagationMode) (runtimeapi.M // getEtcHostsPath returns the full host-side path to a pod's generated /etc/hosts file func getEtcHostsPath(podDir string) string { - hostsFilePath := path.Join(podDir, "etc-hosts") + hostsFilePath := filepath.Join(podDir, "etc-hosts") // Volume Mounts fail on Windows if it is not of the form C:/ return volumeutil.MakeAbsolutePath(runtime.GOOS, hostsFilePath) } diff --git a/pkg/kubelet/kuberuntime/legacy.go b/pkg/kubelet/kuberuntime/legacy.go index 1187123ad7f..296a5f5588a 100644 --- a/pkg/kubelet/kuberuntime/legacy.go +++ b/pkg/kubelet/kuberuntime/legacy.go @@ -18,7 +18,7 @@ package kuberuntime import ( "fmt" - "path" + "path/filepath" "strings" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" @@ -71,5 +71,5 @@ func logSymlink(containerLogsDir, podFullName, containerName, containerID string if len(logPath) > ext4MaxFileNameLen-len(suffix) { logPath = logPath[:ext4MaxFileNameLen-len(suffix)] } - return path.Join(containerLogsDir, logPath+suffix) + return filepath.Join(containerLogsDir, logPath+suffix) } diff --git a/pkg/kubelet/kuberuntime/legacy_test.go b/pkg/kubelet/kuberuntime/legacy_test.go index 48cdcb21fd1..7d99032fd3d 100644 --- a/pkg/kubelet/kuberuntime/legacy_test.go +++ b/pkg/kubelet/kuberuntime/legacy_test.go @@ -19,7 +19,7 @@ package kuberuntime import ( "fmt" "math/rand" - "path" + "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -42,7 +42,7 @@ func TestLogSymLink(t *testing.T) { containerName := randStringBytes(70) dockerID := randStringBytes(80) // The file name cannot exceed 255 characters. Since .log suffix is required, the prefix cannot exceed 251 characters. - expectedPath := path.Join(containerLogsDir, fmt.Sprintf("%s_%s-%s", podFullName, containerName, dockerID)[:251]+".log") + expectedPath := filepath.Join(containerLogsDir, fmt.Sprintf("%s_%s-%s", podFullName, containerName, dockerID)[:251]+".log") as.Equal(expectedPath, logSymlink(containerLogsDir, podFullName, containerName, dockerID)) } @@ -53,6 +53,6 @@ func TestLegacyLogSymLink(t *testing.T) { podName := randStringBytes(128) podNamespace := randStringBytes(10) // The file name cannot exceed 255 characters. Since .log suffix is required, the prefix cannot exceed 251 characters. - expectedPath := path.Join(legacyContainerLogsDir, fmt.Sprintf("%s_%s_%s-%s", podName, podNamespace, containerName, containerID)[:251]+".log") + expectedPath := filepath.Join(legacyContainerLogsDir, fmt.Sprintf("%s_%s_%s-%s", podName, podNamespace, containerName, containerID)[:251]+".log") as.Equal(expectedPath, legacyLogSymlink(containerID, containerName, podName, podNamespace)) } diff --git a/pkg/kubelet/stats/cadvisor_stats_provider.go b/pkg/kubelet/stats/cadvisor_stats_provider.go index a896610c987..be428561d1e 100644 --- a/pkg/kubelet/stats/cadvisor_stats_provider.go +++ b/pkg/kubelet/stats/cadvisor_stats_provider.go @@ -19,7 +19,7 @@ package stats import ( "context" "fmt" - "path" + "path/filepath" "sort" "strings" @@ -318,7 +318,7 @@ func filterTerminatedContainerInfoAndAssembleByPodCgroupKey(containerInfo map[st podCgroupKey = internalCgroupName[len(internalCgroupName)-1] } else { // Take last component only. - podCgroupKey = path.Base(key) + podCgroupKey = filepath.Base(key) } cinfosByPodCgroupKey[podCgroupKey] = cinfo if !isPodManagedContainer(&cinfo) { diff --git a/pkg/kubelet/stats/cri_stats_provider.go b/pkg/kubelet/stats/cri_stats_provider.go index 60c4abe53f0..d1e1d90b275 100644 --- a/pkg/kubelet/stats/cri_stats_provider.go +++ b/pkg/kubelet/stats/cri_stats_provider.go @@ -20,7 +20,7 @@ import ( "context" "errors" "fmt" - "path" + "path/filepath" "sort" "strings" "sync" @@ -899,7 +899,7 @@ func getCRICadvisorStats(infos map[string]cadvisorapiv2.ContainerInfo) (map[stri func extractIDFromCgroupPath(cgroupPath string) string { // case0 == cgroupfs: "/kubepods/burstable/pod2fc932ce-fdcc-454b-97bd-aadfdeb4c340/9be25294016e2dc0340dd605ce1f57b492039b267a6a618a7ad2a7a58a740f32" - id := path.Base(cgroupPath) + id := filepath.Base(cgroupPath) // case1 == systemd: "/kubepods.slice/kubepods-burstable.slice/kubepods-burstable-pod2fc932ce_fdcc_454b_97bd_aadfdeb4c340.slice/cri-containerd-aaefb9d8feed2d453b543f6d928cede7a4dbefa6a0ae7c9b990dd234c56e93b9.scope" // trim anything before the final '-' and suffix .scope diff --git a/pkg/kubelet/volumemanager/reconciler/reconciler_test.go b/pkg/kubelet/volumemanager/reconciler/reconciler_test.go index be7a26338f9..e331f62e4c0 100644 --- a/pkg/kubelet/volumemanager/reconciler/reconciler_test.go +++ b/pkg/kubelet/volumemanager/reconciler/reconciler_test.go @@ -20,7 +20,6 @@ import ( "crypto/md5" "fmt" "os" - "path" "path/filepath" "testing" "time" @@ -2306,8 +2305,8 @@ func TestSyncStates(t *testing.T) { { name: "when two pods are using same volume and both are deleted", volumePaths: []string{ - path.Join("pod1", "volumes", "fake-plugin", "pvc-abcdef"), - path.Join("pod2", "volumes", "fake-plugin", "pvc-abcdef"), + filepath.Join("pod1", "volumes", "fake-plugin", "pvc-abcdef"), + filepath.Join("pod2", "volumes", "fake-plugin", "pvc-abcdef"), }, createMountPoint: true, podInfos: []podInfo{}, @@ -2322,8 +2321,8 @@ func TestSyncStates(t *testing.T) { { name: "when two pods are using same volume and one of them is deleted", volumePaths: []string{ - path.Join("pod1uid", "volumes", "fake-plugin", "volume-name"), - path.Join("pod2uid", "volumes", "fake-plugin", "volume-name"), + filepath.Join("pod1uid", "volumes", "fake-plugin", "volume-name"), + filepath.Join("pod2uid", "volumes", "fake-plugin", "volume-name"), }, createMountPoint: true, podInfos: []podInfo{defaultPodInfo}, @@ -2342,7 +2341,7 @@ func TestSyncStates(t *testing.T) { { name: "when reconstruction fails for a volume, volumes should be cleaned up", volumePaths: []string{ - path.Join("pod1", "volumes", "fake-plugin", "pvc-abcdef"), + filepath.Join("pod1", "volumes", "fake-plugin", "pvc-abcdef"), }, createMountPoint: false, podInfos: []podInfo{}, @@ -2359,7 +2358,7 @@ func TestSyncStates(t *testing.T) { { name: "when volume exists in dsow, volume should be recorded in skipped during reconstruction", volumePaths: []string{ - path.Join("pod1uid", "volumes", "fake-plugin", "volume-name"), + filepath.Join("pod1uid", "volumes", "fake-plugin", "volume-name"), }, createMountPoint: true, podInfos: []podInfo{defaultPodInfo}, diff --git a/pkg/kubelet/volumemanager/reconciler/reconstruct_common.go b/pkg/kubelet/volumemanager/reconciler/reconstruct_common.go index e5a33aa735a..afc3fe63700 100644 --- a/pkg/kubelet/volumemanager/reconciler/reconstruct_common.go +++ b/pkg/kubelet/volumemanager/reconciler/reconstruct_common.go @@ -20,7 +20,6 @@ import ( "fmt" "io/fs" "os" - "path" "path/filepath" "time" @@ -132,16 +131,16 @@ func getVolumesFromPodDir(podDir string) ([]podVolume, error) { continue } podName := podsDirInfo[i].Name() - podDir := path.Join(podDir, podName) + podDir := filepath.Join(podDir, podName) // Find filesystem volume information // ex. filesystem volume: /pods/{podUid}/volume/{escapeQualifiedPluginName}/{volumeName} volumesDirs := map[v1.PersistentVolumeMode]string{ - v1.PersistentVolumeFilesystem: path.Join(podDir, config.DefaultKubeletVolumesDirName), + v1.PersistentVolumeFilesystem: filepath.Join(podDir, config.DefaultKubeletVolumesDirName), } // Find block volume information // ex. block volume: /pods/{podUid}/volumeDevices/{escapeQualifiedPluginName}/{volumeName} - volumesDirs[v1.PersistentVolumeBlock] = path.Join(podDir, config.DefaultKubeletVolumeDevicesDirName) + volumesDirs[v1.PersistentVolumeBlock] = filepath.Join(podDir, config.DefaultKubeletVolumeDevicesDirName) for volumeMode, volumesDir := range volumesDirs { var volumesDirInfo []fs.DirEntry @@ -151,7 +150,7 @@ func getVolumesFromPodDir(podDir string) ([]podVolume, error) { } for _, volumeDir := range volumesDirInfo { pluginName := volumeDir.Name() - volumePluginPath := path.Join(volumesDir, pluginName) + volumePluginPath := filepath.Join(volumesDir, pluginName) volumePluginDirs, err := utilpath.ReadDirNoStat(volumePluginPath) if err != nil { klog.ErrorS(err, "Could not read volume plugin directory", "volumePluginPath", volumePluginPath) @@ -159,7 +158,7 @@ func getVolumesFromPodDir(podDir string) ([]podVolume, error) { } unescapePluginName := utilstrings.UnescapeQualifiedName(pluginName) for _, volumeName := range volumePluginDirs { - volumePath := path.Join(volumePluginPath, volumeName) + volumePath := filepath.Join(volumePluginPath, volumeName) klog.V(5).InfoS("Volume path from volume plugin directory", "podName", podName, "volumePath", volumePath) volumes = append(volumes, podVolume{ podName: volumetypes.UniquePodName(podName), diff --git a/pkg/kubelet/volumemanager/reconciler/reconstruct_new_test.go b/pkg/kubelet/volumemanager/reconciler/reconstruct_new_test.go index 26b04edd171..332c5476a04 100644 --- a/pkg/kubelet/volumemanager/reconciler/reconstruct_new_test.go +++ b/pkg/kubelet/volumemanager/reconciler/reconstruct_new_test.go @@ -19,7 +19,6 @@ package reconciler import ( "fmt" "os" - "path" "path/filepath" "reflect" "testing" @@ -48,8 +47,8 @@ func TestReconstructVolumes(t *testing.T) { { name: "when two pods are using same volume and both are deleted", volumePaths: []string{ - path.Join("pod1", "volumes", "fake-plugin", "pvc-abcdef"), - path.Join("pod2", "volumes", "fake-plugin", "pvc-abcdef"), + filepath.Join("pod1", "volumes", "fake-plugin", "pvc-abcdef"), + filepath.Join("pod2", "volumes", "fake-plugin", "pvc-abcdef"), }, expectedVolumesNeedReportedInUse: []string{"fake-plugin/pvc-abcdef", "fake-plugin/pvc-abcdef"}, expectedVolumesNeedDevicePath: []string{"fake-plugin/pvc-abcdef", "fake-plugin/pvc-abcdef"}, @@ -77,7 +76,7 @@ func TestReconstructVolumes(t *testing.T) { { name: "when reconstruction fails for a volume, volumes should be cleaned up", volumePaths: []string{ - path.Join("pod1", "volumes", "missing-plugin", "pvc-abcdef"), + filepath.Join("pod1", "volumes", "missing-plugin", "pvc-abcdef"), }, expectedVolumesNeedReportedInUse: []string{}, expectedVolumesNeedDevicePath: []string{}, @@ -271,14 +270,14 @@ func TestReconstructVolumesMount(t *testing.T) { }{ { name: "reconstructed volume is mounted", - volumePath: path.Join("pod1uid", "volumes", "fake-plugin", "volumename"), + volumePath: filepath.Join("pod1uid", "volumes", "fake-plugin", "volumename"), expectMount: true, }, { name: "reconstructed volume fails to mount", // FailOnSetupVolumeName: MountDevice succeeds, SetUp fails - volumePath: path.Join("pod1uid", "volumes", "fake-plugin", volumetesting.FailOnSetupVolumeName), + volumePath: filepath.Join("pod1uid", "volumes", "fake-plugin", volumetesting.FailOnSetupVolumeName), expectMount: false, }, }