Merge pull request #183 from JoshuaJSwain/jswain_xdisk_rawfix

Disk: correct volume error - drive is read only
dsccommunity · Feb 14, 2019 · 6251057 · 6251057
2 parents 01c2380 + 48e764c
commit 6251057
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 43 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,14 @@
   CODE\_OF\_CONDUCT.md file.
 - Explicitly removed extra hidden files from release package
 
+- Disk:
+  - Added minimum timetowate of 3s after new-partition using the while loop.
+    The problem occurs when the partition is created and the format-volume
+    is attempted before the volume has completed.
+    There appears to be no property to determine if the partition is
+    sufficiently ready to format and it will often format as a raw volume when
+    the error occurs - fixes [Issue #85](https://github.com/PowerShell/StorageDsc/issues/85).
+
 ## 4.3.0.0
 
 - WaitForDisk:

diff --git a/DSCResources/MSFTDSC_Disk/MSFTDSC_Disk.psm1 b/DSCResources/MSFTDSC_Disk/MSFTDSC_Disk.psm1
@@ -141,7 +141,7 @@ function Get-TargetResource
         DiskId             = $DiskId
         DiskIdType         = $DiskIdType
         DriveLetter        = $partition.DriveLetter
-        PartitionStyle    = $disk.PartitionStyle
+        PartitionStyle     = $disk.PartitionStyle
         Size               = $partition.Size
         FSLabel            = $volume.FileSystemLabel
         AllocationUnitSize = $blockSize
@@ -293,9 +293,9 @@ function Set-TargetResource
     if ($disk.PartitionStyle -eq 'RAW')
     {
         Write-Verbose -Message ( @(
-            "$($MyInvocation.MyCommand): "
-            $($localizedData.InitializingDiskMessage -f $DiskIdType, $DiskId, $PartitionStyle)
-        ) -join '' )
+                "$($MyInvocation.MyCommand): "
+                $($localizedData.InitializingDiskMessage -f $DiskIdType, $DiskId, $PartitionStyle)
+            ) -join '' )
 
         $disk | Initialize-Disk -PartitionStyle $PartitionStyle
     }
@@ -307,7 +307,7 @@ function Set-TargetResource
             Write-Verbose -Message ( @(
                     "$($MyInvocation.MyCommand): "
                     $($localizedData.DiskAlreadyInitializedMessage `
-                        -f $DiskIdType, $DiskId, $disk.PartitionStyle)
+                            -f $DiskIdType, $DiskId, $disk.PartitionStyle)
                 ) -join '' )
 
         }
@@ -334,7 +334,7 @@ function Set-TargetResource
         Write-Verbose -Message ( @(
                 "$($MyInvocation.MyCommand): "
                 $($localizedData.DriveNotFoundOnPartitionMessage `
-                    -f $DiskIdType, $DiskId, $DriveLetter)
+                        -f $DiskIdType, $DiskId, $DriveLetter)
             ) -join '' )
 
         # Are there any partitions defined on this disk?
@@ -443,19 +443,21 @@ function Set-TargetResource
                 After creating the partition it can take a few seconds for it to become writeable
                 Wait for up to 30 seconds for the parition to become writeable
             #>
-            $timeout = (Get-Date) + (New-Timespan -Second 30)
-            while ($partition.IsReadOnly -and (Get-Date) -lt $timeout)
+            $timeAtStart = Get-Date
+            $minimumTimeToWait = $timeAtStart + (New-Timespan -Second 3)
+            $maximumTimeToWait = $timeAtStart + (New-Timespan -Second 30)
+            while (($partitionstate.IsReadOnly -and (Get-Date) -lt $maximumTimeToWait) -or ((Get-Date) -lt $minimumTimeToWait))
             {
                 Write-Verbose -Message ( @(
                         "$($MyInvocation.MyCommand): "
                         ($localizedData.NewPartitionIsReadOnlyMessage `
-                            -f $DiskIdType, $DiskId, $partition.PartitionNumber)
+                                -f $DiskIdType, $DiskId, $partition.PartitionNumber)
                     ) -join '' )
 
                 Start-Sleep -Seconds 1
 
                 # Pull the partition details again to check if it is readonly
-                $partition = $partition | Get-Partition
+                $partitionstate = $partition | Get-Partition
             } # while
         } # if
 
@@ -475,7 +477,7 @@ function Set-TargetResource
         Write-Verbose -Message ( @(
                 "$($MyInvocation.MyCommand): "
                 $($localizedData.PartitionAlreadyAssignedMessage `
-                    -f $DriveLetter, $assignedPartition.PartitionNumber)
+                        -f $DriveLetter, $assignedPartition.PartitionNumber)
             ) -join '' )
 
         $assignDriveLetter = $false
