DhcpServerOptionValue: Applies -Force switch to all Set-DhcpServerV4OptionValue calls in DhcpServerDsc.OptionValueHelper.psm1

realslacker commented 3 years ago

Pull Request (PR) description

The calls to Set-DhcpServerV4OptionValue fail if the internal validation for some options fail. We should apply all changes in a DSC configuration as they are written and not fail on unnecessary validation.

This Pull Request (PR) fixes the following issues

Fixes #56

Task list

Note: This change will not affect any of the currently written DSC code, as such none of the documentation should need to be updated.
[X] Added an entry to the change log under the Unreleased section of the file CHANGELOG.md. Entry should say what was changed and how that affects users (if applicable), and reference the issue being resolved (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 Community Testing Guidelines.
[ ] Integration tests added/updated (where possible). See DSC Community Testing Guidelines.
[ ] New/changed code adheres to DSC Community Style Guidelines.

This change is

codecov[bot] commented 3 years ago

Codecov Report

Merging #78 (7ea9788) into main (57bd18b) will not change coverage. Report is 1 commits behind head on main. The diff coverage is 80%.

@@        Coverage Diff         @@
##           main   #78   +/-   ##
==================================
  Coverage    79%   79%           
==================================
  Files        13    13           
  Lines       962   962           
==================================
  Hits        768   768           
  Misses      194   194

Files Changed	Coverage Δ
...onValueHelper/DhcpServerDsc.OptionValueHelper.psm1	`86% <80%> (ø)`

johlju commented 2 years ago

I think we should make the Force parameter optional by adding the a new parameter Force to the resource. If set we pass Force to the commands.

johlju commented 2 years ago

Need help verifying that using -Force actually works, as per https://github.com/dsccommunity/xDhcpServer/issues/56#issuecomment-1119816710 it should only be applicable when using DnsServer parameter. 🤔

realslacker commented 2 years ago

@johlju it's been a while since I submitted this pull request, but at the time I used this code to push some changes. I would be happy to do some additional testing but it will take me a bit to get a test environment setup.

johlju commented 2 years ago

Would be great if you have the time, thank you 😊

jangins101 commented 1 year ago

since I ran into this issue again myself (#56 ), wanted to bring it back up for review. I added a comment on the issue (#56) before, and have not seen any problems with with the -Force parameter being added in my environments.

johlju commented 1 year ago

Tomorrow I will fix the pipeline in this repo and then merge this. Thanks.

dsccommunity / xDhcpServer