curtin-dev team mailing list archive

[Dan Bungert <mp+448309@xxxxxxxxxxxxxxxxxx>]

Dan Bungert has proposed merging ~dbungert/curtin:zpool-disable-features into curtin:master.

Commit message:
zpool: add no_default_features flag support

And a new test type for validating storage configs.

Requested reviews:
  curtin developers (curtin-dev)

For more details, see:
https://code.launchpad.net/~dbungert/curtin/+git/curtin/+merge/448309
-- 
Your team curtin developers is requested to review the proposed merge of ~dbungert/curtin:zpool-disable-features into curtin:master.

diff --git a/curtin/block/schemas.py b/curtin/block/schemas.py
index 514f791..ad9b1e4 100644
--- a/curtin/block/schemas.py
+++ b/curtin/block/schemas.py
@@ -391,6 +391,7 @@ ZPOOL = {
         'pool': {'$ref': '#/definitions/name'},
         'pool_properties': {'$ref': '#/definitions/params'},
         'fs_properties': {'$ref': '#/definitions/params'},
+        'no_default_features': {'type': 'boolean'},
         'mountpoint': {
             'type': 'string',
             'oneOf': [
diff --git a/curtin/block/zfs.py b/curtin/block/zfs.py
index 25751d3..38a3dd3 100644
--- a/curtin/block/zfs.py
+++ b/curtin/block/zfs.py
@@ -107,6 +107,7 @@ def zfs_assert_supported():
 
 
 def zpool_create(poolname, vdevs, mountpoint=None, altroot=None,
+                 no_default_features=False,
                  pool_properties=None, zfs_properties=None):
     """
     Create a zpool called <poolname> comprised of devices specified in <vdevs>.
@@ -114,6 +115,10 @@ def zpool_create(poolname, vdevs, mountpoint=None, altroot=None,
     :param poolname: String used to name the pool.
     :param vdevs: An iterable of strings of block devices paths which *should*
                   start with '/dev/disk/by-id/' to follow best practices.
+    :param no_default_features: If true, do not enable the default
+                                features.  pool_properties may be used to
+                                enable the desired features.  Corresponds
+                                to the `-d` flag of `zpool create`.
     :param pool_properties: A dictionary of key, value pairs to be passed
                             to `zpool create` with the `-o` flag as properties
                             of the zpool.  If value is None, then
@@ -155,6 +160,9 @@ def zpool_create(poolname, vdevs, mountpoint=None, altroot=None,
     if altroot:
         options.extend(['-R', altroot])
 
+    if no_default_features:
+        options.extend(['-d'])
+
     cmd = ["zpool", "create"] + options + [poolname] + vdevs
     util.subp(cmd, capture=True)
 
diff --git a/curtin/commands/block_meta.py b/curtin/commands/block_meta.py
index 421b048..32b978f 100644
--- a/curtin/commands/block_meta.py
+++ b/curtin/commands/block_meta.py
@@ -1964,6 +1964,7 @@ def zpool_handler(info, storage_config, context):
     mountpoint = info.get('mountpoint')
     pool_properties = info.get('pool_properties', {})
     fs_properties = info.get('fs_properties', {})
+    no_default_features = info.get('no_default_features', False)
     altroot = state['target']
 
     if not vdevs or not poolname:
@@ -1984,6 +1985,7 @@ def zpool_handler(info, storage_config, context):
     LOG.info('Creating zpool %s with vdevs %s', poolname, vdevs_byid)
     zfs.zpool_create(poolname, vdevs_byid,
                      mountpoint=mountpoint, altroot=altroot,
+                     no_default_features=no_default_features,
                      pool_properties=pool_properties,
                      zfs_properties=fs_properties)
 
diff --git a/doc/topics/storage.rst b/doc/topics/storage.rst
index 918f3cd..42dd874 100644
--- a/doc/topics/storage.rst
+++ b/doc/topics/storage.rst
@@ -1121,6 +1121,12 @@ are:
 - canmount: off
 - normalization: formD
 
+**no_default_features**: *true, false*
+
+If true, do not enable the default features.  pool_properties may be used to
+enable the desired features.  Corresponds to the `-d` flag of `zpool create`.
+Default value is *false*, which means the zpool default feature set is used..
+
 **Config Example**::
 
  - type: zpool
diff --git a/tests/unittests/schema/storage/valid/zpool-no-default-features.yaml b/tests/unittests/schema/storage/valid/zpool-no-default-features.yaml
new file mode 100644
index 0000000..165ff77
--- /dev/null
+++ b/tests/unittests/schema/storage/valid/zpool-no-default-features.yaml
@@ -0,0 +1,8 @@
+storage:
+  version: 2
+  config:
+    - type: zpool
+      id: zpool-1
+      pool: mypool
+      vdevs: [/dev/sda]
+      no_default_features: true
diff --git a/tests/unittests/test_block_zfs.py b/tests/unittests/test_block_zfs.py
index e392000..66d82f1 100644
--- a/tests/unittests/test_block_zfs.py
+++ b/tests/unittests/test_block_zfs.py
@@ -210,6 +210,26 @@ class TestBlockZfsZpoolCreate(CiTestCase):
             self.assertEqual(sorted(expected_args[index]), sorted(args[0]))
             self.assertEqual(expected_kwargs, kwargs)
 
+    def test_zpool_no_default_features_absent(self):
+        """ when no_default_features is missing, no -d arg in zpool create """
+        zfs.zpool_create('mypool', ['/dev/disk/by-id/virtio-abcfoo1'])
+        _name, args, _ = self.mock_subp.mock_calls[0]
+        self.assertNotIn('-d', args[0])
+
+    def test_zpool_no_default_features_false(self):
+        """ when no_default_features is false, no -d arg in zpool create """
+        zfs.zpool_create('mypool', ['/dev/disk/by-id/virtio-abcfoo1'],
+                         no_default_features=False)
+        _name, args, _ = self.mock_subp.mock_calls[0]
+        self.assertNotIn('-d', args[0])
+
+    def test_zpool_no_default_features_true(self):
+        """ when no_default_features is true, -d arg in zpool create """
+        zfs.zpool_create('mypool', ['/dev/disk/by-id/virtio-abcfoo1'],
+                         no_default_features=True)
+        _name, args, _ = self.mock_subp.mock_calls[0]
+        self.assertIn('-d', args[0])
+
 
 class TestBlockZfsZfsCreate(CiTestCase):
 
diff --git a/tests/unittests/test_commands_block_meta.py b/tests/unittests/test_commands_block_meta.py
index 028d04e..9464aac 100644
--- a/tests/unittests/test_commands_block_meta.py
+++ b/tests/unittests/test_commands_block_meta.py
@@ -933,6 +933,7 @@ class TestZpoolHandler(CiTestCase):
             info['pool'], [disk_path],
             mountpoint="/",
             altroot="mytarget",
+            no_default_features=False,
             pool_properties={'ashift': 42},
             zfs_properties={'compression': 'lz4'})
 
diff --git a/tests/unittests/test_schemas.py b/tests/unittests/test_schemas.py
new file mode 100644
index 0000000..f20f620
--- /dev/null
+++ b/tests/unittests/test_schemas.py
@@ -0,0 +1,16 @@
+# This file is part of curtin. See LICENSE file for copyright and license info.
+
+import glob
+import unittest
+
+from parameterized import parameterized
+
+from curtin.commands.schema_validate import schema_validate_storage
+
+
+class TestSchemaValidation(unittest.TestCase):
+    @parameterized.expand([
+        f for f in glob.glob("tests/unittests/schema/storage/valid/*")
+    ])
+    def test_storage_valid(self, filename):
+        self.assertEqual(0, schema_validate_storage(filename))