All resources - add to NamespaceRoots, NamespaceFolders, Add ReplicationGroups, Connections & Memberships #126
Conversation
BREAKING CHANGE: - Add support for setting a DFS Namespace root target's referral status (state) to be either Online or Offline in MSFT_DFSNamespaceRoot. State is now a required parameter. - Add support for setting a DFS Namespace folder target's referral status (state) to be either Online or Offline in MSFT_DFSNamespaceFolder. State is now a required parameter. - Update examples to support new required parameter.
… does not exist (fix dsccommunity#125)
…SReplicationGroupConnection plus EnsureEnabled, MinimumFileStagingSize, ConflictAndDeletedQuotaInMB, RemoveDeletedFiles and DfsnPath in DFSReplicationGroupMembership
…property, make it optional (to avoid breaking change)
|
Am aware of failing tests, will work on tomorrow |
|
Sorry about the delay @Borgquite - will continue with this, this week. |
There was a problem hiding this comment.
Any progress? :)
Reviewable status: 33 of 76 files reviewed, 35 unresolved discussions (waiting on @PlagueHO)
|
Hi @Borgquite - Am so sorry for the delay. Have been so slammed in my day job. I've blocked time this weekend. It will happen. |
|
@PlagueHO Another gentle poke here - so I can honour your time and not keep pestering you, do you have any idea when this might be done? Thanks! |
There was a problem hiding this comment.
Awesome @Borgquite - sorry this took so long. Just some really minor tweaks then merge. I think we'll probably leave the if ($Member) one if we can just note the behaviour in the README.md for that particular resource as I don't see it very common to want to remove all members from an RG.
Reviewed 15 of 27 files at r9, 18 of 18 files at r10, 9 of 9 files at r11, 1 of 1 files at r12, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Borgquite)
source/DSCResources/DSC_DFSReplicationGroup/DSC_DFSReplicationGroup.psm1 line 621 at r8 (raw file):
Previously, Borgquite (Chris Hill) wrote…
Sorry - I tried this but it broke the tests in a way that I don't really understand. It seems to work in produciton - is this the end of the world?
Using a $PSBoundParameters.ContainsKey('Members') check is usually best when there needs to be a behavior difference between not passing the parameter at all and passing null/empty array. In this code, both permutations will behave in the same way - so there will be no combination of parameters that can be used to set the replication group to have no members.
Is there a situation where you would want to clear out all the members from a replication group? This is an edge case, but it's more a question of what behavior would you expect if you had two configs:
# The RG should have no members in it
DFSReplicationGroup RemoveAllMembers {
GroupName = 'rg1'
Members = @()
}Compared with
# The members of the RG should not be considered at all
DFSReplicationGroup IgnoreCurrentMembers {
GroupName = 'rg1'
}When using if ($Members) then both configs will always behave like the second config.
It is a subtle nuance and probably not likely to be a concern for most users, but it's a pattern we try to support if we can.
What were the strange issues with the tests that occurred? Could have been our tests weren't the best. 😢
But perhaps we document this in the README.MD for the resource that you can't have a RG with no members by configuring it this way?
source/DSCResources/DSC_DFSReplicationGroup/DSC_DFSReplicationGroup.psm1 line 755 at r8 (raw file):
Previously, Borgquite (Chris Hill) wrote…
See followup above.
See above
source/DSCResources/DSC_DFSReplicationGroupConnection/DSC_DFSReplicationGroupConnection.psm1 line 512 at r8 (raw file):
Previously, Borgquite (Chris Hill) wrote…
I'm not sure. Check the code above - it's a copy/paste of EnsureRDCEnabled. In that variable, inside the DSC module it's a string 'Enabled' but the underlying Get-DFSReplicationGroup returns a boolean ($true/$false). Not sure why it's not just boolean throughout but just trying to keep the setting in line with how the module works at present.
Let's just leave it as is.
source/Examples/Resources/DFSNamespaceFolder/1-DFSNamespaceFolder_Domain_MultipleTarget_Config.ps1 line 23 at r12 (raw file):
.DESCRIPTION Create an AD Domain V2 based DFS namespace called software in the domain contoso.com with a four targets on the servers ca-fileserver, ma-fileserver, ny-fileserver01 and ny-filerserver02. It also
Nit:
Suggestion:
four targets on the servers ca-fileserver, ma-fileserver, ny-fileserver01 and ny-filerserver02. It also
There was a problem hiding this comment.
@PlagueHO Right - I've made the changes requested including managing to fix the dodgy tests. Let me know :)
Reviewable status: 72 of 76 files reviewed, 2 unresolved discussions (waiting on @PlagueHO)
source/DSCResources/DSC_DFSReplicationGroup/DSC_DFSReplicationGroup.psm1 line 621 at r8 (raw file):
Previously, PlagueHO (Daniel Scott-Raynsford) wrote…
Using a
$PSBoundParameters.ContainsKey('Members')check is usually best when there needs to be a behavior difference between not passing the parameter at all and passing null/empty array. In this code, both permutations will behave in the same way - so there will be no combination of parameters that can be used to set the replication group to have no members.Is there a situation where you would want to clear out all the members from a replication group? This is an edge case, but it's more a question of what behavior would you expect if you had two configs:
# The RG should have no members in it DFSReplicationGroup RemoveAllMembers { GroupName = 'rg1' Members = @() }Compared with
# The members of the RG should not be considered at all DFSReplicationGroup IgnoreCurrentMembers { GroupName = 'rg1' }When using
if ($Members)then both configs will always behave like the second config.It is a subtle nuance and probably not likely to be a concern for most users, but it's a pattern we try to support if we can.
What were the strange issues with the tests that occurred? Could have been our tests weren't the best. 😢
But perhaps we document this in the README.MD for the resource that you can't have a RG with no members by configuring it this way?
OK - now that I understand that I was able to fix the test (which was passing $null for Members and testing as though it was undefined).
Should be hopefully all good now :)
source/DSCResources/DSC_DFSReplicationGroup/DSC_DFSReplicationGroup.psm1 line 755 at r8 (raw file):
Previously, PlagueHO (Daniel Scott-Raynsford) wrote…
See above
See above - should be all good now :)
source/Examples/Resources/DFSNamespaceFolder/1-DFSNamespaceFolder_Domain_MultipleTarget_Config.ps1 line 23 at r12 (raw file):
Previously, PlagueHO (Daniel Scott-Raynsford) wrote…
Nit:
All done (also had the same error in DFSNamespaceRoot example)
|
@PlagueHO Just checking you got the notification above. Hopefully this is quick to resolve now! :) |
|
@PlagueHO Let me know how it's going :) |
There was a problem hiding this comment.
Sorry @Borgquite - I missed the first notification 😢 But now...
Thank you so much for the patience!
Lets see if the deploy pipeline works (API keys sometimes expire 😁 but I'll get those fixed if they have).
Reviewed 4 of 4 files at r13, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @Borgquite)
source/DSCResources/DSC_DFSReplicationGroup/DSC_DFSReplicationGroup.psm1 line 621 at r8 (raw file):
Previously, Borgquite (Chris Hill) wrote…
OK - now that I understand that I was able to fix the test (which was passing $null for Members and testing as though it was undefined).
Should be hopefully all good now :)
Looks great!
|
@PlagueHO Fear the deploy pipeline may have that API key issue... https://dev.azure.com/dsccommunity/DFSDsc/_build/results?buildId=8075&view=results |
|
Yep, he asked me to update, will do in a few hours (my evening). |
|
DFSDsc 5.0.0-preview0001 has been published to the PSGallery: https://www.powershellgallery.com/packages/DFSDsc/5.0.0-preview0001 |
|
@gaelcolas Great! I've just downloaded and deployed & seems to be working fine. What's the normal process & timeframe for moving from prerelease to full release, just so I know? |
|
@gaelcolas (Presume you're also aware that the build is still reporting errors?) https://dev.azure.com/dsccommunity/DFSDsc/_build/results?buildId=8082&view=logs&j=5d0a9c4e-8741-5118-af45-406a30fe661c&t=bda0b158-a237-516f-c1d0-31339587c5b5 |
|
I'm just checking this now. @Borgquite - we'll give the preview release a couple of weeks and if no major issues we'll push a release build. |
|
Oh, the issue is that this build has already been pushed. Think it's just had two deployments running for some reason. This should be fixed when we push a release build. |
|
@PlagueHO OK - is this an issue though? (Attempt 1 failed with 'Bad credentials' accessing docs.github.com) Only mention it in case it breaks the release build too. The doco is currently still showing the older preview https://github.com/dsccommunity/DFSDsc/wiki |
|
Hmm might need to update the GitHub API key too @gaelcolas :D |
|
Hmmm, GitHub changed something about auth and/or token expired, gotta look into that. |
|
Any movement on this @PlagueHO @gaelcolas ? |
|
Hmm. I'll connect with @gaelcolas offline and see if we can figure this out - might be this weekend though... :( |
|
@Borgquite - v5.0.0 has gone out. Thank you for all your help! |
Pull Request (PR) description
New functionality and fixes for resources:
OnlineorOfflinein DSC_DFSNamespaceRoot - Fixes Issue #96 (replaces PR BREAKING CHANGE: MSFT_DFSNamespaceRoot/MSFT_DFSNamespaceFolder: DFSNamespace Support for ReferralStatus (State) Modification #97, adding tests)
OnlineorOfflinein DSC_DFSNamespaceFolder - Fixes Fixes Issue #96 (replaces PR BREAKING CHANGE: MSFT_DFSNamespaceRoot/MSFT_DFSNamespaceFolder: DFSNamespace Support for ReferralStatus (State) Modification #97 adding tests)
Sorry for the bulk PR but wasn't sure how to fix #29 otherwise at the same time as adding new resources.
This Pull Request (PR) fixes the following issues
Task list
file CHANGELOG.md. Entry should say what was changed and how that
affects users (if applicable), and reference the issue being resolved
(if applicable).
and comment-based help.
This change is