Vswitch import #625
Vswitch import #625
Conversation
There was a problem hiding this comment.
Looks good! Just have a couple comments and requests on the PR.
In addition to the inline comments, can you please also add an import test (see here for an example), and run make fmt again with go v1.11? We just switched over to go v1.11.0 which has a few minor fmt differences.
| d.Set("host_system_id", hostID) | ||
| d.Set("name", switchName) | ||
|
|
||
| err = resourceVSphereHostVirtualSwitchRead(d, meta) |
There was a problem hiding this comment.
No need to call Read here as it will be called after import.
| return []*schema.ResourceData{}, err | ||
| } | ||
|
|
||
| d.Set("host_system_id", hostID) |
There was a problem hiding this comment.
The error returned from d.Set should be handled.
| } | ||
|
|
||
| d.Set("host_system_id", hostID) | ||
| d.Set("name", switchName) |
There was a problem hiding this comment.
The error returned from d.Set should be handled.
| @@ -174,3 +174,16 @@ the resource. This is set to an ID value unique to Terraform - the convention | |||
| is a prefix, the host system ID, and the virtual switch name. An example would | |||
| be `tf-HostVirtualSwitch:host-10:vSwitchTerraformTest`. | |||
|
|
|||
| ## Importing | |||
|
|
|||
| An existing vSwitch can be [imported][docs-import] into this resource via the id | |||
There was a problem hiding this comment.
The id for vswitches isn't very intuitive. I think having an explaination that the ID is the <prefix (tf-HostVirtualSwitch)>:: would help make it more clear. Sorry the id isn't simpler in this case!
|
I tried to code the import test but I don't have a test vsphere env so it's very difficult... I'll try to set up one and do that next week. |
There was a problem hiding this comment.
Looking very close! Thanks for taking the time to contribute and work through some reviews. On the acceptance test, I'd be happy to run the test for you on my environment. Or if you'd prefer, I can write the test and get it over to you. I'll follow your lead on it!
| ## Importing | ||
|
|
||
| An existing vSwitch can be [imported][docs-import] into this resource by its id. | ||
| The convention of the id is a prefix, the host system ID, and the virtual switch |
There was a problem hiding this comment.
One last thing on this, can host system ID be changed to host system [managed objectID][docs-about-morefs]? And add:
[docs-about-morefs]: /docs/providers/vsphere/index.html#use-of-managed-object-references-by-the-vsphere-provider
There are several different ID types used throughout the provider, so it is just helpful to be extra clear when specifying.
| @@ -174,3 +174,18 @@ the resource. This is set to an ID value unique to Terraform - the convention | |||
| is a prefix, the host system ID, and the virtual switch name. An example would | |||
| be `tf-HostVirtualSwitch:host-10:vSwitchTerraformTest`. | |||
|
|
|||
| ## Importing | |||
|
|
|||
| An existing vSwitch can be [imported][docs-import] into this resource by its id. | |||
ghost
added
size/m
This update adds import functionality on
vsphere_host_virtual_switchresource.