Add optional backups for on-disk databases to rvn-ec2-service - #119
Open
devin-ai-integration[bot] wants to merge 2 commits into
Open
Add optional backups for on-disk databases to rvn-ec2-service#119devin-ai-integration[bot] wants to merge 2 commits into
devin-ai-integration[bot] wants to merge 2 commits into
Conversation
Co-Authored-By: brandon <brandon@flightcontrol.dev>
flybayer
self-requested a review
August 18, 2026 21:02
Contributor
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Ravion Module Publish PlanDry run only. No Ravion API mutations were made.
Diffsrvn-ec2-service n/a -> 1.5.0--- remote
+++ compiled
-description: Runs supervised workloads on a stable EC2 Auto Scaling Group, with optional shared ALB routing and switchable container or manual in-place deploys.
+description: Runs supervised workloads on a stable EC2 Auto Scaling Group, with optional shared ALB routing, local EBS data, and switchable container or manual in-place deploys.
name: EC2 Service
type: rvn-ec2-servicervn-ec2-service 1.4.1 -> 1.5.0--- remote
+++ compiled
show_when:
data_volume_creation_enabled: true
type: string
+ - collapsible: true
+ description: Optional EBS snapshot ID to restore into the data volume when a replacement instance boots. Clear this after the restore is complete.
+ id: data_volume_snapshot_id
+ label: Data volume snapshot ID
+ patterns:
+ - message: Enter an EBS snapshot ID.
+ pattern: ^snap-[a-z0-9]+$
+ placeholder: snap-...
+ required: false
+ show_when:
+ data_volume_creation_enabled: true
+ type: string
+ - description: EBS snapshots and engine-native logical dumps for on-disk application data.
+ id: section_backups
+ label: Backups
+ type: section
- default: false
+ description: Schedule Amazon Data Lifecycle Manager snapshots for this service's instances. Opt in for on-disk databases such as SQLite or Postgres.
+ id: backup_enabled
+ label: Enable EBS backups
+ type: boolean
+ - default: 24
+ description: Maximum age of a scheduled snapshot, and therefore the honest backup RPO.
+ id: backup_interval_hours
+ label: Backup interval (hours)
+ max: 24
+ min: 1
+ show_when:
+ backup_enabled: true
+ type: number
+ values:
+ - label: Every hour
+ value: 1
+ - label: Every 2 hours
+ value: 2
+ - label: Every 3 hours
+ value: 3
+ - label: Every 4 hours
+ value: 4
+ - label: Every 6 hours
+ value: 6
+ - label: Every 8 hours
+ value: 8
+ - label: Every 12 hours
+ value: 12
+ - label: Daily
+ value: 24
+ - default: 05:00
+ description: UTC time in HH:MM format when the daily schedule begins.
+ id: backup_start_time
+ label: Backup start time (UTC)
+ patterns:
+ - message: Use HH:MM in UTC.
+ pattern: ^(0[0-9]|1[0-9]|2[0-3]):[0-5][0-9]$
+ show_when:
+ backup_enabled: true
+ type: string
+ - default: 7
+ description: Number of snapshots retained for this schedule.
+ id: backup_retention_count
+ label: Snapshots to retain
+ min: 1
+ show_when:
+ backup_enabled: true
+ type: number
+ - default: false
+ description: Include the operating system volume in each snapshot set. It is included automatically when no data volume exists.
+ id: backup_root_volume_included
+ label: Include root volume
+ show_when:
+ backup_enabled: true
+ type: boolean
+ - default: filesystem_freeze
+ description: Freeze the data filesystem before snapshots, take crash-consistent snapshots, or provide database-specific pre/post commands.
+ id: backup_consistency_mode
+ label: Backup consistency mode
+ show_when:
+ backup_enabled: true
+ type: string
+ values:
+ - description: Sync and freeze the data volume without touching / or /boot.
+ label: Filesystem freeze
+ value: filesystem_freeze
+ - description: Snapshot without scripts; database engines must recover as after a power loss.
+ label: Crash consistent
+ value: crash_consistent
+ - description: Run your own database quiesce commands before and after the snapshot.
+ label: Custom
+ value: custom
+ - description: Command that quiesces the database before each snapshot. Required for Custom consistency mode.
+ id: backup_pre_script_command
+ label: Backup pre-script command
+ required: true
+ show_when:
+ backup_consistency_mode: custom
+ backup_enabled: true
+ type: text
+ - description: Command that resumes the database after each snapshot. Required for Custom consistency mode.
+ id: backup_post_script_command
+ label: Backup post-script command
+ required: true
+ show_when:
+ backup_consistency_mode: custom
+ backup_enabled: true
+ type: text
+ - collapsible: true
+ description: Optional AWS region for a second snapshot copy. Cross-region copies incur additional storage and transfer costs.
+ id: backup_cross_region_copy_destination
+ label: Cross-region copy destination
+ required: false
+ show_when:
+ backup_enabled: true
+ type: string
+ values: $values:aws/regions
+ - description: Engine-native logical backups shipped to S3 or EFS, with optional restore and termination-time automation.
+ id: section_logical_backups
+ label: Logical dumps
+ type: section
+ - default: false
+ description: Run engine-native dumps on a systemd schedule and ship them off the instance.
+ id: backup_dump_enabled
+ label: Enable logical dumps
+ type: boolean
+ - description: Root command that writes logical backup artifacts into the RAVION_BACKUP_DIR directory.
+ id: backup_dump_command
+ label: Dump command
+ required: true
+ show_when:
+ backup_dump_enabled: true
+ type: text
+ - description: Root command that reads logical backup artifacts from RAVION_BACKUP_DIR.
+ id: backup_dump_restore_command
+ label: Restore command
+ required: true
+ show_when:
+ backup_dump_restore_on_first_boot_enabled: true
+ type: text
+ - default: "*-*-* 04:00:00 UTC"
+ description: systemd OnCalendar expression for logical dumps, not cron syntax.
+ id: backup_dump_schedule
+ label: Dump schedule
+ show_when:
+ backup_dump_enabled: true
+ type: string
+ - default: s3
+ description: Store logical dumps in a module-created or supplied S3 bucket, or on the existing EFS mount.
+ id: backup_dump_destination
+ label: Dump destination
+ show_when:
+ backup_dump_enabled: true
+ type: string
+ values:
+ - label: S3
+ value: s3
+ - label: EFS
+ value: efs
+ - description: Optional existing S3 bucket ARN. Leave blank to create a dedicated encrypted bucket.
+ id: backup_dump_s3_bucket_arn
+ label: S3 bucket ARN
+ placeholder: arn:aws:s3:::bucket-name
+ required: false
+ show_when:
+ backup_dump_destination: s3
+ backup_dump_enabled: true
+ type: string
+ - default: backups/
+ description: Prefix under which this service's timestamped manifests and dump artifacts are stored.
+ id: backup_dump_s3_prefix
+ label: S3 prefix
+ show_when:
+ backup_dump_destination: s3
+ backup_dump_enabled: true
+ type: string
+ - default: 30
+ description: Number of days to retain logical dump artifacts.
+ id: backup_dump_retention_days
+ label: Logical dump retention (days)
+ min: 1
+ show_when:
+ backup_dump_enabled: true
+ type: number
+ - default: 48
+ description: Maximum expected interval between successful dumps; set this higher than the dump schedule interval so the freshness alarm is meaningful.
+ id: backup_dump_max_interval_hours
+ label: Maximum dump interval (hours)
+ min: 1
+ show_when:
+ backup_dump_enabled: true
+ type: number
+ - default: false
+ description: Allow Terraform to delete a module-created S3 backup bucket that still contains backups.
+ id: backup_dump_force_deletion_enabled
+ label: Allow backup bucket deletion
+ show_when:
+ backup_dump_destination: s3
+ backup_dump_enabled: true
+ type: boolean
+ - default: false
+ description: Discover and restore the newest manifest-based logical dump before starting the application on a replacement instance.
+ id: backup_dump_restore_on_first_boot_enabled
+ label: Restore latest dump on first boot
+ show_when:
+ backup_dump_enabled: true
+ type: boolean
+ - description: Refuse to restore and block application startup when the newest dump is older than this limit.
+ id: backup_max_age_hours
+ label: Maximum restore age (hours)
+ min: 1
+ required: false
+ show_when:
+ backup_dump_enabled: true
+ backup_dump_restore_on_first_boot_enabled: true
+ type: number
+ - default: true
+ description: Run a final logical dump through an Auto Scaling lifecycle hook before planned instance termination.
+ id: backup_on_termination_enabled
+ label: Backup on planned termination
+ show_when:
+ backup_dump_enabled: true
+ type: boolean
+ - default: 1800
+ description: Maximum time allowed for the planned-termination dump; the automation step keeps 60 seconds of slack before the lifecycle hook expires.
+ id: backup_on_termination_timeout_seconds
+ label: Planned termination timeout (secs)
+ max: 7200
+ min: 300
+ show_when:
+ backup_dump_enabled: true
+ type: number
+ - default: true
+ description: Alarm when the shared CloudWatch log group has no recent successful logical dump record.
+ id: backup_dump_failure_alarm_enabled
+ label: Logical dump freshness alarm
+ show_when:
+ backup_dump_enabled: true
+ type: boolean
+ - description: Continuous SQLite replication to S3 with Litestream.
+ id: section_replication
+ label: Continuous replication
+ type: section
+ - default: false
+ description: Continuously replicate one SQLite database to S3 with a pinned Litestream release.
+ id: backup_replication_enabled
+ label: Enable continuous replication
+ type: boolean
+ - default: litestream
+ description: The replication engine. Litestream is currently the only supported engine.
+ id: backup_replication_engine
+ label: Replication engine
+ show_when:
+ backup_replication_enabled: true
+ type: string
+ values:
+ - label: Litestream (SQLite)
+ value: litestream
+ - description: Absolute path to the SQLite database on the data volume, such as /data/app.db.
+ id: backup_replication_database_path
+ label: SQLite database path
+ required: true
+ show_when:
+ backup_replication_enabled: true
+ type: string
+ - description: Optional existing S3 bucket ARN for the replica. Leave blank to create a dedicated encrypted bucket.
+ id: backup_replication_s3_bucket_arn
+ label: Replication S3 bucket ARN
+ placeholder: arn:aws:s3:::bucket-name
+ required: false
+ show_when:
+ backup_replication_enabled: true
+ type: string
+ - default: false
+ description: Restore the newest Litestream replica before starting the application on a replacement instance.
+ id: backup_replication_restore_on_first_boot_enabled
+ label: Restore replica on first boot
+ show_when:
+ backup_replication_enabled: true
+ type: boolean
+ - default: 1m
+ description: Litestream full snapshot interval, such as 1m or 1h. Full snapshots are expensive, so choose a value shorter than the retention duration.
+ id: backup_replication_snapshot_interval
+ label: Replica snapshot interval
+ show_when:
+ backup_replication_enabled: true
+ type: string
+ - default: 24h
+ description: Litestream snapshot retention duration, such as 24h or 168h. It must exceed the snapshot interval.
+ id: backup_replication_retention
+ label: Replica retention
+ show_when:
+ backup_replication_enabled: true
+ type: string
+ - description: Refuse to restore and block application startup when the newest replica is older than this limit.
+ id: backup_replication_max_age_hours
+ label: Maximum replica age (hours)
+ min: 1
+ required: false
+ show_when:
+ backup_replication_enabled: true
+ backup_replication_restore_on_first_boot_enabled: true
+ type: number
+ - default: false
description: Mount a shared network filesystem that every instance can access. Use it when multiple instances need the same files, or when files must survive instance replacement without a restore step.
id: efs_enabled
label: EFS file system
@@
Instances are as stable as an EC2 instance you launch yourself in the AWS console. Deploys, app restarts, and stack updates do not replace them, so each instance keeps its root and optional data volume, and everything on those disks, for its whole life. Even changing the AMI leaves running instances alone: the change becomes a new launc
... diff truncated ... |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+25
to
+35
| if [ -n "$DATA_DEVICE" ] && [ -b "$DATA_DEVICE" ]; then | ||
| DATA_FSTYPE=$(lsblk -no FSTYPE "$DATA_DEVICE" | tr -d '[:space:]') | ||
| if [ -z "$DATA_FSTYPE" ]; then | ||
| mkfs -t xfs "$DATA_DEVICE" | ||
| DATA_FSTYPE="xfs" | ||
| fi | ||
| done | ||
| if [ -n "$DATA_DEVICE" ]; then | ||
| mkfs -t xfs "$DATA_DEVICE" | ||
| mkdir -p ${data_volume_mount_path} | ||
| mkdir -p "${data_volume_mount_path}" | ||
| DATA_UUID=$(blkid -s UUID -o value "$DATA_DEVICE") | ||
| echo "UUID=$DATA_UUID ${data_volume_mount_path} xfs defaults,nofail 0 2" >> /etc/fstab | ||
| mount -a | ||
| sed -i "\|[[:space:]]${data_volume_mount_path}[[:space:]]|d" /etc/fstab | ||
| echo "UUID=$${DATA_UUID} ${data_volume_mount_path} $${DATA_FSTYPE} defaults,nofail 0 2" >> /etc/fstab | ||
| mount "${data_volume_mount_path}" || mount -a |
There was a problem hiding this comment.
If a custom AMI lacks the expected NVMe aliases or ebsnvme-id, reports the mapping as /dev/sdf, or the resolved device fails to mount, this block continues without verifying that the data volume backs data_volume_mount_path. Litestream and the application can then initialize on the root filesystem, causing the intended data volume to remain unused and that state to be hidden by a later mount or lost on instance replacement.
Knowledge Base Used: EC2 Service deployment and durability
Prompt To Fix With AI
This is a comment left during a code review.
Path: compute/ec2_service/templates/user_data.sh.tpl
Line: 25-35
Comment:
**Data-volume mount failures fall through**
If a custom AMI lacks the expected NVMe aliases or `ebsnvme-id`, reports the mapping as `/dev/sdf`, or the resolved device fails to mount, this block continues without verifying that the data volume backs `data_volume_mount_path`. Litestream and the application can then initialize on the root filesystem, causing the intended data volume to remain unused and that state to be hidden by a later mount or lost on instance replacement.
**Knowledge Base Used:** [EC2 Service deployment and durability](https://app.greptile.com/flightcontrol/-/custom-context/knowledge-base/ravionhq/modules/-/docs/ec2-service.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Contributor
Author
There was a problem hiding this comment.
Valid, and fixed in f57a249. Falling through was wrong in both directions: an unresolvable device and a mount that reports success while the target stays unmounted (mount -a can do exactly that) both let the app, Litestream, and the backup staging directory initialize on the root filesystem, which then vanishes on replacement.
Since data_volume_creation_enabled means the operator explicitly asked for a separate volume, this is now fatal rather than a fallback:
else
echo "FATAL: Data volume device ${data_volume_device_name} could not be resolved for mount path ${data_volume_mount_path}."
exit 1
fi
if ! findmnt -rn --mountpoint "${data_volume_mount_path}" >/dev/null 2>&1; then
echo "FATAL: Data volume device ${data_volume_device_name} is not mounted at ${data_volume_mount_path}."
exit 1
fiThe verification is findmnt rather than the mount exit code for that reason, and it runs after the mount attempt so it also covers the /dev/sdf-style mapping case you mention. Device resolution order, format-only-if-blank, the fstab rewrite, and mounting an already-formatted snapshot-restored volume are unchanged.
Co-Authored-By: brandon <brandon@flightcontrol.dev>
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.
Summary
Makes
rvn-ec2-servicea defensible home for a service whose database lives on its EBS data volume. Today both volumes aredelete_on_termination = trueand nothing snapshots or restores them, so "back it up yourself" is the entire story — which is why agents refuse to pick this module for stateful workloads. This adds backups as one optional feature (backup_enabledand friends all default off) with three layers the operator can adopt independently:RavionBackup = <name>, with consistency modesfilesystem_freeze(default when a data volume exists:sync,fsfreeze -f, snapshot,fsfreeze -uon the data mount only — the approach AWS documents for MySQL/PostgreSQL),crash_consistent, orcustompre/post commands. Optional cross-region copies. Restore is deliberate: setdata_volume_snapshot_idand recycle.Engine-agnosticism lives in layers 1 and 2: layer 2 is a symmetric command contract you supply, so it covers Postgres, MySQL, Redis or a file tarball equally.
Only layer 3 is engine-specific, unavoidably: continuous replication has to understand the engine's write stream. PostgreSQL continuous archiving stays a documented WAL-G/pgBackRest recipe rather than an input, because it needs
archive_commandinsidepostgresql.confplus engine credentials this module does not own.The parts that aren't obvious from the diff
A live bug fixed on the way.
user_data.sh.tplpicked the data device by looking for the disk without a filesystem and mounted it only inside the formatting branch, so any already-formatted volume — including every snapshot-restored one — silently got no fstab entry and was never mounted. Device resolution now falls back toebsnvme-idmatching, formats only blank disks, and always writes fstab and mounts.Failed discovery must not look like a fresh service. The dangerous failure mode for restore-on-first-boot is coming up empty and healthy when data existed. So the paths are separated explicitly:
Freshness comes from the artifact, not the key. Each artifact writes
manifest.jsonwithcompleted_at_epoch, and the manifest is uploaded after the payload (--exclude manifest.jsonon the recursive copy, then a secondcp; EFS copies it last) so a restore can never observe a completion record for a half-uploaded backup.One timeout, not three. A termination dump's real ceiling is the ASG lifecycle-hook heartbeat, so
backup_on_termination_timeout_secondsdrives the heartbeat and the Automation step takestimeout - 60for slack, guaranteeingCompleteLifecycleActionstill runs. The hook'sCONTINUEresult means a slow or failed dump can never wedge the group.Litestream 0.5, not the blog posts. 0.5 dropped
-if-db-not-exists/-if-replica-existsand movedsnapshot-interval/retentionunder asnapshot:block — and its config parser silently ignores unknown keys, so a 0.3-era config would have shipped retention settings that did nothing. Discovery useslitestream ltx -level all -json; restore uses-integrity-check full. Version is pinned internally (0.5.12) with per-arch checksum verification rather than exposed as an input.Restore-on-first-boot with several instances is documented as diverging rather than gated, per review discussion — it's the operator's call. EBS Multi-Attach remains deliberately unsupported, and the docs now say why: it's a shared block device, not a shared filesystem, so ext4/XFS corrupt under concurrent mounts; EFS is the shared-files answer but is not a live database directory.
Testing
tofu fmt -recursive,tofu validate,tofu test(22 passed) and the module-tools suite (83 passed) all pass, with new tests covering snapshot restore sizing, the consistency-mode preconditions, dump/replication resource wiring, the termination timeout, and replication bucket selection when dumps use EFS.make publish-local-dev MODULE=rvn-ec2-servicecould not run — it needs the Ravion API onlocalhost:8080, which isn't reachable from this environment — so publishing 1.5.0 is still a manual step.Supersedes #115, #116 and #118, which were the same work split into a stack; combined here for review.
Link to Devin session: https://app.devin.ai/sessions/2b20f3ce4e7744a69642a468e5b7d463
Requested by: @flybayer
Greptile Summary
The PR adds optional scheduled EBS snapshots, logical dumps with restore and termination hooks, and Litestream replication to the EC2 service module. It also revises first-boot data-volume discovery and mounting, where a silent failure path can direct state to the root filesystem.
Confidence Score: 4/5
The data-volume mount failure path must be fixed before merging because it can silently place database state and restore markers on the disposable root volume.
Bootstrap does not require successful data-volume resolution and mounting before Litestream or the application begins using the configured data path, so supported custom-AMI and mount-failure conditions can produce misplaced and subsequently lost state.
Files Needing Attention: compute/ec2_service/templates/user_data.sh.tpl, compute/ec2_service/templates/backup_replication.sh.tpl
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[EC2 instance bootstrap] --> B[Resolve and mount data EBS volume] B --> C{Restore enabled?} C -->|Logical dump| D[Discover manifest in S3 or EFS] C -->|Litestream| E[Discover LTX replica in S3] D --> F[Restore database and write marker] E --> F C -->|No| G[Configure runtime] F --> G G --> H[Start scheduled dumps and replication] H --> I[Application deploy] J[ASG planned termination] --> K[Lifecycle hook] K --> L[Final logical dump] L --> M[Continue termination]Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "Add optional backups for on-disk databas..." | Re-trigger Greptile
Context used (3)