feat: add support for revertable snapshots with boot-time rollback capability

[trgill]

Enhancement:

Add support for revertable snapshots with boot-time rollback capability. This feature allows users to create snapshot sets with revert boot menu entries, enabling disaster recovery scenarios where the system can be rolled back to a previous state by first running the revert action and then booting into the revert entry.

Reason:

System administrators need a reliable disaster recovery mechanism for critical operations like system upgrades or configuration changes. The existing bootable snapshot feature allows booting into a snapshot, but does not provide a way to permanently revert the system to that snapshot state. Revertable snapshots fill this gap by providing a two-step process to merge snapshots back into their origin volumes, effectively rolling back the system to the snapshot point-in-time.

Result:

• Added snapshot_lvm_revertable parameter (boolean, default false) that can be set globally or per snapshot set
• Requires snapshot manager (snapm) version 0.5.0 or later
• Version validation ensures snapm supports the revert parameter, with explicit error messages if unavailable
• Revert workflow:
  1. Create snapshots with snapshot_lvm_revertable: true to generate revert boot entries
  2. Run snapshot_lvm_action: revert to initiate the revert operation
  3. Reboot into the revert boot entry to complete the merge
• Comprehensive test coverage with tests_basic_revertable.yml and tests_set_revertable.yml
• Updated documentation with clear explanation of the two-step revert process, including warnings that the revert boot entry alone does NOT perform a revert
• Added complete workflow example showing disaster recovery usage
• Follows the same parameter handling pattern as the existing bootable feature, supporting both global and per-set configuration with conflict detection

Issue Tracker Tickets (Jira or BZ if any):

Related to snapshotmanager/snapm#999 (clarification of revertable behavior)

Summary by CodeRabbit

• New Features
  • Added revert-capable LVM snapshot support that creates/manages a corresponding revert boot entry, with global (snapshot_lvm_revertable) and per set (revertable) controls.
  • Added snapm version gating (min 0.5.0), precedence/conflict detection, and explicit errors for unsupported or conflicting configurations.
• Documentation
  • Updated revert action docs to clarify the two-step “reboot to complete revert” workflow; expanded with a full runnable example and variable details.
• Tests
  • Added new revertable playbooks and improved snapshot status templating and idempotence/check-mode coverage, including tolerant failures for older snapm.
• Chores
  • Updated ignore rules to exclude CLAUDE.md.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Changes

Adds revertable snapshot parameters, snapm version and conflict handling, validation, workflow documentation, and Ansible integration tests covering creation, status, removal, and idempotence.

Revertable snapshot workflow

Layer / File(s)	Summary
Revertable option contract and propagation library/snapshot.py, module_utils/snapshot_lsr/consts.py, module_utils/snapshot_lsr/validate.py	Adds revertable module and snapset options, status codes, and propagation into snapshot-set data.
snapm revert handling module_utils/snapshot_lsr/snapmgr.py, module_utils/snapshot_lsr/lvm.py	Checks snapm support, detects conflicts, passes supported revert parameters, and rejects unsupported revertable snapshots.
Revertable workflow verification tests/get_snapset_status.yml, tests/tests_basic_revertable.yml, tests/tests_set_revertable.yml	Tests creation, status, removal, idempotence, compatibility failures, and cleanup.
Revert workflow documentation README.md, .gitignore	Documents the reboot-based revert workflow, requirements, deferred merging, and adds CLAUDE.md to ignored files.

Suggested reviewers: spetrosi

🚥 Pre-merge checks | ✅ 4 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description Format	⚠️ Warning	PR description has Enhancement/Reason/Result, but it is missing the required Signed-off-by line.	Add a Signed-off-by: Name <email> line to the PR description (or use git commit -s) and resubmit.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title follows Conventional Commits and accurately summarizes the revertable snapshot feature.
Description check	✅ Passed	The description includes the required sections and sufficiently explains the feature, rationale, results, and issue reference.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (2)

README.md (1)

126-198: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Document the global configuration and conflict behavior.

The example only demonstrates per-set revertable, while the new public variable is snapshot_lvm_revertable. Add a global-configuration example and explain how global and per-set values interact, including what happens when they conflict.

As per path instructions, README.md must document all new user-facing variables and include usage examples for new functionality.

