xCluster: Allow cluster to be assigned IP address from a DHCP

[wasabii]

Pull Request (PR) description

• Changes to xCluster
  • Allow cluster to be assigned IP address from a DHCP (issue #109). When parameter StaticIPAddress is not specified then cluster will be configured to use IP address from a DHCP.

This Pull Request (PR) fixes the following issues: Fixes #109 Fixes #28

Task list:

• [x] Change details added to Unreleased section of CHANGELOG.md?
• [ ] Added/updated documentation, comment-based help and descriptions in .schema.mof files where appropriate?
• [ ] Examples appropriately updated?
• [ ] New/changed code adheres to Style Guidelines?
• [ ] Unit and (optional) Integration tests created/updated where possible

---

This change is 

[codecov-io]

Codecov Report

Merging #113 into dev will not change coverage. The diff coverage is n/a.

@@         Coverage Diff         @@
##            dev   #113   +/-   ##
===================================
  Coverage   100%   100%           
===================================
  Files         7      7           
  Lines       540    540           
===================================
  Hits        540    540

[johlju]

@wasabii Thanks for sending in this pull request. Could you please add a test for this change as well?

[wasabii]

Trying to figure out how. First time messing with this test framework.

[johlju]

No worries, there is no hurry. Let me know if you need any assistance in any way. :)

[wasabii]

Yeah. I'd appreciate some help. Where do you think these should go? Part of me says every existing test should simply be run with StaticIPAddress as unspecified. So, like, somehow duplicating the entire thing.

[johlju]

I don't think you need to run every existing test twice. But at least add one more test for each function (Get-/Set-/Test-TargetResource) where you remove the StaticIPAddress from the parameter list.

It would also be good that for the Set-TargetResource the mock for New-Cluster should have a new ParameterFilter $StaticIpAddress -eq $null for the new test in Set-TargetResource, and for one of the old test it should have ParameterFilter $StaticIpAddress -ne $null. And then you Assert-MockCalled with the respectively ParameterFilter so you know that the correct mock was called and not the other.

[johlju]

@wasabii I updated the PR description with the correct template. Let me know if you need further help with the tests. 😄

[johlju]

@wasabii You need to rebase (git rebase) this one again, both because new PR was merged and a new release was done. How's it going with the test? Need any more help?

[johlju]

@wasabii are you still working on this? Let me know if you can't work on this anymore. :)

[wasabii]

It might be a long time before I get back to it. Was pulled off onto other tasks.

[johlju]

Would you mind checking "Allow edits from maintainers" to the right here? The I could bring this to the finish line for you.

[johlju]

Review status: 0 of 4 files reviewed at latest revision, 1 unresolved discussion.

---

DSCResources/MSFT_xCluster/MSFT_xCluster.psm1, line 57 at r3 (raw file):

        }

        $address = Get-ClusterGroup -Cluster $Name -Name 'Cluster IP Address' -ErrorAction SilentlyContinue | Get-ClusterParameter -Name 'Address' -ErrorAction SilentlyContinue

Isn't possible to return the assigned IP-address here from DHCP? That could be of interest when running Get-DscConfiguration. Uncertain if this change does just that.

---

Comments from Reviewable

[johlju]

@wasabii added tests for this change. Just one question, that I need to verify, unless you have an answer, before I can merge this one,

[johlju]

Review status: 0 of 9 files reviewed at latest revision, 1 unresolved discussion.

---

DSCResources/MSFT_xCluster/MSFT_xCluster.psm1, line 57 at r3 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Isn't possible to return the assigned IP-address here from DHCP? That could be of interest when running Get-DscConfiguration. Uncertain if this change does just that.

I meant, "Is is not..."

---

Comments from Reviewable

[johlju]

Reviewed 1 of 3 files at r3, 10 of 10 files at r5. Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[johlju]

---

Reviewed 2 of 2 files at r6. Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[johlju]

Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

---

CHANGELOG.md, line 3 at r6 (raw file):

# Change log for xFailOverCluster

## Unreleased

Need to add an entry for the issue #28

---

Comments from Reviewable

[johlju]

Review status: 10 of 11 files reviewed at latest revision, 1 unresolved discussion.

---

CHANGELOG.md, line 3 at r6 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Need to add an entry for the issue #28

Done

---

Comments from Reviewable

[johlju]

Reviewed 1 of 1 files at r7. Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[johlju]

---

Review status: all files reviewed at latest revision, all discussions resolved, some commit checks pending.

---

Comments from Reviewable

[johlju]

@wasabii Now I merged this one. Thanks for your work on this one! Sorry it took so long getting this thru.

[wasabii]

Oh, cool. I'm sorry I completely vanished on you. Got pulled off into other things, and didn't have the time required to get back into it as needed. Also, I don't know crap about PowerShell testing, apparently.

[johlju]

No worries. I had a lot of work over at SqlServerDsc and a lot of work on the day job as well, so haven't had much time. You should look into Pester when you have time, it's a good knowledge to have. But here to help with that, just glad you started getting the code changes needed. 😄 Hope I will see you back here.