New module zpool #10146

tomhesse · 2025-05-16T11:32:27Z

SUMMARY

Create, destroy, and modify ZFS zpools and their vdev layouts, pool properties, and filesystem properties.

Closes #10123.

ISSUE TYPE

New Module/Plugin Pull Request

COMPONENT NAME

zpool

ADDITIONAL INFORMATION

If any changes are necessary, please let me know.

felixfontein

Thanks for your contribution! I've added some first comments below. Please note that you also need to add tests.

plugins/modules/zpool.py

russoz

hi @tomhesse, thanks for your contribution.

There are some adjustments needed, but the general structure seems good.

You might want to consider using CmdRunner to run the commands. See https://docs.ansible.com/ansible/latest/collections/community/general/docsite/guide_cmdrunner.html for more details.

And if you use VarDict (see https://docs.ansible.com/ansible/latest/collections/community/general/docsite/guide_vardict.html for that one), the management of diff output is super straightforward.

Finally, please bear in mind that new modules are required to come with tests (either unit tests or integration tests - the latter is preferred, but the former is accepted).

plugins/modules/zpool.py

Fix role: none in vdevs

tests/integration/targets/zpool/defaults/main.yml

tests/integration/targets/zpool/aliases

felixfontein · 2025-06-01T18:59:48Z

tests/integration/targets/zpool/defaults/main.yml

+---
+zpool_single_disk_config:
+  - file: /tmp/disk0.img
+    loop: /dev/loop89


FYI, on Alpine you can only use /dev/loop0 up to /dev/loop7. Anything else does not work. Might be best to skip Alpine for now as well...

(You can run mknod -m660 /dev/loopX b 7 X to create /dev/loopX; if you do that in a loop for all devices not there yet, then it should work fine I guess...)

I updated the module to allow usage of files I realized this is probably better that setting up loop devices. This is probably the more convenient way.

russoz

hi @tomhesse, thanks for the updates so far, it is looking good.

I got a couple of few other comments here, but other than that is is looking good indeed. Cheers

russoz · 2025-06-04T07:00:52Z

plugins/modules/zpool.py

+                    role=dict(
+                        type='str',
+                        choices=['log', 'cache', 'spare', 'dedup', 'special'],
+                        required=False,


required=False is redundant in the arg_spec, please remove it here and in the other options as well

russoz · 2025-06-04T07:02:27Z

plugins/modules/zpool.py

+          - List of device paths to include in this vdev.
+        required: true
+        type: list
+        elements: str


These are paths, right? If they are always paths, then use:

Suggested change

elements: str

elements: path

(and adjust accordingly in the arg spec down below)

russoz · 2025-06-04T07:11:18Z

plugins/modules/zpool.py

+            rc, stdout, stderr = ctx.run(
+                subcommand='create',
+                disable_new_features=self.disable_new_features,
+                force=self.force,
+                dry_run=self.module.check_mode,
+                pool_properties=self.pool_properties,
+                filesystem_properties=self.filesystem_properties,
+                mountpoint=self.mountpoint,
+                altroot=self.altroot,
+                temp_name=self.temp_name,
+                name=self.name,
+                vdevs=self.vdevs,
+            )


CmdRunner receives the reference to the module => the arguments that are named and mapped straight into module parameters do not need to be passed in the run() call. I haven't verified thoroughly, but I think you could make this:

Suggested change

rc, stdout, stderr = ctx.run(

subcommand='create',

disable_new_features=self.disable_new_features,

force=self.force,

dry_run=self.module.check_mode,

pool_properties=self.pool_properties,

filesystem_properties=self.filesystem_properties,

mountpoint=self.mountpoint,

altroot=self.altroot,

temp_name=self.temp_name,

name=self.name,

vdevs=self.vdevs,

)

rc, stdout, stderr = ctx.run(subcommand='create')

Maybe some other additional parameters here needs declaring, but the majority there can be skipped. ;-)

russoz · 2025-06-04T07:20:44Z

tests/integration/targets/zpool/defaults/main.yml

+zpool_single_disk_config:
+  - /tmp/disk0.img
+
+zpool_mirror_disk_config:
+  - /tmp/disk1.img
+  - /tmp/disk2.img
+
+zpool_raidz_disk_config:
+  - /tmp/disk3.img
+  - /tmp/disk4.img
+
+zpool_vdevs_disk_config:
+  vdev1:
+    - /tmp/disk5.img
+  vdev2:
+    - /tmp/disk6.img
+  vdev3:
+    - /tmp/disk7.img
+    - /tmp/disk8.img
+  vdev4:
+    - /tmp/disk9.img
+    - /tmp/disk10.img


As a good practice I would recommend you use remote_tmp_dir (see other integration tests, for example run git grep remote_tmp_dir tests/integration/targets/snap/) instead of /tmp.

russoz · 2025-06-04T07:26:29Z

This PR fixes #10123

russoz

LGTM! Thank you!

felixfontein · 2025-06-07T15:52:14Z

@tomhesse thanks for your contribution!
@russoz thanks for reviewing!

mhalano · 2025-06-07T17:04:52Z

@tomhesse You have my sword, my friend. Thanks!

* Add zpool module * Add botmeta * Use str.format instead of f-strings * Remove nonlocal usage * Add check to only pass ashift to zpool add * Extend ansible_spec and remove unnecessary validation * Apply suggestions and fix style * Fix indentation of yaml lists * Add method to normalize vdevs Fix role: none in vdevs * Use CmdRunner instead of run_command * Fix styling and documentation * Use str.format instead of f-strings * Make sure vdevs are only required when state is present * Add support for loop devices and normalize vdev type * Add integration tests * Add missing test dependencies for alpine and redhat * Skip integration tests on rhel10 until there there packages available * Use package module for better auto detection of package manager on rhel * Add copyright header * Skip tests on rhel and remove redhat install requirements * Ensure loop devices under /dev exist * Enable usage of files as pool devices * Remove disk setup * Use files as disks * Apply suggestions * Fix argument_spec

SandiyosDev · 2025-09-28T02:58:10Z

Adding some feedback from #10771 and #10744, as get_current_layout in class Zpool of this module is not currently idempotent when some users are using symlinked persistent device names

Some users uses /dev/disk/by-id/... paths for vdevs which is best practice; on the first run, state: present does create the pool, but on second run, this module would compare /dev/disk/by-id/ paths with real paths /dev/sdX or /dev/nvmeXnXpX returned by zpool status, which would find a mismatch, then incorrectly tries to zpool add existing disks to the pool, which fails and shows-
eg /dev/disk/by-id/nvme-Samsung_SSD_980_PRO_1TB_SERIAL is in use and contains a unknown filesystem.

felixfontein · 2025-11-01T07:21:31Z

@SandiyosDev please check out #11020.

tomhesse added 2 commits May 16, 2025 11:56

Add zpool module

8acbf53

Add botmeta

2ffd213

ansibullbot added module module new_contributor Help guide this first time contributor plugins plugin (any type) labels May 16, 2025

This comment was marked as outdated.

Sign in to view

ansibullbot added ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels May 16, 2025

Use str.format instead of f-strings

ae53b61

ansibullbot removed the ci_verified Push fixes to PR branch to re-run CI label May 16, 2025

This comment was marked as outdated.

Sign in to view

ansibullbot added the ci_verified Push fixes to PR branch to re-run CI label May 16, 2025

tomhesse added 2 commits May 16, 2025 18:26

Remove nonlocal usage

a57f980

Add check to only pass ashift to zpool add

7e594e2

ansibullbot removed the ci_verified Push fixes to PR branch to re-run CI label May 16, 2025

felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-10 Automatically create a backport for the stable-10 branch labels May 16, 2025

ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label May 17, 2025

felixfontein removed the backport-10 Automatically create a backport for the stable-10 branch label May 24, 2025

felixfontein reviewed May 24, 2025

View reviewed changes

ansibullbot added the stale_ci CI is older than 7 days, rerun before merging label May 24, 2025

russoz reviewed May 25, 2025

View reviewed changes

plugins/modules/zpool.py Outdated Show resolved Hide resolved

plugins/modules/zpool.py Show resolved Hide resolved

plugins/modules/zpool.py Outdated Show resolved Hide resolved

plugins/modules/zpool.py Outdated Show resolved Hide resolved

tomhesse added 5 commits May 26, 2025 17:04

Extend ansible_spec and remove unnecessary validation

9ef2d7f

Apply suggestions and fix style

a88604d

Fix indentation of yaml lists

84e5cb1

Add method to normalize vdevs

0de390f

Fix role: none in vdevs

Use CmdRunner instead of run_command

a3ce8d0

ansibullbot removed the stale_ci CI is older than 7 days, rerun before merging label May 29, 2025

This comment was marked as outdated.

Sign in to view

ansibullbot added the ci_verified Push fixes to PR branch to re-run CI label May 29, 2025

ansibullbot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label May 30, 2025

tomhesse added 3 commits May 30, 2025 17:35

Add missing test dependencies for alpine and redhat

dec372e

Skip integration tests on rhel10 until there there packages available

4c5d11f

Use package module for better auto detection of package manager on rhel

4848d03

felixfontein reviewed May 30, 2025

View reviewed changes

tests/integration/targets/zpool/defaults/main.yml Show resolved Hide resolved

tests/integration/targets/zpool/aliases Outdated Show resolved Hide resolved

Add copyright header

6db397f

felixfontein reviewed Jun 1, 2025

View reviewed changes

tomhesse added 5 commits June 2, 2025 17:41

Skip tests on rhel and remove redhat install requirements

3488ba6

Ensure loop devices under /dev exist

17e2242

Enable usage of files as pool devices

cdd601d

Remove disk setup

9edd5eb

Use files as disks

638361f

ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Jun 2, 2025

russoz reviewed Jun 4, 2025

View reviewed changes

felixfontein mentioned this pull request Jun 4, 2025

Remove and prevent trailing whitespace #10199

Merged

tomhesse added 2 commits June 4, 2025 18:52

Apply suggestions

2a3e840

Fix argument_spec

15e3779

russoz approved these changes Jun 7, 2025

View reviewed changes

russoz mentioned this pull request Jun 7, 2025

lvm_pv: new module for LVM Physical Volumes #10070

Merged

felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Jun 7, 2025

felixfontein merged commit 9286227 into ansible-collections:main Jun 7, 2025
110 checks passed

tomhesse deleted the zpool branch June 8, 2025 06:23

felixfontein mentioned this pull request Nov 1, 2025

Fix(zpool): Ensure consistent device path normalization for idempotency #11020

Open

New module zpool #10146

New module zpool #10146

Uh oh!

Conversation

tomhesse commented May 16, 2025 • edited by felixfontein Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

felixfontein left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

russoz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

felixfontein Jun 1, 2025

Choose a reason for hiding this comment

Uh oh!

felixfontein Jun 1, 2025

Choose a reason for hiding this comment

Uh oh!

tomhesse Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

russoz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

russoz Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

russoz Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

russoz Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

russoz Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

russoz commented Jun 4, 2025

Uh oh!

russoz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

felixfontein commented Jun 7, 2025

Uh oh!

mhalano commented Jun 7, 2025

Uh oh!

SandiyosDev commented Sep 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

felixfontein commented Nov 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

tomhesse commented May 16, 2025 •

edited by felixfontein

Loading

russoz left a comment •

edited

Loading

SandiyosDev commented Sep 28, 2025 •

edited

Loading