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 <gabrioux@ibm.com>

diff --git a/src/ceph-volume/ceph_volume/devices/lvm/common.py b/src/ceph-volume/ceph_volume/devices/lvm/common.py
index f9dc2ce60aef047e48141902175ef89610a589ac..298c49d0e6af9124c2947e2ca91da75c09a1ebb2 100644 (file)
--- 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 0300cb772d4c20f890e367e7d427aa6d4bfcf174..c78d33af157085e860619728d610e57a5d592333 100644 (file)
--- 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 fe792d5ab99a2743f5e8d6b9e6de4eb258d1c075..4dcd24c2d9ea585535b6fe8ebc36e1eb1f394f5f 100644 (file)
--- 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