search

containerd / nerdctl

contaiNERD CTL - Docker-compatible CLI for containerd, with support for Compose, Rootless, eStargz, OCIcrypt, IPFS, ...
Apache License 2.0
8.09k stars 598 forks source link

TestReadRotatedLog fails on windows #3554

Open apostasie opened 1 week ago

apostasie commented 1 week ago

Description

=== RUN   TestReadRotatedLog
    cri_logger_test.go:285: failed to rotate log "C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\TestReadRotatedLog3469684812\\001\\logfile2318891333" to "C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\TestReadRotatedLog3469684812\\001\\logfile2318891333.1620241016-014354", error: rename C:\Users\RUNNER~1\AppData\Local\Temp\TestReadRotatedLog3469684812\001\logfile2318891333 C:\Users\RUNNER~1\AppData\Local\Temp\TestReadRotatedLog3469684812\001\logfile2318891333.1620241016-014354: The process cannot access the file because it is being used by another process.
    testing.go:1232: TempDir RemoveAll cleanup: remove C:\Users\RUNNER~1\AppData\Local\Temp\TestReadRotatedLog3469684812\001\logfile2318891333: The process cannot access the file because it is being used by another process.
--- FAIL: TestReadRotatedLog (2.05s)

Steps to reproduce the issue

Run unit tests for windows.

Describe the results you received and expected

This does not seem like a non-compatible test, but rather an actual issue?

I'll skip the test for the time being so that we can enable unit tests for windows, but this should be looked into?

What version of nerdctl are you using?

main

Are you using a variant of nerdctl? (e.g., Rancher Desktop)

None

Host information

No response

fahedouch commented 1 week ago

it seems like the file is not closed correctly or cri is playing with the file, I think we can lock the file before processing the renaming operation

apostasie commented 1 week ago

@fahedouch would you claim dibs on this?

Pretty sure a fix on this would significantly improve windows logs stability.