Also applies to: 335-351

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@README.md` around lines 126 - 198, The README’s revertable documentation
omits the global snapshot_lvm_revertable setting and its precedence rules.
Update the revertable configuration section and workflow examples to show how to
enable it globally, document how global and per-set values interact, and
explicitly describe the behavior when they conflict; ensure all new user-facing
variables have usage examples.

Source: Path instructions

tests/tests_set_revertable.yml (1)

63-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Missing idempotence check for the revertable creation step.

Unlike tests/tests_basic_revertable.yml, which re-runs the snapshot creation step and asserts not snapshot_cmd["changed"] (lines 62-72 there), this file only checks idempotence for the remove action, not for the initial revertable creation. Per-set revertable creation idempotence is untested.

♻️ Suggested addition mirroring tests_basic_revertable.yml

         - name: Fail if not revertable
           assert:
             that: is_revertable

+        - name: Run the snapshot role again to check idempotence
+          include_tasks: tasks/run_role_with_clear_facts.yml
+          vars:
+            snapshot_lvm_action: snapshot
+            snapshot_lvm_revertable: true
+            snapshot_lvm_set: "{{ snapshot_test_set }}"
+
+        - name: Assert no changes for create snapset idempotence
+          assert:
+            that: not snapshot_cmd["changed"]
+
         - name: Remove the snapshot set

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/tests_set_revertable.yml` around lines 63 - 97, The revertable
snapshot-set creation flow in tests_set_revertable.yml lacks an idempotence
assertion. Re-run the existing snapshot creation task using the same
snapshot_lvm_action, snapshot_lvm_revertable, and snapshot_lvm_set values, then
assert that snapshot_cmd["changed"] is false before continuing to the snapset
status checks.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@module_utils/snapshot_lsr/snapmgr.py`:
- Around line 254-257: Replace the truthiness checks for snapshot_lvm_revertable
with explicit None checks so provided False values are preserved. Apply this in
module_utils/snapshot_lsr/snapmgr.py lines 254-257 and
module_utils/snapshot_lsr/validate.py lines 39-41 and 347-349; also inspect
adjacent snapshot_lvm_bootable checks and apply the same correction only if they
exhibit the same omission-versus-False behavior.

In `@tests/tests_basic_revertable.yml`:
- Around line 41-84: Add verification after the revertable snapshot creation in
the test flow around “Assert changes for creation” by including the existing
get_snapset_status.yml task and asserting the returned snapset status has
is_revertable set. Mirror the established verification pattern from
tests_set_revertable.yml, while preserving the existing creation, idempotence,
and snapshot verification checks.

In `@tests/tests_set_revertable.yml`:
- Around line 98-104: The rescue matchers in tests/tests_set_revertable.yml
lines 98-104 and tests/tests_basic_revertable.yml lines 124-131 must match
revertable-specific snapm failures instead of bootable messages. Replace the
existing msg1/msg2 patterns in both rescues with patterns covering “Revertable
snapshots require snapm version %s or later” and “Revertable snapshots are not
supported without snapm”, preserving the current assertion structure.

---

Nitpick comments:
In `@README.md`:
- Around line 126-198: The README’s revertable documentation omits the global
snapshot_lvm_revertable setting and its precedence rules. Update the revertable
configuration section and workflow examples to show how to enable it globally,
document how global and per-set values interact, and explicitly describe the
behavior when they conflict; ensure all new user-facing variables have usage
examples.

In `@tests/tests_set_revertable.yml`:
- Around line 63-97: The revertable snapshot-set creation flow in
tests_set_revertable.yml lacks an idempotence assertion. Re-run the existing
snapshot creation task using the same snapshot_lvm_action,
snapshot_lvm_revertable, and snapshot_lvm_set values, then assert that
snapshot_cmd["changed"] is false before continuing to the snapset status checks.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f18c9105-5165-446e-a2ee-cb02899c33d3

📥 Commits

Reviewing files that changed from the base of the PR and between 20514dd and 6f2b5cf.

📒 Files selected for processing (10)

• .gitignore
• README.md
• library/snapshot.py
• module_utils/snapshot_lsr/consts.py
• module_utils/snapshot_lsr/lvm.py
• module_utils/snapshot_lsr/snapmgr.py
• module_utils/snapshot_lsr/validate.py
• tests/get_snapset_status.yml
• tests/tests_basic_revertable.yml
• tests/tests_set_revertable.yml

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@README.md`:
- Around line 126-143: Update the README sections documenting
snapshot_lvm_set.revertable and global snapshot_lvm_revertable to explicitly
describe their precedence and the validation behavior when both scopes are
configured inconsistently. Cover both configuration locations, state which value
takes effect, and document the conflict outcome supported by the implementation.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 05d804bd-bdec-49f4-9887-ce6556d87743

📥 Commits

Reviewing files that changed from the base of the PR and between b593ed8 and 027c1ab.

📒 Files selected for processing (10)

• .gitignore
• README.md
• library/snapshot.py
• module_utils/snapshot_lsr/consts.py
• module_utils/snapshot_lsr/lvm.py
• module_utils/snapshot_lsr/snapmgr.py
• module_utils/snapshot_lsr/validate.py
• tests/get_snapset_status.yml
• tests/tests_basic_revertable.yml
• tests/tests_set_revertable.yml

🚧 Files skipped from review as they are similar to previous changes (9)

• module_utils/snapshot_lsr/consts.py
• .gitignore
• tests/tests_set_revertable.yml
• tests/tests_basic_revertable.yml
• tests/get_snapset_status.yml
• module_utils/snapshot_lsr/validate.py
• library/snapshot.py
• module_utils/snapshot_lsr/lvm.py
• module_utils/snapshot_lsr/snapmgr.py

[richm]

[citest]

[richm]

note that Testing Farm is having some issues, so it will be several hours before we get TF test results

[richm]

[citest]

[spetrosi]

tests_set_revertable fails please take a look

[spetrosi]

Have you written your custom CLAUDE.md?
The role has a file .coderabbit.yaml with best-practices that CodeRabbit uses for review, and it can also be used for best-practices while developing. I wonder if creating a symlink from .coderabbit.yaml to CLAUDE.md would help?

[richm]

Or in CLAUDE.md, tell it to use the best-practices from .coderabbit.yaml

[trgill]

yes, I wrote a CLAUDE.md - but it isn't polished. I was just giving claude tips for testing, naming conventions, python version compatability etc. It doesn't look like the other roles use it, so I added it to the .gitignore.

[richm]

[citest]

[richm]

lgtm - @spetrosi ?