From 15b4164c9c1b2c083f9821b71e5c51617e9014ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Rop=C3=A9?= Date: Thu, 28 Aug 2025 16:19:40 +0200 Subject: [PATCH 1/2] runtime_vm: Implement the ReopenContainerLog function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit implements the ReopenContainerLog function. This fixes an issue where kata container logs could not be rotated. This required duplicating part of the createContainerIO function, so I am moving this in its own subfunction for reuse. Signed-off-by: Julien Ropé --- internal/oci/runtime_vm.go | 64 +++++++++++++++++++++++++++++--------- 1 file changed, 50 insertions(+), 14 deletions(-) diff --git a/internal/oci/runtime_vm.go b/internal/oci/runtime_vm.go index 25b5bd851b6..9d087abea62 100644 --- a/internal/oci/runtime_vm.go +++ b/internal/oci/runtime_vm.go @@ -897,37 +897,50 @@ func (r *runtimeVM) createContainerIO(ctx context.Context, c *Container, cioOpts } }() - f, err := os.OpenFile(c.LogPath(), os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0o600) + stdout, stderr, err := r.createContainerLoggers(ctx, c.LogPath()) if err != nil { return nil, err } + containerIO.AddOutput(c.LogPath(), stdout, stderr) + containerIO.Pipe() + + r.Lock() + r.ctrs[c.ID()] = containerInfo{ + cio: containerIO, + } + r.Unlock() + + return containerIO, nil +} + +// createContainerLoggers creates container loggers and return write closer for stdout and stderr. +func (r *runtimeVM) createContainerLoggers(ctx context.Context, logPath string) (stdout, stderr io.WriteCloser, err error) { + f, err := os.OpenFile(logPath, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0o600) + if err != nil { + return nil, nil, err + } + var stdoutCh, stderrCh <-chan struct{} + wc := cioutil.NewSerialWriteCloser(f) - stdout, stdoutCh := cio.NewCRILogger(c.LogPath(), wc, cio.Stdout, -1) - stderr, stderrCh := cio.NewCRILogger(c.LogPath(), wc, cio.Stderr, -1) + stdout, stdoutCh = cio.NewCRILogger(logPath, wc, cio.Stdout, -1) + stderr, stderrCh = cio.NewCRILogger(logPath, wc, cio.Stderr, -1) go func() { if stdoutCh != nil { <-stdoutCh } + if stderrCh != nil { <-stderrCh } - log.Debugf(ctx, "Finish redirecting log file %q, closing it", c.LogPath()) + + log.Debugf(ctx, "Finish redirecting log file %q, closing it", logPath) f.Close() }() - containerIO.AddOutput(c.LogPath(), stdout, stderr) - containerIO.Pipe() - - r.Lock() - r.ctrs[c.ID()] = containerInfo{ - cio: containerIO, - } - r.Unlock() - - return containerIO, nil + return stdout, stderr, nil } // PauseContainer pauses a container. @@ -1170,6 +1183,29 @@ func (r *runtimeVM) ReopenContainerLog(ctx context.Context, c *Container) error log.Debugf(ctx, "RuntimeVM.ReopenContainerLog() start") defer log.Debugf(ctx, "RuntimeVM.ReopenContainerLog() end") + r.Lock() + cInfo, ok := r.ctrs[c.ID()] + r.Unlock() + + if !ok { + return errors.New("could not retrieve container information") + } + + // Create new container logger and replace the existing ones. + stdoutWC, stderrWC, err := r.createContainerLoggers(ctx, c.LogPath()) + if err != nil { + return err + } + + oldStdoutWC, oldStderrWC := cInfo.cio.AddOutput(c.LogPath(), stdoutWC, stderrWC) + if oldStdoutWC != nil { + oldStdoutWC.Close() + } + + if oldStderrWC != nil { + oldStderrWC.Close() + } + return nil } From bca299989018fcd938bc72caf6789fa338d81273 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Rop=C3=A9?= Date: Mon, 1 Sep 2025 11:00:25 +0200 Subject: [PATCH 2/2] tests: add a unit test for log rotation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit adds a test that simulates log rotation, making sure that the log file can be re-opened and used as expected. Signed-off-by: Julien Ropé --- test/logs.bats | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/logs.bats b/test/logs.bats index d703f7247e4..3218c8057c8 100644 --- a/test/logs.bats +++ b/test/logs.bats @@ -51,3 +51,32 @@ function teardown() { output=$(crictl inspect "$ctr_id" | jq -r ".status.state") [[ "$output" == "CONTAINER_RUNNING" ]] } + +@test "Log file rotation should work" { + start_crio + + jq '.metadata.name = "logger" + | .command = ["/bin/sh", "-c", "while true; do echo hello; sleep 1; done"]' \ + "$TESTDATA"/container_config.json > "$TESTDIR"/logger.json + + ctr_id=$(crictl run "$TESTDIR"/logger.json "$TESTDATA"/sandbox_config.json) + # Especially when using kata, it sometimes takes a few seconds to actually run container + sleep 5 + + logpath=$(crictl inspect "$ctr_id" | jq -r ".status.logPath") + [[ -f "$logpath" ]] + + # Move log file away, then ask for re-open. + # It will fail if the new log file is not created + mv "$logpath" "$logpath".rotated + crictl logs -r "$ctr_id" + + [[ -f "$logpath" ]] + + # Verify that the rotated log file is not written to anymore + initial_size=$(stat -c %s "$logpath.rotated") + [ "$initial_size" -gt 0 ] + sleep 2 # our logger writes every second, leave enough time for at least one write + new_size=$(stat -c %s "$logpath.rotated") + [ "$new_size" -eq "$initial_size" ] +}