Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow setting journal size and tag size for integrity metadata pre-allocations on create #1110

Merged
merged 1 commit into from
Jan 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion docs/stratis.txt
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ GLOBAL OPTIONS

COMMANDS
--------
pool create [--key-desc <key_desc>] [--clevis <(nbde|tang|tpm2)> [--tang-url <tang_url>] [<(--thumbprint <thp> | --trust-url)>] [--no-overprovision] <pool_name> <blockdev> [<blockdev>..]::
pool create [--key-desc <key_desc>] [--clevis <(nbde|tang|tpm2)> [--tang-url <tang_url>] [<(--thumbprint <thp> | --trust-url)>] [--no-overprovision] [--integrity <(no,pre-allocate)>] [--journal-size <journal_size>] [--tag-spec <tag_spec>] <pool_name> <blockdev> [<blockdev>..]::
Create a pool from one or more block devices, with the given pool name.
The --tag-spec and --journal-size options are used to configure the amount
of space to reserve for integrity metadata.
pool stop <(--uuid <uuid> |--name <name>)>::
Stop a pool, specifying the pool by its UUID or by its name. Tear down
the storage stack but leave all metadata intact.
Expand Down
3 changes: 3 additions & 0 deletions src/stratis_cli/_actions/_introspect.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
<arg name="devices" type="as" direction="in" />
<arg name="key_desc" type="(bs)" direction="in" />
<arg name="clevis_info" type="(b(ss))" direction="in" />
<arg name="journal_size" type="(bt)" direction="in" />
<arg name="tag_spec" type="(bs)" direction="in" />
<arg name="allocate_superblock" type="(bb)" direction="in" />
<arg name="result" type="(b(oao))" direction="out" />
<arg name="return_code" type="q" direction="out" />
<arg name="return_string" type="s" direction="out" />
Expand Down
17 changes: 15 additions & 2 deletions src/stratis_cli/_actions/_pool.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
# isort: THIRDPARTY
from justbytes import Range

from .._constants import Id, IdType, UnlockMethod
from .._constants import Id, IdType, IntegrityOption, IntegrityTagSpec, UnlockMethod
from .._error_codes import PoolErrorCode
from .._errors import (
StratisCliEngineError,
Expand Down Expand Up @@ -180,6 +180,16 @@ def create_pool(namespace): # pylint: disable=too-many-locals
None if namespace.clevis is None else ClevisInfo.get_info(namespace.clevis)
)

(journal_size, tag_spec, allocate_superblock) = (
((True, 0), (True, IntegrityTagSpec.B0), (True, False))
if namespace.integrity.integrity is IntegrityOption.NO
else (
(True, namespace.integrity.journal_size.magnitude.numerator),
(True, namespace.integrity.tag_spec),
(True, True),
)
)

(
(changed, (pool_object_path, _)),
return_code,
Expand All @@ -199,10 +209,13 @@ def create_pool(namespace): # pylint: disable=too-many-locals
if clevis_info is None
else (True, (clevis_info.pin, json.dumps(clevis_info.config)))
),
"journal_size": journal_size,
"tag_spec": tag_spec,
"allocate_superblock": allocate_superblock,
},
)

if return_code != StratisdErrors.OK: # pragma: no cover
if return_code != StratisdErrors.OK:
raise StratisCliEngineError(return_code, message)

if not changed: # pragma: no cover
Expand Down
25 changes: 25 additions & 0 deletions src/stratis_cli/_constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,3 +116,28 @@ class Clevis(Enum):

def __str__(self):
return self.value


class IntegrityTagSpec(Enum):
"""
Options for specifying integrity tag size.
"""

B0 = "0b"
B32 = "32b"
B512 = "512b"

def __str__(self):
return self.value


class IntegrityOption(Enum):
"""
Options for specifying integrity choices on create.
"""

NO = "no"
PRE_ALLOCATE = "pre-allocate"

