ClusterIPAddress: New Resource to add IP Addresses to clusters

[nickgw]

Pull Request (PR) description

ClusterIPAddress

• New resource to add IP Addresses to Clusters if the ClusterNetwork exists in the Cluster.
• This needs to be separate from Cluster because creating the object will fail until the ClusterNetwork has been added either via DSC_ClusterNetwork or a node in that network joins the cluster.

This Pull Request (PR) fixes the following issues

• Fixes #151

Task list

• [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).
• [x] Resource documentation added/updated in README.md.
• [x] Resource parameter descriptions added/updated in README.md, schema.mof and comment-based help.
• [x] Comment-based help added/updated.
• [x] Localization strings added/updated in all localization files as appropriate.
• [x] Examples appropriately added/updated.
• [x] Unit tests added/updated. See DSC Community Testing Guidelines.
• [x] Integration tests added/updated (where possible). See DSC Community Testing Guidelines.
• [x] New/changed code adheres to DSC Community Style Guidelines.

---

This change is 

[codecov[bot]]

Codecov Report

Merging #270 (d138ea5) into main (605481d) will not change coverage. The diff coverage is 100%.

@@         Coverage Diff          @@
##           main   #270    +/-   ##
====================================
  Coverage   100%   100%            
====================================
  Files         7      8     +1     
  Lines       460    611   +151     
====================================
+ Hits        460    611   +151     

Impacted Files	Coverage Δ
...ces/DSC_ClusterIPAddress/DSC_ClusterIPAddress.psm1	100% <100%> (ø)

[nickgw]

I think I've resolved everything in your comments. Just futzing with tests now. I think I'm having issues figuring out how to mock the foreach loop in Get-ClusterIPResourceFromIPAddress.

As for your two questions:

1. I'm curious, can this potentially remove the IP address that is added by the resource ClusterNetwork if a configuration is passing 'Absent' for the same IP address with this new resource?

Yes, it will remove that IP address, leaving the cluster in an offline state. I don't know why anyone would want to do that, and if you think it's the right path I can write some logic to throw when there is only one IP address and DSC is trying to remove it.

2. Since netmask is mandatory parameter should it not also look that the netmask is correct. But maybe there is not possible to add the same IP adress with multiple netmasks, so maybe thatis why it only checks for IP address? 🤔

I think generally, the IP address is what matters to be added. If you try and add an IP address that falls in the same address scope as an already added network, but with a different AddressMask, Test-ClusterNetwork will catch that in Set and throw.

3. Instead of assigning to a variable that is then never used, can be output to null instead? (In Test-IPAddress)

I'm assigning it a variable because otherwise the System.Net.IPAddress object is returned. The resource doesn't care about that information, just that the IP address String passed is able to be casted to an IPAddress object.

I don't know best practices here, but I just tested quickly and [System.Net.IPAddress]::Parse($IPAddress) | Out-Null serves the same purpose as assigning a variable and never using it. Let me know what you think here.

[johlju]

I think I'm having issues figuring out how to mock the foreach loop in Get-ClusterIPResourceFromIPAddress

Debugging the test it was that you wanted to return a hashtable, but the return statement was on one row and hashtable on the next line, so it just ran return and never returned the hashtable. 🙂 But when debugging the tests I had to fix a few extra things, so they are in a PR here for you to merge if you like: https://github.com/nickgw/FailOverClusterDsc/pull/1

[johlju]

Yes, it will remove that IP address, leaving the cluster in an offline state. I don't know why anyone would want to do that, and if you think it's the right path I can write some logic to throw when there is only one IP address and DSC is trying to remove it.

I think I misunderstood the purpose of ClusterNetwork - after digging in further it does not create anything new, just update an existing network. So my initial thought that these will collide in some way was not true. So no need to do anything. 🙂

[johlju]

I think generally, the IP address is what matters to be added. If you try and add an IP address that falls in the same address scope as an already added network, but with a different AddressMask, Test-ClusterNetwork will catch that in Set and throw.

Sounds good, lets go with that.

[johlju]

I don't know best practices here, but I just tested quickly and [System.Net.IPAddress]::Parse($IPAddress) | Out-Null serves the same purpose as assigning a variable and never using it. Let me know what you think here.

Using either | Out-Null or $null = ... will avoid getting the lint error "variable is assigned but never used". So rather see we either assign it or pipe it to null.

[johlju]

@nickgw I won't have time to look over this tonight or tomorrow. So will look at it after a workday next week.

[nickgw]

@johlju no worries! I think I've updated everything, and am ready for hopefully your final review! I appreciate all the time you've spent on this.