Merge pull request #20 from GoodOlClint/feat/storage-iscsi-nfs-params

Add iSCSI/NFS parameters to New-PveStorage and shared storage integration tests

src/PSProxmoxVE/Cmdlets/Storage/NewPveStorageCmdlet.cs

@@ -1,3 +1,4 @@
+using System;
 using System.Collections.Generic;
 using System.Management.Automation;
 using Newtonsoft.Json.Linq;
@@ -63,6 +64,14 @@ namespace PSProxmoxVE.Cmdlets.Storage
         [Parameter(Mandatory = false, HelpMessage = "Ceph monitor host list.")]
         public string? MonHost { get; set; }
 
+        /// <summary>iSCSI target IQN (for "iscsi" type).</summary>
+        [Parameter(Mandatory = false, HelpMessage = "iSCSI target IQN (e.g. iqn.2024-01.com.example:storage).")]
+        public string? Target { get; set; }
+
+        /// <summary>iSCSI portal address (for "iscsi" type). Defaults to server:3260 if not specified.</summary>
+        [Parameter(Mandatory = false, HelpMessage = "iSCSI portal address (host:port).")]
+        public string? Portal { get; set; }
+
         /// <summary>Whether this storage is shared across cluster nodes.</summary>
         [Parameter(Mandatory = false, HelpMessage = "Storage is shared across cluster nodes.")]
         public SwitchParameter Shared { get; set; }
@@ -75,6 +84,12 @@ namespace PSProxmoxVE.Cmdlets.Storage
         [Parameter(Mandatory = false, HelpMessage = "Limit access to these nodes (comma-separated).")]
         public string? Nodes { get; set; }
 
+        private static void AddIfNotEmpty(Dictionary<string, string> data, string key, string? value)
+        {
+            if (!string.IsNullOrEmpty(value))
+                data[key] = value!;
+        }
+
         protected override void ProcessRecord()
         {
             if (!ShouldProcess(Storage, "Create PVE Storage"))
@@ -98,16 +113,35 @@ namespace PSProxmoxVE.Cmdlets.Storage
                 ["type"]    = Type
             };
 
-            if (!string.IsNullOrEmpty(Content))  data["content"]  = Content!;
+            AddIfNotEmpty(data, "content", Content);
-            if (!string.IsNullOrEmpty(Path))      data["path"]     = Path!;
+            AddIfNotEmpty(data, "path", Path);
-            if (!string.IsNullOrEmpty(Server))    data["server"]   = Server!;
+            AddIfNotEmpty(data, "export", Export);
-            if (!string.IsNullOrEmpty(Export))    data["export"]   = Export!;
+            AddIfNotEmpty(data, "vgname", VgName);
-            if (!string.IsNullOrEmpty(VgName))    data["vgname"]   = VgName!;
+            AddIfNotEmpty(data, "thinpool", ThinPool);
-            if (!string.IsNullOrEmpty(ThinPool))  data["thinpool"] = ThinPool!;
+            AddIfNotEmpty(data, "pool", !string.IsNullOrEmpty(Pool) ? Pool : CephPool);
-            if (!string.IsNullOrEmpty(Pool))      data["pool"]     = Pool!;
+            AddIfNotEmpty(data, "monhost", MonHost);
-            if (!string.IsNullOrEmpty(CephPool))  data["pool"]     = CephPool!;
+            AddIfNotEmpty(data, "target", Target);
-            if (!string.IsNullOrEmpty(MonHost))   data["monhost"]  = MonHost!;
+
-            if (!string.IsNullOrEmpty(Nodes))     data["nodes"]    = Nodes!;
+            // For iSCSI types, 'server' is not a valid API field; derive portal from Server if Portal omitted.
+            bool isIscsiType = string.Equals(Type, "iscsi", StringComparison.OrdinalIgnoreCase)
+                            || string.Equals(Type, "iscsidirect", StringComparison.OrdinalIgnoreCase);
+            if (isIscsiType)
+            {
+                string? portalValue;
+                if (!string.IsNullOrEmpty(Portal))
+                    portalValue = Portal;
+                else if (!string.IsNullOrEmpty(Server))
+                    portalValue = $"{Server}:3260";
+                else
+                    portalValue = null;
+                AddIfNotEmpty(data, "portal", portalValue);
+            }
+            else
+            {
+                AddIfNotEmpty(data, "server", Server);
+                AddIfNotEmpty(data, "portal", Portal);
+            }
+            AddIfNotEmpty(data, "nodes", Nodes);
             if (Shared.IsPresent)  data["shared"]  = "1";
             if (Disable.IsPresent) data["disable"] = "1";
 

