[TDnorthgarden]

Problem

With --log-rotate --log-max-files 1 (--log-max-files defaults to 1), every rotation
from the second one on leaves two backup files (.1 and .2) behind instead of the
single one configured, so the disk space used by logs exceeds the --log-max-files limit.

shift_backup_files() computes the loop start as
(opt_log_max_files > 1) ? opt_log_max_files : 2, so with --log-max-files 1 the shifting
loop runs once and moves .1 to .2 on every rotation; perform_file_rotation() then
renames the current log to .1. The result is log.1 + log.2.

This is a bug, not a design choice: --log-max-files is documented as "Number of backup
log files to keep", and rotation exists to bound log disk usage. The : 2 special case
looks like an attempt to move .1 out of the way before the new .1 is placed, without
noticing that rename() already replaces the destination atomically — a property the same
function relies on elsewhere ("Direct atomic rename - overwrites destination if it
exists"). Introduced by 61c8da9 ("Add log rotation functionality as alternative to log
truncation").

Reproducer (container writes ~2 KB+ of output, log-size-max 1024):

$ conmon --cid repro --cuuid repro --runtime runc --bundle ./bundle \
      --log-path k8s-file:/tmp/repro.log \
      --log-rotate --log-max-files 1 --log-size-max 1024
$ ls /tmp/repro.log*
/tmp/repro.log  /tmp/repro.log.1  /tmp/repro.log.2    # .2 must not exist

Fix

• Start the shifting loop at opt_log_max_files directly and drop the : 2 special
  case. With --log-max-files 1 the loop no longer runs; the old .1 backup is
  atomically replaced by the rename() in perform_file_rotation(), keeping exactly one
  backup.
• Behavior for --log-max-files >= 2 is unchanged: the loop start equals
  opt_log_max_files in both versions.

Testing

New regression test log management: should keep only log-max-files backup files in
test/06-log-management.bats: runs a real container producing ~300 log lines (≈20 KB)
against --log-size-max 1024 (many rotations) and asserts $LOG_PATH.1 exists while
$LOG_PATH.2 does not.

$ bats test/06-log-management.bats          # before the fix
not ok 4 log management: should keep only log-max-files backup files
$ bats test/06-log-management.bats          # after the fix
ok 4 log management: should keep only log-max-files backup files

Full suite result after the fix: 06-log-management.bats 14/14, 03-k8s-log-rotation.bats
3/3, clean build with the project's -Werror flag set.

[giuseppe]

looks like a AI artifact to explain the change itself, instead we need to explain what the code below does

[TDnorthgarden]

Done — removed those two lines. The pre-existing first line already describes what the loop does; the change rationale stays in the commit message.

[giuseppe]

please use your real name for the commit author and DCO line

[TDnorthgarden]

please use your real name for the commit author and DCO line

Done — amended the author and DCO line to "Aucoto Zhang": Zhang is my real surname; Aucoto is the contributor name I sign my contributions under across projects (e.g. docker/compose#14390), and I'd rather not publish my legal given name. Happy to adjust if this doesn't work for you.

[giuseppe]

LGTM

[giuseppe]

lint fix here: #713

[kolyshkin]

LGTM, thanks