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 : Test-TargetResource does not fail when Allocation Unit size changes #157

Open
bbollard opened this issue Jul 12, 2018 · 5 comments
Open
Labels
bug The issue is a bug. help wanted The issue is up for grabs for anyone in the community.

Comments

@bbollard
Copy link

Details of the scenario you tried and the problem that is occurring:
When using the Disk resource to set a non-default AllocationUnitSize such as:

    WaitForDisk Disk6
    {
         DiskId = 6
         RetryIntervalSec = 60
         RetryCount = 60
    }

    Disk ZVolume
    {
         DiskId = 6
         DriveLetter = 'Z'
         FSFormat = 'NTFS'
         AllocationUnitSize = 64KB
         DependsOn = '[WaitForDisk]Disk6'
         AllowDestructive = $false
         ClearDisk = $true
    }

The Allocation Unit Size is set properly. However, if the volume is re-formatted to a different allocation unit size, the Test-TargetResource operation notes the issue (somewhat incorrectly) but does not mark the resource as not in desired state:

"VERBOSE: [XXXXX]: [[Disk]ZVolume] Test-TargetResource: Volume assigned to drive Z has allocation unit size 0 KB does not match expected
allocation unit size 64 KB."

Version of the Operating System and PowerShell the DSC Target Node is running:
Windows Server 2016

PSVersion 5.1.14393.2312
PSEdition Desktop
PSCompatibleVersions {1.0, 2.0, 3.0, 4.0...}
BuildVersion 10.0.14393.2312
CLRVersion 4.0.30319.42000
WSManStackVersion 3.0
PSRemotingProtocolVersion 2.3
SerializationVersion 1.1.0.1

Version of the DSC module you're using, or 'dev' if you're using current dev branch:
4.0.0.0

@johlju
Copy link
Member

johlju commented Jul 12, 2018

This sounds like a bug. Labeling it as such and help wanted so that anyone in the community can run with it.

@johlju johlju added bug The issue is a bug. help wanted The issue is up for grabs for anyone in the community. labels Jul 12, 2018
@PlagueHO
Copy link
Member

Hi @bbollard - you are correct, however it is by design, although it could be in need of improved design. If you set the AllowDestructive to $true then Test-TargetResource will return that the disk is not in state. Set-TargetResource will be called and the disk will be reformatted with the correct blocksize.

It could be argued however that AllowDestructive should just block the reformat from firing in the Set-TargetResource and still allow the Test-TargetResource to return $false - and the more I think about this, the more I think it might be the correct behavior.

@johlju - any thoughts on the best behavior here? It does mean that the Set-TargetResource will end up firing even though we know it will never make any changes in this situation due to the AllowDestructive being $false.

@bbollard - in the mean time, does setting the AllowDestructive to true enable the behavior you are after? Or is it just the behavior of Test-TargetResource that you're concerned with?

@johlju
Copy link
Member

johlju commented Jul 19, 2018

Oh, I see now, thanks for commenting on this @PlagueHO!

@PlagueHO we could either document what functionality is "skipped" when AllowDestructive equals to $false (unless it is already done), or Test-TargetResource will always return $false and we write out a warning saying what steps was "skipped" due to the AllowDestructive equals to $false. But not sure warning is seen, unless running manually, so just might be pointless.
I like the idea that it does not run the Set-TargetResource when AllowDestructive equals to $false, since it would be no point when it can't do anything regardless (just takes more CPU cycles figuring that out in Set-TargetResource).
So I think it's better to make it clear in the documentation what will not happen when AllowDestructive equals to $false.

@bbollard
Copy link
Author

I believe this particular case is a classic situation where 'one size does not fit all', since we are dealing with File Systems and where one person may want the volume re-formatted while someone else may not. In my case, I would not want the volume re-formatted, however, given this is a compliance toolset, I would want to know that the volume is out of compliance so that the reason why could be investigated.

So @PlagueHO, I am just concerned with the Test-TargetResource behavior in this particular case. I would rather spin some extra CPU cycles during Set-TargetResource and not re-format if AllowDestructive doesn't allow if it means getting visibility into this resource being out of compliance.

Either way, the warning message did not display the current Allocation Unit Size correctly so perhaps if the behavior isn't changed, the message is fixed.

Thanks for discussing.

@johlju
Copy link
Member

johlju commented Jul 19, 2018

If we would leave it at the current behavior, could we add a new property that would make the Test-TargetResource to report $false if any destructive change needs to occur. Or vice-verse, making the default so it always return $false in the above scenario, but that could be overridden by a new property, to make the Test-TargetResource to report $true.

Just thinking if we can cover more scenarios with a new property. 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug. help wanted The issue is up for grabs for anyone in the community.
Projects
None yet
Development

No branches or pull requests

3 participants