tests/PSProxmoxVE.Tests/Integration/SharedStorage.Tests.ps1

@@ -0,0 +1,187 @@
+#Requires -Module Pester
+<#
+.SYNOPSIS
+    Pester 5 integration tests for shared storage backends (NFS, iSCSI).
+
+    These tests require the multi-node integration test infrastructure with
+    Docker-based storage services (iSCSI target + NFS server). They are
+    SKIPPED when the storage env vars are not set.
+
+    Required environment variables (in addition to base integration vars):
+        PVETEST_STORAGE_VM_IP   - IP of the storage host running iSCSI/NFS
+        PVETEST_ISCSI_IQN       - iSCSI target IQN
+        PVETEST_NFS_EXPORT      - NFS export path (e.g. 10.0.0.1:/srv/nfs/shared)
+
+    WARNING: These tests CREATE and DESTROY storage definitions on the target
+    PVE node. Never run against a production cluster.
+#>
+
+BeforeAll {
+    . $PSScriptRoot/../_TestHelper.ps1
+
+    # --- Base integration vars ---
+    $baseVars = @('PVETEST_HOST', 'PVETEST_PORT', 'PVETEST_APITOKEN', 'PVETEST_NODE')
+    $script:SkipReason = $null
+
+    foreach ($var in $baseVars) {
+        if (-not [System.Environment]::GetEnvironmentVariable($var)) {
+            $script:SkipReason = "No live Proxmox VE target configured. Set: $($baseVars -join ', ')"
+            break
+        }
+    }
+
+    # --- Shared storage vars ---
+    $script:StorageVmIp = [System.Environment]::GetEnvironmentVariable('PVETEST_STORAGE_VM_IP')
+    $script:IscsiIqn    = [System.Environment]::GetEnvironmentVariable('PVETEST_ISCSI_IQN')
+    $script:NfsExport   = [System.Environment]::GetEnvironmentVariable('PVETEST_NFS_EXPORT')
+
+    $script:Host_    = [System.Environment]::GetEnvironmentVariable('PVETEST_HOST')
+    $portEnv = [System.Environment]::GetEnvironmentVariable('PVETEST_PORT')
+    $script:Port     = [int]$(if ($portEnv) { $portEnv } else { '8006' })
+    $script:ApiToken = [System.Environment]::GetEnvironmentVariable('PVETEST_APITOKEN')
+    $script:Node     = [System.Environment]::GetEnvironmentVariable('PVETEST_NODE')
+
+    # Track created storages for cleanup
+    $script:CreatedStorages = [System.Collections.Generic.List[string]]::new()
+
+    function script:Skip-IfNoTarget {
+        if ($script:SkipReason) {
+            Set-ItResult -Skipped -Because $script:SkipReason
+            return $true
+        }
+        return $false
+    }
+
+    function script:Skip-IfNoSharedStorage {
+        if (Skip-IfNoTarget) { return $true }
+        if (-not $script:StorageVmIp -or -not $script:IscsiIqn -or -not $script:NfsExport) {
+            Set-ItResult -Skipped -Because 'Shared storage env vars not set (PVETEST_STORAGE_VM_IP, PVETEST_ISCSI_IQN, PVETEST_NFS_EXPORT)'
+            return $true
+        }
+        return $false
+    }
+}
+
+AfterAll {
+    # Best-effort cleanup of any storages we created
+    foreach ($name in $script:CreatedStorages) {
+        try { Remove-PveStorage -Storage $name -Confirm:$false -ErrorAction Stop }
+        catch { Write-Warning "Cleanup: failed to remove storage '$name': $_" }
+    }
+}
+
+Describe 'Shared Storage — Integration' -Tag 'Integration' {
+
+    # -------------------------------------------------------------------
+    Context 'Connection' {
+        It 'Should connect to PVE node' {
+            if (Skip-IfNoTarget) { return }
+
+            $session = Connect-PveServer `
+                -Server $script:Host_ `
+                -Port $script:Port `
+                -ApiToken $script:ApiToken `
+                -SkipCertificateCheck `
+                -PassThru
+
+            $session | Should -Not -BeNullOrEmpty
+        }
+    }
+
+    # -------------------------------------------------------------------
+    Context 'NFS Storage' {
+        It 'Should create NFS storage' {
+            if (Skip-IfNoSharedStorage) { return }
+
+            # Parse server and export from the full NFS export path (e.g. "10.0.0.1:/srv/nfs/shared")
+            $nfsParts = $script:NfsExport -split ':', 2
+            if (-not $nfsParts -or
+                $nfsParts.Count -ne 2 -or
+                [string]::IsNullOrWhiteSpace($nfsParts[0]) -or
+                [string]::IsNullOrWhiteSpace($nfsParts[1])) {
+                Set-ItResult -Skipped -Because "PVETEST_NFS_EXPORT must be in 'server:/export/path' format (current value: '$($script:NfsExport)')"
+                return
+            }
+            $nfsServer = $nfsParts[0]
+            $nfsExportPath = $nfsParts[1]
+
+            # Note: NFS is implicitly shared in PVE — do not pass -Shared
+            $result = New-PveStorage -Storage 'pester-nfs' -Type 'nfs' `
+                -Server $nfsServer `
+                -Export $nfsExportPath `
+                -Content 'images,iso,backup' `
+                -ErrorAction Stop
+
+            $result | Should -Not -BeNullOrEmpty
+            $result.Storage | Should -Be 'pester-nfs'
+            $script:CreatedStorages.Add('pester-nfs')
+        }
+
+        It 'Should list and find the NFS storage' {
+            if (Skip-IfNoSharedStorage) { return }
+
+            $storages = Get-PveStorage -Node $script:Node
+            $nfs = $storages | Where-Object { $_.Storage -eq 'pester-nfs' }
+            $nfs | Should -Not -BeNullOrEmpty
+            $nfs.Type | Should -Be 'nfs'
+        }
+
+        It 'Should get NFS storage status' {
+            if (Skip-IfNoSharedStorage) { return }
+
+            $status = Get-PveStorageStatus -Node $script:Node -Storage 'pester-nfs' -ErrorAction Stop
+            $status | Should -Not -BeNullOrEmpty
+            $status.Active | Should -Be 1
+        }
+
+        It 'Should remove NFS storage' {
+            if (Skip-IfNoSharedStorage) { return }
+
+            { Remove-PveStorage -Storage 'pester-nfs' -Confirm:$false -ErrorAction Stop } |
+                Should -Not -Throw
+            $script:CreatedStorages.Remove('pester-nfs')
+        }
+    }
+
+    # -------------------------------------------------------------------
+    Context 'iSCSI Storage' {
+        It 'Should create iSCSI storage with -Target parameter' {
+            if (Skip-IfNoSharedStorage) { return }
+
+            # iSCSI uses -Portal (not -Server) for the target address.
+            # iSCSI is implicitly shared in PVE — do not pass -Shared.
+            $result = New-PveStorage -Storage 'pester-iscsi' -Type 'iscsi' `
+                -Portal $script:StorageVmIp `
+                -Target $script:IscsiIqn `
+                -ErrorAction Stop
+
+            $result | Should -Not -BeNullOrEmpty
+            $result.Storage | Should -Be 'pester-iscsi'
+            $script:CreatedStorages.Add('pester-iscsi')
+        }
+
+        It 'Should list and find the iSCSI storage' {
+            if (Skip-IfNoSharedStorage) { return }
+
+            $storages = Get-PveStorage -Node $script:Node
+            $iscsi = $storages | Where-Object { $_.Storage -eq 'pester-iscsi' }
+            $iscsi | Should -Not -BeNullOrEmpty
+            $iscsi.Type | Should -Be 'iscsi'
+        }
+
+        It 'Should get iSCSI storage status' {
+            if (Skip-IfNoSharedStorage) { return }
+
+            $status = Get-PveStorageStatus -Node $script:Node -Storage 'pester-iscsi' -ErrorAction Stop
+            $status | Should -Not -BeNullOrEmpty
+        }
+
+        It 'Should remove iSCSI storage' {
+            if (Skip-IfNoSharedStorage) { return }
+
+            { Remove-PveStorage -Storage 'pester-iscsi' -Confirm:$false -ErrorAction Stop } |
+                Should -Not -Throw
+            $script:CreatedStorages.Remove('pester-iscsi')
+        }
+    }
+}