def __str__(self):
return self.value
142 changes: 138 additions & 4 deletions src/stratis_cli/_parser/_pool.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,22 @@
from argparse import SUPPRESS, ArgumentTypeError
from uuid import UUID

# isort: THIRDPARTY
from justbytes import MiB, Range

from .._actions import BindActions, PoolActions
from .._constants import Clevis, EncryptionMethod, UnlockMethod, YesOrNo
from .._constants import (
Clevis,
EncryptionMethod,
IntegrityOption,
IntegrityTagSpec,
UnlockMethod,
YesOrNo,
)
from .._error_codes import PoolErrorCode
from ._bind import BIND_SUBCMDS, REBIND_SUBCMDS
from ._debug import POOL_DEBUG_SUBCMDS
from ._range import RejectAction
from ._range import DefaultAction, RejectAction, parse_range


class ClevisEncryptionOptions: # pylint: disable=too-few-public-methods
Expand Down Expand Up @@ -81,6 +91,62 @@ def verify(self, namespace, parser):
namespace.clevis = None if self.clevis is None else self


class IntegrityOptions: # pylint: disable=too-few-public-methods
"""
Gathers and verifies integrity options.
"""

def __init__(self, namespace):
self.integrity = copy.copy(namespace.integrity)
del namespace.integrity

self.journal_size = copy.copy(namespace.journal_size)
del namespace.journal_size

self.journal_size_default = getattr(namespace, "journal_size_default", True)

self.tag_spec = copy.copy(namespace.tag_spec)
del namespace.tag_spec

self.tag_spec_default = getattr(namespace, "tag_spec_default", True)

def verify(self, namespace, parser):
"""
Do supplementary parsing of conditional arguments.
"""

if self.integrity is IntegrityOption.NO and not self.journal_size_default:
parser.error(
f'--integrity value "{IntegrityOption.NO}" and --journal-size '
"option can not be specified together"
)

if self.integrity is IntegrityOption.NO and not self.tag_spec_default:
parser.error(
f'--integrity value "{IntegrityOption.NO}" and --tag-spec '
"option can not be specified together"
)

namespace.integrity = self


class CreateOptions: # pylint: disable=too-few-public-methods
"""
Gathers and verifies options specified on pool create.
"""

def __init__(self, namespace):
self.clevis_encryption_options = ClevisEncryptionOptions(namespace)
self.integrity_options = IntegrityOptions(namespace)

def verify(self, namespace, parser):
"""
Verify that create command-line is formed correctly.
"""
self.clevis_encryption_options.verify(namespace, parser)
self.integrity_options.verify(namespace, parser)


