From: Guillaume Abrioux Date: Wed, 6 May 2026 11:48:13 +0000 (+0200) Subject: ceph-volume: fix argparse dmcrypt opts: use str type X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F68769%2Fhead;p=ceph.git ceph-volume: fix argparse dmcrypt opts: use str type argparse's `type` has to be a callable that turns the CLI string into the stored value. Optional[str] is a typing construct. Newer argparse treats that specially and fails with "invalid Optional value". default=None already means the flag is optional. Fixes: https://tracker.ceph.com/issues/76433 Signed-off-by: Guillaume Abrioux --- diff --git a/src/ceph-volume/ceph_volume/devices/lvm/common.py b/src/ceph-volume/ceph_volume/devices/lvm/common.py index f9dc2ce60aef..298c49d0e6af 100644 --- a/src/ceph-volume/ceph_volume/devices/lvm/common.py +++ b/src/ceph-volume/ceph_volume/devices/lvm/common.py @@ -86,11 +86,11 @@ common_args: Dict[str, Any] = { }, '--dmcrypt-format-opts': { 'default': None, - 'type': Optional[str], + 'type': str, }, '--dmcrypt-open-opts': { 'default': None, - 'type': Optional[str], + 'type': str, }, '--with-tpm': { 'dest': 'with_tpm', diff --git a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_batch.py b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_batch.py index 0300cb772d4c..c78d33af1570 100644 --- a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_batch.py +++ b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_batch.py @@ -22,6 +22,25 @@ class TestBatch(object): with pytest.raises(SystemExit): batch.Batch(argv=['--osd-ids', '1', 'foo']).main() + def test_batch_dmcrypt_open_opts_accepts_cryptsetup_style_value(self): + b = batch.Batch([ + '--dmcrypt-open-opts', '--persistent --debug-json', + ]) + assert b.args.dmcrypt_open_opts == '--persistent --debug-json' + assert b.args.dmcrypt_format_opts is None + + def test_batch_dmcrypt_format_opts_accepts_cryptsetup_style_value(self): + b = batch.Batch([ + '--dmcrypt-format-opts', '--foo bar', + ]) + assert b.args.dmcrypt_format_opts == '--foo bar' + assert b.args.dmcrypt_open_opts is None + + def test_batch_dmcrypt_opts_default_none(self): + b = batch.Batch([]) + assert b.args.dmcrypt_open_opts is None + assert b.args.dmcrypt_format_opts is None + def test_disjoint_device_lists(self, mock_device_generator: Callable) -> None: device1 = mock_device_generator(used_by_ceph=False, available=True, abspath='/dev/sda') device2 = mock_device_generator(used_by_ceph=False, available=True, abspath='/dev/sdb') diff --git a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_common.py b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_common.py index fe792d5ab99a..4dcd24c2d9ea 100644 --- a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_common.py +++ b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_common.py @@ -1,3 +1,5 @@ +from unittest.mock import MagicMock, patch + from ceph_volume.devices.lvm import common @@ -6,3 +8,68 @@ class TestCommon(object): def test_get_default_args_smoke(self): default_args = common.get_default_args() assert default_args + + @patch('ceph_volume.util.arg_validators.disk.has_bluestore_label', return_value=False) + @patch('ceph_volume.util.arg_validators.Device') + def test_prepare_parser_dmcrypt_open_opts_string_with_leading_dashes( + self, mocked_device, _mock_bs_label + ): + mocked_device.return_value = MagicMock( + exists=True, + has_gpt_headers=False, + has_partitions=False, + used_by_ceph=False, + has_fs=False, + is_lv=False, + path='/dev/nvme8n1', + vg_name='vg', + lv_name='lv', + ) + parser = common.prepare_parser('ceph-volume lvm prepare', 'test') + ns = parser.parse_args([ + '--data', '/dev/nvme8n1', + '--dmcrypt-open-opts', '--persistent --debug-json', + ]) + assert ns.dmcrypt_open_opts == '--persistent --debug-json' + + @patch('ceph_volume.util.arg_validators.disk.has_bluestore_label', return_value=False) + @patch('ceph_volume.util.arg_validators.Device') + def test_prepare_parser_dmcrypt_format_opts_same_shape( + self, mocked_device, _mock_bs_label + ): + mocked_device.return_value = MagicMock( + exists=True, + has_gpt_headers=False, + has_partitions=False, + used_by_ceph=False, + has_fs=False, + is_lv=False, + path='/dev/nvme0n1', + vg_name='vg', + lv_name='lv', + ) + parser = common.prepare_parser('ceph-volume lvm prepare', 'test') + ns = parser.parse_args([ + '--data', '/dev/nvme0n1', + '--dmcrypt-format-opts', '--foo bar', + ]) + assert ns.dmcrypt_format_opts == '--foo bar' + + @patch('ceph_volume.util.arg_validators.disk.has_bluestore_label', return_value=False) + @patch('ceph_volume.util.arg_validators.Device') + def test_prepare_parser_dmcrypt_opts_default_none(self, mocked_device, _mock_bs_label): + mocked_device.return_value = MagicMock( + exists=True, + has_gpt_headers=False, + has_partitions=False, + used_by_ceph=False, + has_fs=False, + is_lv=False, + path='/dev/sdz', + vg_name='vg', + lv_name='lv', + ) + parser = common.prepare_parser('ceph-volume lvm prepare', 'test') + ns = parser.parse_args(['--data', '/dev/sdz']) + assert ns.dmcrypt_open_opts is None + assert ns.dmcrypt_format_opts is None