tests/PSProxmoxVE.Tests/Storage/Get-PveStorage.Tests.ps1

@@ -171,6 +171,48 @@ Describe 'New-PveStorage' {
             $isMandatory | Should -Not -BeNullOrEmpty
         }
     }
+
+    Context 'iSCSI/NFS parameters' {
+        It 'Should have Target parameter for iSCSI IQN' {
+            Skip-IfMissing 'New-PveStorage'
+            $script:Cmd.Parameters.ContainsKey('Target') | Should -BeTrue
+        }
+
+        It 'Target should not be Mandatory' {
+            Skip-IfMissing 'New-PveStorage'
+            $isMandatory = $script:Cmd.Parameters['Target'].ParameterSets.Values |
+                Where-Object { $_.IsMandatory }
+            $isMandatory | Should -BeNullOrEmpty
+        }
+
+        It 'Should have Portal parameter for iSCSI portal' {
+            Skip-IfMissing 'New-PveStorage'
+            $script:Cmd.Parameters.ContainsKey('Portal') | Should -BeTrue
+        }
+
+        It 'Portal should not be Mandatory' {
+            Skip-IfMissing 'New-PveStorage'
+            $isMandatory = $script:Cmd.Parameters['Portal'].ParameterSets.Values |
+                Where-Object { $_.IsMandatory }
+            $isMandatory | Should -BeNullOrEmpty
+        }
+
+        It 'Should have Server parameter for NFS/iSCSI server' {
+            Skip-IfMissing 'New-PveStorage'
+            $script:Cmd.Parameters.ContainsKey('Server') | Should -BeTrue
+        }
+
+        It 'Should have Export parameter for NFS export path' {
+            Skip-IfMissing 'New-PveStorage'
+            $script:Cmd.Parameters.ContainsKey('Export') | Should -BeTrue
+        }
+
+        It 'Should have Shared switch parameter' {
+            Skip-IfMissing 'New-PveStorage'
+            $script:Cmd.Parameters.ContainsKey('Shared') | Should -BeTrue
+            $script:Cmd.Parameters['Shared'].ParameterType | Should -Be ([System.Management.Automation.SwitchParameter])
+        }
+    }
 }
 
 # ---------------------------------------------------------------------------

tests/infrastructure/docker-compose.storage.yml

@@ -53,6 +53,10 @@ services:
     privileged: true
     volumes:
       - nfs-data:/srv/nfs/shared
+      # NOTE: /lib/modules is required by the NFS kernel module loader.
+      # This mount is Linux-host-specific and will fail on macOS/Windows Docker Desktop.
+      # Integration tests using this compose file must run on a Linux host (e.g. GitHub-hosted runners or the dev container).
+      - /lib/modules:/lib/modules:ro
     environment:
       NFS_EXPORT_0: /srv/nfs/shared *(rw,sync,no_subtree_check,no_root_squash)
     restart: unless-stopped