migrate cluster info module from community #80
Conversation
mikemorency
requested review from
shellymiron,
elsapassaro,
anna-savina,
bardielle and
machacekondra
as code owners
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #80 +/- ##
==========================================
+ Coverage 26.09% 27.19% +1.10%
==========================================
Files 19 23 +4
Lines 1667 2015 +348
Branches 331 375 +44
==========================================
+ Hits 435 548 +113
- Misses 1232 1467 +235
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
mikemorency
force-pushed
the
mm-feature/migrate-cluster-info
branch
from
cdb2d81 to
5313bde
Compare
| for host in self.cluster.host: | ||
| hosts.append({ | ||
| 'name': host.name, | ||
| 'folder': get_folder_path_of_vm(host), |
There was a problem hiding this comment.
The function is called get_folder_path_of_vm but it gets an host.... why?
There was a problem hiding this comment.
I think when it was copied it just looked up vm paths but now its more generic.
I updated the name to get_folder_path_of_vsphere_object
| try: | ||
| output_facts["dpm_host_power_action_rate"] = 6 - int(dpm_config.hostPowerActionRate) | ||
| except (TypeError, AttributeError): | ||
| output_facts["dpm_host_power_action_rate"] = 3 |
There was a problem hiding this comment.
Can you use consts here? (for the values 3 as a default and for 6)
There was a problem hiding this comment.
I made a constant for the default value, and a static method for the adjustment (6 - whatever).
I updated the drs and dpm modules to use those instead of having their own copies
| datacenter_name: datacenter | ||
| register: _out | ||
|
|
||
| - name: Gather Specific Properties About a Cluster |
There was a problem hiding this comment.
If there are multiple datacenters what will it return? the first one? please explain
I'm not sure why to use it without a DC name...
There was a problem hiding this comment.
If there are multiple datacenters and the user doesnt supply a datacenter name, this will search for clusters in any datacenter.
If the user does supply a name, each datacenter must have a unique name (per VMware) so there can only ever be one.
The community version has the datacenter as optional. I guess you might want to gather info on every cluster you have, even if you have multiple datacenters?
| - name: Check DRS Settings Were Applied | ||
| ansible.builtin.assert: | ||
| that: | ||
| - _idempotence_check is not changed | ||
| - _config.drs_default_vm_behavior == drs_default_vm_behavior | ||
| - _config.drs_enable_vm_behavior_overrides == drs_enable_vm_behavior_overrides | ||
| - _config.drs_vmotion_rate == (6 - drs_vmotion_rate) | ||
| - _config.enabled_drs == drs_enable | ||
| - _config.drs_vmotion_rate == drs_vmotion_rate |
There was a problem hiding this comment.
why did you remove the calculation? I think it was a bug in the past the @shellymiron fixed
There was a problem hiding this comment.
its no longer needed, the new cluster_info module does the calculation before returning the value
SUMMARY
This migrates the vmware_cluster_info module from the community.vmware collection to this collection.
The inputs are mostly the same. There is one input that was renamed to match the guest_info module (gather_tags).
The outputs are mostly the same. However, there are some changes worth noting:
<service>_enabledwhere service could bedpm,drs,ha, etcISSUE TYPE
COMPONENT NAME
cluster_info