From 2ee1d355790bbf800d85f92ea86580301360c1a5 Mon Sep 17 00:00:00 2001 From: Todd Gill Date: Thu, 16 Jul 2026 11:46:24 -0400 Subject: [PATCH 1/7] feat: add snapshot_lvm_revertable parameter to module interface Add the snapshot_lvm_revertable parameter to the snapshot module, allowing users to create revert boot entries for snapshot sets. This parameter is supported at both the global level and within snapshot_lvm_set definitions. A revert boot entry allows booting into a special mode that automatically merges the snapshot back into the origin volume, providing a boot-time rollback mechanism for disaster recovery. Signed-off-by: Todd Gill --- library/snapshot.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/library/snapshot.py b/library/snapshot.py index d2194a8c..c3547392 100644 --- a/library/snapshot.py +++ b/library/snapshot.py @@ -100,6 +100,13 @@ When set to true, and passed to the 'snapshot' command, the snapshot created will have a corresponding boot entry. The boot entry will be removed when the snapset is removed. type: bool + snapshot_lvm_revertable: + description: Only supported on operating systems that support snapshot manager (snapm). + When set to true, and passed to the 'snapshot' command, the snapshot created will have + a corresponding revert boot entry. Booting into the revert entry will trigger an automatic + revert operation, merging the snapshot back into the origin volume. The revert boot entry + will be removed when the snapset is removed. + type: bool snapshot_lvm_set: description: set of volumes type: dict @@ -110,6 +117,9 @@ bootable: description: create snapshot with boot menu entry (requires snapm) type: bool + revertable: + description: create snapshot with revert boot menu entry (requires snapm) + type: bool volumes: description: list of volumes type: list @@ -291,6 +301,7 @@ def run_module(): snapshot_lvm_verify_only=dict(type="bool"), snapshot_lvm_mount_origin=dict(type="bool"), snapshot_lvm_bootable=dict(type="bool"), + snapshot_lvm_revertable=dict(type="bool"), snapshot_lvm_mountpoint_create=dict(type="bool"), snapshot_lvm_unmount_all=dict(type="bool"), snapshot_lvm_percent_space_required=dict(type="str"), @@ -306,6 +317,7 @@ def run_module(): options=dict( name=dict(type="str"), bootable=dict(type="bool"), + revertable=dict(type="bool"), volumes=dict( type="list", elements="dict", From 886f98e480c69a2a399d7ff41ca28b2e220f44a5 Mon Sep 17 00:00:00 2001 From: Todd Gill Date: Thu, 16 Jul 2026 11:46:24 -0400 Subject: [PATCH 2/7] feat: implement revertable snapshot support with version validation Add support for creating revert boot entries when snapm is available. This allows users to boot into a special mode that automatically merges snapshots back into their origin volumes for disaster recovery. The revertable parameter now properly validates snapm availability and version support, failing explicitly rather than being silently ignored. Changes: - Add SNAPM_REVERT_PARAM_MIN_VERSION constant (0.5.0) - Add has_revert_parameter() version check function - Implement revertable parameter handling with conflict detection - Add explicit version validation for bootable and revertable features - Fail with ERROR_REVERTABLE_NOT_SUPPORTED when snapm is unavailable or version doesn't support revert parameter - Pass bootable/revert parameters to snapm create_snapshot_set via kwargs pattern for clean version-conditional support - Propagate revertable parameter through validation layer - Add ERROR_REVERTABLE_NOT_SUPPORTED and ERROR_REVERTABLE_CONFLICT status codes - Add debug logging for create_snapshot_set calls The implementation mirrors the existing bootable parameter handling, supporting both global-level and snapset-level configuration with conflict detection. Both parameters now validate properly instead of silently ignoring unsupported features. Signed-off-by: Todd Gill --- module_utils/snapshot_lsr/consts.py | 2 + module_utils/snapshot_lsr/lvm.py | 14 +++++ module_utils/snapshot_lsr/snapmgr.py | 83 +++++++++++++++++++++++---- module_utils/snapshot_lsr/validate.py | 6 ++ 4 files changed, 93 insertions(+), 12 deletions(-) diff --git a/module_utils/snapshot_lsr/consts.py b/module_utils/snapshot_lsr/consts.py index 819cb6d4..19d347d2 100644 --- a/module_utils/snapshot_lsr/consts.py +++ b/module_utils/snapshot_lsr/consts.py @@ -61,6 +61,8 @@ class SnapshotStatus: ERROR_SNAPM_INTERNAL_ERROR = 42 ERROR_BOOTABLE_NOT_SUPPORTED = 43 ERROR_BOOTABLE_CONFLICT = 44 + ERROR_REVERTABLE_NOT_SUPPORTED = 45 + ERROR_REVERTABLE_CONFLICT = 46 COMMAND_ENV = {"LC_ALL": "C", "LVM_COMMAND_PROFILE": "lvmdbusd"} diff --git a/module_utils/snapshot_lsr/lvm.py b/module_utils/snapshot_lsr/lvm.py index 092a4349..5c0a15e4 100644 --- a/module_utils/snapshot_lsr/lvm.py +++ b/module_utils/snapshot_lsr/lvm.py @@ -1235,6 +1235,20 @@ def snapshot_set(module, snapset_json, check_mode): changed, ) + # If this function has been called with revertable snapshot requested, return error + # because snapm is required for revertable snapshots. + if any( + ( + snapset_json.get("snapshot_lvm_revertable", False), + snapset_json.get("revertable", False), + ) + ): + return ( + SnapshotStatus.ERROR_REVERTABLE_NOT_SUPPORTED, + "Revertable snapshots are not supported without snapm", + changed, + ) + rc, message = verify_snapset_source_lvs_exist(module, snapset_json) if rc != SnapshotStatus.SNAPSHOT_OK: return rc, message, changed diff --git a/module_utils/snapshot_lsr/snapmgr.py b/module_utils/snapshot_lsr/snapmgr.py index 9d5823e2..560c9852 100644 --- a/module_utils/snapshot_lsr/snapmgr.py +++ b/module_utils/snapshot_lsr/snapmgr.py @@ -35,6 +35,9 @@ # Minimum version of snapm that supports the boot parameter # in the create_snapshot_set function. SNAPM_BOOT_PARAM_MIN_VERSION = "0.4.3" +# Minimum version of snapm that supports the revert parameter +# in the create_snapshot_set function. +SNAPM_REVERT_PARAM_MIN_VERSION = "0.5.0" # NOTE: Because of PEP632, we cannot use distutils. @@ -73,6 +76,16 @@ def has_boot_parameter(): return True +def has_revert_parameter(): + + if not snapshot_manager_imported or lsr_parse_version( + snapm.__version__ + ) < lsr_parse_version(SNAPM_REVERT_PARAM_MIN_VERSION): + return False + + return True + + def mgr_get_snapshot_lv(module, origin_vg, origin_lv, snapshot_set): for snapshot in snapshot_set[0].snapshots: @@ -202,6 +215,7 @@ def mgr_check_verify_lvs_set(manager, module, snapset_json): def mgr_snapshot_cmd(module, module_args, snapset_json): bootable = None + revertable = None snapset_name = snapset_json["name"] logger.info("mgr_snapshot_cmd: %s", snapset_name) changed = False @@ -237,8 +251,48 @@ def mgr_snapshot_cmd(module, module_args, snapset_json): "changed": False, } + # Revertable global variable is set + if module_args["snapshot_lvm_revertable"]: + revertable = module_args["snapshot_lvm_revertable"] + + # Global is not set, check the snapset + if revertable is None: + if "revertable" in snapset_json: + revertable = snapset_json["revertable"] + else: + revertable = False + else: # Global is set, check for conflict + if ( + "revertable" in snapset_json + and snapset_json["revertable"] is not None + and revertable != snapset_json["revertable"] + ): + return { + "return_code": SnapshotStatus.ERROR_REVERTABLE_CONFLICT, + "errors": "Conflicting values for revertable", + "changed": False, + } + source_list = mgr_get_source_list_for_create(volume_list) + # Check if bootable is requested but not supported by snapm version + if bootable and not has_boot_parameter(): + return { + "return_code": SnapshotStatus.ERROR_BOOTABLE_NOT_SUPPORTED, + "errors": "Bootable snapshots require snapm version %s or later" + % SNAPM_BOOT_PARAM_MIN_VERSION, + "changed": False, + } + + # Check if revertable is requested but not supported by snapm version + if revertable and not has_revert_parameter(): + return { + "return_code": SnapshotStatus.ERROR_REVERTABLE_NOT_SUPPORTED, + "errors": "Revertable snapshots require snapm version %s or later" + % SNAPM_REVERT_PARAM_MIN_VERSION, + "changed": False, + } + if check_mode: return { "return_code": SnapshotStatus.SNAPSHOT_OK, @@ -249,19 +303,24 @@ def mgr_snapshot_cmd(module, module_args, snapset_json): manager = snap_manager.Manager() try: + # Build kwargs for create_snapshot_set based on available parameters + kwargs = {} if has_boot_parameter(): - manager.create_snapshot_set( - snapset_name, - source_list, - SNAPM_DEFAULT_SIZE_POLICY, - boot=bootable, - ) - else: - manager.create_snapshot_set( - snapset_name, - source_list, - SNAPM_DEFAULT_SIZE_POLICY, - ) + kwargs["boot"] = bootable + if has_revert_parameter(): + kwargs["revert"] = revertable + + logger.info( + "DEBUG: Calling create_snapshot_set with snapset_name=%s, " + "source_list=%s, default_size_policy=%s, kwargs=%s", + snapset_name, + source_list, + SNAPM_DEFAULT_SIZE_POLICY, + kwargs, + ) + manager.create_snapshot_set( + snapset_name, source_list, SNAPM_DEFAULT_SIZE_POLICY, **kwargs + ) changed = True except snapm.SnapmError as snap_err: # if the set already exists, return ok diff --git a/module_utils/snapshot_lsr/validate.py b/module_utils/snapshot_lsr/validate.py index b72defac..52f966ac 100644 --- a/module_utils/snapshot_lsr/validate.py +++ b/module_utils/snapshot_lsr/validate.py @@ -36,6 +36,9 @@ def get_json_from_args(module, module_args, vg_include): if module_args["snapshot_lvm_bootable"]: args_dict["bootable"] = module_args["snapshot_lvm_bootable"] + if module_args["snapshot_lvm_revertable"]: + args_dict["revertable"] = module_args["snapshot_lvm_revertable"] + for vg, lv_list in vgs_lvs_iterator( module, module_args["snapshot_lvm_vg"], @@ -341,6 +344,9 @@ def validate_snapset_json(cmd, module_args, verify_only): if module_args["snapshot_lvm_bootable"]: snapset_dict["snapshot_lvm_bootable"] = module_args["snapshot_lvm_bootable"] + if module_args["snapshot_lvm_revertable"]: + snapset_dict["snapshot_lvm_revertable"] = module_args["snapshot_lvm_revertable"] + if cmd == SnapshotCommand.SNAPSHOT: rc, message = validate_json_request(snapset_dict, True) elif cmd == SnapshotCommand.CHECK and not verify_only: From 424c43328f60ca1ff74bf19517d98e6a67d5452c Mon Sep 17 00:00:00 2001 From: Todd Gill Date: Thu, 16 Jul 2026 11:46:24 -0400 Subject: [PATCH 3/7] docs(tests): add test coverage and documentation for revertable snapshots Add comprehensive test coverage for the revertable snapshot feature: - tests_basic_revertable.yml: Test global snapshot_lvm_revertable parameter - tests_set_revertable.yml: Test revertable parameter in snapshot sets - Update get_snapset_status.yml to extract revert entry information Add documentation in README.md: - Document snapshot_lvm_revertable parameter and requirements - Add example showing bootable and revertable used together - Explain boot-time rollback mechanism Tests follow the existing bootable test patterns and include proper version checks and rescue blocks for older snapm versions. Signed-off-by: Todd Gill --- README.md | 34 +++++++ module_utils/snapshot_lsr/snapmgr.py | 6 +- tests/get_snapset_status.yml | 7 +- tests/tests_basic_revertable.yml | 136 +++++++++++++++++++++++++++ tests/tests_set_revertable.yml | 109 +++++++++++++++++++++ 5 files changed, 290 insertions(+), 2 deletions(-) create mode 100644 tests/tests_basic_revertable.yml create mode 100644 tests/tests_set_revertable.yml diff --git a/README.md b/README.md index 3940c0f6..7c6a9ba2 100644 --- a/README.md +++ b/README.md @@ -120,6 +120,30 @@ that it applies, for example: The mount_origin flag defaults to false, so it is not necessary when the user is mounting the snapshot rather than the origin. +To create a snapshot set with a revert boot entry for disaster recovery, the set would be +defined with the "revertable" field: + +```yaml + snapshot_lvm_set: + name: snapset1 + bootable: true + revertable: true + volumes: + - name: snapshot VG1 LV1 + vg: test_vg1 + lv: lv1 + percent_space_required: 20 + - name: snapshot VG2 LV3 + vg: test_vg2 + lv: lv3 + percent_space_required: 15 +``` + +When `revertable: true` is set, a revert boot entry will be created in the boot menu. +Booting into this entry will automatically merge the snapshots back into their origin volumes, +providing a boot-time rollback mechanism. Both bootable and revertable can be set independently +or together. + ### snapshot_lvm_snapset_name This variable is required. snapshot_lvm_snapset_name is a string that will be @@ -256,6 +280,16 @@ support snapshot manager (snapm). When set to true, and passed to the 'snapshot' command, the snapshot created will have a corresponding boot entry. The boot entry will be removed when the snapset is removed. +### snapshot_lvm_revertable + +Boolean - default is false. Only supported on operating systems that +support snapshot manager (snapm) version 0.5.0 or later. When set to true, +and passed to the 'snapshot' command, the snapshot created will have a +corresponding revert boot entry. Booting into the revert entry will trigger +an automatic revert operation, merging the snapshot back into the origin +volume. This provides a boot-time rollback mechanism for disaster recovery. +The revert boot entry will be removed when the snapset is removed. + ### snapshot_use_copr (EXPERIMENTAL) Boolean - default is unset - if you want to enable the copr repo to use the diff --git a/module_utils/snapshot_lsr/snapmgr.py b/module_utils/snapshot_lsr/snapmgr.py index 560c9852..e45464fb 100644 --- a/module_utils/snapshot_lsr/snapmgr.py +++ b/module_utils/snapshot_lsr/snapmgr.py @@ -303,7 +303,11 @@ def mgr_snapshot_cmd(module, module_args, snapset_json): manager = snap_manager.Manager() try: - # Build kwargs for create_snapshot_set based on available parameters + # Build kwargs for create_snapshot_set based on available parameters. + # Use kwargs dict to conditionally pass optional parameters based on + # snapm version support. This avoids needing separate code paths for + # each combination (e.g., boot-only, revert-only, both, neither). + # Each version check adds its parameter to kwargs if supported. kwargs = {} if has_boot_parameter(): kwargs["boot"] = bootable diff --git a/tests/get_snapset_status.yml b/tests/get_snapset_status.yml index 55769b1c..f4add7b7 100644 --- a/tests/get_snapset_status.yml +++ b/tests/get_snapset_status.yml @@ -13,4 +13,9 @@ - name: Define reusable variables ansible.builtin.set_fact: is_bootable: "{{ _raw_data.Bootable | bool }}" - boot_entries_list: "{{ _raw_data.BootEntries.values() | list }}" + boot_entries_list: "{{ (_raw_data.BootEntries | d({})).values() | list }}" + _revert_entry: "{{ (_raw_data.BootEntries | d({})).RevertEntry | d('') }}" + +- name: Set is_revertable based on revert entry + ansible.builtin.set_fact: + is_revertable: "{{ _revert_entry | length > 0 }}" diff --git a/tests/tests_basic_revertable.yml b/tests/tests_basic_revertable.yml new file mode 100644 index 00000000..ec747a3b --- /dev/null +++ b/tests/tests_basic_revertable.yml @@ -0,0 +1,136 @@ +--- +- name: Basic snapshot test with revertable boot entry + hosts: all + vars: + # only use vgs matching this pattern + snapshot_lvm_vg_include: "^test_" + test_disk_min_size: "1g" + test_disk_count: 10 + test_storage_pools: + - name: test_vg1 + disks: "{{ range(0, 3) | map('extract', unused_disks) | list }}" + volumes: + - name: lv1 + size: "15%" + - name: lv2 + size: "50%" + - name: test_vg2 + disks: "{{ range(3, 6) | map('extract', unused_disks) | list }}" + volumes: + - name: lv3 + size: "10%" + - name: lv4 + size: "20%" + - name: test_vg3 + disks: "{{ range(6, 10) | map('extract', unused_disks) | list }}" + volumes: + - name: lv5 + size: "30%" + - name: lv6 + size: "25%" + - name: lv7 + size: "10%" + - name: lv8 + size: "10%" + tasks: + - name: Run tests + block: + - name: Setup + include_tasks: tasks/setup.yml + + - name: Run the snapshot role to create snapshot LVs with revert boot entry + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + snapshot_lvm_percent_space_required: 15 + snapshot_lvm_all_vgs: true + snapshot_lvm_snapset_name: snapset1 + snapshot_lvm_action: snapshot + snapshot_lvm_revertable: true + + - name: Assert changes for creation + assert: + that: snapshot_cmd["changed"] + + - name: Verify the snapshot LVs are created + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + snapshot_lvm_all_vgs: true + snapshot_lvm_snapset_name: snapset1 + snapshot_lvm_verify_only: true + snapshot_lvm_action: check + + - name: Run the snapshot role again to check idempotence + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + snapshot_lvm_percent_space_required: 15 + snapshot_lvm_all_vgs: true + snapshot_lvm_snapset_name: snapset1 + snapshot_lvm_action: snapshot + + - name: Assert no changes for creation + assert: + that: not snapshot_cmd["changed"] + + - name: Verify again to check idempotence + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + snapshot_lvm_all_vgs: true + snapshot_lvm_snapset_name: snapset1 + snapshot_lvm_verify_only: true + snapshot_lvm_action: check + + - name: Assert no changes for verify + assert: + that: not snapshot_cmd["changed"] + + - name: Run the snapshot role remove the snapshot LVs + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + snapshot_lvm_snapset_name: snapset1 + snapshot_lvm_action: remove + + - name: Assert changes for removal + assert: + that: snapshot_cmd["changed"] + + - name: Use the snapshot_lvm_verify option to make sure remove is done + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + snapshot_lvm_snapset_name: snapset1 + snapshot_lvm_verify_only: true + snapshot_lvm_action: remove + + - name: Remove again to check idempotence + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + snapshot_lvm_snapset_name: snapset1 + snapshot_lvm_action: remove + + - name: Assert no changes for remove + assert: + that: not snapshot_cmd["changed"] + + - name: Verify remove again to check idempotence + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + snapshot_lvm_snapset_name: snapset1 + snapshot_lvm_verify_only: true + snapshot_lvm_action: remove + + - name: Assert no changes for remove verify + assert: + that: not snapshot_cmd["changed"] + + rescue: + - name: Do not fail if snapm is not available or too old for test + assert: + that: ansible_failed_result.msg is search(msg1) or ansible_failed_result.msg is search(msg2) or ansible_failed_result.msg is search(msg3) + vars: + msg1: "Package snapm version .* is too old" + msg2: "Package snapm version 0.5 or later is required to use revertable snapsets" + msg3: "Revertable snapshots are not supported without snapm" + + always: + - name: Cleanup + include_tasks: tasks/cleanup.yml + tags: tests::cleanup diff --git a/tests/tests_set_revertable.yml b/tests/tests_set_revertable.yml new file mode 100644 index 00000000..629c8d1b --- /dev/null +++ b/tests/tests_set_revertable.yml @@ -0,0 +1,109 @@ +--- +- name: Snapshot a set of volumes with snapshot_lvm_revertable set to true + hosts: all + vars: + test_disk_min_size: "1g" + test_disk_count: 10 + test_storage_pools: + - name: test_vg1 + disks: "{{ range(0, 3) | map('extract', unused_disks) | list }}" + volumes: + - name: lv1 + size: "15%" + - name: lv2 + size: "50%" + - name: test_vg2 + disks: "{{ range(3, 6) | map('extract', unused_disks) | list }}" + volumes: + - name: lv3 + size: "10%" + - name: lv4 + size: "20%" + - name: test_vg3 + disks: "{{ range(6, 10) | map('extract', unused_disks) | list }}" + volumes: + - name: lv5 + size: "30%" + - name: lv6 + size: "25%" + - name: lv7 + size: "10%" + - name: lv8 + size: "10%" + snapshot_test_set: + name: snapset1 + revertable: true + volumes: + - name: snapshot VG1 LV1 + vg: test_vg1 + lv: lv1 + percent_space_required: 20 + - name: snapshot VG2 LV3 + vg: test_vg2 + lv: lv3 + percent_space_required: 15 + - name: snapshot VG2 LV4 + vg: test_vg2 + lv: lv4 + percent_space_required: 15 + - name: snapshot VG3 LV7 + vg: test_vg3 + lv: lv7 + percent_space_required: 15 + tasks: + - name: Load test variables + include_vars: + file: vars/rh_distros_vars.yml + when: __snapshot_is_ostree is not defined + + - name: Run tests + block: + - name: Setup + include_tasks: tasks/setup.yml + + - name: Run the snapshot role to create snapshot set of LVs with revert entry + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + snapshot_lvm_action: snapshot + snapshot_lvm_set: "{{ snapshot_test_set }}" + + - name: Assert changes for create snapset + assert: + that: snapshot_cmd["changed"] + + - name: Get snapset details + ansible.builtin.include_tasks: get_snapset_status.yml + vars: + snapset_name: "{{ snapshot_test_set.name }}" + + - name: Fail if not revertable + assert: + that: is_revertable + + - name: Remove the snapshot set + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + snapshot_lvm_action: remove + snapshot_lvm_set: "{{ snapshot_test_set }}" + + - name: Remove again to check idempotence + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + snapshot_lvm_action: remove + snapshot_lvm_set: "{{ snapshot_test_set }}" + + - name: Assert no changes for remove snapset + assert: + that: not snapshot_cmd["changed"] + rescue: + - name: Do not fail if snapm is not available or too old for test + assert: + that: ansible_failed_result.msg is search(msg1) or ansible_failed_result.msg is search(msg2) or ansible_failed_result.msg is search(msg3) + vars: + msg1: "Package snapm version .* is too old" + msg2: "Package snapm version 0.5 or later is required to use revertable snapsets" + msg3: "Revertable snapshots are not supported without snapm" + always: + - name: Cleanup + include_tasks: tasks/cleanup.yml + tags: tests::cleanup From 055bd984804c09adb7d72d70fc72d06bb3b4ddee Mon Sep 17 00:00:00 2001 From: Todd Gill Date: Thu, 16 Jul 2026 11:46:24 -0400 Subject: [PATCH 4/7] fix: add CLAUDE.md to .gitignore Signed-off-by: Todd Gill --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 10cccca1..f4507c68 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,4 @@ artifacts/ __pycache__/ *~ .pytest_cache/ +CLAUDE.md From 9e5cdff85e06bd1dfb0ed464eae35895b82a3df2 Mon Sep 17 00:00:00 2001 From: Todd Gill Date: Thu, 16 Jul 2026 11:46:24 -0400 Subject: [PATCH 5/7] docs: clarify revertable snapshot two-step revert process Update documentation to accurately explain how revertable snapshots work based on snapm behavior. The revert boot entry does NOT automatically perform a revert - users must first run the revert action, then boot into the revert entry. Changes: - Clarify snapshot_lvm_action: revert for revertable vs regular snapshots - Add explicit two-step process documentation with warnings - Add complete workflow example showing disaster recovery usage - Update snapshot_lvm_revertable parameter description - Emphasize that boot entry alone does not initiate revert This addresses confusion from snapshotmanager/snapm#999 where users expected automatic revert behavior from the boot entry alone. Signed-off-by: Todd Gill --- README.md | 77 ++++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 68 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 7c6a9ba2..b367e272 100644 --- a/README.md +++ b/README.md @@ -35,8 +35,11 @@ This variable is required. It supports one of the following values: - `remove`: Remove snapshots that conform to the specified prefix and pattern -- `revert`: Revert to snapshots that are specified by either the pattern or set. - If either the source LV or snapshot are open, the merge is deferred +- `revert`: Initiate a revert to merge snapshots back into their origin volumes. + For snapshots created with `revertable: true`, this command initiates + the revert operation, and you must then reboot into the revert boot + entry to complete the operation. For regular (non-revertable) snapshots, + if either the source LV or snapshot are open, the merge is deferred until the next time the server reboots and the source logical volume is activated. @@ -140,9 +143,58 @@ defined with the "revertable" field: ``` When `revertable: true` is set, a revert boot entry will be created in the boot menu. -Booting into this entry will automatically merge the snapshots back into their origin volumes, -providing a boot-time rollback mechanism. Both bootable and revertable can be set independently -or together. +To perform a revert operation: + +1. First run `snapshot_lvm_action: revert` to initiate the revert +2. Then reboot into the revert boot entry to complete the revert operation + +The revert boot entry alone does NOT initiate a revert - it must be preceded by running +the revert action. Booting into the revert entry after running the revert action will +start the merge operation on all affected volumes. The snapshot set will show a status +of "Reverting" while in progress, and will be destroyed once the revert completes. + +Both bootable and revertable can be set independently or together. + +**Important**: Simply booting into the revert entry without first running the revert +action will NOT perform a revert. The two-step process (revert action + revert boot) is +required for disaster recovery. + +#### Complete Revertable Snapshot Workflow Example + +```yaml +# 1. Create revertable snapshots before a risky operation (e.g., system upgrade) +- name: Create revertable snapshot set + include_role: + name: linux-system-roles.snapshot + vars: + snapshot_lvm_action: snapshot + snapshot_lvm_set: + name: before-upgrade + revertable: true + volumes: + - vg: system_vg + lv: root + percent_space_required: 20 + - vg: system_vg + lv: var + percent_space_required: 20 + +# 2. Perform your risky operation (upgrade, configuration change, etc.) +# ... + +# 3. If something goes wrong and you need to revert: +- name: Initiate revert operation + include_role: + name: linux-system-roles.snapshot + vars: + snapshot_lvm_action: revert + snapshot_lvm_set: + name: before-upgrade + +# 4. After running the revert action, reboot the system and select the +# "Revert before-upgrade" entry from the boot menu to complete the revert. +# The system will boot and merge the snapshots, reverting to the pre-upgrade state. +``` ### snapshot_lvm_snapset_name @@ -285,10 +337,17 @@ entry. The boot entry will be removed when the snapset is removed. Boolean - default is false. Only supported on operating systems that support snapshot manager (snapm) version 0.5.0 or later. When set to true, and passed to the 'snapshot' command, the snapshot created will have a -corresponding revert boot entry. Booting into the revert entry will trigger -an automatic revert operation, merging the snapshot back into the origin -volume. This provides a boot-time rollback mechanism for disaster recovery. -The revert boot entry will be removed when the snapset is removed. +corresponding revert boot entry. + +To perform a disaster recovery revert: + +1. Run `snapshot_lvm_action: revert` to initiate the revert operation +2. Reboot into the revert boot entry to complete the revert + +The revert boot entry is part of a two-step process and does NOT initiate a +revert on its own - you must first run the revert action, then boot into the +revert entry. The revert boot entry will be removed when the snapset is removed +or after a successful revert operation completes. ### snapshot_use_copr (EXPERIMENTAL) From 8162396eddcf5be0393c47f7abbfda01704fc5b4 Mon Sep 17 00:00:00 2001 From: Todd Gill Date: Thu, 16 Jul 2026 14:42:44 -0400 Subject: [PATCH 6/7] docs: clarify bootable and revertable parameter precedence and conflict handling Update documentation for both snapshot_lvm_bootable and snapshot_lvm_revertable to explicitly describe: - The two configuration locations (global vs. set-level) - Precedence rules (global takes precedence when set) - Conflict behavior (ERROR_BOOTABLE_CONFLICT or ERROR_REVERTABLE_CONFLICT) - That no snapshot is created when conflicting values are detected This addresses potential confusion about parameter precedence and makes it clear that conflicting values will cause the role to fail rather than silently choosing one value over the other. Signed-off-by: Todd Gill --- README.md | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/README.md b/README.md index b367e272..e0228e33 100644 --- a/README.md +++ b/README.md @@ -332,6 +332,20 @@ support snapshot manager (snapm). When set to true, and passed to the 'snapshot' command, the snapshot created will have a corresponding boot entry. The boot entry will be removed when the snapset is removed. +This parameter can be set in two locations: + +- **Global level**: `snapshot_lvm_bootable: true` applies to all snapshots +- **Set level**: `bootable: true` in the `snapshot_lvm_set` definition + +**Precedence and conflict handling:** + +- If the global parameter is set, it takes precedence +- If both global and set-level parameters are set to **different** values + (e.g., global is `true` and set-level is `false`), the role will fail with + `ERROR_BOOTABLE_CONFLICT` and no snapshot will be created +- If both are set to the **same** value, the operation proceeds normally +- If only one is set, that value is used + ### snapshot_lvm_revertable Boolean - default is false. Only supported on operating systems that @@ -339,6 +353,20 @@ support snapshot manager (snapm) version 0.5.0 or later. When set to true, and passed to the 'snapshot' command, the snapshot created will have a corresponding revert boot entry. +This parameter can be set in two locations: + +- **Global level**: `snapshot_lvm_revertable: true` applies to all snapshots +- **Set level**: `revertable: true` in the `snapshot_lvm_set` definition + +**Precedence and conflict handling:** + +- If the global parameter is set, it takes precedence +- If both global and set-level parameters are set to **different** values + (e.g., global is `true` and set-level is `false`), the role will fail with + `ERROR_REVERTABLE_CONFLICT` and no snapshot will be created +- If both are set to the **same** value, the operation proceeds normally +- If only one is set, that value is used + To perform a disaster recovery revert: 1. Run `snapshot_lvm_action: revert` to initiate the revert operation From 23fb519faf58cc9fd44a810ba548a24308865956 Mon Sep 17 00:00:00 2001 From: Todd Gill Date: Thu, 16 Jul 2026 17:41:29 -0400 Subject: [PATCH 7/7] fix: handle explicit False for bootable and revertable parameters Fix boolean parameter checking to distinguish between explicit False and unset (None) values. Previously, truthy checks treated False the same as None, causing silent conflicts when: - Global parameter set to False - Set-level parameter set to True The global False was silently ignored instead of raising a conflict error. Changes: - Use `module_args.get("param") is not None` instead of `if module_args["param"]:` - Applies to both snapshot_lvm_bootable and snapshot_lvm_revertable - Fixed in snapmgr.py (conflict detection) and validate.py (parameter propagation) This ensures ERROR_BOOTABLE_CONFLICT and ERROR_REVERTABLE_CONFLICT are correctly raised for all conflicting configurations, not just when the global value is True. Signed-off-by: Todd Gill --- module_utils/snapshot_lsr/snapmgr.py | 10 ++++------ module_utils/snapshot_lsr/validate.py | 8 ++++---- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/module_utils/snapshot_lsr/snapmgr.py b/module_utils/snapshot_lsr/snapmgr.py index e45464fb..1e768aaa 100644 --- a/module_utils/snapshot_lsr/snapmgr.py +++ b/module_utils/snapshot_lsr/snapmgr.py @@ -229,9 +229,8 @@ def mgr_snapshot_cmd(module, module_args, snapset_json): snapset_name = snapset_json["name"] volume_list = snapset_json["volumes"] - # Bootable global variable is set - if module_args["snapshot_lvm_bootable"]: - bootable = module_args["snapshot_lvm_bootable"] + # Bootable global variable + bootable = module_args.get("snapshot_lvm_bootable") # Global is not set, check the snapset if bootable is None: @@ -251,9 +250,8 @@ def mgr_snapshot_cmd(module, module_args, snapset_json): "changed": False, } - # Revertable global variable is set - if module_args["snapshot_lvm_revertable"]: - revertable = module_args["snapshot_lvm_revertable"] + # Revertable global variable + revertable = module_args.get("snapshot_lvm_revertable") # Global is not set, check the snapset if revertable is None: diff --git a/module_utils/snapshot_lsr/validate.py b/module_utils/snapshot_lsr/validate.py index 52f966ac..bb454feb 100644 --- a/module_utils/snapshot_lsr/validate.py +++ b/module_utils/snapshot_lsr/validate.py @@ -33,10 +33,10 @@ def get_json_from_args(module, module_args, vg_include): if module_args["snapshot_lvm_snapset_name"]: args_dict["name"] = module_args["snapshot_lvm_snapset_name"] - if module_args["snapshot_lvm_bootable"]: + if module_args.get("snapshot_lvm_bootable") is not None: args_dict["bootable"] = module_args["snapshot_lvm_bootable"] - if module_args["snapshot_lvm_revertable"]: + if module_args.get("snapshot_lvm_revertable") is not None: args_dict["revertable"] = module_args["snapshot_lvm_revertable"] for vg, lv_list in vgs_lvs_iterator( @@ -341,10 +341,10 @@ def validate_snapset_json(cmd, module_args, verify_only): snapset_dict = module_args["snapshot_lvm_set"] - if module_args["snapshot_lvm_bootable"]: + if module_args.get("snapshot_lvm_bootable") is not None: snapset_dict["snapshot_lvm_bootable"] = module_args["snapshot_lvm_bootable"] - if module_args["snapshot_lvm_revertable"]: + if module_args.get("snapshot_lvm_revertable") is not None: snapset_dict["snapshot_lvm_revertable"] = module_args["snapshot_lvm_revertable"] if cmd == SnapshotCommand.SNAPSHOT: