-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: enable log rotation + install CW Agent #48
Conversation
WalkthroughThe pull request introduces enhancements to the Tailscale Subnet Router module, focusing on system logging, monitoring, and configuration management. Key changes include updating the module version, adding new input variables for journald settings, modifying the instance type, and improving the user data script. The modifications aim to provide more flexible logging configuration, better system resource management, and enhanced CloudWatch integration. Changes
Sequence DiagramsequenceDiagram
participant EC2 as Tailscale Subnet Router
participant CW as CloudWatch
participant Journald as System Logging
EC2->>Journald: Configure log retention
EC2->>Journald: Set max disk usage
EC2->>CW: Install CloudWatch Agent
EC2->>CW: Report system metrics
EC2->>Tailscale: Initialize service
Possibly Related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
userdata.sh.tmpl (1)
17-27
: Consider adding a sanity check before restarting journald.While creating the config directory and installing these drop-ins is straightforward, you might log a validation step to check if the file was written correctly before calling
systemctl restart systemd-journald
.variables.tf (1)
118-128
: Configurable journald retention is a helpful addition.Exposing journald limits via variables is a solid approach for controlling log growth. You might document recommended values for various workloads or highlight potential constraints for smaller instance types.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.md
(4 hunks)main.tf
(3 hunks)userdata.sh.tmpl
(2 hunks)variables.tf
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
main.tf (1)
Pattern **/*.tf
: You're a Terraform expert who has thoroughly studied all the documentation from Hashicorp https://developer.hashicorp.com/terraform/docs and OpenTofu https://opentofu.org/docs/.
You have a strong grasp of Terraform syntax and prioritize providing accurate and insightful code suggestions.
As a fan of the Cloud Posse / SweetOps ecosystem, you incorporate many of their best practices https://docs.cloudposse.com/best-practices/terraform/ while balancing them with general Terraform guidelines.
variables.tf (1)
Pattern **/*.tf
: You're a Terraform expert who has thoroughly studied all the documentation from Hashicorp https://developer.hashicorp.com/terraform/docs and OpenTofu https://opentofu.org/docs/.
You have a strong grasp of Terraform syntax and prioritize providing accurate and insightful code suggestions.
As a fan of the Cloud Posse / SweetOps ecosystem, you incorporate many of their best practices https://docs.cloudposse.com/best-practices/terraform/ while balancing them with general Terraform guidelines.
🔇 Additional comments (13)
userdata.sh.tmpl (3)
11-16
: Good use of Administrator drop-ins for journald.This approach to placing the custom journald config in
/etc/systemd/journald.conf.d
with a high precedence filename (99-custom.conf) is a clean way to override default settings.
57-61
: Great use of a retry mechanism for CloudWatch Agent installation.The retry command ensures resilience in case of transient network failures. Confirm that this step is being logged or monitored so any persistent install failures are visible.
62-62
: Confirm Tailscale has enough disk space under journald constraints.Ensure that the journald limits (e.g., 200M) do not negatively impact Tailscale’s logs during peak usage or debugging sessions.
main.tf (3)
29-31
: Good practice: externalizing journald parameters.Passing
journald_system_max_use
andjournald_max_retention_sec
via locals helps keep the user data flexible.
40-40
: Module version upgraded to 1.3.0.Since the update includes journald configurations and CloudWatch Agent setup, make sure to document any new usage assumptions for downstream users.
127-131
: CloudWatchAgentServerPolicy role attachment looks correct.Attaching this policy to the same role used by the Tailscale instance is a clean way to provide CloudWatch Agent permissions without introducing extra roles.
variables.tf (1)
83-83
: Switching instance type to t4g.nano.Note that t4g.* is ARM-based. Ensure your software suite (including Tailscale dependencies or any other native binaries) is compatible with ARM architecture.
README.md (6)
62-69
: Clear explanation of the new system logging and monitoring setup.Documenting journald retention and CloudWatch Agent installation ensures that users understand and can easily customize these new features.
82-85
: Updated provider constraints appear consistent.Specifying a minimum supported provider version helps users avoid unexpected compatibility issues.
93-93
: Module version reference (1.3.0).Ensure you’ve tested a full run with this version to confirm stable provisioning.
98-102
: Resource listing includes the new IAM policy attachment.Nicely documented in the table, ensuring discoverability of the new resource.
126-128
: Updated instance_type and journald variables.These new variable defaults reinforce the changes in the user data script, ensuring consistent behavior out of the box.
147-147
: Enhanced explanation of ssm_state_enabled.The updated description helps users better understand storing Tailscale state in AWS SSM, complementing the journald and CloudWatch enhancements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, love it!
what
why
references
Summary by CodeRabbit
Documentation
New Features
Chores