search

WahlNetwork / vester

Easily validate and remediate your vSphere configuration
https://wahlnetwork.com
Apache License 2.0
146 stars 45 forks source link

vSAN Cluster + vSAN Host Scope Additions Proposal #185

Open aaronwsmith opened 7 years ago

aaronwsmith commented 7 years ago

During the VMworld 2017 Hackathon, the team work several tests that were added to the Host and Cluster scopes, but pertains only to hosts/clusters that have vSAN enabled. Continuing to add tests of this type to these categories means user may have to filter out tests that either waste cycles checking settings they don't care about, or potentially fail because vSAN related properties don't exist on those inventory objects.

Adding two new scopes would allow for the previously written vSAN tests to be moved to those categories whereby users can easily exclude the scopes entirely if they do not apply to their environment. It also creates clear separation of such tests.

The proposed code has been added to my forked repo which is now a number of changes behind, but provides the code enhancements needed to make this work as expected.

Details of the commit to my forked repo are found here: https://github.com/aaronwsmith/Vester/commit/bd7576e2d36996a7891fbb9cf60dfe632e48f0e6#diff-4800bf613339f1d044cd5c65d8394154

To implement the scope, the following is done:

  1. Two new private functions are added for getting vSAN Clusters and vSAN Hosts. This essentially masks the extended properties of these objects that are checked to determine if vSAN is enabled.

  2. Select-InventoryObject.ps1 has a minor proposed enhancement that is optional but I feel adds value in that it sorts the inventory objects by name before presenting them to the user to make their selections for running tests against. This generally allows for entering a consistent numbering input when performing repeated tests. It also makes it easy in general for users to find the object they want to select because they'll easily that the list is sorted alphabetically.

  3. VesterTemplate.Tests.ps1 invokes the new private functions to assist in obtaining the appropriate inventory objects for vSAN.

** Note that the VM inventory scope had to be modified to account for the possibility that a user would specify the same cluster for the $Cluster scope as they did for the $vSANCluster scope. In such cases the $VM object list would list the same VMs twice. Therefore the two cluster-type objects must be explicitly added to arrays, combined, then piped through Get-VM along with Select -Unique to ensure such duplicates are eliminated before being passed along to the user.

  1. Get-VesterTest.ps1 simply adds awareness of the new scopes.

  2. New-VesterConfig.ps1 adds the same new vSAN scopes, and deals with the $VM object list in the same manner as described for #3 for VesterTemplate.Tests.ps1.

  3. My fork was based on Vester 1.2, so Vester.psd1 was modified for 1.3.0 version to signal the enhancements.

If this change is acceptable and we process a pull request, the next step would be to move the vSAN related tests into the new scope folders. I did this with the files from my local repo without committing those changes in GitHub at this time, as I wasn't sure the best way handle that aspect upstream.

Let me know if there's any questions about the rational of the proposed change, or the specifics of how the code was implemented.

aaronwsmith commented 7 years ago

I've since trashed the original fork since I didn't branch from master to keep it clean and couldn't figure out how to recover it (GitHub newbie.) Also since my code was several changes behind, seems best to "start over" I also decided it best to pull out item #2 from above (Select-InventoryObject) and make that its own PR since it wasn't directly related to this.

aaronwsmith commented 7 years ago

I will close this for now until I retest the vSAN scopes against the latest code and submit a PR for it instead.

brianbunke commented 7 years ago

I am here for this. 👏 Reopening so we can refer to this (and solicit feedback from any other interested parties) until a PR is merged.

Item 2 will be accepted in #186.

Please don't include Item 6 (the version increment) in your PR. I totally agree that vSAN inclusion will trigger 1.3, but I'd like to avoid any potential merge conflicts coming from other PRs. We'll increment it when we're ready to publish to the gallery.

Thanks so much for this effort, it looks awesome!