From 1cbb6cf2e16caec516e62a082736c449cfce97a5 Mon Sep 17 00:00:00 2001 From: Daniel Scott-Raynsford Date: Sun, 22 Sep 2019 07:48:28 +1200 Subject: [PATCH 1/4] Fix maximum size discrepancy --- CHANGELOG.md | 3 + DSCResources/MSFTDSC_Disk/MSFTDSC_Disk.psm1 | 15 ++- .../MSFTDSC_Disk.Integration.Tests.ps1 | 110 ++++++++++++++++++ Tests/Unit/MSFTDSC_Disk.Tests.ps1 | 59 +++++++++- 4 files changed, 184 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1137bc95..d3e9d8b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,9 @@ - WaitForDisk: - Added `Location` as a possible value for `DiskIdType`. This will select the disk based on the `Location` property returned by `Get-Disk` +- Maximum size calculation now uses workaround so that + Test-TargetResource works properly - workaround for + [Issue #181](https://github.com/PowerShell/StorageDsc/issues/181). ## 4.8.0.0 diff --git a/DSCResources/MSFTDSC_Disk/MSFTDSC_Disk.psm1 b/DSCResources/MSFTDSC_Disk/MSFTDSC_Disk.psm1 index 7137113a..b4beeef6 100644 --- a/DSCResources/MSFTDSC_Disk/MSFTDSC_Disk.psm1 +++ b/DSCResources/MSFTDSC_Disk/MSFTDSC_Disk.psm1 @@ -480,7 +480,20 @@ function Set-TargetResource #> if (-not ($PSBoundParameters.ContainsKey('Size'))) { - $Size = $supportedSize.SizeMax + <# + If the difference in size between the supported partition size + and the current partition size is less than 1MB then set the + required partition size to the current size. This will prevent + the partition from being resized. + #> + if (($supportedSize.SizeMax - $partition.Size) -lt 1MB) + { + $Size = $partition.Size + } + else + { + $Size = $supportedSize.SizeMax + } } if ($assignedPartition.Size -ne $Size) diff --git a/Tests/Integration/MSFTDSC_Disk.Integration.Tests.ps1 b/Tests/Integration/MSFTDSC_Disk.Integration.Tests.ps1 index 7ef0567c..c81ab52a 100644 --- a/Tests/Integration/MSFTDSC_Disk.Integration.Tests.ps1 +++ b/Tests/Integration/MSFTDSC_Disk.Integration.Tests.ps1 @@ -891,6 +891,116 @@ try } } #endregion + + #region Integration Tests for size and maximum size + Context 'When maximum size is used, Test-TargetResource needs to report true even though Size and SizeMax are different.' { + BeforeAll { + # Create a VHD and attach it to the computer + $VHDPath = Join-Path -Path $TestDrive ` + -ChildPath 'TestDisk.vhd' + $null = New-VDisk -Path $VHDPath -SizeInMB 1024 -Initialize + $null = Mount-DiskImage -ImagePath $VHDPath -StorageType VHD -NoDriveLetter + $diskImage = Get-DiskImage -ImagePath $VHDPath + $disk = Get-Disk -Number $diskImage.Number + $FSLabelA = 'TestDiskA' + $FSLabelB = 'TestDiskB' + + # Get a spare drive letter + $lastDrive = ((Get-Volume).DriveLetter | Sort-Object | Select-Object -Last 1) + $driveLetterA = [char](([int][char]$lastDrive) + 1) + $driveLetterB = [char](([int][char]$lastDrive) + 2) + } + + Context "When using fixed size Disk Number $($disk.Number)" { + It 'Should compile and apply the MOF without throwing' { + { + # This is to pass to the Config + $configData = @{ + AllNodes = @( + @{ + NodeName = 'localhost' + DriveLetter = $driveLetterA + DiskId = $disk.Number + DiskIdType = 'Number' + PartitionStyle = 'GPT' + FSLabel = $FSLabelA + Size = 100MB + } + ) + } + + & "$($script:DSCResourceName)_Config" ` + -OutputPath $TestDrive ` + -ConfigurationData $configData + + Start-DscConfiguration ` + -Path $TestDrive ` + -ComputerName localhost ` + -Wait ` + -Verbose ` + -Force ` + -ErrorAction Stop + } | Should -Not -Throw + } + + It 'Should be able to call Get-DscConfiguration without throwing' { + { $script:currentConfiguration = Get-DscConfiguration -Verbose -ErrorAction Stop } | Should -Not -Throw + } + + It 'Should have set the resource and size should match' { + $current = $script:currentConfiguration | Where-Object -FilterScript { + $_.ConfigurationName -eq "$($script:DSCResourceName)_Config" + } + $current.Size | Should -Be 100MB + } + } + + Context "When using maximum size for new volume on Disk Number $($disk.Number)" { + It 'Should compile and apply the MOF without throwing' { + { + # This is to pass to the Config + $configData = @{ + AllNodes = @( + @{ + NodeName = 'localhost' + DriveLetter = $driveLetterA + DiskId = $disk.Number + DiskIdType = 'Number' + PartitionStyle = 'GPT' + FSLabel = $FSLabelA + } + ) + } + + & "$($script:DSCResourceName)_Config" ` + -OutputPath $TestDrive ` + -ConfigurationData $configData + + Start-DscConfiguration ` + -Path $TestDrive ` + -ComputerName localhost ` + -Wait ` + -Verbose ` + -Force ` + -ErrorAction Stop + } | Should -Not -Throw + } + + It 'Should be able to call Get-DscConfiguration without throwing' { + { $script:currentConfiguration = Get-DscConfiguration -Verbose -ErrorAction Stop } | Should -Not -Throw + } + + It 'Test-DscConfiguration should return True, indicating that partition size instead of SizeMax was used' { + Test-DscConfiguration | Should -Be $true + } + } + + AfterAll { + Dismount-DiskImage -ImagePath $VHDPath -StorageType VHD + Remove-Item -Path $VHDPath -Force + } + } + #endregion } } finally diff --git a/Tests/Unit/MSFTDSC_Disk.Tests.ps1 b/Tests/Unit/MSFTDSC_Disk.Tests.ps1 index 417037a3..f052c390 100644 --- a/Tests/Unit/MSFTDSC_Disk.Tests.ps1 +++ b/Tests/Unit/MSFTDSC_Disk.Tests.ps1 @@ -2667,7 +2667,8 @@ try -MockWith { return @{ SizeMin = 0 - SizeMax = $script:mockedPartition.Size + 1024 + # Adding >1MB, otherwise workaround for wrong SizeMax is triggered + SizeMax = $script:mockedPartition.Size + 1.1MB } } ` -Verifiable @@ -2727,7 +2728,8 @@ try -MockWith { return @{ SizeMin = 0 - SizeMax = $script:mockedPartition.Size + 1024 + # Adding >1MB, otherwise workaround for wrong SizeMax is triggered + SizeMax = $script:mockedPartition.Size + 1.1MB } } ` -Verifiable @@ -2764,6 +2766,59 @@ try } } + Context 'When testing matching partition size with AllowDestructive and without Size specified using Disk Number' { + # verifiable (should be called) mocks + Mock ` + -CommandName Get-DiskByIdentifier ` + -ParameterFilter $script:parameterFilter_MockedDisk0Number ` + -MockWith { $script:mockedDisk0Gpt } ` + -Verifiable + + Mock ` + -CommandName Get-Partition ` + -MockWith { $script:mockedPartition } ` + -Verifiable + + Mock ` + -CommandName Get-PartitionSupportedSize ` + -MockWith { + return @{ + SizeMin = 0 + SizeMax = $script:mockedPartition.Size + } + } ` + -Verifiable + Mock -CommandName Get-Volume -Verifiable + Mock -CommandName Get-CimInstance -Verifiable + + $script:result = $null + + It 'Should not throw an exception' { + { + $script:result = Test-TargetResource ` + -DiskId $script:mockedDisk0Gpt.Number ` + -DriveLetter $script:testDriveLetter ` + -AllocationUnitSize 4096 ` + -AllowDestructive $true ` + -Verbose + } | Should -Not -Throw + } + + It 'Should be true' { + $script:result | Should -Be $true + } + + It 'Should call the correct mocks' { + 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-PartitionSupportedSize -Exactly -Times 1 + Assert-MockCalled -CommandName Get-Volume -Exactly -Times 1 + Assert-MockCalled -CommandName Get-CimInstance -Exactly -Times 1 + } + } + Context 'When testing mismatched AllocationUnitSize using Disk Number' { # verifiable (should be called) mocks Mock ` From 239bbdab3d5b0b34ccbaa43acd408bcd41067626 Mon Sep 17 00:00:00 2001 From: Daniel Scott-Raynsford Date: Sun, 22 Sep 2019 08:15:55 +1200 Subject: [PATCH 2/4] Correct location of size check --- CHANGELOG.md | 6 ++-- DSCResources/MSFTDSC_Disk/MSFTDSC_Disk.psm1 | 31 +++++++++++---------- Tests/Unit/MSFTDSC_Disk.Tests.ps1 | 4 +-- 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d3e9d8b0..a80a6d6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,15 +5,15 @@ - Disk: - Added `Location` as a possible value for `DiskIdType`. This will select the disk based on the `Location` property returned by `Get-Disk` + - Maximum size calculation now uses workaround so that + Test-TargetResource works properly - workaround for + [Issue #181](https://github.com/PowerShell/StorageDsc/issues/181). - DiskAccessPath: - Added `Location` as a possible value for `DiskIdType`. This will select the disk based on the `Location` property returned by `Get-Disk` - WaitForDisk: - Added `Location` as a possible value for `DiskIdType`. This will select the disk based on the `Location` property returned by `Get-Disk` -- Maximum size calculation now uses workaround so that - Test-TargetResource works properly - workaround for - [Issue #181](https://github.com/PowerShell/StorageDsc/issues/181). ## 4.8.0.0 diff --git a/DSCResources/MSFTDSC_Disk/MSFTDSC_Disk.psm1 b/DSCResources/MSFTDSC_Disk/MSFTDSC_Disk.psm1 index b4beeef6..0525edfe 100644 --- a/DSCResources/MSFTDSC_Disk/MSFTDSC_Disk.psm1 +++ b/DSCResources/MSFTDSC_Disk/MSFTDSC_Disk.psm1 @@ -480,20 +480,7 @@ function Set-TargetResource #> if (-not ($PSBoundParameters.ContainsKey('Size'))) { - <# - If the difference in size between the supported partition size - and the current partition size is less than 1MB then set the - required partition size to the current size. This will prevent - the partition from being resized. - #> - if (($supportedSize.SizeMax - $partition.Size) -lt 1MB) - { - $Size = $partition.Size - } - else - { - $Size = $supportedSize.SizeMax - } + $Size = $supportedSize.SizeMax } if ($assignedPartition.Size -ne $Size) @@ -822,7 +809,21 @@ function Test-TargetResource { $supportedSize = ($partition | Get-PartitionSupportedSize) - $Size = $supportedSize.SizeMax + <# + If the difference in size between the supported partition size + and the current partition size is less than 1MB then set the + desired partition size to the current size. This will prevent + any size difference less than 1MB from trying to contiuously + resize. See https://github.com/PowerShell/StorageDsc/issues/181 + #> + if (($supportedSize.SizeMax - $partition.Size) -lt 1MB) + { + $Size = $partition.Size + } + else + { + $Size = $supportedSize.SizeMax + } } if ($Size) diff --git a/Tests/Unit/MSFTDSC_Disk.Tests.ps1 b/Tests/Unit/MSFTDSC_Disk.Tests.ps1 index f052c390..93f1f7fb 100644 --- a/Tests/Unit/MSFTDSC_Disk.Tests.ps1 +++ b/Tests/Unit/MSFTDSC_Disk.Tests.ps1 @@ -2766,7 +2766,7 @@ try } } - Context 'When testing matching partition size with AllowDestructive and without Size specified using Disk Number' { + Context 'When testing matching partition size with a less than 1MB difference in desired size and with AllowDestructive and without Size specified using Disk Number' { # verifiable (should be called) mocks Mock ` -CommandName Get-DiskByIdentifier ` @@ -2784,7 +2784,7 @@ try -MockWith { return @{ SizeMin = 0 - SizeMax = $script:mockedPartition.Size + SizeMax = $script:mockedPartition.Size + 0.98MB } } ` -Verifiable From 6b2b35f9db2334450c7152a5f0f59e66a617efde Mon Sep 17 00:00:00 2001 From: Daniel Scott-Raynsford Date: Sun, 22 Sep 2019 08:27:29 +1200 Subject: [PATCH 3/4] Added more information to the README.MD --- DSCResources/MSFTDSC_Disk/README.md | 12 ++++++++++++ DSCResources/MSFT_DiskAccessPath/README.md | 2 ++ DSCResources/MSFT_WaitForDisk/README.md | 2 ++ 3 files changed, 16 insertions(+) diff --git a/DSCResources/MSFTDSC_Disk/README.md b/DSCResources/MSFTDSC_Disk/README.md index bdb09fa6..2166ea36 100644 --- a/DSCResources/MSFTDSC_Disk/README.md +++ b/DSCResources/MSFTDSC_Disk/README.md @@ -25,6 +25,8 @@ table for the disk has been created. ## Known Issues +### Defragsvc Conflict + The 'defragsvc' service ('Optimize Drives') may cause the following errors when enabled with this resource. The following error may occur when testing the state of the resource: @@ -41,6 +43,16 @@ this error. Use the `Service` resource in either the 'xPSDesiredStateConfgiurati or 'PSDSCResources' resource module to set the 'defragsvc' service is always stopped and set to manual start up. +### Null Location + The _Location_ for a disk may be `null` for some types of disk, e.g. file-based virtual disks. Physical disks or Virtual disks provided via a hypervisor or other hardware virtualization platform should not be affected. + +### Maximum Supported Partition Size + +On some disks the _maximum supported partition size_ may differ from the actual +size of a partition created when specifying the maximum size. This difference +in reported size is always less than **1MB**, so if the reported _maximum supported +partition size_ is less than **1MB** then the partition will be considered to be +in the correct state. diff --git a/DSCResources/MSFT_DiskAccessPath/README.md b/DSCResources/MSFT_DiskAccessPath/README.md index 2c393601..72592f33 100644 --- a/DSCResources/MSFT_DiskAccessPath/README.md +++ b/DSCResources/MSFT_DiskAccessPath/README.md @@ -23,6 +23,8 @@ of disk selection the disk must have been initialized by some other method. ## Known Issues +### Null Location + The _Location_ for a disk may be `null` for some types of disk, e.g. file-based virtual disks. Physical disks or Virtual disks provided via a hypervisor or other hardware virtualization platform should not be affected. diff --git a/DSCResources/MSFT_WaitForDisk/README.md b/DSCResources/MSFT_WaitForDisk/README.md index 67636444..242e0d90 100644 --- a/DSCResources/MSFT_WaitForDisk/README.md +++ b/DSCResources/MSFT_WaitForDisk/README.md @@ -22,6 +22,8 @@ of disk selection the disk must have been initialized by some other method. ## Known Issues +### Null Location + The _Location_ for a disk may be `null` for some types of disk, e.g. file-based virtual disks. Physical disks or Virtual disks provided via a hypervisor or other hardware virtualization platform should not be affected. From 507b24adadfcb71ab3287e63431d70f86a0e7b4c Mon Sep 17 00:00:00 2001 From: Daniel Scott-Raynsford Date: Mon, 23 Sep 2019 21:48:27 +1200 Subject: [PATCH 4/4] Changes as per PR comments --- DSCResources/MSFTDSC_Disk/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/DSCResources/MSFTDSC_Disk/README.md b/DSCResources/MSFTDSC_Disk/README.md index 2166ea36..fcd27d87 100644 --- a/DSCResources/MSFTDSC_Disk/README.md +++ b/DSCResources/MSFTDSC_Disk/README.md @@ -55,4 +55,5 @@ On some disks the _maximum supported partition size_ may differ from the actual size of a partition created when specifying the maximum size. This difference in reported size is always less than **1MB**, so if the reported _maximum supported partition size_ is less than **1MB** then the partition will be considered to be -in the correct state. +in the correct state. This is a work around for [this issue](https://windowsserver.uservoice.com/forums/301869-powershell/suggestions/36967870-get-partitionsupportedsize-and-msft-partition-clas) +that has been reported on user voice and also discussed in [issue #181](https://github.com/PowerShell/StorageDsc/issues/181).