Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New resource: vsphere_compute_cluster_host_group #508

Merged
merged 5 commits into from
May 10, 2018

Conversation

vancluever
Copy link
Contributor

This is the other group resource, which manages hosts, and is mainly used in the vm/host affinity rules.

This sets the ground work for the cluster rules to be added now.

@vancluever vancluever added enhancement Type: Enhancement new-resource Feature: New Resource labels May 8, 2018
@vancluever vancluever requested a review from a team May 8, 2018 04:15
@vancluever vancluever force-pushed the f-vsphere-cluster-host-group-resource branch from a949d66 to d70ea1a Compare May 8, 2018 19:04
Copied/updated from vsphere_compute_cluster_vm_group. Not much to
change. Tests and docs to follow.
Resource is now functional, docs next.
Copy/paste problems :P
Docs are complete, resource is ready for PR barring any necessary final
edits.
Some minor corrections, and also adding warning blurbs to both the host
group and cluster group resources.
@vancluever vancluever force-pushed the f-vsphere-cluster-host-group-resource branch from d70ea1a to 5c99bd3 Compare May 10, 2018 14:09
Copy link

@paultyng paultyng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super minor issue I'll leave to your judgement, otherwise LGTM

func resourceVSphereComputeClusterHostGroupImport(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
var data map[string]string
if err := json.Unmarshal([]byte(d.Id()), &data); err != nil {
return nil, err

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add some additional context on this err before returning?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will just return the JSON error which will hopefully be clear enough to the user. I think there are a couple of other resources that have similar import patterns that don't necessarily include any additional error context, so might as well just leave it for consistency for now.

@vancluever vancluever merged commit af98a4a into master May 10, 2018
@bill-rich bill-rich deleted the f-vsphere-cluster-host-group-resource branch November 17, 2018 00:34
@ghost ghost locked and limited conversation to collaborators Apr 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Type: Enhancement new-resource Feature: New Resource
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants