Added more storage types (#184)

* Added dir and zfspool storage types

* Added unit tests for dir and zfspool

* Added changelog

* Fix sanity

* Fix more sanity

* Add validation tests for missing required parameters in storage types

* Fix sanity

---------

Co-authored-by: Kevin Quick <[email protected]>

Author: Thulium-Drake

changelogs/fragments/184-add-storage-types.yml

@@ -0,0 +1,3 @@
+---
+minor_changes:
+  - proxmox_storage - added `dir` and `zfspool` storage types (https://github.com/ansible-collections/community.proxmox/pull/184)

plugins/modules/proxmox_storage.py

@@ -43,7 +43,7 @@
       - The storage type/protocol to use when adding the storage.
     type: str
     required: true
-    choices: ['cephfs', 'cifs', 'iscsi', 'nfs', 'pbs']
+    choices: ['cephfs', 'cifs', 'dir', 'iscsi', 'nfs', 'pbs', 'zfspool']
   cephfs_options:
     description:
       - Extended information for adding CephFS storage.
@@ -121,6 +121,16 @@
           - The minimum SMB version to use for.
         type: str
         required: false
+  dir_options:
+    description:
+      - Extended information for adding Directory storage.
+    type: dict
+    suboptions:
+      path:
+        description:
+          - The path of the direcotry on the node(s).
+        type: str
+        required: false
   nfs_options:
     description:
       - Extended information for adding NFS storage.
@@ -186,6 +196,16 @@
           - The required fingerprint of the Proxmox Backup Server system.
         type: str
         required: false
+  zfspool_options:
+    description:
+      - Extended information for adding ZFS storage.
+    type: dict
+    suboptions:
+      pool:
+        description:
+          - The name of the ZFS pool to use.
+        type: str
+        required: false
   content:
     description:
       - The desired content that should be used with this storage type.
@@ -322,6 +342,14 @@ def add_storage(self):
                 payload['server'] = server
                 payload['share'] = share
 
+        if storage_type == "dir":
+            dir_options = self.module.params.get(f'{storage_type}_options', {})
+            path = dir_options.get('path')
+            if not all([path]):
+                self.module.fail_json(msg="Directory storage requires 'path' parameter.")
+            else:
+                payload['path'] = path
+
         if storage_type == "iscsi":
             iscsi_options = self.module.params.get(f'{storage_type}_options', {})
             portal = iscsi_options.get('portal')
@@ -359,6 +387,14 @@ def add_storage(self):
                 if fingerprint:
                     payload['fingerprint'] = fingerprint
 
+        if storage_type == "zfspool":
+            zfspool_options = self.module.params.get(f'{storage_type}_options', {})
+            pool = zfspool_options.get('pool')
+            if not all([pool]):
+                self.module.fail_json(msg="ZFS storage requires 'pool' parameter.")
+            else:
+                payload['pool'] = pool
+
         # Check Mode validation
         if self.module.check_mode:
             try:
@@ -440,7 +476,10 @@ def main():
         nodes=dict(type='list', elements='str',),
         name=dict(type='str', required=True),
         state=dict(choices=['present', 'absent']),
-        type=dict(choices=['cephfs', 'cifs', 'iscsi', 'nfs', 'pbs'], required=True),
+        type=dict(choices=['cephfs', 'cifs', 'dir', 'iscsi', 'nfs', 'pbs', 'zfspool'], required=True),
+        dir_options=dict(type='dict', options={
+            'path': dict(type='str')
+        }),
         cephfs_options=dict(type='dict', options={
             'monhost': dict(type='list', elements='str'),
             'username': dict(type='str'),
@@ -474,6 +513,9 @@ def main():
             'datastore': dict(type='str'),
             'fingerprint': dict(type='str')
         }),
+        zfspool_options=dict(type='dict', options={
+            'pool': dict(type='str')
+        }),
         content=dict(type='list', elements='str', choices=["images", "snippets", "import", "iso", "backup", "rootdir", "vztmpl"]),
     )
 

tests/unit/plugins/modules/test_proxmox_storage.py

@@ -11,6 +11,25 @@
 from ansible_collections.community.proxmox.plugins.module_utils.proxmox import ProxmoxAnsible
 
 
+@pytest.fixture
+def dir_storage_args():
+    return {
+        "api_host": "localhost",
+        "api_user": "root@pam",
+        "api_password": "secret",
+        "validate_certs": False,
+        "node_name": "pve01",
+        "nodes": ["pve01", "pve02"],
+        "state": "present",
+        "name": "dir-storage",
+        "type": "dir",
+        "dir_options": {
+            "path": "/dir",
+        },
+        "content": ["images"]
+    }
+
+
 @pytest.fixture
 def pbs_storage_args():
     return {
@@ -54,6 +73,25 @@ def nfs_storage_args():
     }
 
 
+@pytest.fixture
+def zfspool_storage_args():
+    return {
+        "api_host": "localhost",
+        "api_user": "root@pam",
+        "api_password": "secret",
+        "validate_certs": False,
+        "node_name": "pve01",
+        "nodes": ["pve01", "pve02"],
+        "state": "present",
+        "name": "zfspool-storage",
+        "type": "zfspooldir",
+        "zfspool_options": {
+            "pool": "mypool",
+        },
+        "content": ["images"]
+    }
+
+
 @pytest.fixture
 def existing_storages():
     return [
@@ -62,6 +100,29 @@ def existing_storages():
     ]
 
 
+@patch.object(ProxmoxAnsible, "__init__", return_value=None)
+@patch.object(ProxmoxAnsible, "proxmox_api", create=True)
+def test_add_dir_storage(mock_api, mock_init, dir_storage_args):
+    module = MagicMock(spec=AnsibleModule)
+    module.params = dir_storage_args
+    module.check_mode = False
+
+    mock_api_instance = MagicMock()
+    mock_api.return_value = mock_api_instance
+    mock_api_instance.nodes.get.return_value = [{"node": "pve01", "status": "online"}]
+    mock_api_instance.storage.get.return_value = []
+    mock_api_instance.storage.post.return_value = {}
+
+    proxmox = proxmox_storage.ProxmoxNodeAnsible(module)
+    proxmox.module = module
+    proxmox.proxmox_api = mock_api_instance
+
+    changed, msg = proxmox.add_storage()
+
+    assert changed is True
+    assert "created successfully" in msg
+
+
 @patch.object(ProxmoxAnsible, "__init__", return_value=None)
 @patch.object(ProxmoxAnsible, "proxmox_api", create=True)
 def test_add_pbs_storage(mock_api, mock_init, pbs_storage_args):
@@ -231,3 +292,98 @@ def test_add_cephfs_storage(mock_api, mock_init):
 
     assert changed is True
     assert "created successfully" in msg
+
+
+@patch.object(ProxmoxAnsible, "__init__", return_value=None)
+@patch.object(ProxmoxAnsible, "proxmox_api", create=True)
+def test_add_zfspool_storage(mock_api, mock_init, zfspool_storage_args):
+    module = MagicMock(spec=AnsibleModule)
+    module.params = zfspool_storage_args
+    module.check_mode = False
+
+    mock_api_instance = MagicMock()
+    mock_api.return_value = mock_api_instance
+    mock_api_instance.nodes.get.return_value = [{"node": "pve01", "status": "online"}]
+    mock_api_instance.storage.get.return_value = []
+    mock_api_instance.storage.post.return_value = {}
+
+    proxmox = proxmox_storage.ProxmoxNodeAnsible(module)
+    proxmox.module = module
+    proxmox.proxmox_api = mock_api_instance
+
+    changed, msg = proxmox.add_storage()
+
+    assert changed is True
+    assert "created successfully" in msg
+
+
+@patch.object(ProxmoxAnsible, "__init__", return_value=None)
+@patch.object(ProxmoxAnsible, "proxmox_api", create=True)
+def test_add_dir_missing_required_path(mock_api, mock_init):
+    dir_args = {
+        "api_host": "localhost",
+        "api_user": "root@pam",
+        "api_password": "secret",
+        "validate_certs": False,
+        "node_name": "pve01",
+        "nodes": ["pve01"],
+        "state": "present",
+        "name": "dir-storage",
+        "type": "dir",
+        "dir_options": {},  # Missing 'path' parameter
+        "content": ["images"]
+    }
+
+    module = MagicMock(spec=AnsibleModule)
+    module.params = dir_args
+    module.check_mode = False
+    module.fail_json = lambda **kwargs: (result for result in ()).throw(SystemExit(kwargs))
+
+    mock_api_instance = MagicMock()
+    mock_api.return_value = mock_api_instance
+
+    proxmox = proxmox_storage.ProxmoxNodeAnsible(module)
+    proxmox.module = module
+    proxmox.proxmox_api = mock_api_instance
+
+    with pytest.raises(SystemExit) as exc:
+        proxmox.add_storage()
+
+    result = exc.value.args[0]
+    assert "Directory storage requires 'path' parameter" in result["msg"]
+
+
+@patch.object(ProxmoxAnsible, "__init__", return_value=None)
+@patch.object(ProxmoxAnsible, "proxmox_api", create=True)
+def test_add_zfspool_missing_required_pool(mock_api, mock_init):
+    zfspool_args = {
+        "api_host": "localhost",
+        "api_user": "root@pam",
+        "api_password": "secret",
+        "validate_certs": False,
+        "node_name": "pve01",
+        "nodes": ["pve01"],
+        "state": "present",
+        "name": "zfspool-storage",
+        "type": "zfspool",
+        "zfspool_options": {},  # Missing 'pool' parameter
+        "content": ["images"]
+    }
+
+    module = MagicMock(spec=AnsibleModule)
+    module.params = zfspool_args
+    module.check_mode = False
+    module.fail_json = lambda **kwargs: (result for result in ()).throw(SystemExit(kwargs))
+
+    mock_api_instance = MagicMock()
+    mock_api.return_value = mock_api_instance
+
+    proxmox = proxmox_storage.ProxmoxNodeAnsible(module)
+    proxmox.module = module
+    proxmox.proxmox_api = mock_api_instance
+
+    with pytest.raises(SystemExit) as exc:
+        proxmox.add_storage()
+
+    result = exc.value.args[0]
+    assert "ZFS storage requires 'pool' parameter" in result["msg"]