Support for encryption enhancements #1207
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds whole-pool encryption commands (on/off/reencrypt) with an in-place requirement, implements CryptActions and new D-Bus pool methods, updates CLI parser and docs, records last reencrypt timestamp in listings, introduces long-running-operation handling for DBus NoReply, lowers DBus timeout, adjusts tests and build instrumentation, and enables PR Copr builds. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant CLI as stratis-cli
participant Parser as ArgParser
participant Act as CryptActions
participant D as stratisd (DBus)
U->>CLI: pool encryption on --in-place --name <pool> [--key-desc/--clevis...]
CLI->>Parser: parse args
Parser-->>CLI: Namespace
CLI->>Act: CryptActions.encrypt(namespace)
Act->>Act: validate --in-place, resolve pool state
Act->>D: EncryptPool(key_descs, clevis_infos)
alt synchronous reply
D-->>Act: results, return_code, return_string
Act-->>CLI: success or raises error
CLI-->>U: prints result
else asynchronous (DBus NoReply)
D--x Act: NoReply
Act->>Act: long_running_operation swallows NoReply
Act-->>CLI: writes "Operation initiated" to stderr
CLI-->>U: exits without error
end
sequenceDiagram
autonumber
participant CLI as stratis-cli
participant Act as CryptActions
participant D as stratisd (DBus)
CLI->>Act: CryptActions.unencrypt(namespace with --in-place)
Act->>D: DecryptPool()
alt pool not encrypted / no-change
D-->>Act: return_code indicates no change
Act-->>CLI: raise StratisCliNoChangeError
else success
D-->>Act: results true
Act-->>CLI: success
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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. Comment |
|
Congratulations! One of the builds has completed. 🍾 You can install the built RPMs by following these steps:
Please note that the RPMs should be used only in a testing environment. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (20)
src/stratis_cli/_actions/_list_pool.py (1)
364-371: Make timestamp parsing robust; optionally remove external dependencyCurrent code assumes a valid ISO timestamp when valid is True. A malformed value would raise and break the detail view. Consider guarding the parse and, if desired, switch to stdlib parsing to avoid the extra dependency.
Option A (keep dateutil, handle parse errors):
- reencrypted = ( - date_parser.isoparse(timestamp).astimezone().strftime("%b %d %Y %H:%M") - if valid - else "Never" - ) + if valid: + try: + reencrypted = date_parser.isoparse(timestamp).astimezone().strftime("%b %d %Y %H:%M") + except Exception: # pragma: no cover + reencrypted = "<INVALID TIMESTAMP>" + else: + reencrypted = "Never"Option B (drop dependency; use stdlib):
- from dateutil import parser as date_parser + from datetime import datetime @@ - reencrypted = ( - date_parser.isoparse(timestamp).astimezone().strftime("%b %d %Y %H:%M") - if valid - else "Never" - ) + if valid: + try: + ts = datetime.fromisoformat(timestamp.replace("Z", "+00:00")) + reencrypted = ts.astimezone().strftime("%b %d %Y %H:%M") + except Exception: # pragma: no cover + reencrypted = "<INVALID TIMESTAMP>" + else: + reencrypted = "Never"src/stratis_cli/_actions/_utils.py (2)
277-287: Address Ruff B009 and simplify exception chaininggetattr with constant attribute names is not safer and triggers B009. Direct attribute access is clearer and marginally faster.
Apply:
- while True: - yield exc - exc = getattr(exc, "__cause__") or getattr(exc, "__context__") - if exc is None: - return + while True: + yield exc + exc = exc.__cause__ or exc.__context__ + if exc is None: + return
289-308: Preserve return value and generalize wrapper signatureTwo small improvements:
- Return the wrapped function’s result to preserve semantics.
- Accept *args, **kwargs to match the wrapped callable’s signature (still works for Namespace).
Proposed diff:
- @wraps(func) - def wrapper(namespace: Namespace): + @wraps(func) + def wrapper(*args, **kwargs): try: - func(namespace) + return func(*args, **kwargs) except DPClientInvocationError as err: if not any( isinstance(e, DBusException) and e.get_dbus_name() == "org.freedesktop.DBus.Error.NoReply" for e in get_errors(err) ): raise err print("Operation initiated", file=sys.stderr) + return Nonetests/whitebox/unit/test_running.py (2)
37-51: Optional: assert the user-facing stderr message as part of the contractThe wrapper’s contract includes printing “Operation initiated” on NoReply. Asserting stderr strengthens the test against regressions.
Example patch:
+ import io + import contextlib @@ - self.assertIsNone(long_running_operation(raises_error)(None)) + buf = io.StringIO() + with contextlib.redirect_stderr(buf): + self.assertIsNone(long_running_operation(raises_error)(None)) + self.assertIn("Operation initiated", buf.getvalue())
52-61: Good negative-path coverageThis test correctly ensures non-NoReply DPClientInvocationError propagates. Consider a third case with a DBusException of a different name to pin the predicate more tightly, but not strictly necessary.
Makefile (1)
7-10: Optional: keep MONKEYTYPE_MODULES list sorted to reduce merge conflictsNot required, but keeping the module list sorted alphabetically tends to minimize diffs and future merge noise.
MONKEYTYPE_MODULES = stratis_cli._actions._bind \ stratis_cli._actions._constants \ - stratis_cli._actions._crypt \ + stratis_cli._actions._crypt \ stratis_cli._actions._data \src/stratis_cli/_actions/__init__.py (2)
20-20: Resolve Ruff F401 for CryptActions re-exportThis import exists to re-export, but Ruff flags it as unused. Either add a local
# noqa: F401or, preferably, define__all__to explicitly export it.-from ._crypt import CryptActions +from ._crypt import CryptActions # noqa: F401Alternatively (preferred): define
__all__to document the public API. Example snippet to add near the end of this module:# Public re-exports for consumers of stratis_cli._actions __all__ = ( "BindActions", "RebindActions", "BLOCKDEV_INTERFACE", "FILESYSTEM_INTERFACE", "POOL_INTERFACE", "CryptActions", "LogicalActions", "PhysicalActions", "PoolActions", "StratisActions", "check_stratisd_version", "TopActions", "get_errors", )
33-34: Resolve Ruff F401 for get_errors re-exportSame as above; mark as intentional re-export or add to
__all__.-from ._utils import get_errors +from ._utils import get_errors # noqa: F401tests/whitebox/integration/test_parser.py (1)
251-256: Apply Clevis negative cases to encryption-on — LGTM; consider also covering forbidden Clevis flags for other subcommandsGood to extend these cases to
pool encryption on. If the parser forbids Clevis options onencryption offandencryption reencrypt(as expected), consider adding a couple of negative tests to lock that behavior in.for subcommand in [ ["pool", "create", "pn", "/dev/n"], ["pool", "encryption", "on", "--name=pn"], + # Clevis options should be rejected on these subcommands as well: + ["pool", "encryption", "off", "--name=pn"], + ["pool", "encryption", "reencrypt", "--name=pn"], ]: self._do_test(subcommand + clevis_args)src/stratis_cli/_parser/_shared.py (1)
278-299: CLEVIS_AND_KERNEL constant looks correct and consistent with parser expectations
- --key-desc, --clevis (type Clevis, choices list(Clevis)), and --tang-url are captured here; verification flags remain in TRUST_URL_OR_THUMBPRINT, which is appropriate.
- Help strings are clear and align with docs.
Optional nits:
- Consider adding metavar hints for readability, e.g., "--tang-url ", "--key-desc <key_desc>" to match the docs synopsis.
docs/stratis.txt (1)
277-289: Use “cannot” instead of “can not” for consistencyMinor wording improvement matching the project’s existing style elsewhere.
- for example, extending filesystems, can not be taken on the pool. + for example, extending filesystems, cannot be taken on the pool.src/stratis_cli/_actions/_crypt.py (3)
66-89: Optional: pre-validate presence of at least one encryption method for a clearer CLI errorRight now, the daemon enforces “no encryption params” for encrypt and returns an engine error (tests expect this). If you want a more user-friendly CLI-side message, you could check for both key_desc and clevis being absent and raise a StratisCliUserError with an actionable hint. This would require adjusting tests that currently expect the engine error.
Example (non-diff, illustrative):
if namespace.key_desc is None and namespace.clevis is None: # raise a CLI-layer user error with a helpful hint raise StratisCliUserError( "No encryption parameters specified; use --key-desc or --clevis." )
149-171: Reencrypt relies on daemon-side validation; consider early state check (optional)Current behavior delegates “unencrypted pools” rejection to stratisd (tests validate the engine error path). For a more consistent UX, you could mirror the encrypt/unencrypt pattern and check MOPool(...).Encrypted() before calling ReencryptPool, raising StratisCliNoChangeError on unencrypted pools. If you decide to do this, tests expecting engine errors will need to be updated.
53-65: Minor: reduce duplication in pool resolution logicThe pool lookup pattern repeats in all three methods. Extracting a tiny helper (e.g., _get_pool_object_path(namespace) -> object_path, mopool) would trim duplication and centralize any future changes to selection semantics.
Illustrative helper (non-diff):
@staticmethod def _get_pool_object_path(namespace): from ._data import ObjectManager, pools pool_id = PoolId.from_parser_namespace(namespace) assert pool_id is not None proxy = get_object(TOP_OBJECT) managed_objects = ObjectManager.Methods.GetManagedObjects(proxy, {}) return next( pools(props=pool_id.managed_objects_key()) .require_unique_match(True) .search(managed_objects) )tests/whitebox/integration/pool/test_encryption.py (2)
433-473: OffTestCase: validates decrypt-on-encrypted path; consider asserting post-stateThe test exercises the success path. Optionally, add a quick verification (e.g., pool list detail) to assert encryption is now disabled to strengthen the signal.
498-542: ReencryptTestCase: exercising list after reencryption is good — optionally assert presence of the new fieldSince docs introduce “Last Time Reencrypted,” consider asserting the detail view includes it (and is not “Never”) to validate the end-to-end exposure. If the harness doesn’t capture stdout easily, we can extend RUNNER/TEST_RUNNER to return output for assertions.
src/stratis_cli/_parser/_encryption.py (4)
402-419: Polish help text for “encryption off”Functionality and wiring look good. Consider clearer phrasing.
- "help": "Make unencrypted a previously encrypted pool", + "help": "Decrypt a previously encrypted pool",
421-438: Polish help text for “reencrypt”Looks good. Minor wording improvement for user-facing help.
- "help": "Reencrypt an encrypted pool with a new master key", + "help": "Rotate the pool’s encryption master key (re-encrypt with a new master key)",
25-26: Constants Verified; Consider Enforcing Mutual Exclusivity for Encryption ArgumentsI’ve confirmed that both
CLEVIS_AND_KERNEL(line 278) andIN_PLACE(line 301) insrc/stratis_cli/_parser/_shared.pyare defined as list literals of argument specs, so their reuse here is correct.To ensure users select exactly one encryption method at parse time (rather than catching it post‐parse), you may wish to leverage an argparse mutually‐exclusive group. For example, in
src/stratis_cli/_parser/_encryption.py, you could replace:- { - "description": "Arguments controlling encryption", - "args": CLEVIS_AND_KERNEL, - }, + { + "description": "Arguments controlling encryption", + "mut_ex_args": [(True, CLEVIS_AND_KERNEL)], + },This change will enforce exclusivity when parsing
encryption on.
- src/stratis_cli/_parser/_shared.py: CLEVIS_AND_KERNEL (lines 278–299) and IN_PLACE (lines 301–303) confirmed as lists
- src/stratis_cli/_parser/_encryption.py: consider swapping
"args": CLEVIS_AND_KERNELfor a"mut_ex_args"entry
357-400: Tighten help text for “encryption on” subcommand• Confirmed the “Tang Server Verification” group label correctly refers to
--tang-urlas defined insrc/stratis_cli/_parser/_shared.py(line 294), so no change needed there.
• Nit: simplify the help text for theencryption onsubcommand for clarity.Suggested update in
src/stratis_cli/_parser/_encryption.py(around line 360):- "help": "Make encrypted a previously unencrypted pool", + "help": "Encrypt a previously unencrypted pool",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (17)
.packit.yaml(0 hunks)Makefile(1 hunks)docs/stratis.txt(3 hunks)src/stratis_cli/_actions/__init__.py(2 hunks)src/stratis_cli/_actions/_crypt.py(1 hunks)src/stratis_cli/_actions/_data.py(1 hunks)src/stratis_cli/_actions/_introspect.py(3 hunks)src/stratis_cli/_actions/_list_pool.py(2 hunks)src/stratis_cli/_actions/_utils.py(2 hunks)src/stratis_cli/_error_reporting.py(1 hunks)src/stratis_cli/_errors.py(1 hunks)src/stratis_cli/_parser/_encryption.py(2 hunks)src/stratis_cli/_parser/_pool.py(2 hunks)src/stratis_cli/_parser/_shared.py(1 hunks)tests/whitebox/integration/pool/test_encryption.py(2 hunks)tests/whitebox/integration/test_parser.py(1 hunks)tests/whitebox/unit/test_running.py(1 hunks)
💤 Files with no reviewable changes (1)
- .packit.yaml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-18T15:25:05.145Z
Learnt from: mulkieran
PR: stratis-storage/stratis-cli#0
File: :0-0
Timestamp: 2025-08-18T15:25:05.145Z
Learning: For the stratis-cli project, use Python's standard unittest framework instead of pytest. The codebase follows unittest.TestCase patterns with standard unittest assertions and test discovery.
Applied to files:
tests/whitebox/integration/pool/test_encryption.py
🪛 Ruff (0.12.2)
src/stratis_cli/_actions/__init__.py
20-20: ._crypt.CryptActions imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
33-33: ._utils.get_errors imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
src/stratis_cli/_actions/_utils.py
284-284: Do not call getattr with a constant attribute value. It is not any safer than normal property access.
Replace getattr with attribute access
(B009)
284-284: Do not call getattr with a constant attribute value. It is not any safer than normal property access.
Replace getattr with attribute access
(B009)
🪛 LanguageTool
docs/stratis.txt
[grammar] ~123-~123: There might be a mistake here.
Context: ...oportional to the size of the pool. pool encryption off --in-place <(--uuid ...
(QB_NEW_EN)
[grammar] ~127-~127: There might be a mistake here.
Context: ...oportional to the size of the pool. pool encryption reencrypt --in-place <(-...
(QB_NEW_EN)
[grammar] ~131-~131: There might be a mistake here.
Context: ... proportional to the size of the pool. pool encryption bind <(nbde|tang)> <(--u...
(QB_NEW_EN)
[grammar] ~135-~135: There might be a mistake here.
Context: ...on). tang is an alias for nbde. pool encryption bind tpm2 <(--uuid <uuid...
(QB_NEW_EN)
[style] ~285-~285: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ... for example, extending filesystems, can not be taken on the pool. Furthermo...
(CAN_NOT_PREMIUM)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (52)
- GitHub Check: osh-diff-scan:fedora-rawhide-x86_64:copr_pull
- GitHub Check: osh-diff-scan:fedora-rawhide-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-42-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-rawhide-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-43-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-41-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-43-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-42-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-41-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-rawhide-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-rawhide-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-43-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-41-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-42-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-43-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-41-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-42-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-rawhide-x86_64:copr_pull
- GitHub Check: osh-diff-scan:fedora-rawhide-x86_64:copr_pull
- GitHub Check: osh-diff-scan:fedora-rawhide-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-41-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-rawhide-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-42-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-43-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-42-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-43-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-41-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-rawhide-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-43-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-41-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-42-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-rawhide-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-rawhide-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-43-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-41-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-42-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-42-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-rawhide-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-43-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-41-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-42-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-43-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-41-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-rawhide-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-rawhide-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-43-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-41-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-42-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-42-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-43-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-rawhide-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-41-x86_64:copr_pull
🔇 Additional comments (26)
src/stratis_cli/_actions/_data.py (1)
46-46: The above will show us how_data.pyusesget_timeout(and whether any long-running methods use explicit overrides or decorators). Once we see the methods and their timeout handling, we can confirm no uncovered operations remain.src/stratis_cli/_errors.py (1)
459-478: Clear, user-focused error for in-place requirementThe new StratisCliInPlaceNotSpecified is well-scoped and the message is explicit about implications and next steps. Looks good.
src/stratis_cli/_actions/_list_pool.py (1)
25-25: New dependency on python-dateutil — confirm packaging/runtime availabilityImporting dateutil.parser introduces/assumes a runtime dependency. If this project targets environments where python-dateutil might not be present by default, please add it to the packaging manifest (setup/config) or the distro spec, and update installation docs.
Optional: you can avoid the dependency by using the stdlib datetime.fromisoformat with a small adapter for trailing 'Z' timestamps. See the companion comment below for a concrete diff.
src/stratis_cli/_actions/_utils.py (1)
289-308: Behavioral acknowledgmentCatching DPClientInvocationError chained to NoReply and emitting a one-line status to stderr is an appropriate UX for initiating long-running actions over D-Bus where replies may not arrive before timeout.
Makefile (1)
7-10: Include new encryption actions module in MonkeyType tracing — LGTMAdding
stratis_cli._actions._crypttoMONKEYTYPE_MODULESensures type traces are collected for the new actions. This will help keep annotations current as the encryption workflows evolve.src/stratis_cli/_actions/_introspect.py (5)
180-184: New DecryptPool method signature — LGTMThe out-parameters
(results: b, return_code: q, return_string: s)match the existing conventions for similar single-result methods on the pool interface.
191-197: New EncryptPool method signature aligns with Manager.CreatePool types — LGTMIn-params
key_descs: a((bu)s)andclevis_infos: a((bu)ss)mirror the types used byManager.r9.CreatePool, ensuring consistency in how keys and Clevis metadata are conveyed.
236-240: New ReencryptPool method signature — LGTMShape and naming consistent with related pool actions returning a single boolean
resultsplus return code/string.
269-269: NoEncryptedproperty-change listeners detected; safe to remove EmitsChangedSignal annotationSearch for subscription patterns (
.connect('PropertiesChanged',.subscribe,add_signal_receiver) and occurrences ofEncryptedin signal contexts returned no matches. All references toEncryptedin the codebase are direct, synchronous property reads (e.g.,bool(MOPool(...).Encrypted())in_crypt.pyand_list_pool.py). Therefore, there are no listeners that depend on property-changed signals forEncrypted.
274-275: LastReencryptedTimestamp usage verifiedThe pool-listing code in src/stratis_cli/_actions/_list_pool.py (around lines 363–370) correctly unpacks
(valid, timestamp)and uses a conditional expression to format the timestamp only whenvalidis true, falling back to"Never"otherwise. This confirms downstream formatting gracefully handles the boolean flag and absent/empty timestamp.tests/whitebox/integration/test_parser.py (1)
244-245: Docstring scope widened appropriately — LGTMThe updated description now covers encryption options beyond creation; matches the broadened test coverage.
src/stratis_cli/_error_reporting.py (1)
36-41: No import cycle detected between_actionsand_error_reportingI’ve verified that:
src/stratis_cli/_actions/__init__.pyre-exportsget_errors(from_utils.py) and several other helpers, but it does not importsrc/stratis_cli/_error_reporting.py(script output lines 18–33).src/stratis_cli/_actions/_utils.pycontains no references or imports of_error_reporting(script output “No references found”).src/stratis_cli/_error_reporting.pyimports from_actions, but there is no reverse import path.The re-export pattern for
get_errorsis safe and introduces no import cycle.src/stratis_cli/_parser/_pool.py (2)
38-38: Good move: centralizing encryption args via CLEVIS_AND_KERNELImporting CLEVIS_AND_KERNEL from _shared reduces duplication and keeps pool create aligned with the new encryption subcommands. No issues spotted with the import set.
114-119: Pool create “Encryption” group now reuses CLEVIS_AND_KERNEL — verify option coverageSwitching "args" to CLEVIS_AND_KERNEL keeps --key-desc / --clevis / --tang-url consistent with the encryption subcommands. The follow-on "Tang Server Verification" group still supplies --thumbprint/--trust-url, so ClevisEncryptionOptions will find everything it needs.
Two checks to consider:
- Ensure there are no legacy callers/tests relying on a different help/ordering for these options for pool create.
- Confirm CreateOptions’ ClevisEncryptionOptions(namespace) is only constructed when these members exist; here they do, so this is safe.
If desired, we can add an explicit doc string here that both --key-desc and --clevis may be specified together (to allow multiple bindings) to avoid ambiguity for users.
src/stratis_cli/_parser/_shared.py (1)
301-313: IN_PLACE switch is clear; message communicates operational constraints wellThe mandatory nature and runtime implications are well articulated. This matches enforcement in CryptActions.
docs/stratis.txt (2)
120-151: New pool encryption subcommands documented comprehensively — build the man page to catch formatting edge casesThe synopsis for on/off/reencrypt mirrors the parser. AsciiDoc with nested brackets can be sensitive; please run the doc build (manpage) to ensure the new lines render as intended and don’t trip the formatter on nested option groups.
Would you like me to provide a script to build and sanity-check the generated man page locally?
385-389: “Last Time Reencrypted” field description is precise and scoped correctlyClearly states conditions (metadata v2 and encryption enabled) and the “Never” behavior. Looks good.
src/stratis_cli/_actions/_crypt.py (2)
47-49: Hard requirement of --in-place is enforced early — goodFast-failing on missing in-place matches the new CLI contract and simplifies subsequent logic.
101-142: Decrypt flow mirrors encrypt with coherent no-change and error handlingChecks encrypted state, calls DecryptPool, and handles Stratisd return codes consistently. Looks solid.
tests/whitebox/integration/pool/test_encryption.py (6)
20-24: Imports for new error types are correct and match behavior under testBringing in StratisCliInPlaceNotSpecified and friends aligns with the new enforcement and engine errors.
475-496: OffTestCase2: correct “no change” assertion for decrypting unencrypted poolsMatches CryptActions.unencrypt pre-check behavior.
544-565: ReencryptTestCase2: expecting engine error on unencrypted reencryption is consistent with current implementationGood negative coverage.
567-608: EncryptTestCase: asserting no-change when already encrypted is correctThis aligns with the early MOPool().Encrypted() guard in CryptActions.encrypt.
610-647: EncryptTestCase2: covers both success and “missing params” error
- Success path with --clevis=tpm2 is validated.
- Error path without any encryption params matches current engine enforcement. If you ever add CLI-side validation, this assertion will need to change accordingly.
649-705: NoInPlaceTestCase: great coverage of the mandatory --in-place requirement across on/off/reencryptThese tests directly backstop the new policy enforced in CryptActions.
src/stratis_cli/_parser/_encryption.py (1)
22-22: CryptActions import verified
TheCryptActionsclass is defined insrc/stratis_cli/_actions/_crypt.py(line 35) with the requiredencrypt,unencrypt, andreencryptmethods (lines 42, 103, 145), and it’s re-exported insrc/stratis_cli/_actions/__init__.py(line 20). The import insrc/stratis_cli/_parser/_encryption.pyis correct—no changes needed.
mulkieran
force-pushed
the
issue-stratisd-3597-2
branch
from
c87558d to
35a90f3
Compare
mulkieran
force-pushed
the
issue-stratisd-3597-2
branch
3 times, most recently
from
10d9e7a to
abfcfce
Compare
mulkieran
force-pushed
the
issue-stratisd-3597-2
branch
from
abfcfce to
e04f847
Compare
|
rebased, conflicts resolved |
mulkieran
force-pushed
the
issue-stratisd-3597-2
branch
from
e04f847 to
f48cec2
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (11)
src/stratis_cli/_actions/_utils.py (1)
277-286: Broaden exception-chain walk and add return typingConsider including context in addition to cause so wrapped DBus errors aren’t missed, and add an explicit iterator return type.
-from typing import Callable, Tuple +from typing import Callable, Tuple, Iterator @@ -def get_errors(exc: BaseException): +def get_errors(exc: BaseException) -> Iterator[BaseException]: @@ - while exc.__cause__ is not None: - yield exc.__cause__ - exc = exc.__cause__ + while True: + nxt = exc.__cause__ or exc.__context__ + if nxt is None: + break + yield nxt + exc = nxtdocs/stratis.txt (3)
120-132: Clarify requirement for specifying an encryption method on “encryption on”Users otherwise hit an engine error (covered by tests). Add one sentence.
pool encryption on --in-place <(--uuid <uuid> |--name <name>)> [--key-desc <key_desc>] [--clevis <(nbde|tang|tpm2)> [--tang-url <tang_url>] [<(--thumbprint <thp> | --trust-url)>]:: Turn encryption on for the specified pool. This operation must encrypt every block in the pool and takes time proportional to the size of the pool. + Either --key-desc or --clevis must be supplied to specify the encryption method.
277-291: Fix typos and minor grammar in --in-place option text“can not” -> “cannot”; “you data” -> “your data”.
- for example, extending filesystems, can not be taken on the pool. + for example, extending filesystems, cannot be taken on the pool. @@ - necessary while the encryption operation is running. Consider - backing up you data before initiating this operation. + necessary while the encryption operation is running. Consider + backing up your data before initiating this operation.
386-390: Consistency: “re-encrypted” vs “reencrypted”Elsewhere the label uses “Reencrypted”. Consider using “Reencrypted” here for consistency.
- The last time the pool was re-encrypted. If the pool has never been + The last time the pool was reencrypted. If the pool has never beensrc/stratis_cli/_actions/_crypt.py (3)
22-29: Import StratisdErrors from its home module, not via _errorsAvoids layering confusion; _errors doesn’t own StratisdErrors.
-from .._errors import ( - StratisCliEngineError, - StratisCliIncoherenceError, - StratisCliInPlaceNotSpecified, - StratisCliNoChangeError, - StratisdErrors, -) +from .._errors import ( + StratisCliEngineError, + StratisCliIncoherenceError, + StratisCliInPlaceNotSpecified, + StratisCliNoChangeError, +) +from .._stratisd_constants import StratisdErrors
53-56: Replace assert with an explicit checkAsserts may be optimized out; raise a CLI error instead for robustness.
If PoolId.from_parser_namespace(namespace) can ever return None, apply:
- pool_id = PoolId.from_parser_namespace(namespace) - assert pool_id is not None + pool_id = PoolId.from_parser_namespace(namespace) + if pool_id is None: # Parser should prevent this; fail defensively. + raise StratisCliEngineError(StratisdErrors.ERROR, "Missing pool identifier")If None is truly unreachable, add a brief comment:
- pool_id = PoolId.from_parser_namespace(namespace) - assert pool_id is not None + pool_id = PoolId.from_parser_namespace(namespace) + assert pool_id is not None # guaranteed by parser
69-89: Validate mixed method inputs or document intentencrypt() accepts both key_desc and clevis simultaneously and forwards both. If this is intended (multiple bindings on enable), great; if not, fail fast at the CLI with a clear message.
Would you like a small guard to enforce “exactly one of --key-desc or --clevis”?
tests/whitebox/integration/pool/test_encryption.py (2)
438-463: Minor test nits: annotate class constants; prefer star-unpacking
- Annotate class-level constants as ClassVar to satisfy RUF012 (optional in tests).
- Use iterable unpacking for command lines to satisfy RUF005.
Example pattern (apply similarly across classes):
-from .._misc import RUNNER, TEST_RUNNER, SimTestCase, device_name_list +from .._misc import RUNNER, TEST_RUNNER, SimTestCase, device_name_list +from typing import ClassVar, List @@ -class OffTestCase(SimTestCase): +class OffTestCase(SimTestCase): @@ - _MENU = ["--propagate", "pool", "encryption", "off", "--in-place"] - _POOLNAME = "poolname" - _KEY_DESC = "keydesc" + _MENU: ClassVar[List[str]] = ["--propagate", "pool", "encryption", "off", "--in-place"] + _POOLNAME: ClassVar[str] = "poolname" + _KEY_DESC: ClassVar[str] = "keydesc" @@ - command_line = ["pool", "create", self._POOLNAME] + _DEVICE_STRATEGY() + command_line = ["pool", "create", self._POOLNAME, *_DEVICE_STRATEGY()] @@ - command_line = self._MENU + [f"--name={self._POOLNAME}"] + command_line = [*self._MENU, f"--name={self._POOLNAME}"]Also applies to: 480-487, 503-529, 549-556, 572-598, 615-647
621-636: Optional: factor repeated setup logicPool creation and key setup sequences are repeated across classes. Consider small helpers to DRY the tests.
src/stratis_cli/_parser/_encryption.py (2)
361-372: Use iterable unpacking instead of list concatenation (RUF005)Cleaner and satisfies Ruff by avoiding
+ IN_PLACE.- "args": [ - ( - "--post-parser", - { - "action": RejectAction, - "default": ClevisEncryptionOptions, - "help": SUPPRESS, - "nargs": "?", - }, - ) - ] - + IN_PLACE, + "args": [ + ( + "--post-parser", + { + "action": RejectAction, + "default": ClevisEncryptionOptions, + "help": SUPPRESS, + "nargs": "?", + }, + ), + *IN_PLACE, + ],
360-361: Polish user-facing help stringsSlight rewording for clarity and consistency.
- "help": "Make encrypted a previously unencrypted pool", + "help": "Encrypt a previously unencrypted pool", - "help": "Make unencrypted a previously encrypted pool", + "help": "Unencrypt a previously encrypted pool", - "help": "Reencrypt an encrypted pool with a new master key", + "help": "Re-encrypt an encrypted pool with a new master key",Also applies to: 404-405, 423-424
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (17)
.packit.yaml(0 hunks)Makefile(1 hunks)docs/stratis.txt(3 hunks)src/stratis_cli/_actions/__init__.py(2 hunks)src/stratis_cli/_actions/_crypt.py(1 hunks)src/stratis_cli/_actions/_data.py(1 hunks)src/stratis_cli/_actions/_introspect.py(3 hunks)src/stratis_cli/_actions/_list_pool.py(2 hunks)src/stratis_cli/_actions/_utils.py(2 hunks)src/stratis_cli/_error_reporting.py(1 hunks)src/stratis_cli/_errors.py(1 hunks)src/stratis_cli/_parser/_encryption.py(2 hunks)src/stratis_cli/_parser/_pool.py(2 hunks)src/stratis_cli/_parser/_shared.py(1 hunks)tests/whitebox/integration/pool/test_encryption.py(2 hunks)tests/whitebox/integration/test_parser.py(1 hunks)tests/whitebox/unit/test_running.py(1 hunks)
💤 Files with no reviewable changes (1)
- .packit.yaml
🚧 Files skipped from review as they are similar to previous changes (10)
- src/stratis_cli/_actions/init.py
- src/stratis_cli/_parser/_shared.py
- src/stratis_cli/_actions/_data.py
- tests/whitebox/unit/test_running.py
- src/stratis_cli/_parser/_pool.py
- src/stratis_cli/_error_reporting.py
- tests/whitebox/integration/test_parser.py
- Makefile
- src/stratis_cli/_actions/_introspect.py
- src/stratis_cli/_actions/_list_pool.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-18T15:25:05.145Z
Learnt from: mulkieran
PR: stratis-storage/stratis-cli#0
File: :0-0
Timestamp: 2025-08-18T15:25:05.145Z
Learning: For the stratis-cli project, use Python's standard unittest framework instead of pytest. The codebase follows unittest.TestCase patterns with standard unittest assertions and test discovery.
Applied to files:
tests/whitebox/integration/pool/test_encryption.py
🪛 LanguageTool
docs/stratis.txt
[grammar] ~123-~123: There might be a mistake here.
Context: ...oportional to the size of the pool. pool encryption off --in-place <(--uuid ...
(QB_NEW_EN)
[grammar] ~127-~127: There might be a mistake here.
Context: ...oportional to the size of the pool. pool encryption reencrypt --in-place <(-...
(QB_NEW_EN)
[grammar] ~131-~131: There might be a mistake here.
Context: ... proportional to the size of the pool. pool encryption bind <(nbde|tang)> <(--u...
(QB_NEW_EN)
[grammar] ~135-~135: There might be a mistake here.
Context: ...on). tang is an alias for nbde. pool encryption bind tpm2 <(--uuid <uuid...
(QB_NEW_EN)
[style] ~285-~285: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ... for example, extending filesystems, can not be taken on the pool. Furthermo...
(CAN_NOT_PREMIUM)
[grammar] ~290-~290: Ensure spelling is correct
Context: ...is running. Consider backing up you data before initiating this operation. ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 Ruff (0.12.2)
src/stratis_cli/_actions/_crypt.py
54-54: Use of assert detected
(S101)
67-67: Avoid specifying long messages outside the exception class
(TRY003)
95-99: Avoid specifying long messages outside the exception class
(TRY003)
114-114: Use of assert detected
(S101)
127-127: Avoid specifying long messages outside the exception class
(TRY003)
137-141: Avoid specifying long messages outside the exception class
(TRY003)
156-156: Use of assert detected
(S101)
176-179: Avoid specifying long messages outside the exception class
(TRY003)
src/stratis_cli/_actions/_utils.py
304-304: Use raise without specifying exception name
Remove exception name
(TRY201)
src/stratis_cli/_parser/_encryption.py
361-372: Consider iterable unpacking instead of concatenation
Replace with iterable unpacking
(RUF005)
tests/whitebox/integration/pool/test_encryption.py
438-438: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
455-462: Consider iterable unpacking instead of concatenation
Replace with iterable unpacking
(RUF005)
469-471: Consider [*self._MENU, f"--name={self._POOLNAME}"] instead of concatenation
Replace with [*self._MENU, f"--name={self._POOLNAME}"]
(RUF005)
480-480: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
485-485: Consider iterable unpacking instead of concatenation
Replace with iterable unpacking
(RUF005)
492-494: Consider [*self._MENU, f"--name={self._POOLNAME}"] instead of concatenation
Replace with [*self._MENU, f"--name={self._POOLNAME}"]
(RUF005)
503-503: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
520-527: Consider iterable unpacking instead of concatenation
Replace with iterable unpacking
(RUF005)
534-536: Consider [*self._MENU, f"--name={self._POOLNAME}"] instead of concatenation
Replace with [*self._MENU, f"--name={self._POOLNAME}"]
(RUF005)
549-549: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
554-554: Consider iterable unpacking instead of concatenation
Replace with iterable unpacking
(RUF005)
561-563: Consider [*self._MENU, f"--name={self._POOLNAME}"] instead of concatenation
Replace with [*self._MENU, f"--name={self._POOLNAME}"]
(RUF005)
572-572: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
589-596: Consider iterable unpacking instead of concatenation
Replace with iterable unpacking
(RUF005)
603-606: Consider iterable unpacking instead of concatenation
Replace with iterable unpacking
(RUF005)
615-615: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
621-626: Consider iterable unpacking instead of concatenation
Replace with iterable unpacking
(RUF005)
633-636: Consider iterable unpacking instead of concatenation
Replace with iterable unpacking
(RUF005)
643-645: Consider [*self._MENU, f"--name={self._POOLNAME}"] instead of concatenation
Replace with [*self._MENU, f"--name={self._POOLNAME}"]
(RUF005)
659-664: Consider iterable unpacking instead of concatenation
Replace with iterable unpacking
(RUF005)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: rpm-build:fedora-43-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-41-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-rawhide-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-43-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-42-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-41-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-43-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-42-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-rawhide-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-41-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-41-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-rawhide-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-43-x86_64:copr_pull
- GitHub Check: rpm-build:fedora-42-x86_64:copr_pull
- GitHub Check: test-runs (3.13)
- GitHub Check: test-runs (3.12)
🔇 Additional comments (11)
src/stratis_cli/_errors.py (1)
459-472: New user-facing error reads well; API addition LGTMClear message, consistent with the new mandatory --in-place option. No functional concerns.
src/stratis_cli/_actions/_crypt.py (1)
101-109: In-place enforcement is correctly appliedEarly fail on missing --in-place matches the docs and tests.
tests/whitebox/integration/pool/test_encryption.py (7)
433-473: Good coverage for “off” happy path and setupSetup and name-based targeting look correct; test asserts on errors are precise.
475-496: No-change path validation reads wellAsserting StratisCliNoChangeError on decrypt when unencrypted matches action semantics.
498-542: Reencrypt flow and detail view exercise are solidCovers operation and the detail listing that includes last reencrypt time.
544-565: Engine error expectation on reencrypt when unencrypted is appropriateMatches daemon-side validation.
567-608: Encrypt-on when already encrypted; negative case looks correctAsserting StratisCliNoChangeError is consistent with CryptActions.encrypt().
610-647: Positive encrypt-on and missing-params error both coveredNice pairing of success and failure tests.
649-706: Mandatory --in-place tests are on pointExplicitly verifies enforcement across on/off/reencrypt.
src/stratis_cli/_parser/_encryption.py (2)
22-27: Imports for CryptActions and shared argument sets look correctNo issues wiring up CryptActions and pulling in CLEVIS_AND_KERNEL/IN_PLACE.
384-389: CLEVIS_AND_KERNEL is a list of (name, opts) tuples and correctly used under “args”
| def long_running_operation(func: Callable) -> Callable: | ||
| """ | ||
| Mark a function as a long running operation and catch and ignore NoReply | ||
| D-Bus exception. | ||
| """ | ||
|
|
||
| @wraps(func) | ||
| def wrapper(namespace: Namespace): | ||
| try: | ||
| func(namespace) | ||
| except DPClientInvocationError as err: | ||
| if not any( | ||
| isinstance(e, DBusException) | ||
| and e.get_dbus_name() == "org.freedesktop.DBus.Error.NoReply" | ||
| for e in get_errors(err) | ||
| ): | ||
| raise err | ||
| print("Operation initiated", file=sys.stderr) | ||
|
|
||
| return wrapper |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Preserve traceback; pass through return; accept any signature
- Use bare raise to keep the original traceback (TRY201).
- Return the wrapped function’s result.
- Support arbitrary signatures so the decorator is reusable beyond Namespace-only call sites.
-from typing import Callable, Tuple
+from typing import Callable, Tuple, Any
@@
-def long_running_operation(func: Callable) -> Callable:
+def long_running_operation(func: Callable[..., Any]) -> Callable[..., Any]:
@@
- def wrapper(namespace: Namespace):
+ def wrapper(*args: Any, **kwargs: Any) -> Any:
try:
- func(namespace)
+ return func(*args, **kwargs)
except DPClientInvocationError as err:
if not any(
isinstance(e, DBusException)
and e.get_dbus_name() == "org.freedesktop.DBus.Error.NoReply"
for e in get_errors(err)
):
- raise err
+ raise
print("Operation initiated", file=sys.stderr)
+ return None📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def long_running_operation(func: Callable) -> Callable: | |
| """ | |
| Mark a function as a long running operation and catch and ignore NoReply | |
| D-Bus exception. | |
| """ | |
| @wraps(func) | |
| def wrapper(namespace: Namespace): | |
| try: | |
| func(namespace) | |
| except DPClientInvocationError as err: | |
| if not any( | |
| isinstance(e, DBusException) | |
| and e.get_dbus_name() == "org.freedesktop.DBus.Error.NoReply" | |
| for e in get_errors(err) | |
| ): | |
| raise err | |
| print("Operation initiated", file=sys.stderr) | |
| return wrapper | |
| from typing import Callable, Tuple, Any | |
| def long_running_operation(func: Callable[..., Any]) -> Callable[..., Any]: | |
| """ | |
| Mark a function as a long running operation and catch and ignore NoReply | |
| D-Bus exception. | |
| """ | |
| @wraps(func) | |
| def wrapper(*args: Any, **kwargs: Any) -> Any: | |
| try: | |
| return func(*args, **kwargs) | |
| except DPClientInvocationError as err: | |
| if not any( | |
| isinstance(e, DBusException) | |
| and e.get_dbus_name() == "org.freedesktop.DBus.Error.NoReply" | |
| for e in get_errors(err) | |
| ): | |
| raise | |
| print("Operation initiated", file=sys.stderr) | |
| return None | |
| return wrapper |
🧰 Tools
🪛 Ruff (0.12.2)
304-304: Use raise without specifying exception name
Remove exception name
(TRY201)
🤖 Prompt for AI Agents
In src/stratis_cli/_actions/_utils.py around lines 288 to 307, the decorator
currently forces a Namespace-only signature, drops the wrapped function’s return
value, and re-raises exceptions with "raise err" which loses the original
traceback; change wrapper to accept arbitrary signatures (def wrapper(*args,
**kwargs)), call the wrapped function as result = func(*args, **kwargs) inside
the try, return that result, and in the except block re-raise the original
exception with a bare "raise" when it is not the NoReply DBus error so the
original traceback is preserved; keep the existing NoReply-check and stderr
print behavior unchanged.
mulkieran
force-pushed
the
issue-stratisd-3597-2
branch
from
1606146 to
c7d4f82
Compare
|
rebased, conflicts resolved |
mulkieran
force-pushed
the
issue-stratisd-3597-2
branch
2 times, most recently
from
1c9b1b8 to
dbe1553
Compare
mulkieran
force-pushed
the
issue-stratisd-3597-2
branch
from
dbe1553 to
fee0c44
Compare
|
rebased, conflicts resolved |
mulkieran
force-pushed
the
issue-stratisd-3597-2
branch
from
8583b69 to
c1b9bce
Compare
|
rebased, conflicts resolved. |
|
Test errors are from the change on stratisd side. If the D-Bus lock isn't FIFO, then the tests will have to read the D-Bus until the newly created D-Bus object is inserted in the tree before operating on it. Until now, many of the tests have relied on the lock being FIFO. To change that...can't wait for the signal in this code, 'cause the signal might appear too early, so need to scan the D-Bus until a new dbus object path appears. For that, should really design a context manager for any command that should produce a new object path on success. The problem is, if reads can get in before writes, incessant reads will block the write that is being waited for. So need to give that write a chance by an intelligent wait using one of our favorite retry libraries, e.g., tenacity. |
mulkieran
force-pushed
the
issue-stratisd-3597-2
branch
from
c1b9bce to
722b0bb
Compare
Signed-off-by: mulhern <amulhern@redhat.com>
Signed-off-by: mulhern <amulhern@redhat.com>
Signed-off-by: mulhern <amulhern@redhat.com>
Signed-off-by: mulhern <amulhern@redhat.com>
Signed-off-by: mulhern <amulhern@redhat.com>
We had it as high as two minutes to give a chance of returning on fairly long-running task, like creating encrypted pools, since each device had to be separately encrypted. We do not do that anymore, as the whole pool is encrypted these days, so the create method returns faster. We are about to introduce really long running commands, where 120 minutes will not be enough in almost all cases. So shortening the D-Bus timeout seems like a reasonable thing to do. 60 seconds is a plenty long time to wait for any error messsages that stratisd might return. Signed-off-by: mulhern <amulhern@redhat.com>
This way we can relatively cheaply avoid printing the timeout error message and return a zero exit code on the timeout. Signed-off-by: mulhern <amulhern@redhat.com>
Signed-off-by: mulhern <amulhern@redhat.com>
Signed-off-by: mulhern <amulhern@redhat.com>
These are bind, unbind, and rebind. The new commands use the mandatory --name, --uuid option mechanism for specifying the pool to operate on, while the old commands just used name. Signed-off-by: mulhern <amulhern@redhat.com>
Signed-off-by: mulhern <amulhern@redhat.com>
Signed-off-by: mulhern <amulhern@redhat.com>
Signed-off-by: mulhern <amulhern@redhat.com>
2 participants
Related stratis-storage/stratisd#3597
Supercedes #1186
Summary by CodeRabbit
New Features
Documentation
Tests
Chores