Extend bash record function export#16
Merged
freol35241 merged 15 commits intoNov 18, 2025
Merged
Conversation
- Add optional --interval parameter (hourly, daily, weekly, monthly) to configure log rotation frequency - Add optional --rotate-count parameter (defaults to 7) to specify number of rotated files to keep - Automatically create logrotate configuration file when interval is specified - Automatically set up cronjob to run logrotate at the specified interval - Gracefully handle permission issues by falling back to user directory for logrotate config - Update README.md with new parameters and example usage
- Rename --interval to --rotate-interval for consistency - Add logrotate and cron packages to Dockerfile - Add automatic cron daemon startup when log rotation is configured - Update all references in bash-init.sh to use rotate_interval variable - Update README.md documentation and examples with new parameter name
…ndling - Add early validation of rotate_interval parameter with helpful error message - Error out (return 1) if invalid interval is provided instead of continuing - Add dateext option to use date suffixes for rotated files - Add dateyesterday option to use yesterday's date in rotated file names - Add inline comments explaining delaycompress behavior - Simplify cronjob setup logic by removing redundant validation
Since we use copytruncate, rotated files can be compressed immediately. The copytruncate option keeps the original file in place (just empties it), so there's no risk of processes writing to the rotated file.
- Fix SC2155 warning by splitting declaration and assignment of logrotate_conf - Add test for invalid rotate_interval parameter (should fail with error) - Add test for valid rotate_interval (verifies cron daemon, logrotate config, and cronjob) - Add test to ensure record function works normally without rotate_interval - All three tests verify different aspects of the log rotation functionality
…iners - Remove docker exec and long-running container approach - Chain all commands in single docker run for immediate completion - Record function sets up rotation, then cat config and crontab in same run - Verify both logrotate config content and crontab entry in one test - All tests now complete quickly and work in GitHub Actions CI
- Test now checks /etc/logrotate.d/ first, then falls back to $HOME/.porla/logrotate.d/ - This handles both cases: when running as root with write access to /etc, and when not - Add tests/bats-helpers/ to .gitignore since they're installed by post-create script
- Verify that restarting a container with same logrotate config works correctly - First run creates config and cronjob - Second run (simulating restart) succeeds without errors - Both runs successfully append to the same log file - Tests the idempotent nature of the logrotate setup
- Remove overly specific output assertions that might vary - Focus on what matters: both runs succeed without errors - Verify log file exists and contains exactly 2 lines with "run" - This tests the core functionality: containers can restart with same config
- Add error handling around cron daemon startup - Add error handling around crontab commands - Log warnings instead of failing if cron/crontab don't work - Allow record function to continue even if cron setup fails - This fixes the container restart test which was failing due to crontab errors - With set -euo pipefail, any unchecked command failure causes script to exit
- New test starts a detached container with record using rotation - Verifies cron daemon is actually running - Checks crontab configuration is present - Validates logrotate config file exists - This will help diagnose whether cron works in real-world scenarios
- Create /var/run directory and crond.pid file if needed - Set correct permissions (1730) on cron spool directory - Add 0.5s sleep after starting cron to allow initialization - Verify cron daemon is actually running after start attempt - Only attempt to add cronjob if cron daemon is confirmed running - Provide helpful manual rotation command if cron setup fails - Better error messages to help users understand automation status This fixes Option 1: ensuring cron works properly in containers by handling directory setup and permissions before starting the daemon.
Inspired by https://github.com/linkyard/docker-logrotate approach: - Write cronjob directly to /var/spool/cron/crontabs/root instead of using crontab command - Create /var/spool/cron/crontabs directory with correct permissions (1730) - Set crontab file permissions to 0600 - Remove unnecessary /var/run/crond.pid touch - Simplify cron initialization This is more reliable than using the crontab command which can fail in container environments. Direct file manipulation is the standard approach for Docker-based cron setups.
…tating log files using logrotate and cron).
freol35241
deleted the
claude/extend-record-function-011CV3Rrzuv4mjgRKm8H2wxY
branch
November 18, 2025 12:20
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.