Improve VM Placement Policy data source for tenant users #948

adambarreiro · 2022-12-01T15:21:00Z

Overview

This PR depends on Fix multiple resource pools issue when fetching VM Groups go-vcloud-director#530
This PR helps in solving Datasource vcd_org_vdc not returning provider_vdc_name #945

This PR modifies the vcd_vm_placement_policy data source to add a new attribute: vdc_id. This attribute helps tenant users to fetch VM Placement Policies, as they cannot obtain provider_vdc_id in any way.

With vdc_id, they can obtain the VDC identifier from the desired VDC and obtain the assigned VM Placement Policies. If the policies are not assigned, this data source will not find them (which makes sense even looking in UI).

As this PR bumps latest Go SDK, in this library it is included a fix that avoids an error when the Provider VDC is backed by many Resource pools (see vmware/go-vcloud-director#530).

Detailed changes

provider_vdc_id is now optional and conflicts with vdc_id
vdc_id is a new optional attribute that conflicts with provider_vdc_id
Improved the filtering by adding policyType==VdcVmPolicy (this one is from v2.0.0 endpoint),isSizingOnly==false and the vGPU policy filter. Not having these options could have lead to bugs in future.
Improved error handling. There were also some d.SetId("") which didn't make much sense as the ID was already "" in that conditional branch.
Improved documentation to add an example for bot System administrators and tenant users.

Side changes

Applied the boy scout rule to fix some broken links in documentation :)
Prepare files for v3.8.1

Tests

Improved TestAccVcdVmPlacementPolicy to use the VDC data source (more realistic scenario) and created TestAccVcdVmPlacementPolicyInVdc to test the new scenario, partially (as it is run as System Administrator due to the need of creating admin resources, see the test docs for more).

A manual test of vcd_vm_placement_policy using only a tenant user is required.

TestAccVcdVmPlacementPolicy and TestAccVcdVmPlacementPolicyInVdc pass
data.vcd_vm_placement_policy using only a tenant user:

provider "vcd" {
  user                 = "foo"
  password             = "*****"
  auth_type            = "integrated"
  url                  = "*****"
  org                  = "test"
  vdc                  = "vdc-test"
  allow_unverified_ssl = true
}

data "vcd_org_vdc" "org-vdc" {
  org      = "test"
  name     = "vdc-test"
}

data "vcd_vm_placement_policy" "policy" {
  name   = "policy1"
  vdc_id = data.vcd_org_vdc.org-vdc.id
}

Note: policy1 was created and assigned to VDC beforehand

data.vcd_vm_placement_policy with no vdc_id nor provider_vdc_id set returns Error: either provider_vdc_id or vdc_id needs to be set

Signed-off-by: abarreiro <abarreiro@vmware.com>

terraform-provider-vcd-bot · 2022-12-01T15:22:25Z

go.mod

@@ -59,3 +59,5 @@ require (
 	google.golang.org/grpc v1.50.1 // indirect
 	google.golang.org/protobuf v1.28.1 // indirect
 )
+
+replace github.com/vmware/go-vcloud-director/v2 => github.com/adambarreiro/go-vcloud-director/v2 v2.17.0-alpha.1.0.20221201150414-93ae0f2f6136


Please remove this module replacement before you merge this PR,
once you completely finish your code changes and the PR is ready to be merged.
You don't need to resolve this conversation, it will be automatically removed once
go.mod is in a correct state.
Thanks! 😸

Signed-off-by: abarreiro <abarreiro@vmware.com>

website/docs/d/vm_placement_policy.html.markdown

vcd/datasource_vcd_vm_placement_policy.go

vcd/resource_vcd_vm_placement_policy.go

Signed-off-by: abarreiro <abarreiro@vmware.com>

lvirbalas · 2022-12-05T11:11:51Z

website/docs/d/vm_placement_policy.html.markdown

 ## Argument Reference

 The following arguments are supported:

 * `name` - (Required) The name VM Placement Policy.
-* `provider_vdc_id` - (Required) The ID of the [Provider VDC](/providers/vmware/vcd/latest/docs/data-sources/vcd_provider_vdc) to which the VM Placement Policy belongs.
+* `provider_vdc_id` - (Required for System admins) The ID of the [Provider VDC](/providers/vmware/vcd/latest/docs/data-sources/provider_vdc) to which the VM Placement Policy belongs.
+* `vdc_id` - (Required for tenant users; *v3.8.1+*) The ID of the [VDC](/providers/vmware/vcd/latest/docs/data-sources/org_vdc) to which the VM Placement Policy is assigned.


This is a generic question/suggestion about "tenant" vs. "org" term across the whole PR docs. Should we use "org users" instead of "tenant users"?

E.g. here:

Suggested change

* `vdc_id` - (Required for tenant users; *v3.8.1+*) The ID of the [VDC](/providers/vmware/vcd/latest/docs/data-sources/org_vdc) to which the VM Placement Policy is assigned.