@@ -501,7 +503,7 @@ function Set-TargetResource
                     Write-Warning -Message ( @(
                             "$($MyInvocation.MyCommand): "
                             $($localizedData.ResizeRefsNotPossibleMessage `
-                                -f $DriveLetter, $assignedPartition.Size, $Size)
+                                    -f $DriveLetter, $assignedPartition.Size, $Size)
                         ) -join '' )
 
                 }
@@ -510,7 +512,7 @@ function Set-TargetResource
                     Write-Verbose -Message ( @(
                             "$($MyInvocation.MyCommand): "
                             $($localizedData.SizeMismatchCorrectionMessage `
-                                -f $DriveLetter, $assignedPartition.Size, $Size)
+                                    -f $DriveLetter, $assignedPartition.Size, $Size)
                         ) -join '' )
 
                     if ($Size -gt $supportedSize.SizeMax)
@@ -529,10 +531,10 @@ function Set-TargetResource
             {
                 # A partition resize was required but is not allowed
                 Write-Warning -Message ( @(
-                    "$($MyInvocation.MyCommand): "
-                    $($localizedData.ResizeNotAllowedMessage `
-                            -f $DriveLetter, $assignedPartition.Size, $Size)
-                ) -join '' )
+                        "$($MyInvocation.MyCommand): "
+                        $($localizedData.ResizeNotAllowedMessage `
+                                -f $DriveLetter, $assignedPartition.Size, $Size)
+                    ) -join '' )
             }
         }
     }
@@ -582,15 +584,15 @@ function Set-TargetResource
                 Write-Verbose -Message ( @(
                         "$($MyInvocation.MyCommand): "
                         $($localizedData.FileSystemFormatMismatch `
-                            -f $DriveLetter, $fileSystem, $FSFormat)
+                                -f $DriveLetter, $fileSystem, $FSFormat)
                     ) -join '' )
 
                 if ($AllowDestructive)
                 {
                     Write-Verbose -Message ( @(
                             "$($MyInvocation.MyCommand): "
                             $($localizedData.VolumeFormatInProgressMessage `
-                                -f $DriveLetter, $fileSystem, $FSFormat)
+                                    -f $DriveLetter, $fileSystem, $FSFormat)
                         ) -join '' )
 
                     $formatParam = @{
@@ -618,7 +620,7 @@ function Set-TargetResource
                 Write-Verbose -Message ( @(
                         "$($MyInvocation.MyCommand): "
                         $($localizedData.ChangingVolumeLabelMessage `
-                            -f $DriveLetter, $FSLabel)
+                                -f $DriveLetter, $FSLabel)
                     ) -join '' )
 
                 $volume | Set-Volume -NewFileSystemLabel $FSLabel
@@ -766,7 +768,7 @@ function Test-TargetResource
         Write-Verbose -Message ( @(
                 "$($MyInvocation.MyCommand): "
                 $($localizedData.DiskReadOnlyMessage `
-                    -f $DiskIdType, $DiskId)
+                        -f $DiskIdType, $DiskId)
             ) -join '' )
 
         return $false