def _ensure_nat(arg):
"""
Raise error if argument is not an natural number.
Expand Down Expand Up @@ -110,7 +176,7 @@ def _ensure_nat(arg):
"--post-parser",
{
"action": RejectAction,
"default": ClevisEncryptionOptions,
"default": CreateOptions,
"help": SUPPRESS,
"nargs": "?",
},
Expand Down Expand Up @@ -163,7 +229,75 @@ def _ensure_nat(arg):
)
],
},
)
),
(
"integrity",
{
"description": (
"Optional parameters for configuring integrity "
"metadata pre-allocation"
),
"args": [
(
"--integrity",
{
"help": (
"Integrity options for this pool. If "
f'"{IntegrityOption.NO}" no space will '
"be allocated for integrity metadata "
"and it will never be possible to turn "
"on integrity functionality for this "
"pool. "
f'If "{IntegrityOption.PRE_ALLOCATE}" '
"then space will be allocated for "
"integrity metadata and it will be "
"possible to switch on integrity "
"functionality in future. The default "
'is "%(default)s".'
),
"choices": list(IntegrityOption),
"default": IntegrityOption.PRE_ALLOCATE,
"type": IntegrityOption,
},
),
(
"--journal-size",
{
"help": (
"Size of integrity device's journal. "
"Each block is written to this journal "
"before being written to its address. "
"The default is %(default)s."
),
"action": DefaultAction,
"default": Range(128, MiB),
"type": parse_range,
},
),
(
"--tag-spec",
{
"help": (
"Integrity tag specification defining "
"the size of the tag used to store a "
"checksum or other value for each "
"block on a device. All size "
"specifications are in bits. The "
"default is %(default)s."
),
"action": DefaultAction,
"default": IntegrityTagSpec.B512,
"choices": [
x
for x in list(IntegrityTagSpec)
if x is not IntegrityTagSpec.B0
],
"type": IntegrityTagSpec,
},
),
],
},
),
],
"args": [
("pool_name", {"help": "Name of new pool"}),
Expand Down
10 changes: 10 additions & 0 deletions src/stratis_cli/_parser/_range.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,13 @@ def __call__(self, parser, namespace, values, option_string=None):
raise argparse.ArgumentError(
self, f"Option {option_string} can not be assigned to or set."
)


class DefaultAction(argparse.Action):
"""
Detect if the default value was set.
"""

def __call__(self, parser, namespace, values, option_string=None):
setattr(namespace, self.dest, values)
setattr(namespace, self.dest + "_default", False)
20 changes: 20 additions & 0 deletions tests/whitebox/integration/pool/test_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
# isort: LOCAL
from stratis_cli import StratisCliErrorCodes
from stratis_cli._errors import (
StratisCliEngineError,
StratisCliInUseSameTierError,
StratisCliNameConflictError,
)
Expand Down Expand Up @@ -165,3 +166,22 @@ def test_no_overprovision(self):
self._MENU + [self._POOLNAME] + self._DEVICES + ["--no-overprovision"]
)
TEST_RUNNER(command_line)


class Create7TestCase(SimTestCase):
"""
Test create with integrity options.
"""

_MENU = ["--propagate", "pool", "create"]
_DEVICES = _DEVICE_STRATEGY()
_POOLNAME = "thispool"

def test_invalid_journal_size(self):
"""
Test creating with an invalid journal size.
"""
command_line = (
self._MENU + [self._POOLNAME] + self._DEVICES + ["--journal-size=131079KiB"]
)
self.check_error(StratisCliEngineError, command_line, _ERROR)
48 changes: 47 additions & 1 deletion tests/whitebox/integration/test_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
_PARSE_ERROR = StratisCliErrorCodes.PARSE_ERROR


class ParserTestCase(RunTestCase):
class ParserTestCase(RunTestCase): # pylint: disable=too-many-public-methods
"""
Test parser behavior. The behavior should be identical, regardless of
whether the "--propagate" flag is set. That is, stratis should never produce
Expand Down Expand Up @@ -221,6 +221,52 @@ def test_create_with_post_parser_set(self):
for prefix in [[], ["--propagate"]]:
self.check_system_exit(prefix + command_line, _PARSE_ERROR)

def test_create_with_bad_tag_value(self):
"""
Verify that an unrecognized tag value causes an error.
"""
command_line = [
"pool",
"create",
"pn",
"/dev/n",
"--tag-spec=512",
]
for prefix in [[], ["--propagate"]]:
self.check_system_exit(prefix + command_line, _PARSE_ERROR)

def test_create_with_integrity_no_journal_size(self):
"""
Verify that creating with integrity = no plus good journal-size
results in a parse error.
"""
command_line = [
"pool",
"create",
"pn",
"/dev/n",
"--integrity=no",
"--journal-size=128MiB",
]
for prefix in [[], ["--propagate"]]:
self.check_system_exit(prefix + command_line, _PARSE_ERROR)

def test_create_with_integrity_no_tag_spec(self):
"""
Verify that creating with integrity = no plus good tag-size
results in a parse error.
"""
command_line = [
"pool",
"create",
"pn",
"/dev/n",
"--integrity=no",
"--tag-spec=32b",
]
for prefix in [[], ["--propagate"]]:
self.check_system_exit(prefix + command_line, _PARSE_ERROR)

def test_stratis_list_filesystem_with_name_no_pool(self):
"""
We want to get a parse error if filesystem UUID is specified but no
Expand Down
Loading