* `vdc_id` - (Required for Org users; *v3.8.1+*) The ID of the [VDC](/providers/vmware/vcd/latest/docs/data-sources/org_vdc) to which the VM Placement Policy is assigned.

The thought behind this is that "tenant" is more a business term, while inside the CD itself we see "organization".

Comments are welcome!
CC: @dataclouder @Didainius

Either we use provider/tenant (as it is sometimes appropriate for wide operations guide) or we use System administrator/Org user.
While I have found the provider/tenant pair clearer in some contexts, I think that in this case it should be Org users.

I agree that we could use provider/tenant in "soft" text, while in technical docs we should use System administrrator/Org user.

We had to fork this PR so that we can resolve conflicts and merge it therefore the resolution is in PR #953 commit 1f7897c

vmwclabot · 2022-12-07T09:55:28Z

@adambarreiro, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding Signed-off-by: John Doe <john.doe@email.org> to the last line of each Git commit message. The e-mail address used to sign must match the e-mail address of the Git author. Click here to view the Developer Certificate of Origin agreement.

Commit c2ebc30 has a no-reply/unreachable author email address: Didainius@users.noreply.github.com

vmwclabot · 2022-12-07T09:55:28Z

@adambarreiro, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding Signed-off-by: John Doe <john.doe@email.org> to the last line of each Git commit message. The e-mail address used to sign must match the e-mail address of the Git author. Click here to view the Developer Certificate of Origin agreement.

Commit c2ebc30 has a no-reply/unreachable author email address: Didainius@users.noreply.github.com

vmwclabot · 2022-12-07T10:02:10Z

@adambarreiro, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding Signed-off-by: John Doe <john.doe@email.org> to the last line of each Git commit message. The e-mail address used to sign must match the e-mail address of the Git author. Click here to view the Developer Certificate of Origin agreement.

Commit 8cb78fa has a no-reply/unreachable author email address: Didainius@users.noreply.github.com

vmwclabot · 2022-12-07T10:02:10Z

@adambarreiro, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding Signed-off-by: John Doe <john.doe@email.org> to the last line of each Git commit message. The e-mail address used to sign must match the e-mail address of the Git author. Click here to view the Developer Certificate of Origin agreement.

Commit 8cb78fa has a no-reply/unreachable author email address: Didainius@users.noreply.github.com

vmwclabot · 2022-12-07T10:04:51Z

@adambarreiro, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding Signed-off-by: John Doe <john.doe@email.org> to the last line of each Git commit message. The e-mail address used to sign must match the e-mail address of the Git author. Click here to view the Developer Certificate of Origin agreement.

Commit 26287f1 has a no-reply/unreachable author email address: Didainius@users.noreply.github.com

vmwclabot · 2022-12-07T10:04:51Z

@adambarreiro, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding Signed-off-by: John Doe <john.doe@email.org> to the last line of each Git commit message. The e-mail address used to sign must match the e-mail address of the Git author. Click here to view the Developer Certificate of Origin agreement.

Commit 26287f1 has a no-reply/unreachable author email address: Didainius@users.noreply.github.com

vmwclabot · 2022-12-07T10:09:19Z

@adambarreiro, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding Signed-off-by: John Doe <john.doe@email.org> to the last line of each Git commit message. The e-mail address used to sign must match the e-mail address of the Git author. Click here to view the Developer Certificate of Origin agreement.

Commit 8cac8f2 has a no-reply/unreachable author email address: Didainius@users.noreply.github.com

vmwclabot · 2022-12-07T10:09:19Z

@adambarreiro, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding Signed-off-by: John Doe <john.doe@email.org> to the last line of each Git commit message. The e-mail address used to sign must match the e-mail address of the Git author. Click here to view the Developer Certificate of Origin agreement.

Commit 8cac8f2 has a no-reply/unreachable author email address: Didainius@users.noreply.github.com

Signed-off-by: Dainius Serplis <dserplis@vmware.com>

Didainius · 2022-12-07T10:38:27Z

Closing in favor of #953 as we need to merge conflicts while @adambarreiro is away

…953)

adambarreiro added 3 commits December 1, 2022 15:21

Fixes in docs

e3d0441

Signed-off-by: abarreiro <abarreiro@vmware.com>

Checkpoint

46ae2ef

Signed-off-by: abarreiro <abarreiro@vmware.com>

bump sdk

448701e

Signed-off-by: abarreiro <abarreiro@vmware.com>

terraform-provider-vcd-bot reviewed Dec 1, 2022

View reviewed changes

adambarreiro added 5 commits December 1, 2022 16:26

Add check

02961f8

Signed-off-by: abarreiro <abarreiro@vmware.com>

Add test

0642889

Signed-off-by: abarreiro <abarreiro@vmware.com>

Fix test

be23f7e

Signed-off-by: abarreiro <abarreiro@vmware.com>

Changelog

621a564

Signed-off-by: abarreiro <abarreiro@vmware.com>

#

7e28170

Signed-off-by: abarreiro <abarreiro@vmware.com>

