Merge pull request #9450 from littlejawa/backport_logrotate_fix_1.31

openshift-merge-bot[bot] · web-flow · commit dc59c782f3e6 · 2025-09-04T11:15:15.000Z
[release-1.31] Backport kata logrotate fix
diff --git 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
 }
 
diff --git 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" ]
+}
