ExchangeDsc icon indicating copy to clipboard operation
ExchangeDsc copied to clipboard

xExchangeHelper: Fix/workaround for Test-Path error

Open malauter opened this issue 6 years ago • 5 comments

Pull Request (PR) description

Sometimes the Test-Path cmdlet is a little bit buggy and return false even if the path is available. For example if you mount the Exchange ISO image with Mount-DiskImage, the first Test-Path returns false although setup.exe is accessible. To prevent this, it is helpful to run a Get-PSDrive before running Test-Path the first time.

This Pull Request (PR) fixes the following issues

None

Task list

  • [x ] Added an entry under the Unreleased section of the change log in the CHANGELOG.md. Entry should say what was changed, and how that affects users (if applicable).
  • [ ] Resource documentation added/updated in README.md.
  • [ ] Resource parameter descriptions added/updated in README.md, schema.mof and comment-based help.
  • [ ] Comment-based help added/updated.
  • [ ] Localization strings added/updated in all localization files as appropriate.
  • [ ] Examples appropriately added/updated.
  • [ ] Unit tests added/updated. See DSC Resource Testing Guidelines.
  • [ ] Integration tests added/updated (where possible). See DSC Resource Testing Guidelines.
  • [ ] New/changed code adheres to DSC Resource Style Guidelines and Best Practices.

This change is Reviewable

malauter avatar Nov 01 '19 18:11 malauter

Hi @malauter , thanks for this submission as well! Do you have any references where this buggyness has been discussed, and Get-PSDrive has been agreed on as the workaround? It sounds plausible, but I want to make sure we're identifying the appropriate root cause issue and solving it as far upstream as possible (i.e. if this is a problem caused by the DSC resource for mounting ISO's, maybe we should address it there instead; or if this is a known issue solved by a PowerShell hotfix or something, perhaps we should check for that fix instead).

mhendric avatar Nov 01 '19 19:11 mhendric

Hi @mhendric, the root cause is in the Test-Path cmdlet. But since we are not able to fix this, we can only implement a workaround here to prevent issues during Exchange setup. Please have a look at the following links where this issue has been discussed (some behavior for UNC paths as well as for mounted disk images). In the environment of my customer I mounted the Exchange ISO and ran into the problem. After adding Get-PSDrive before running Test-Path, it worked.

  • https://stackoverflow.com/questions/22394904/powershell-test-path-returns-false-when-testing-a-network-share
  • https://github.com/PowerShell/PowerShell/issues/6864
  • https://stackoverflow.com/questions/42290665/check-if-iso-is-already-mounted-in-powershell-if-not-then-mount (here you will find the Get-PSDrive workaround)

malauter avatar Nov 06 '19 19:11 malauter

Hey @malauter , sorry for the late response on this one as well. I don't love the idea of fixing this in xExchange rather than in PowerShell itself, but if we don't reasonably think we can get a PowerShell fix out to all xExchange users, we may have no other choice. That said, do you believe this is 100% reproducible (i.e. could we create a unit test that mounted a sample .iso and then repro'd this bug)? If so, we should probably add a unit test for that. Also, there's another setup related call to Test-Path in Assert-ExchangeSetupArgumentsComplete, so this should probably be addressed there as well. Perhaps you can make a Test-SafePath helper function, and move the Get-PSDrive / Test-Path combo into there. And add comments linking to the articles you found and maybe that we should revisit this at a later point. Sound good?

mhendric avatar Jan 04 '20 19:01 mhendric

@malauter , can you update the base branch of this to point to Master per step 'Remove branch dev from upstream repository' in https://dsccommunity.org/blog/convert-a-module-for-continuous-delivery?

mhendric avatar Jan 27 '20 17:01 mhendric

@malauter , please disregard my request to point this to Master. I was able to do it myself. Thanks.

mhendric avatar Jan 28 '20 17:01 mhendric

I'm closing this issue as there has been no work to adress the comment for a long time. This can re-opened or another PR can be sent in to propose the appropriate changes.

johlju avatar Apr 01 '23 14:04 johlju