adambarreiro self-assigned this Dec 1, 2022

#

9a88ed8

Signed-off-by: abarreiro <abarreiro@vmware.com>

adambarreiro marked this pull request as ready for review December 1, 2022 17:12

adambarreiro requested review from lvirbalas, Didainius and dataclouder as code owners December 1, 2022 17:12

Improve weird writing of a comment

686b9f5

Signed-off-by: abarreiro <abarreiro@vmware.com>

adambarreiro commented Dec 1, 2022

View reviewed changes

website/docs/d/vm_placement_policy.html.markdown Outdated Show resolved Hide resolved

Didainius reviewed Dec 2, 2022

View reviewed changes

adambarreiro added 3 commits December 2, 2022 10:23

Address some suggestions

56fc35f

Signed-off-by: abarreiro <abarreiro@vmware.com>

Simplify code

ef0d59c

Signed-off-by: abarreiro <abarreiro@vmware.com>

Add attribute version in docs

12c889b

Signed-off-by: abarreiro <abarreiro@vmware.com>

dataclouder approved these changes Dec 2, 2022

View reviewed changes

Didainius approved these changes Dec 2, 2022

View reviewed changes

This was referenced Dec 2, 2022

Datasource vcd_org_vdc not returning provider_vdc_name #945

Closed

VDC VM placement policies #898

Closed

lvirbalas reviewed Dec 5, 2022

View reviewed changes

vmwclabot added the dco-required label Dec 7, 2022

Didainius force-pushed the fix-vm-placement-policy-ds branch from c2ebc30 to 8cb78fa Compare December 7, 2022 10:02

Didainius force-pushed the fix-vm-placement-policy-ds branch from 8cb78fa to 26287f1 Compare December 7, 2022 10:04

Didainius force-pushed the fix-vm-placement-policy-ds branch from 26287f1 to 12c889b Compare December 7, 2022 10:07

vmwclabot removed dco-required labels Dec 7, 2022

vmwclabot added the dco-required label Dec 7, 2022

Didainius force-pushed the fix-vm-placement-policy-ds branch from 8cac8f2 to 12c889b Compare December 7, 2022 10:12

vmwclabot removed the dco-required label Dec 7, 2022

Didainius added a commit to Didainius/terraform-provider-vcd that referenced this pull request Dec 7, 2022

Fork PR vmware#948 to resolve conflicts

9026484

Signed-off-by: Dainius Serplis <dserplis@vmware.com>

Didainius mentioned this pull request Dec 7, 2022

Improve VM Placement Policy data source for tenant users (fork #948) #953

Merged

Didainius closed this Dec 7, 2022

Didainius added a commit that referenced this pull request Dec 7, 2022

Improve VM Placement Policy data source for tenant users (fork #948) (#…

317557b

…953)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve VM Placement Policy data source for tenant users #948

Improve VM Placement Policy data source for tenant users #948

adambarreiro commented Dec 1, 2022 •

edited

terraform-provider-vcd-bot Dec 1, 2022

lvirbalas Dec 5, 2022

dataclouder Dec 7, 2022

lvirbalas Dec 7, 2022

Didainius Dec 7, 2022

vmwclabot commented Dec 7, 2022

vmwclabot commented Dec 7, 2022

vmwclabot commented Dec 7, 2022

vmwclabot commented Dec 7, 2022

vmwclabot commented Dec 7, 2022

vmwclabot commented Dec 7, 2022

vmwclabot commented Dec 7, 2022

vmwclabot commented Dec 7, 2022

Didainius commented Dec 7, 2022

	* `vdc_id` - (Required for tenant users; v3.8.1+) The ID of the [VDC](/providers/vmware/vcd/latest/docs/data-sources/org_vdc) to which the VM Placement Policy is assigned.
	* `vdc_id` - (Required for Org users; v3.8.1+) The ID of the [VDC](/providers/vmware/vcd/latest/docs/data-sources/org_vdc) to which the VM Placement Policy is assigned.

Improve VM Placement Policy data source for tenant users #948

Improve VM Placement Policy data source for tenant users #948

Conversation

adambarreiro commented Dec 1, 2022 • edited

Overview

Detailed changes

Side changes

Tests

terraform-provider-vcd-bot Dec 1, 2022

Choose a reason for hiding this comment

lvirbalas Dec 5, 2022

Choose a reason for hiding this comment

dataclouder Dec 7, 2022

Choose a reason for hiding this comment

lvirbalas Dec 7, 2022

Choose a reason for hiding this comment

Didainius Dec 7, 2022

Choose a reason for hiding this comment

vmwclabot commented Dec 7, 2022

vmwclabot commented Dec 7, 2022

vmwclabot commented Dec 7, 2022

vmwclabot commented Dec 7, 2022

vmwclabot commented Dec 7, 2022

vmwclabot commented Dec 7, 2022

vmwclabot commented Dec 7, 2022

vmwclabot commented Dec 7, 2022

Didainius commented Dec 7, 2022

adambarreiro commented Dec 1, 2022 •

edited