Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disk: Fix Partition MaxSize discrepency - Fixes #181 #213

Merged
merged 4 commits into from
Sep 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
- 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`
Expand Down
16 changes: 15 additions & 1 deletion DSCResources/MSFTDSC_Disk/MSFTDSC_Disk.psm1
Original file line number Diff line number Diff line change
Expand Up @@ -809,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)
Expand Down
13 changes: 13 additions & 0 deletions DSCResources/MSFTDSC_Disk/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -41,6 +43,17 @@ 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. 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).
2 changes: 2 additions & 0 deletions DSCResources/MSFT_DiskAccessPath/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
2 changes: 2 additions & 0 deletions DSCResources/MSFT_WaitForDisk/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
110 changes: 110 additions & 0 deletions Tests/Integration/MSFTDSC_Disk.Integration.Tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
59 changes: 57 additions & 2 deletions Tests/Unit/MSFTDSC_Disk.Tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -2764,6 +2766,59 @@ try
}
}

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 `
-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 + 0.98MB
}
} `
-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 `
Expand Down