From 62b40ffa3bbebbea1ba599d1e3ab066c82cbdce5 Mon Sep 17 00:00:00 2001 From: mulhern Date: Tue, 24 Sep 2024 21:58:54 -0400 Subject: [PATCH 1/8] Update changelog Signed-off-by: mulhern --- CHANGES.txt | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/CHANGES.txt b/CHANGES.txt index d0f82a376..3777eea90 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,3 +1,87 @@ +stratis-cli 3.7.0 (UNRELEASED) +================= +Required stratisd version 3.7.0 + +Recommended development environment: Fedora 40 +Lowest supported Python interpreter: 3.9.16 + +* indicates new since stratis-cli 3.6.2 patch release + +* Update for r7 D-Bus revision: + https://github.com/stratis-storage/stratis-cli/pull/1035 + +* Display snapshot origin in filesystem detail view: + https://github.com/stratis-storage/stratis-cli/pull/1045 + +* Add filesystem detail view: + https://github.com/stratis-storage/stratis-cli/pull/1051 + +* Add commands to print filesystem metadata: + https://github.com/stratis-storage/stratis-cli/pull/1089 + +* Add commands to print pool-level metadata: + https://github.com/stratis-storage/stratis-cli/pull/1075 + +* Improve parsing of Clevis create options: + https://github.com/stratis-storage/stratis-cli/pull/1076 + https://github.com/stratis-storage/stratis-cli/pull/1073 + +- Implement Clevis-related parser errors as parser errors: + https://github.com/stratis-storage/stratis-cli/pull/1060 + https://github.com/stratis-storage/stratis-cli/pull/1057 + https://github.com/stratis-storage/stratis-cli/pull/1056 + https://github.com/stratis-storage/stratis-cli/pull/1053 + +* Sort JSON output from report by default: + https://github.com/stratis-storage/stratis-cli/pull/1048 + +* man: Add description of field in filesystem detail view: + https://github.com/stratis-storage/stratis-cli/pull/1097 + +* man: Add description of fields in pool detail view: + https://github.com/stratis-storage/stratis-cli/pull/1080 + +* Improve help text for the unbind command: + https://github.com/stratis-storage/stratis-cli/pull/1083 + +* Refactor filesystem listing code: + https://github.com/stratis-storage/stratis-cli/pull/1050 + +- Replace use of getpass with custom implementation: + https://github.com/stratis-storage/stratis-cli/pull/1068 + +- Tidies and Maintenance: + https://github.com/stratis-storage/stratis-cli/pull/1096 + https://github.com/stratis-storage/stratis-cli/pull/1095 + https://github.com/stratis-storage/stratis-cli/pull/1094 + https://github.com/stratis-storage/stratis-cli/pull/1093 + https://github.com/stratis-storage/stratis-cli/pull/1092 + https://github.com/stratis-storage/stratis-cli/pull/1090 + https://github.com/stratis-storage/stratis-cli/pull/1088 + https://github.com/stratis-storage/stratis-cli/pull/1086 + https://github.com/stratis-storage/stratis-cli/pull/1084 + https://github.com/stratis-storage/stratis-cli/pull/1079 + https://github.com/stratis-storage/stratis-cli/pull/1077 + https://github.com/stratis-storage/stratis-cli/pull/1074 + https://github.com/stratis-storage/stratis-cli/pull/1072 + https://github.com/stratis-storage/stratis-cli/pull/1070 + https://github.com/stratis-storage/stratis-cli/pull/1067 + https://github.com/stratis-storage/stratis-cli/pull/1065 + https://github.com/stratis-storage/stratis-cli/pull/1064 + https://github.com/stratis-storage/stratis-cli/pull/1063 + https://github.com/stratis-storage/stratis-cli/pull/1061 + https://github.com/stratis-storage/stratis-cli/pull/1059 + https://github.com/stratis-storage/stratis-cli/pull/1058 + https://github.com/stratis-storage/stratis-cli/pull/1054 + https://github.com/stratis-storage/stratis-cli/pull/1052 + https://github.com/stratis-storage/stratis-cli/pull/1049 + https://github.com/stratis-storage/stratis-cli/pull/1046 + https://github.com/stratis-storage/stratis-cli/pull/1043 + https://github.com/stratis-storage/stratis-cli/pull/1040 + https://github.com/stratis-storage/stratis-cli/pull/1038 + https://github.com/stratis-storage/stratis-cli/pull/1036 + + stratis-cli 3.6.0 ================= Required stratisd version: 3.6.0 From 2c04d1dbf0635feed7a9d6b8e5027ac8252e5a45 Mon Sep 17 00:00:00 2001 From: mulhern Date: Tue, 23 Jul 2024 15:52:55 -0400 Subject: [PATCH 2/8] Update introspection data Signed-off-by: mulhern --- src/stratis_cli/_actions/_introspect.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/stratis_cli/_actions/_introspect.py b/src/stratis_cli/_actions/_introspect.py index 2c4be82fc..b06ee7684 100644 --- a/src/stratis_cli/_actions/_introspect.py +++ b/src/stratis_cli/_actions/_introspect.py @@ -123,6 +123,7 @@ + From 0e174b0903819e11c87bd6acb3ce0ba0ec8a733a Mon Sep 17 00:00:00 2001 From: mulhern Date: Tue, 23 Jul 2024 15:18:12 -0400 Subject: [PATCH 3/8] Add functions to schedule or cancel a revert Just return the engine error if origin not set. When trying to cancel a revert, and the revert has not been scheduled because the filesystem has no origin, better to reject the cancel action because of the missing origin, rather than because the merge status will not be changed. Signed-off-by: mulhern --- src/stratis_cli/_actions/_logical.py | 62 ++++++++++++ src/stratis_cli/_errors.py | 12 +++ src/stratis_cli/_parser/_logical.py | 45 +++++++++ .../integration/logical/test_cancel_revert.py | 94 +++++++++++++++++++ .../logical/test_schedule_revert.py | 92 ++++++++++++++++++ 5 files changed, 305 insertions(+) create mode 100644 tests/whitebox/integration/logical/test_cancel_revert.py create mode 100644 tests/whitebox/integration/logical/test_schedule_revert.py diff --git a/src/stratis_cli/_actions/_logical.py b/src/stratis_cli/_actions/_logical.py index 9b032f9e1..98bd9aeda 100644 --- a/src/stratis_cli/_actions/_logical.py +++ b/src/stratis_cli/_actions/_logical.py @@ -21,6 +21,7 @@ from .._constants import Id, IdType from .._errors import ( StratisCliEngineError, + StratisCliFsMergeScheduledChangeError, StratisCliFsSizeLimitChangeError, StratisCliIncoherenceError, StratisCliNoChangeError, @@ -336,3 +337,64 @@ def unset_size_limit(namespace): raise StratisCliFsSizeLimitChangeError(None) Filesystem.Properties.SizeLimit.Set(get_object(fs_object_path), (False, "")) + + @staticmethod + def schedule_revert(namespace): + """ + Schedule reverting a snapshot into its origin. + """ + # pylint: disable=import-outside-toplevel + from ._data import Filesystem, MOFilesystem, ObjectManager, filesystems, pools + + proxy = get_object(TOP_OBJECT) + managed_objects = ObjectManager.Methods.GetManagedObjects(proxy, {}) + (pool_object_path, _) = next( + pools(props={"Name": namespace.pool_name}) + .require_unique_match(True) + .search(managed_objects) + ) + (fs_object_path, fs_info) = next( + filesystems( + props={"Name": namespace.snapshot_name, "Pool": pool_object_path} + ) + .require_unique_match(True) + .search(managed_objects) + ) + + merge_requested = MOFilesystem(fs_info).MergeScheduled() + + if bool(merge_requested): + raise StratisCliFsMergeScheduledChangeError(True) + + Filesystem.Properties.MergeScheduled.Set(get_object(fs_object_path), True) + + @staticmethod + def cancel_revert(namespace): + """ + Cancel reverting a snapshot into its origin. + """ + # pylint: disable=import-outside-toplevel + from ._data import Filesystem, MOFilesystem, ObjectManager, filesystems, pools + + proxy = get_object(TOP_OBJECT) + managed_objects = ObjectManager.Methods.GetManagedObjects(proxy, {}) + (pool_object_path, _) = next( + pools(props={"Name": namespace.pool_name}) + .require_unique_match(True) + .search(managed_objects) + ) + (fs_object_path, fs_info) = next( + filesystems( + props={"Name": namespace.snapshot_name, "Pool": pool_object_path} + ) + .require_unique_match(True) + .search(managed_objects) + ) + + mofs = MOFilesystem(fs_info) + (merge_requested, (origin_set, _)) = (mofs.MergeScheduled(), mofs.Origin()) + + if origin_set and not bool(merge_requested): + raise StratisCliFsMergeScheduledChangeError(False) + + Filesystem.Properties.MergeScheduled.Set(get_object(fs_object_path), False) diff --git a/src/stratis_cli/_errors.py b/src/stratis_cli/_errors.py index ef6cf0012..ea0eca04d 100644 --- a/src/stratis_cli/_errors.py +++ b/src/stratis_cli/_errors.py @@ -103,6 +103,18 @@ def __str__(self): return f"Filesystem's size limit is exactly {self.value}" +class StratisCliFsMergeScheduledChangeError(StratisCliNoPropertyChangeError): + """ + Raised when the user requests the same filesystems MergeScheduled value + that the filesystem already has. + """ + + def __str__(self): + if self.value: + return "Filesystem is already scheduled for a revert operation." + return "Filesystem is not currently scheduled for a revert operation." + + class StratisCliHasCacheChangeError(StratisCliNoPropertyChangeError): """ Raised when the user request cache initialization, but a cache is already diff --git a/src/stratis_cli/_parser/_logical.py b/src/stratis_cli/_parser/_logical.py index e156878a4..c860374bd 100644 --- a/src/stratis_cli/_parser/_logical.py +++ b/src/stratis_cli/_parser/_logical.py @@ -235,6 +235,51 @@ def verify(self, namespace, parser): "func": LogicalActions.unset_size_limit, }, ), + ( + "schedule-revert", + { + "help": ( + "Schedule a revert operation for this snapshot filesystem " + "into its origin filesystem" + ), + "args": [ + ( + "pool_name", + { + "help": ( + "Name of the pool the snapshot and its origin belong to" + ), + }, + ), + ( + "snapshot_name", + {"help": "Name of the snapshot filesystem"}, + ), + ], + "func": LogicalActions.schedule_revert, + }, + ), + ( + "cancel-revert", + { + "help": "Cancel a previously scheduled revert operation", + "args": [ + ( + "pool_name", + { + "help": ( + "Name of the pool the snapshot and its origin belong to" + ), + }, + ), + ( + "snapshot_name", + {"help": "Name of the snapshot filesystem"}, + ), + ], + "func": LogicalActions.cancel_revert, + }, + ), ( "debug", { diff --git a/tests/whitebox/integration/logical/test_cancel_revert.py b/tests/whitebox/integration/logical/test_cancel_revert.py new file mode 100644 index 000000000..9e7d31f6b --- /dev/null +++ b/tests/whitebox/integration/logical/test_cancel_revert.py @@ -0,0 +1,94 @@ +# Copyright 2023 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +""" +Test 'cancel-revert'. +""" + +# isort: FIRSTPARTY +from dbus_python_client_gen import DPClientInvocationError + +# isort: LOCAL +from stratis_cli import StratisCliErrorCodes +from stratis_cli._errors import StratisCliFsMergeScheduledChangeError + +from .._misc import RUNNER, SimTestCase, device_name_list + +_DEVICE_STRATEGY = device_name_list(1) +_ERROR = StratisCliErrorCodes.ERROR + + +class FsCancelRevertTestCase(SimTestCase): + """ + Test canceling a filesystem revert. + """ + + _MENU = ["--propagate", "filesystem", "cancel-revert"] + _POOLNAME = "pool" + _FSNAME = "nofs" + _SNAPNAME = "snofs" + + def setUp(self): + """ + Start the stratisd daemon with the simulator. + """ + super().setUp() + command_line = ["pool", "create", self._POOLNAME] + _DEVICE_STRATEGY() + RUNNER(command_line) + + command_line = [ + "filesystem", + "create", + self._POOLNAME, + self._FSNAME, + ] + RUNNER(command_line) + + command_line = [ + "filesystem", + "snapshot", + self._POOLNAME, + self._FSNAME, + self._SNAPNAME, + ] + RUNNER(command_line) + + def test_cancel_revert_when_unscheduled(self): + """ + Cancelling an unscheduled revert should fail. + """ + command_line = self._MENU + [self._POOLNAME, self._SNAPNAME] + self.check_error(StratisCliFsMergeScheduledChangeError, command_line, _ERROR) + + def test_cancel_revert_when_no_origin(self): + """ + Cancelling a revert on a filesystem with no origin should fail. + """ + command_line = self._MENU + [self._POOLNAME, self._FSNAME] + self.check_error(DPClientInvocationError, command_line, _ERROR) + + def test_cancel_a_revert_twice(self): + """ + Cancelling a revert twice should fail. + """ + command_line = [ + "--propagate", + "filesystem", + "schedule-revert", + self._POOLNAME, + self._SNAPNAME, + ] + RUNNER(command_line) + command_line = self._MENU + [self._POOLNAME, self._SNAPNAME] + RUNNER(command_line) + self.check_error(StratisCliFsMergeScheduledChangeError, command_line, _ERROR) diff --git a/tests/whitebox/integration/logical/test_schedule_revert.py b/tests/whitebox/integration/logical/test_schedule_revert.py new file mode 100644 index 000000000..fa2dc0152 --- /dev/null +++ b/tests/whitebox/integration/logical/test_schedule_revert.py @@ -0,0 +1,92 @@ +# Copyright 2023 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +""" +Test 'schedule-revert'. +""" + +# isort: FIRSTPARTY +from dbus_python_client_gen import DPClientInvocationError + +# isort: LOCAL +from stratis_cli import StratisCliErrorCodes +from stratis_cli._errors import StratisCliFsMergeScheduledChangeError + +from .._misc import RUNNER, TEST_RUNNER, SimTestCase, device_name_list + +_DEVICE_STRATEGY = device_name_list(1) +_ERROR = StratisCliErrorCodes.ERROR + + +class FsScheduleRevertTestCase(SimTestCase): + """ + Test scheduling a revert for a filesystem. + """ + + _MENU = ["--propagate", "filesystem", "schedule-revert"] + _POOLNAME = "pool" + _FSNAME = "nofs" + _SNAPNAME = "snofs" + + def setUp(self): + """ + Start the stratisd daemon with the simulator. + """ + super().setUp() + command_line = ["pool", "create", self._POOLNAME] + _DEVICE_STRATEGY() + RUNNER(command_line) + + command_line = [ + "filesystem", + "create", + self._POOLNAME, + self._FSNAME, + ] + RUNNER(command_line) + + command_line = [ + "filesystem", + "snapshot", + self._POOLNAME, + self._FSNAME, + self._SNAPNAME, + ] + RUNNER(command_line) + + def test_schedule_revert_twice(self): + """ + Scheduling a revert twice should fail. + """ + command_line = self._MENU + [ + self._POOLNAME, + self._SNAPNAME, + ] + RUNNER(command_line) + self.check_error(StratisCliFsMergeScheduledChangeError, command_line, _ERROR) + + def test_schedule_revert_once(self): + """ + Scheduling a revert once should succeed. + """ + command_line = self._MENU + [self._POOLNAME, self._SNAPNAME] + TEST_RUNNER(command_line) + + def test_schedule_revert_on_filesystem_without_origin(self): + """ + Scheduling a revert on a filesystem without an origin should fail. + """ + command_line = self._MENU + [ + self._POOLNAME, + self._FSNAME, + ] + self.check_error(DPClientInvocationError, command_line, _ERROR) From 015cc0e917ad3ccaf0f81dfe3102f8138be4e7ae Mon Sep 17 00:00:00 2001 From: mulhern Date: Wed, 24 Jul 2024 11:57:42 -0400 Subject: [PATCH 4/8] Display filesystem revert information in detail view Signed-off-by: mulhern --- src/stratis_cli/_actions/_list_filesystem.py | 7 +++++-- tests/whitebox/integration/logical/test_list.py | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/stratis_cli/_actions/_list_filesystem.py b/src/stratis_cli/_actions/_list_filesystem.py index 958a83a12..a92edd1b6 100644 --- a/src/stratis_cli/_actions/_list_filesystem.py +++ b/src/stratis_cli/_actions/_list_filesystem.py @@ -176,7 +176,7 @@ def display(self): date_parser.parse(fs.Created()).astimezone().strftime("%b %d %Y %H:%M") ) - origin = get_property(fs.Origin(), self.uuid_formatter, "None") + origin = get_property(fs.Origin(), self.uuid_formatter, None) print(f"UUID: {self.uuid_formatter(fs.Uuid())}") print(f"Name: {fs.Name()}") @@ -186,7 +186,10 @@ def display(self): print() print(f"Created: {created}") print() - print(f"Snapshot origin: {origin}") + print(f'Snapshot origin: {"None" if origin is None else origin}') + if origin is not None: + scheduled = "Yes" if fs.MergeScheduled() else "No" + print(f" Revert scheduled: {scheduled}") print() print("Sizes:") print(f" Logical size of thin device: {total}") diff --git a/tests/whitebox/integration/logical/test_list.py b/tests/whitebox/integration/logical/test_list.py index a40058f57..afca8943b 100644 --- a/tests/whitebox/integration/logical/test_list.py +++ b/tests/whitebox/integration/logical/test_list.py @@ -144,6 +144,14 @@ def setUp(self): RUNNER(command_line) command_line = ["filesystem", "create", self._POOLNAMES[0], self._VOLUMES[1]] RUNNER(command_line) + command_line = [ + "filesystem", + "snapshot", + self._POOLNAMES[0], + self._VOLUMES[1], + self._VOLUMES[2], + ] + RUNNER(command_line) command_line = ["pool", "create", self._POOLNAMES[1]] + device_lists[1] RUNNER(command_line) command_line = ["filesystem", "create", self._POOLNAMES[1], self._VOLUMES[2]] @@ -191,3 +199,10 @@ def test_list_fs_name_pool_name_not_match(self): self.check_error( DbusClientUniqueResultError, command_line, StratisCliErrorCodes.ERROR ) + + def test_list_fs_name_snapshot(self): + """ + Test list detailed view of a snapshot to test printing of revert information. + """ + command_line = self._MENU + [self._POOLNAMES[0], f"--name={self._VOLUMES[2]}"] + TEST_RUNNER(command_line) From 749c9d483a701ec07ee4ba46889cec8e07df93cd Mon Sep 17 00:00:00 2001 From: mulhern Date: Wed, 11 Sep 2024 14:18:41 -0400 Subject: [PATCH 5/8] Add man page entries for new commands Signed-off-by: mulhern (cherry picked from commit 39c87a85d3b437b3143e9ae0f277b25f9452478c) --- docs/stratis.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/stratis.txt b/docs/stratis.txt index 79fd7f535..690f8dc83 100644 --- a/docs/stratis.txt +++ b/docs/stratis.txt @@ -134,6 +134,11 @@ filesystem set-size-limit :: as the filesystem's current size, use the keyword "current". filesystem unset-size-limit :: Remove a filesystem size limit previously set. +filesystem schedule-revert :: + Set a flag so that when the pool is next started, the specified snapshot + will overwrite its origin filesystem. +filesystem cancel-revert :: + Cancel a scheduled revert. filesystem debug get-object-path <(--uuid |--name )> :: Look up the D-Bus object path for a filesystem given the UUID or name. filesystem debug get-metadata [--pretty] [--written] [--fs-name ] :: From cd44c9afe8768daff9c50476d94daff06761fc3a Mon Sep 17 00:00:00 2001 From: mulhern Date: Thu, 12 Sep 2024 12:38:03 -0400 Subject: [PATCH 6/8] Add man text about snapshot revert scheduled status The revised text for the "Snapshot origin" field now looks like this: Snapshot origin If this filesystem is a snapshot, its origin. If the filesystem is a snapshot, whether or not it is scheduled to replace its origin next time the pool is started. Signed-off-by: mulhern --- docs/stratis.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/stratis.txt b/docs/stratis.txt index 690f8dc83..c2cb30fca 100644 --- a/docs/stratis.txt +++ b/docs/stratis.txt @@ -348,7 +348,9 @@ Created:: The time the filesystem was created. Snapshot origin:: - If this filesystem is a snapshot, its origin. + If this filesystem is a snapshot, its origin. If the filesystem is + a snapshot, whether or not it is scheduled to replace its origin + next time the pool is started. Sizes:: The logical size of the Stratis filesystem, the total space actually From e6630ff367039ddd247d7d99ef77d3140c6873cb Mon Sep 17 00:00:00 2001 From: mulhern Date: Tue, 24 Sep 2024 16:44:21 -0400 Subject: [PATCH 7/8] TEMPORARY Signed-off-by: mulhern --- .github/workflows/main.yml | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 655e919aa..0081a6ab9 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -129,7 +129,10 @@ jobs: components: cargo toolchain: 1.71.1 # LOWEST SUPPORTED RUST TOOLCHAIN - name: Check out stratisd - run: git clone https://github.com/stratis-storage/stratisd.git + run: > + git clone + https://github.com/mulkieran/stratisd.git + --branch version-3.7.0 - name: Build stratisd run: PROFILEDIR=debug make build-all working-directory: ./stratisd @@ -187,7 +190,10 @@ jobs: components: cargo toolchain: 1.71.1 # LOWEST SUPPORTED RUST TOOLCHAIN - name: Check out stratisd - run: git clone https://github.com/stratis-storage/stratisd.git + run: > + git clone + https://github.com/mulkieran/stratisd.git + --branch version-3.7.0 - name: Build stratisd run: PROFILEDIR=debug make build-all working-directory: ./stratisd From 62e6922e1b794b72dd80fcd18f13364c14934f68 Mon Sep 17 00:00:00 2001 From: mulhern Date: Wed, 25 Sep 2024 21:40:28 -0400 Subject: [PATCH 8/8] TEMPORARY Signed-off-by: mulhern --- .github/workflows/main.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 0081a6ab9..e29c5eae2 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -132,7 +132,7 @@ jobs: run: > git clone https://github.com/mulkieran/stratisd.git - --branch version-3.7.0 + --branch snapshot-support - name: Build stratisd run: PROFILEDIR=debug make build-all working-directory: ./stratisd @@ -193,7 +193,7 @@ jobs: run: > git clone https://github.com/mulkieran/stratisd.git - --branch version-3.7.0 + --branch snapshot-support - name: Build stratisd run: PROFILEDIR=debug make build-all working-directory: ./stratisd