Get-DbaWsfc* - Fix the State property read from an undefined variable, and report the witness path (#10588)

Author: andreasjordan

private/functions/Get-ResourceGroupState.ps1

@@ -0,0 +1,9 @@
+function Get-ResourceGroupState ($state) {
+    switch ($state) {
+        -1 { "Unknown" }
+        0 { "Online" }
+        1 { "Offline" }
+        2 { "Failed" }
+        default { $state }
+    }
+}

private/testing/Get-TestConfig.ps1

@@ -17,6 +17,14 @@ function Get-TestConfig {
         # When testing a remote SQL Server instance this must be a network share
         # where both the SQL Server instance and the test script can write to.
         Temp             = 'C:\Temp'
+        # The Windows failover clusters that the Get-DbaWsfc* tests need. No CI environment has a
+        # cluster, so these stay empty there and the cluster tests skip themselves. A local
+        # configuration whose lab has clusters sets them to the cluster names.
+        # ClusterStorage needs shared storage: one cluster shared volume, and one disk that every
+        # node can see but that is deliberately not part of the cluster.
+        ClusterStorage   = $null
+        # ClusterWitness needs a file share witness.
+        ClusterWitness   = $null
     }
 
     if (Test-Path $LocalConfigPath) {

public/Get-DbaWsfcAvailableDisk.ps1

@@ -37,7 +37,6 @@ function Get-DbaWsfcAvailableDisk {
         All properties from the MSCluster_AvailableDisk WMI class are returned, including:
 
         Properties added via Add-Member:
-        - State: Current operational state of the disk
         - ClusterName: Name of the cluster
         - ClusterFqdn: Fully qualified domain name of the cluster
 
@@ -80,7 +79,6 @@ function Get-DbaWsfcAvailableDisk {
             $disk = Get-DbaCmObject -Computername $computer -Credential $Credential -Namespace root\MSCluster -ClassName MSCluster_AvailableDisk
 
             # I don't have an available disk, so I can't see how to clean this up: Passthru
-            $disk | Add-Member -Force -NotePropertyName State -NotePropertyValue (Get-ResourceState $resource.State)
             $disk | Add-Member -Force -NotePropertyName ClusterName -NotePropertyValue $cluster.Name
             $disk | Add-Member -Force -NotePropertyName ClusterFqdn -NotePropertyValue $cluster.Fqdn -PassThru
         }

public/Get-DbaWsfcCluster.ps1

@@ -39,14 +39,14 @@ function Get-DbaWsfcCluster {
         Default display properties (via Select-DefaultView):
         - Name: The name of the cluster
         - Fqdn: Fully qualified domain name of the cluster
-        - State: Current operational state of the cluster (added via NoteProperty)
         - DrainOnShutdown: Boolean indicating if nodes drain resources during service shutdown (uint32)
         - DynamicQuorumEnabled: Boolean indicating if dynamic quorum adjustment is enabled (uint32)
         - EnableSharedVolumes: Boolean indicating if Cluster Shared Volumes feature is enabled (uint32)
         - SharedVolumesRoot: The root directory path for Cluster Shared Volumes
         - QuorumPath: File system path where quorum files are maintained
         - QuorumType: Current quorum type as a string (Majority Node Majority, Node and Disk Majority, No Majority - Disk Only, Node Majority, or Witness)
         - QuorumTypeValue: Numeric identifier representing the quorum type (uint32)
+        - WitnessPath: Location of the witness, whichever kind it is - the UNC path of the share for a file share witness, the value of QuorumPath for a disk witness, and empty for a cluster with no witness (added via NoteProperty)
         - RequestReplyTimeout: Timeout period in milliseconds for request-reply operations (uint32)
 
         Additional properties from MSCluster_Cluster WMI class (accessible via Select-Object *):
@@ -108,8 +108,24 @@ function Get-DbaWsfcCluster {
     process {
         foreach ($computer in $computername) {
             $cluster = Get-DbaCmObject -Computername $computer -Credential $Credential -Namespace root\MSCluster -ClassName MSCluster_Cluster
-            $cluster | Add-Member -Force -NotePropertyName State -NotePropertyValue (Get-ResourceState $resource.State)
-            $cluster | Select-DefaultView -Property Name, Fqdn, State, DrainOnShutdown, DynamicQuorumEnabled, EnableSharedVolumes, SharedVolumesRoot, QuorumPath, QuorumType, QuorumTypeValue, RequestReplyTimeout
+
+            # One property for "where is the witness", whichever kind of witness the cluster uses.
+            # QuorumPath already holds it for a disk witness and is empty for every other quorum type.
+            $witnessPath = $cluster.QuorumPath
+
+            # A file share witness keeps its path in the private properties of its own resource and
+            # nowhere else - MSCluster_Cluster does not expose it at all. Ask for that resource only
+            # when the quorum type says there is one, so no other cluster pays for the extra query.
+            if ($cluster.QuorumTypeValue -eq 2) {
+                $witnessQuery = "SELECT * FROM MSCluster_Resource WHERE Type = `"File Share Witness`""
+                $witnessResource = Get-DbaCmObject -Computername $computer -Credential $Credential -Namespace root\MSCluster -Query $witnessQuery
+                if ($witnessResource.PrivateProperties.SharePath) {
+                    $witnessPath = $witnessResource.PrivateProperties.SharePath
+                }
+            }
+
+            $cluster | Add-Member -Force -NotePropertyName WitnessPath -NotePropertyValue $witnessPath
+            $cluster | Select-DefaultView -Property Name, Fqdn, DrainOnShutdown, DynamicQuorumEnabled, EnableSharedVolumes, SharedVolumesRoot, QuorumPath, QuorumType, QuorumTypeValue, WitnessPath, RequestReplyTimeout
         }
     }
 }

public/Get-DbaWsfcResourceGroup.ps1

@@ -81,17 +81,6 @@ function Get-DbaWsfcResourceGroup {
         [string[]]$Name,
         [switch]$EnableException
     )
-    begin {
-        function Get-ResourceGroupState ($state) {
-            switch ($state) {
-                -1 { "Unknown" }
-                0 { "Online" }
-                1 { "Offline" }
-                2 { "Failed" }
-                default { $state }
-            }
-        }
-    }
     process {
         foreach ($computer in $computername) {
             $cluster = Get-DbaWsfcCluster -ComputerName $computer -Credential $Credential

public/Get-DbaWsfcRole.ps1

@@ -40,7 +40,7 @@ function Get-DbaWsfcRole {
         - ClusterFqdn: Fully qualified domain name of the cluster
         - Name: Name of the resource group (key property), typically the cluster role name (e.g., SQL Server instance name)
         - OwnerNode: Name of the node currently hosting this resource group
-        - State: Current state of the resource group translated to readable format (Online, Offline, Failed, Partial Online, Pending, or Unknown)
+        - State: Current state of the resource group translated to readable format (Online, Offline, Failed, or Unknown)
 
         Additional properties available (from WMI MSCluster_ResourceGroup object):
         - Caption: Short textual description of the resource group
@@ -89,11 +89,13 @@ function Get-DbaWsfcRole {
     process {
         foreach ($computer in $computername) {
             $cluster = Get-DbaWsfcCluster -ComputerName $computer -Credential $Credential
-            $role = Get-DbaCmObject -Computername $computer -Credential $Credential -Namespace root\MSCluster -ClassName MSCluster_ResourceGroup
-            $role | Add-Member -Force -NotePropertyName State -NotePropertyValue (Get-ResourceState $resource.State)
-            $role | Add-Member -Force -NotePropertyName ClusterName -NotePropertyValue $cluster.Name
-            $role | Add-Member -Force -NotePropertyName ClusterFqdn -NotePropertyValue $cluster.Fqdn
-            $role | Select-DefaultView -Property ClusterName, ClusterFqdn, Name, OwnerNode, State
+            $roles = Get-DbaCmObject -Computername $computer -Credential $Credential -Namespace root\MSCluster -ClassName MSCluster_ResourceGroup
+            foreach ($role in $roles) {
+                $role | Add-Member -Force -NotePropertyName State -NotePropertyValue (Get-ResourceGroupState $role.State)
+                $role | Add-Member -Force -NotePropertyName ClusterName -NotePropertyValue $cluster.Name
+                $role | Add-Member -Force -NotePropertyName ClusterFqdn -NotePropertyValue $cluster.Fqdn
+                $role | Select-DefaultView -Property ClusterName, ClusterFqdn, Name, OwnerNode, State
+            }
         }
     }
 }

public/Get-DbaWsfcSharedVolume.ps1

@@ -33,14 +33,15 @@ function Get-DbaWsfcSharedVolume {
     .OUTPUTS
         System.Management.ManagementObject
 
-        Returns one ClusterSharedVolume WMI object per shared volume found on the cluster, with three added NoteProperties providing cluster context.
+        Returns one MSCluster_ClusterSharedVolume WMI object per shared volume found on the cluster, with two added NoteProperties providing cluster context.
 
-        Default display properties (ClusterSharedVolume WMI class properties plus):
+        Default display properties (MSCluster_ClusterSharedVolume WMI class properties plus):
         - ClusterName: Name of the Windows Server Failover Cluster
         - ClusterFqdn: Fully qualified domain name of the failover cluster
-        - State: Current state of the cluster shared volume (Online, Offline, Failed, etc.), converted from numeric value
 
-        All properties from the underlying ClusterSharedVolume WMI class are accessible using Select-Object *.
+        The WMI class carries BackupState and FaultState for the volume; there is no single State property.
+
+        All properties from the underlying MSCluster_ClusterSharedVolume WMI class are accessible using Select-Object *.
 
     .LINK
         https://dbatools.io/Get-DbaWsfcSharedVolume
@@ -60,11 +61,10 @@ function Get-DbaWsfcSharedVolume {
     process {
         foreach ($computer in $computername) {
             $cluster = Get-DbaWsfcCluster -ComputerName $computer -Credential $Credential
-            $volume = Get-DbaCmObject -Computername $computer -Credential $Credential -Namespace root\MSCluster -ClassName ClusterSharedVolume
+            $volume = Get-DbaCmObject -Computername $computer -Credential $Credential -Namespace root\MSCluster -ClassName MSCluster_ClusterSharedVolume
             # I don't have a shared volume, so I can't see how to clean this up: Passthru
             $volume | Add-Member -Force -NotePropertyName ClusterName -NotePropertyValue $cluster.Name
-            $volume | Add-Member -Force -NotePropertyName ClusterFqdn -NotePropertyValue $cluster.Fqdn
-            $volume | Add-Member -Force -NotePropertyName State -NotePropertyValue (Get-ResourceState $resource.State) -PassThru
+            $volume | Add-Member -Force -NotePropertyName ClusterFqdn -NotePropertyValue $cluster.Fqdn -PassThru
         }
     }
 }

tests/CLAUDE.md

@@ -468,6 +468,16 @@ A boundary is an external system such as SQL Server, S3, Azure Storage, SMTP, LD
 - A skipped placeholder is not coverage. If a required dependency was not provisioned, the behavioral test and runner setup must fail rather than skip. Skipping remains appropriate when the tested server version does not support the targeted feature.
 - Regression integration tests are expected when a bug crosses a boundary. Add the smallest focused test that fails for the old behavior and passes for the fix.
 
+### The one boundary CI does not have: Windows failover clusters
+
+`Get-DbaWsfc*` is the documented exception to "must fail rather than skip". A Windows failover cluster cannot be provisioned on any current runner - not on the container workflow, not on the self-hosted Azure runners - and there is deliberately no `CLUSTER` scenario in `pester.groups.ps1`, because scenarios exist to schedule CI lanes and no CI lane can host a cluster.
+
+So these tests read their cluster from `$TestConfig.ClusterStorage` and `$TestConfig.ClusterWitness`, which are `$null` in `Get-TestConfig` and set only by a local configuration whose lab has clusters. The integration `Context` carries `-Skip:(-not $TestConfig.ClusterStorage)`, so it skips in CI and runs for real against a lab.
+
+The lab those names describe needs more than a bare cluster: one cluster shared volume, one shared disk that every node can see but that is deliberately *not* part of the cluster, and a file share witness. Without them `Get-DbaWsfcSharedVolume` and `Get-DbaWsfcAvailableDisk` return nothing at all and cannot fail, which is exactly how a wrong WMI class name survived in `Get-DbaWsfcSharedVolume` from 2018 until August 2026.
+
+Do not copy this exception to any other command family. It applies where the boundary is a Windows feature that no runner can provide, not where provisioning is merely inconvenient.
+
 ## TEST MANAGEMENT GUIDELINES
 
 The dbatools test suite must remain manageable in size while ensuring adequate coverage for important functionality.

tests/Get-DbaWsfcAvailableDisk.Tests.ps1

@@ -18,4 +18,37 @@ Describe $CommandName -Tag UnitTests {
             Compare-Object -ReferenceObject $expectedParameters -DifferenceObject $hasParameters | Should -BeNullOrEmpty
         }
     }
+}
+
+Describe $CommandName -Tag IntegrationTests {
+    # These tests need a Windows failover cluster that can see a shared disk which is not part of the
+    # cluster. No CI environment has one, so $TestConfig.ClusterStorage is empty there and these tests
+    # skip. A lab that has a cluster sets it, and then a regression has to fail these tests.
+    Context "Retrieving the available disks" -Skip:(-not $TestConfig.ClusterStorage) {
+        BeforeAll {
+            $PSDefaultParameterValues["*-Dba*:EnableException"] = $true
+
+            $availableDisks = @(Get-DbaWsfcAvailableDisk -ComputerName $TestConfig.ClusterStorage)
+            $wsfcCluster = Get-DbaWsfcCluster -ComputerName $TestConfig.ClusterStorage
+
+            $PSDefaultParameterValues.Remove("*-Dba*:EnableException")
+        }
+
+        It "Returns at least one available disk" {
+            $availableDisks | Should -Not -BeNullOrEmpty
+        }
+
+        It "Adds the cluster name and fqdn to every disk" {
+            # The -PassThru that emits these objects used to sit on a NoteProperty that has been
+            # removed, so this also guards against the command returning nothing at all.
+            ($availableDisks | Where-Object ClusterName -ne $wsfcCluster.Name).Name | Should -BeNullOrEmpty
+            ($availableDisks | Where-Object ClusterFqdn -ne $wsfcCluster.Fqdn).Name | Should -BeNullOrEmpty
+        }
+
+        It "Does not add an empty State property" {
+            # MSCluster_AvailableDisk has no State property, so the command built its State
+            # NoteProperty from an undefined variable and it was always empty.
+            $availableDisks[0].PSObject.Properties.Name | Should -Not -Contain "State"
+        }
+    }
 }

tests/Get-DbaWsfcCluster.Tests.ps1

@@ -18,4 +18,65 @@ Describe $CommandName -Tag UnitTests {
             Compare-Object -ReferenceObject $expectedParameters -DifferenceObject $hasParameters | Should -BeNullOrEmpty
         }
     }
+}
+
+Describe $CommandName -Tag IntegrationTests {
+    # These tests need a Windows failover cluster. No CI environment has one, so
+    # $TestConfig.ClusterStorage is empty there and these tests skip. A lab that has a cluster sets
+    # it, and then a regression has to fail these tests.
+    Context "Retrieving the cluster" -Skip:(-not $TestConfig.ClusterStorage) {
+        BeforeAll {
+            $PSDefaultParameterValues["*-Dba*:EnableException"] = $true
+
+            $wsfcCluster = Get-DbaWsfcCluster -ComputerName $TestConfig.ClusterStorage
+
+            $PSDefaultParameterValues.Remove("*-Dba*:EnableException")
+        }
+
+        It "Returns the name and fqdn of the cluster" {
+            $wsfcCluster.Name | Should -Be $TestConfig.ClusterStorage
+            $wsfcCluster.Fqdn | Should -BeLike "$($TestConfig.ClusterStorage).*"
+        }
+
+        It "Returns the quorum type as a readable string" {
+            $wsfcCluster.QuorumType | Should -Not -BeNullOrEmpty
+            $wsfcCluster.QuorumTypeValue | Should -BeOfType [uint32]
+        }
+
+        It "Does not add an empty State property" {
+            # MSCluster_Cluster has no State property, so the command built its State NoteProperty
+            # from an undefined variable and it was always empty, in the object and in the default view.
+            $wsfcCluster.PSObject.Properties.Name | Should -Not -Contain "State"
+        }
+
+        It "Reports the quorum path as the witness path of a disk witness" {
+            $wsfcCluster.WitnessPath | Should -Be $wsfcCluster.QuorumPath
+        }
+    }
+
+    # This needs a cluster whose witness is a file share, which is a different cluster from the one
+    # with the shared storage. Same rule as above: empty in CI, set by a lab that has one.
+    Context "Retrieving the witness of a file share witness cluster" -Skip:(-not $TestConfig.ClusterWitness) {
+        BeforeAll {
+            $PSDefaultParameterValues["*-Dba*:EnableException"] = $true
+
+            $witnessCluster = Get-DbaWsfcCluster -ComputerName $TestConfig.ClusterWitness
+            $witnessResource = Get-DbaWsfcResource -ComputerName $TestConfig.ClusterWitness | Where-Object Type -eq "File Share Witness"
+
+            $PSDefaultParameterValues.Remove("*-Dba*:EnableException")
+        }
+
+        It "Uses a file share witness" {
+            $witnessResource | Should -Not -BeNullOrEmpty -Because "the rest of this context tests what such a cluster reports"
+        }
+
+        It "Returns the share path of the witness" {
+            # This is what issue #10573 asked for. MSCluster_Cluster has no property that carries it:
+            # QuorumPath stays empty for a file share witness, and the path only exists in the private
+            # properties of the witness resource.
+            $witnessCluster.QuorumPath | Should -BeNullOrEmpty
+            $witnessCluster.WitnessPath | Should -Be $witnessResource.PrivateProperties.SharePath
+            $witnessCluster.WitnessPath | Should -BeLike "\\*"
+        }
+    }
 }