diff --git a/staging/src/k8s.io/cri-client/pkg/logs/logs.go b/staging/src/k8s.io/cri-client/pkg/logs/logs.go index 8642f5a180a..a2f07483d53 100644 --- a/staging/src/k8s.io/cri-client/pkg/logs/logs.go +++ b/staging/src/k8s.io/cri-client/pkg/logs/logs.go @@ -295,7 +295,7 @@ func ReadLogs(ctx context.Context, logger *klog.Logger, path, containerID string return fmt.Errorf("failed to try resolving symlinks in path %q: %v", path, err) } path = evaluated - f, err := os.Open(path) + f, err := openFileShareDelete(path) if err != nil { return fmt.Errorf("failed to open log file %q: %v", path, err) } @@ -364,7 +364,7 @@ func ReadLogs(ctx context.Context, logger *klog.Logger, path, containerID string return err } if recreated { - newF, err := os.Open(path) + newF, err := openFileShareDelete(path) if err != nil { if os.IsNotExist(err) { continue diff --git a/staging/src/k8s.io/cri-client/pkg/logs/logs_other.go b/staging/src/k8s.io/cri-client/pkg/logs/logs_other.go new file mode 100644 index 00000000000..4aa17b4a207 --- /dev/null +++ b/staging/src/k8s.io/cri-client/pkg/logs/logs_other.go @@ -0,0 +1,29 @@ +//go:build !windows +// +build !windows + +/* +Copyright 2024 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 logs + +import ( + "os" +) + +func openFileShareDelete(path string) (*os.File, error) { + // Noop. Only relevant for Windows. + return os.Open(path) +} diff --git a/staging/src/k8s.io/cri-client/pkg/logs/logs_test.go b/staging/src/k8s.io/cri-client/pkg/logs/logs_test.go index 59febfc00ba..cf53d7a5f5f 100644 --- a/staging/src/k8s.io/cri-client/pkg/logs/logs_test.go +++ b/staging/src/k8s.io/cri-client/pkg/logs/logs_test.go @@ -24,7 +24,6 @@ import ( "io" "os" "path/filepath" - goruntime "runtime" "testing" "time" @@ -212,10 +211,6 @@ func TestReadLogs(t *testing.T) { } func TestReadRotatedLog(t *testing.T) { - if goruntime.GOOS == "windows" { - // TODO: remove skip once the failing test has been fixed. - t.Skip("Skip failing test on Windows.") - } tmpDir := t.TempDir() file, err := os.CreateTemp(tmpDir, "logfile") if err != nil { @@ -288,6 +283,10 @@ func TestReadRotatedLog(t *testing.T) { } } + // Finished writing into the file, close it, so we can delete it later. + err = file.Close() + assert.NoErrorf(t, err, "could not close file.") + time.Sleep(20 * time.Millisecond) // Make the function ReadLogs end. fakeRuntimeService.Lock() diff --git a/staging/src/k8s.io/cri-client/pkg/logs/logs_windows.go b/staging/src/k8s.io/cri-client/pkg/logs/logs_windows.go new file mode 100644 index 00000000000..83d07d8df79 --- /dev/null +++ b/staging/src/k8s.io/cri-client/pkg/logs/logs_windows.go @@ -0,0 +1,49 @@ +//go:build windows +// +build windows + +/* +Copyright 2024 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 logs + +import ( + "os" + "syscall" +) + +// Based on Windows implementation of Windows' syscall.Open +// https://cs.opensource.google/go/go/+/refs/tags/go1.22.2:src/syscall/syscall_windows.go;l=342 +// In addition to syscall.Open, this function also adds the syscall.FILE_SHARE_DELETE flag to sharemode, +// which will allow us to read from the file without blocking the file from being deleted or renamed. +// This is essential for Log Rotation which is done by renaming the open file. Without this, the file rename would fail. +func openFileShareDelete(path string) (*os.File, error) { + pathp, err := syscall.UTF16PtrFromString(path) + if err != nil { + return nil, err + } + + var access uint32 = syscall.GENERIC_READ + var sharemode uint32 = syscall.FILE_SHARE_READ | syscall.FILE_SHARE_WRITE | syscall.FILE_SHARE_DELETE + var createmode uint32 = syscall.OPEN_EXISTING + var attrs uint32 = syscall.FILE_ATTRIBUTE_NORMAL + + handle, err := syscall.CreateFile(pathp, access, sharemode, nil, createmode, attrs, 0) + if err != nil { + return nil, err + } + + return os.NewFile(uintptr(handle), path), nil +}