fix(logger): preserve log file across commands

[pbrissaud]

Summary

• Remove os.Truncate call in PersistentPreRun that was wiping the log file on every CLI invocation
• Remove now-unused fmt import
• The logger already opens files with O_APPEND and rotates at 1000 lines, making truncation redundant

Test plan

• task build passes
• Run two commands back-to-back with --debug and verify both runs appear in kubeasy-cli.log

🤖 Generated with Claude Code

[claude]

Claude finished @pbrissaud's task in 1m 11s —— View job

---

PR Review: fix(logger): preserve log file across commands

• Gather context
• Read relevant files (cmd/root.go, internal/logger/logger.go)
• Analyze the change and post review

---

Summary

This is a clean and correct fix. The removed os.Truncate call in PersistentPreRun (cmd/root.go:31-34) was silently wiping the log file before every CLI invocation, which directly contradicted the logger's own design intent.

Why the change is correct

The logger in internal/logger/logger.go:86 opens files with os.O_APPEND:

file, err := os.OpenFile(opts.FilePath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0644)

It also has a built-in rotation mechanism (checkAndRotateFile / truncateFile) that kicks in at constants.MaxLogLines and keeps the newest 50% of lines. The os.Truncate at startup was entirely redundant and harmful — it prevented any log persistence between runs.

Removing the fmt import is correct cleanup since it was only used for the fmt.Fprintf in the truncate error handler.

No issues with this PR

The change is minimal, safe, and well-reasoned. Both deletions are justified.

---

Unrelated pre-existing observation (not blocking)

There is a subtle bug in the rotation logic in logger.go that is not introduced by this PR but worth noting: after rewriteFile executes os.Rename(tempFile, l.filePath), the old file handle stored in l.outputs still points to the original inode (now unlinked from the filesystem path). Subsequent log writes after a rotation would go to the orphaned inode and won't appear in the log file at constants.LogFilePath. If log rotation matters for debugging, this could cause silent log loss post-rotation. Fix this →

---

Verdict: LGTM ✓ — straightforward fix, no issues introduced.

[claude]

Claude finished @pbrissaud's task in 1m 24s —— View job

---

PR Review: fix(logger): preserve log file across commands

• Gather context
• Read changed files (cmd/root.go, internal/logger/logger.go)
• Analyze changes and post review

---

Summary

This PR addresses two bugs across two commits: the original startup truncation fix and a follow-up fix for stale file handles after log rotation. Both are correct and well-reasoned.

---

Commit 1 — fix(logger): preserve log file across commands

cmd/root.go:31-34 — Removes the os.Truncate call in PersistentPreRun. Clean and correct. The logger already uses O_APPEND, so this truncation was both redundant and harmful (silently destroying all prior log content on every invocation). Removing the now-unused fmt import is the correct cleanup.

No issues here.

---

Commit 2 — fix(logger): reopen file handle after rotation to prevent silent log loss

This addresses the pre-existing bug noted in the previous review: after a temp-file rename, the old *os.File in l.outputs pointed to an orphaned inode.

The new approach (internal/logger/logger.go:312-332):

l.file.Seek(0, io.SeekStart)
l.file.Truncate(0)
// write lines via l.file

This is the correct in-place rotation strategy. By seeking + truncating the existing open handle rather than renaming a temp file, the inode stays stable and all writers (l.outputs, klog's separate handle) remain valid after rotation.

One minor observation — the Seek(0, io.SeekStart) call is technically unnecessary. Since l.file was opened with O_APPEND, all writes always go to end-of-file regardless of the seek position, and ftruncate (underlying file.Truncate) does not use the file position either. It's harmless and arguably aids readability, so this is not a blocking issue.

Existing behavior, not introduced here — if rewriteFile fails (e.g., a permission error), l.lineCount is not reset and rotation will be retried on every subsequent write. This is pre-existing and low-risk for a CLI.

---

No issues with this PR

Both changes are minimal, correct, and well-scoped. The in-place seek+truncate approach is superior to the prior temp-file rename strategy.

Verdict: LGTM ✓

[sentry]

Codecov Report

❌ Patch coverage is 20.00000% with 8 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/logger/logger.go	20.00%	8 Missing ⚠️

📢 Thoughts on this report? Let us know!