@@ -777,7 +779,7 @@ function Test-TargetResource
         Write-Verbose -Message ( @(
                 "$($MyInvocation.MyCommand): "
                 $($localizedData.DiskPartitionStyleNotMatchMessage `
-                    -f $DiskIdType, $DiskId, $disk.PartitionStyle, $PartitionStyle)
+                        -f $DiskIdType, $DiskId, $disk.PartitionStyle, $PartitionStyle)
             ) -join '' )
 
         if ($disk.PartitionStyle -eq 'RAW' -or ($AllowDestructive -and $ClearDisk))
@@ -823,20 +825,20 @@ function Test-TargetResource
             if ($AllowDestructive)
             {
                 Write-Verbose -Message ( @(
-                    "$($MyInvocation.MyCommand): "
-                    $($localizedData.SizeMismatchWithAllowDestructiveMessage `
-                            -f $DriveLetter, $Partition.Size, $Size)
-                ) -join '' )
+                        "$($MyInvocation.MyCommand): "
+                        $($localizedData.SizeMismatchWithAllowDestructiveMessage `
+                                -f $DriveLetter, $Partition.Size, $Size)
+                    ) -join '' )
 
                 return $false
             }
             else
             {
                 Write-Verbose -Message ( @(
-                    "$($MyInvocation.MyCommand): "
-                    $($localizedData.SizeMismatchMessage `
-                            -f $DriveLetter, $Partition.Size, $Size)
-                ) -join '' )
+                        "$($MyInvocation.MyCommand): "
+                        $($localizedData.SizeMismatchMessage `
+                                -f $DriveLetter, $Partition.Size, $Size)
+                    ) -join '' )
             }
         } # if
     } # if
@@ -877,7 +879,7 @@ function Test-TargetResource
             Write-Verbose -Message ( @(
                     "$($MyInvocation.MyCommand): "
                     $($localizedData.FileSystemFormatMismatch `
-                        -f $DriveLetter, $fileSystem, $FSFormat)
+                            -f $DriveLetter, $fileSystem, $FSFormat)
                 ) -join '' )
 
             if ($AllowDestructive)
