SPBM storage policy enablement for VM provisioning. #881

mnshtiwari · 2019-10-23T13:04:14Z

Storage policy can be applied to VM and virtual disk in Create and Update operations.
Website docs added for the code changes.

Storage policy can be applied to VM and virtual disk in Create and Update operations. Website docs added for the code changes. Adding the docs related to storage policy for website. ng description in docs of data source storage policy.

markpeek

LGTM

bill-rich · 2019-10-25T23:46:46Z

vsphere/internal/helper/spbm/spbm_helper.go

+// pbmClientFromGovmomiClient creates a new pbm client from given govmomi client.
+// Can we have it in govmomi client as a field similar to tag client?
+// We should not create a new pbm client every time we need it.
+func pbmClientFromGovmomiClient(ctx context.Context, client *govmomi.Client) (*pbm.Client, error) {


I agree this would be better create the pbm client as part of the global config. Given the current structure, I think its ok to leave here for now. There is another feature in progress that needs some modification to the client structure, so I'll see about including the pbm client in that work.

Just merged a PR that adds the pbm client to the vSphereClient.

Is there a way to get vSphereClient from govmomi.Client?
We have govmomi.Client handle in VM operations and need pbm.Client from it. The vSphereClient struct will have pbm.Client.

bill-rich · 2019-10-25T23:51:13Z

vsphere/internal/virtualdevice/virtual_machine_disk_subresource.go

+		if err != nil {
+			return err
+		}
+		vmMOID := vm.Reference().Value


We've got a convenience function to simplify getting the MOID from the UUID you can use here.

bill-rich

Looks great! Thanks for the submission!

I added a couple comments outside of the review. One other thing I'd like to add before merging this PR is acceptance tests for the data source and updating the create and update tests for the virtual machine.

markpeek · 2019-10-28T14:18:17Z

@bill-rich thank you for the review. Since there isn't a terraform way to create a SPBM policy (possible future work), how would you suggest structuring the acceptance tests to allow querying the data source and the VM tests?

bill-rich · 2019-10-28T22:03:48Z

@markpeek Since we can't create the SPBM, for now I'd recommend having the tests use an env variable to specify an existing one. You can add it here. Ideally we can create most test fixtures at test time, but (as you can see in that file), we've got quite a few exceptions.

Sometimes govmomi will return a PortGroup spec where the failure criteria property of the nic teaming policy can be nil. When this happens we crash because the code doesn't check for this case. Addresses the crash in hashicorp#869

…_govmomi_tags Switch to govmomi tags client

Storage policy can be applied to VM and virtual disk in Create and Update operations. Website docs added for the code changes. Adding the docs related to storage policy for website. ng description in docs of data source storage policy.

mnshtiwari · 2019-11-01T11:03:43Z

@bill-rich I am trying to run the Virtual Machine acceptance tests before adding the tests related to storage policy. I went through the "tf-vsphere-devrc.mk.example" and added the env variables for storage policy.

Is there any guideline or requirement of vSphere infrastructure to run the tests? For example, VC should have 2 clusters with each 3 hosts each and certain type of datastores. Any pointers in this regard will be very helpful.

markpeek · 2019-11-18T17:32:03Z

@bill-rich @mnshtiwari I know there are people wanting to use this functionality but it looks like this has stalled. What needs to be done to get this merged?

bill-rich · 2019-11-21T01:02:17Z

@mnshtiwari @markpeek I'm currently working on a document to describe the environment requirements to run the acceptance tests, and am hoping to have it ready to share by the end of the week. Once that is ready, hopefully it'll be easier to get this finished up.

mnshtiwari · 2019-11-28T06:39:06Z

@mnshtiwari @markpeek I'm currently working on a document to describe the environment requirements to run the acceptance tests, and am hoping to have it ready to share by the end of the week. Once that is ready, hopefully it'll be easier to get this finished up.

Thanks @markpeek.

bill-rich · 2019-12-13T06:22:28Z

Since the process of getting the acceptance test environment requirements is taking a while, I can help out and get the acceptance tests written. That way we can get this merged and not be blocking.

mnshtiwari · 2019-12-17T07:45:50Z

Since the process of getting the acceptance test environment requirements is taking a while, I can help out and get the acceptance tests written. That way we can get this merged and not be blocking.

Thanks @bill-rich . I would start adding tests keeping in mind the requirements for storage policy tests. I'll put new env variables as required.
Testing storage policy would require a vSAN cluster to be present in vSphere. So it will be added as requirement for it in env variables.
I'll update the pull request with all these changes and you can advise and review the test cases.

bill-rich · 2019-12-20T00:18:37Z

@mnshtiwari & @markpeek Please take a look at https://github.com/terraform-providers/terraform-provider-vsphere/tree/pr/881 and let me know what you think of the added tests.

mnshtiwari · 2019-12-20T10:57:13Z

@mnshtiwari & @markpeek Please take a look at https://github.com/terraform-providers/terraform-provider-vsphere/tree/pr/881 and let me know what you think of the added tests.

Tests look good. Two points to add:

1- Need to add read and set the property "storage_policy_id" in "flattenVirtualMachineConfigInfo" for test to work. Only expand is added for now. Should I add this in a new pull request?
2- Need to add similar test for virtual disk.

One question I have about running tests. As stated in docs,
make testacc TESTARGS="-run=TestAccVSphereVirtualMachine"
will run the tests under this category. To run a specific test, e.g. "TestAccVSphereVirtualMachine_spbm", will the below work:
make testacc TESTARGS="-run=TestAccVSphereVirtualMachine_spbm"

…orm-provider-vsphere into f-spbm-support Adding storage policy Read in virtual machine resource.

bill-rich · 2020-01-17T06:38:59Z

I was looking over the PR, and I think you're right @mnshtiwari, we need to get the reading and updating of virtual machine storage policies added before we can release. I had to spend a little time getting familiar with the PR again (sorry, its been a while). If anyone on your side has the bandwidth to get that implemented, I can take a look first thing. Otherwise, I'll look at taking care of it since I've been blocking for so long. Either way, I'm holding the release for this PR so that we can get it out ASAP.

bill-rich · 2020-01-17T07:06:21Z

We ran into a problem with the rest client with govmomi, so we had to revert the earlier PR. I see you've pulled in those changes. I'd like to switch to govmomi, but it'll take a little investigation. I'm looking into that now and will provide updates by tomorrow.

markpeek · 2020-01-21T15:01:00Z

@bill-rich were you able to look at the changes @mnshtiwari made to address your review request on reading and updating the virtual machine storage policies? Also, what is the govmomi issue? Is there an Issue opened for it in the gvmomi repo?

bill-rich

LGTM

…pport Pulling in additional acceptance tests and doc updates

bill-rich · 2020-01-24T23:30:47Z

Merged via CLI to pull in a few additional changes such as acceptance tests.

mnshtiwari · 2020-01-27T02:00:13Z

LGTM

Thanks Bill for your help.

Rodrigo0461 · 2020-02-05T18:27:06Z

Hi all.
For default, my repo it take the 1.16 version (last release) then I get this error

vsphere_virtual_machine.xxxxl_node: ServerFaultCode: NoPermission
can anybody help me?

ty

Koleon · 2020-02-06T15:52:34Z

@Rodrigo0461 getting similar error while deploying VM via module.

Error: disk.0: validation failed (ServerFaultCode: NoPermission)

  on .terraform/modules/misp/vsphere_virtual_machine_linux.tf line 8, in resource "vsphere_virtual_machine" "virtual_machine_linux":
    8: resource "vsphere_virtual_machine" "virtual_machine_linux" {

With this, works without problem.

terraform {
  required_providers {
    vsphere = "= 1.15"
  }
}

mnshtiwari · 2020-02-07T08:34:26Z

@Rodrigo0461 getting similar error while deploying VM via module.

Error: disk.0: validation failed (ServerFaultCode: NoPermission)

  on .terraform/modules/misp/vsphere_virtual_machine_linux.tf line 8, in resource "vsphere_virtual_machine" "virtual_machine_linux":
    8: resource "vsphere_virtual_machine" "virtual_machine_linux" {

With this, works without problem.

terraform {
  required_providers {
    vsphere = "= 1.15"
  }
}

Can we get vCenter vpxd log where the call "CreateVM_Task" is failing?

mnshtiwari · 2020-02-07T09:17:49Z

@Rodrigo0461 getting similar error while deploying VM via module.
Error: disk.0: validation failed (ServerFaultCode: NoPermission)

  on .terraform/modules/misp/vsphere_virtual_machine_linux.tf line 8, in resource "vsphere_virtual_machine" "virtual_machine_linux":
    8: resource "vsphere_virtual_machine" "virtual_machine_linux" {
With this, works without problem.
terraform {
  required_providers {
    vsphere = "= 1.15"
  }
}
Can we get vCenter vpxd log where the call "CreateVM_Task" is failing?

Also what type of datastore is being used? VMFS, NFS or vSAN?

ghost added size/l Relative Sizing: Large documentation Type: Documentation labels Oct 23, 2019

hashibot added the enhancement Type: Enhancement label Oct 23, 2019

markpeek approved these changes Oct 23, 2019

View reviewed changes

bill-rich reviewed Oct 25, 2019

View reviewed changes

bill-rich suggested changes Oct 25, 2019

View reviewed changes

bill-rich and others added 8 commits October 30, 2019 17:53

Switch to govmomi tags client

9bfdd2e

Check if portgroup failure criteria is nil

a4b6343

Sometimes govmomi will return a PortGroup spec where the failure criteria property of the nic teaming policy can be nil. When this happens we crash because the code doesn't check for this case. Addresses the crash in hashicorp#869

Clean up comments and docs

38bb95e

Update dependencies

a9e97be

Merge pull request hashicorp#890 from terraform-providers/i-switch_to…

f09b64a

…_govmomi_tags Switch to govmomi tags client

Using convenience function to get the MOID from the UUID of a VM.

21ed097

Using convenience function to get the MOID from the UUID of a VM.

b774158

Merge branch 'f-spbm-support' of https://github.com/mnshtiwari/terraf…

1a22777

…orm-provider-vsphere into f-spbm-support Adding storage policy Read in virtual machine resource.

ghost added size/xxl Relative Sizing: Extra-Extra-Large and removed size/l Relative Sizing: Large labels Jan 16, 2020

Adding Read for storage policy in virtual machine

12e2fc9

bill-rich approved these changes Jan 24, 2020

View reviewed changes

bill-rich added a commit that referenced this pull request Jan 24, 2020

Merge pull request #881 from terraform-providers/mnshtiwari:f-spbm-su…

bc1eb53

…pport Pulling in additional acceptance tests and doc updates

bill-rich closed this Jan 24, 2020

neogen19 mentioned this pull request Feb 19, 2020

Issue with resource creation using vmware vsphere plugin version 1.16.1 #974

Closed

ghost locked and limited conversation to collaborators Apr 18, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SPBM storage policy enablement for VM provisioning. #881

SPBM storage policy enablement for VM provisioning. #881

mnshtiwari commented Oct 23, 2019

markpeek left a comment

bill-rich Oct 25, 2019

bill-rich Oct 31, 2019

mnshtiwari Nov 1, 2019

bill-rich Oct 25, 2019

bill-rich left a comment

markpeek commented Oct 28, 2019

bill-rich commented Oct 28, 2019

mnshtiwari commented Nov 1, 2019

markpeek commented Nov 18, 2019

bill-rich commented Nov 21, 2019

mnshtiwari commented Nov 28, 2019

bill-rich commented Dec 13, 2019

mnshtiwari commented Dec 17, 2019

bill-rich commented Dec 20, 2019

mnshtiwari commented Dec 20, 2019 •

edited

Loading

bill-rich commented Jan 17, 2020

bill-rich commented Jan 17, 2020

markpeek commented Jan 21, 2020

bill-rich left a comment

bill-rich commented Jan 24, 2020

mnshtiwari commented Jan 27, 2020

Rodrigo0461 commented Feb 5, 2020

Koleon commented Feb 6, 2020 •

edited

Loading

mnshtiwari commented Feb 7, 2020

mnshtiwari commented Feb 7, 2020

SPBM storage policy enablement for VM provisioning. #881

SPBM storage policy enablement for VM provisioning. #881

Conversation

mnshtiwari commented Oct 23, 2019

markpeek left a comment

Choose a reason for hiding this comment

bill-rich Oct 25, 2019

Choose a reason for hiding this comment

bill-rich Oct 31, 2019

Choose a reason for hiding this comment

mnshtiwari Nov 1, 2019

Choose a reason for hiding this comment

bill-rich Oct 25, 2019

Choose a reason for hiding this comment

bill-rich left a comment

Choose a reason for hiding this comment

markpeek commented Oct 28, 2019

bill-rich commented Oct 28, 2019

mnshtiwari commented Nov 1, 2019

markpeek commented Nov 18, 2019

bill-rich commented Nov 21, 2019

mnshtiwari commented Nov 28, 2019

bill-rich commented Dec 13, 2019

mnshtiwari commented Dec 17, 2019

bill-rich commented Dec 20, 2019

mnshtiwari commented Dec 20, 2019 • edited Loading

bill-rich commented Jan 17, 2020

bill-rich commented Jan 17, 2020

markpeek commented Jan 21, 2020

bill-rich left a comment

Choose a reason for hiding this comment

bill-rich commented Jan 24, 2020

mnshtiwari commented Jan 27, 2020

Rodrigo0461 commented Feb 5, 2020

Koleon commented Feb 6, 2020 • edited Loading

mnshtiwari commented Feb 7, 2020

mnshtiwari commented Feb 7, 2020

mnshtiwari commented Dec 20, 2019 •

edited

Loading

Koleon commented Feb 6, 2020 •

edited

Loading