diff --git a/.gitignore b/.gitignore index 10cccca1..f4507c68 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,4 @@ artifacts/ __pycache__/ *~ .pytest_cache/ +CLAUDE.md diff --git a/README.md b/README.md index 3940c0f6..e0228e33 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. @@ -120,6 +123,79 @@ 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. +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 This variable is required. snapshot_lvm_snapset_name is a string that will be @@ -256,6 +332,51 @@ 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 +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 +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) Boolean - default is unset - if you want to enable the copr repo to use the 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", 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..1e768aaa 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 @@ -215,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: @@ -237,8 +250,47 @@ def mgr_snapshot_cmd(module, module_args, snapset_json): "changed": False, } + # Revertable global variable + revertable = module_args.get("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 +301,28 @@ def mgr_snapshot_cmd(module, module_args, snapset_json): manager = snap_manager.Manager() try: + # 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(): - 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..bb454feb 100644 --- a/module_utils/snapshot_lsr/validate.py +++ b/module_utils/snapshot_lsr/validate.py @@ -33,9 +33,12 @@ 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.get("snapshot_lvm_revertable") is not None: + args_dict["revertable"] = module_args["snapshot_lvm_revertable"] + for vg, lv_list in vgs_lvs_iterator( module, module_args["snapshot_lvm_vg"], @@ -338,9 +341,12 @@ 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.get("snapshot_lvm_revertable") is not None: + 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: 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