@@ -897,7 +899,7 @@ function Test-TargetResource
             Write-Verbose -Message ( @(
                     "$($MyInvocation.MyCommand): "
                     $($localizedData.DriveLabelMismatch `
-                        -f $DriveLetter, $label, $FSLabel)
+                            -f $DriveLetter, $label, $FSLabel)
                 ) -join '' )
 
             return $false

diff --git a/Tests/Unit/MSFTDSC_Disk.Tests.ps1 b/Tests/Unit/MSFTDSC_Disk.Tests.ps1
@@ -821,7 +821,7 @@ try
                         -ParameterFilter { $DiskId -eq $script:mockedDisk0GptOffline.Number -and $DiskIdType -eq 'Number' }
                     Assert-MockCalled -CommandName Set-Disk -Exactly -Times 1
                     Assert-MockCalled -CommandName Initialize-Disk -Exactly -Times 0
-                    Assert-MockCalled -CommandName Get-Partition -Exactly -Times 1
+                    Assert-MockCalled -CommandName Get-Partition -Exactly -Times 4
                     Assert-MockCalled -CommandName Get-Volume -Exactly -Times 1
                     Assert-MockCalled -CommandName New-Partition -Exactly -Times 1 `
                         -ParameterFilter { $DriveLetter -eq $script:testDriveLetter }
@@ -886,7 +886,7 @@ try
                         -ParameterFilter { $DiskId -eq $script:mockedDisk0GptOffline.UniqueId -and $DiskIdType -eq 'UniqueId' }
                     Assert-MockCalled -CommandName Set-Disk -Exactly -Times 1
                     Assert-MockCalled -CommandName Initialize-Disk -Exactly -Times 0
-                    Assert-MockCalled -CommandName Get-Partition -Exactly -Times 1
+                    Assert-MockCalled -CommandName Get-Partition -Exactly -Times 4
                     Assert-MockCalled -CommandName Get-Volume -Exactly -Times 1
                     Assert-MockCalled -CommandName New-Partition -Exactly -Times 1 `
                         -ParameterFilter {
@@ -953,7 +953,7 @@ try
                         -ParameterFilter { $DiskId -eq $script:mockedDisk0GptOffline.Guid -and $DiskIdType -eq 'Guid' }
                     Assert-MockCalled -CommandName Set-Disk -Exactly -Times 1
                     Assert-MockCalled -CommandName Initialize-Disk -Exactly -Times 0
-                    Assert-MockCalled -CommandName Get-Partition -Exactly -Times 1
+                    Assert-MockCalled -CommandName Get-Partition -Exactly -Times 4
                     Assert-MockCalled -CommandName Get-Volume -Exactly -Times 1
                     Assert-MockCalled -CommandName New-Partition -Exactly -Times 1 `
                         -ParameterFilter {
@@ -1019,7 +1019,7 @@ try
                         -ParameterFilter $script:parameterFilter_MockedDisk0Number
                     Assert-MockCalled -CommandName Set-Disk -Exactly -Times 1
                     Assert-MockCalled -CommandName Initialize-Disk -Exactly -Times 0
-                    Assert-MockCalled -CommandName Get-Partition -Exactly -Times 1
+                    Assert-MockCalled -CommandName Get-Partition -Exactly -Times 4
                     Assert-MockCalled -CommandName Get-Volume -Exactly -Times 1
                     Assert-MockCalled -CommandName New-Partition -Exactly -Times 1 `
                         -ParameterFilter {
@@ -1086,7 +1086,7 @@ try
                         -ParameterFilter $script:parameterFilter_MockedDisk0Number
                     Assert-MockCalled -CommandName Set-Disk -Exactly -Times 1
                     Assert-MockCalled -CommandName Initialize-Disk -Exactly -Times 1
-                    Assert-MockCalled -CommandName Get-Partition -Exactly -Times 1
+                    Assert-MockCalled -CommandName Get-Partition -Exactly -Times 4
                     Assert-MockCalled -CommandName Get-Volume -Exactly -Times 1
                     Assert-MockCalled -CommandName New-Partition -Exactly -Times 1 `
                         -ParameterFilter {
@@ -1155,7 +1155,7 @@ try
                         -ParameterFilter $script:parameterFilter_MockedDisk0Number
                     Assert-MockCalled -CommandName Set-Disk -Exactly -Times 0
                     Assert-MockCalled -CommandName Initialize-Disk -Exactly -Times 1
-                    Assert-MockCalled -CommandName Get-Partition -Exactly -Times 1
+                    Assert-MockCalled -CommandName Get-Partition -Exactly -Times 4
                     Assert-MockCalled -CommandName Get-Volume -Exactly -Times 1
                     Assert-MockCalled -CommandName New-Partition -Exactly -Times 1 `
                         -ParameterFilter {
@@ -1218,7 +1218,7 @@ try
                         -ParameterFilter $script:parameterFilter_MockedDisk0Number
                     Assert-MockCalled -CommandName Set-Disk -Exactly -Times 0
                     Assert-MockCalled -CommandName Initialize-Disk -Exactly -Times 0
-                    Assert-MockCalled -CommandName Get-Partition -Exactly -Times 1
+                    Assert-MockCalled -CommandName Get-Partition -Exactly -Times 4
                     Assert-MockCalled -CommandName Get-Volume -Exactly -Times 1
                     Assert-MockCalled -CommandName New-Partition -Exactly -Times 1 `
                         -ParameterFilter {
@@ -1275,8 +1275,8 @@ try
 
                 $endTime = Get-Date
 
-                It 'Should take at least 30s' {
-                    ($endTime - $startTime).TotalSeconds | Should -BeGreaterThan 29
+                It 'Should take at least 3s' {
+                    ($endTime - $startTime).TotalSeconds | Should -BeGreaterThan 2
                 }
 
                 It 'Should call the correct mocks' {
@@ -1535,7 +1535,7 @@ try
                         -ParameterFilter $script:parameterFilter_MockedDisk0Number
                     Assert-MockCalled -CommandName Set-Disk -Exactly -Times 0
                     Assert-MockCalled -CommandName Initialize-Disk -Exactly -Times 0
-                    Assert-MockCalled -CommandName Get-Partition -Exactly -Times 1
+                    Assert-MockCalled -CommandName Get-Partition -Exactly -Times 4
                     Assert-MockCalled -CommandName Get-Volume -Exactly -Times 1
                     Assert-MockCalled -CommandName New-Partition -Exactly -Times 1
                     Assert-MockCalled -CommandName Format-Volume -Exactly -Times 0
@@ -1699,7 +1699,7 @@ try
                     Assert-VerifiableMock
                     Assert-MockCalled -CommandName Get-DiskByIdentifier -Exactly -Times 1 `
                         -ParameterFilter $script:parameterFilter_MockedDisk0Number
-                    Assert-MockCalled -CommandName Get-Partition -Exactly -Times 1
+                    Assert-MockCalled -CommandName Get-Partition -Exactly -Times 4
                     Assert-MockCalled -CommandName New-Partition -Exactly -Times 1
                     Assert-MockCalled -CommandName Get-Volume -Exactly -Times 1
                     Assert-MockCalled -CommandName Set-Disk -Exactly -Times 0