Add support for Service Engine Group assignment to NSX-T Edge Gateway #738

Didainius · 2021-11-05T14:21:00Z

This PR adds resource and datasource to manage NSX-T ALB Service Engine Group assignment to Edge Gateways. It is named quite lengthy vcd_nsxt_alb_edgegateway_service_engine_group, but should reflect that this resource handle Service Engine Group assignments to Edge gateways as opposed to resource vcd_nsxt_alb_service_engine_group defining Service Engine Groups for provider in general.

Note. This is the last "setup" resource that requires System user. The next resources should be manageable by Org users and allow instantiate load balancers based on this setup.

Also:

updates Code Owners to GitHub configuration so that automatic code reviews are requested.
Adds a missing resource link to vcd_library_certificate in vcd.erb.
Cleans up main page shown in GitHub repo homepage README.md - changed old links to new ones, fixed broken logo link and rebranded remaining "vCloud Director" names to "VMware Cloud Director"
~~Improves make test-binary-validate so that it tests out terraform fmt -check on binary test files~~ (moved out to separate PR due to many fixes Fix 'make test-binary-validate' command and all HCL errors #746)

Acceptance and binary tests passed on 10.2 and 10.3

Signed-off-by: Dainius Serplis <[email protected]>

dataclouder

LGTM, with some minor formatting issues

dataclouder · 2021-11-15T14:16:23Z

vcd/resource_vcd_nsxt_alb_edgegateway_service_engine_group_test.go

+`
+
+const testAccVcdNsxtAlbEdgeGatewayServiceEngineGroupDedicatedDS = testAccVcdNsxtAlbEdgeGatewayServiceEngineGroupDedicated + `
+data "vcd_nsxt_alb_edgegateway_service_engine_group" "test" {


This script is not formatted.
OT: It seems that make test-binary-validate doesn't check formatting. Needs to be updated

Fixed and ~~updated make test-binary-validate~~ (HCL validator fixes went to separate PR #746)

dataclouder · 2021-11-15T14:16:50Z

vcd/resource_vcd_nsxt_alb_edgegateway_service_engine_group_test.go

+}
+
+const testAccVcdNsxtAlbEdgeGatewayServiceEngineGroupShared = testAccVcdNsxtAlbGeneralSettings + `
+resource "vcd_nsxt_alb_edgegateway_service_engine_group" "test" {


This script is not formatted

Fixed and ~~updated make test-binary-validate~~ (HCL validator fixes went to separate PR #746)

dataclouder · 2021-11-15T14:17:44Z

vcd/resource_vcd_nsxt_alb_edgegateway_service_engine_group_test.go

+`
+
+const testAccVcdNsxtAlbEdgeServiceEngineGroupSharedDS = testAccVcdNsxtAlbEdgeGatewayServiceEngineGroupDedicated + `
+data "vcd_nsxt_alb_edgegateway_service_engine_group" "test" {


This script is not formatted

Fixed and ~~updated make test-binary-validate~~ (HCL validator fixes went to separate PR #746)

dataclouder · 2021-11-15T14:17:50Z

vcd/resource_vcd_nsxt_alb_edgegateway_service_engine_group_test.go

+`
+
+const testAccVcdNsxtAlbEdgeGatewayServiceEngineGroupSharedStep3 = testAccVcdNsxtAlbGeneralSettings + `
+resource "vcd_nsxt_alb_edgegateway_service_engine_group" "test" {


This script is not formatted

Fixed and ~~updated make test-binary-validate~~ (HCL validator fixes went to separate PR #746)

vbauzys · 2021-11-16T15:33:24Z

vcd/resource_vcd_nsxt_alb_edgegateway_service_engine_group.go

+		return diag.Errorf("error reading ALB Service Engine Group assignment to Edge Gateway: %s", err)
+	}
+	edgeAlbServiceEngineGroupAssignmentConfig := getAlbServiceEngineGroupAssignmentType(d)
+	// Inject correct ID for update


Suggested change

// Inject correct ID for update

// Add correct ID for update

Maybe just Add is enough to avoid to much geekness :))))

vbauzys · 2021-11-16T15:37:27Z

vcd/resource_vcd_nsxt_alb_edgegateway_service_engine_group.go

+	}
+
+	return edgeAlbServiceEngineAssigmentConfig
+}


It's just me a little lost. Is AlbServiceEngineGroup or AlbServiceEngineGroupAssigment. In govcd I see AlbServiceEngineGroupAssigment in terraform I see both things. Are they different or the same thing with two names?

The Assignment word in govcd comes from API (loadBalancer/serviceEngineGroups/assignments/) however VCD UI does not use the term Assignment. The idea is that govcd follows API, while in Terraform we follow UI naming. In this case this was a leftover variable name from initial coding. I have removed the 'assignment' word.

vbauzys · 2021-11-16T15:38:38Z

vcd/resource_vcd_nsxt_alb_edgegateway_service_engine_group_test.go

+
+const testAccVcdNsxtAlbEdgeGatewayServiceEngineGroupDedicated = testAccVcdNsxtAlbGeneralSettings + `
+resource "vcd_nsxt_alb_edgegateway_service_engine_group" "test" {
+	edge_gateway_id = data.vcd_nsxt_edgegateway.existing.id


script not formatted

Fixed and ~~updated make test-binary-validate~~ (HCL validator fixes went to separate PR #746)

vbauzys · 2021-11-16T15:45:23Z

website/docs/r/nsxt_alb_edgegateway_service_engine_group.html.markdown

+[docs-import]: https://www.terraform.io/docs/import/
+
+```
+terraform import vcd_nsxt_alb_settings.imported my-org.my-vdc.my-nsxt-edge-gateway-name.service-engine-group-name


Is service-engine-group-name is unique?

It is an interesting story, because this name comes exactly as in the nsxt_alb_service_engine_group resource. It can't be specified and comes in Unique from a layer above.

Signed-off-by: Dainius Serplis <[email protected]>

This reverts commit 3919d0c. Signed-off-by: Dainius Serplis <[email protected]>

lvirbalas

Very clean, just one nit pick :)

lvirbalas · 2021-11-16T13:50:33Z

website/docs/d/nsxt_alb_edgegateway_service_engine_group.html.markdown

+  name = "first-se"
+}
+
+


Extra new line. BTW, could we capture such double new lines with the script? It's a rather common slip :)

Fixed this one. Will try to do an automated check.

Signed-off-by: Dainius Serplis <[email protected]>

Didainius force-pushed the alb_seg-pr branch from 306ec45 to 1561e15 Compare November 5, 2021 14:22

Didainius self-assigned this Nov 5, 2021

Didainius force-pushed the alb_seg-pr branch from 1561e15 to 900e400 Compare November 5, 2021 14:47

Add support for Service Engine Groupp assignment to NSX-T Edge Gateway

d1f38c2

Signed-off-by: Dainius Serplis <[email protected]>

Didainius force-pushed the alb_seg-pr branch from 900e400 to d1f38c2 Compare November 8, 2021 08:20

Name -> vcd_nsxt_alb_edgegateway_service_engine_group

cdf5880

Signed-off-by: Dainius Serplis <[email protected]>

Didainius force-pushed the alb_seg-pr branch 3 times, most recently from 07c7fce to f0c559c Compare November 9, 2021 07:28

Didainius added 2 commits November 9, 2021 10:18

More naming cleanup in functions and comments

7c09db0

Signed-off-by: Dainius Serplis <[email protected]>

Merge branch 'master' into alb_seg-pr

19f9e27

Signed-off-by: Dainius Serplis <[email protected]>

Didainius force-pushed the alb_seg-pr branch from f0c559c to 19f9e27 Compare November 9, 2021 08:27

Didainius added 5 commits November 9, 2021 10:31

Fix HCL check

52acf8f

Signed-off-by: Dainius Serplis <[email protected]>

Adjust code owners

ef2d261

Signed-off-by: Dainius Serplis <[email protected]>

[skip ci] fix PR number in changelog

7ff85bf

Signed-off-by: Dainius Serplis <[email protected]>

bump govcd

ebc8d21

Signed-off-by: Dainius Serplis <[email protected]>

Merge master

59617bb

Signed-off-by: Dainius Serplis <[email protected]>

Didainius changed the title ~~Add support for Service Engine Groupp assignment to NSX-T Edge Gateway~~ Add support for Service Engine Group assignment to NSX-T Edge Gateway Nov 9, 2021

Didainius marked this pull request as ready for review November 9, 2021 10:17

Didainius requested review from dataclouder and lvirbalas as code owners November 9, 2021 10:17

Didainius requested a review from vbauzys November 9, 2021 10:17

Didainius added 3 commits November 9, 2021 13:39

Merge branch 'master' into alb_seg-pr

5ba39cd

Signed-off-by: Dainius Serplis <[email protected]>

convert d.Set to dSet

ad7af24

Signed-off-by: Dainius Serplis <[email protected]>

[skip ci] - cleanup of README.md

571d324

Signed-off-by: Dainius Serplis <[email protected]>

dataclouder approved these changes Nov 15, 2021

View reviewed changes

vbauzys reviewed Nov 16, 2021

View reviewed changes

vbauzys approved these changes Nov 16, 2021

View reviewed changes

Merge branch 'master' into alb_seg-pr

c9d1883

Signed-off-by: Dainius Serplis <[email protected]>

Didainius added 4 commits November 17, 2021 14:23

fmt HCL formatting in tests

27c1aaa

Signed-off-by: Dainius Serplis <[email protected]>

Include fmt validation in make 'test-binary-validate'

3919d0c

Signed-off-by: Dainius Serplis <[email protected]>

change comment

61b1ba9

Signed-off-by: Dainius Serplis <[email protected]>

Revert "Include fmt validation in make 'test-binary-validate'"

592c3ce

This reverts commit 3919d0c. Signed-off-by: Dainius Serplis <[email protected]>

Didainius mentioned this pull request Nov 19, 2021

Fix 'make test-binary-validate' command and all HCL errors #746

Closed

lvirbalas approved these changes Nov 19, 2021

View reviewed changes

Didainius added 3 commits November 19, 2021 14:42

Remove extraneus newline in HCL example

8f2919a

Signed-off-by: Dainius Serplis <[email protected]>

bump govcd

74dcbfd

Signed-off-by: Dainius Serplis <[email protected]>

Include govcd v2.14.0-alpha.4

90c467c

Signed-off-by: Dainius Serplis <[email protected]>

Didainius merged commit 72cb07a into vmware:master Nov 19, 2021

Didainius deleted the alb_seg-pr branch November 19, 2021 13:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Service Engine Group assignment to NSX-T Edge Gateway #738

Add support for Service Engine Group assignment to NSX-T Edge Gateway #738

Didainius commented Nov 5, 2021 •

edited

Loading

dataclouder left a comment

dataclouder Nov 15, 2021

Didainius Nov 17, 2021 •

edited

Loading

dataclouder Nov 15, 2021

Didainius Nov 18, 2021

dataclouder Nov 15, 2021

Didainius Nov 17, 2021 •

edited

Loading

dataclouder Nov 15, 2021

Didainius Nov 17, 2021 •

edited

Loading

vbauzys Nov 16, 2021

Didainius Nov 17, 2021

vbauzys Nov 16, 2021 •

edited

Loading

Didainius Nov 18, 2021

vbauzys Nov 16, 2021

Didainius Nov 17, 2021 •

edited

Loading

vbauzys Nov 16, 2021

Didainius Nov 17, 2021 •

edited

Loading

lvirbalas left a comment

lvirbalas Nov 16, 2021

Didainius Nov 19, 2021

Add support for Service Engine Group assignment to NSX-T Edge Gateway #738

Add support for Service Engine Group assignment to NSX-T Edge Gateway #738

Conversation

Didainius commented Nov 5, 2021 • edited Loading

dataclouder left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Didainius Nov 17, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Didainius Nov 17, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Didainius Nov 17, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vbauzys Nov 16, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Didainius Nov 17, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Didainius Nov 17, 2021 • edited Loading

Choose a reason for hiding this comment

lvirbalas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Didainius commented Nov 5, 2021 •

edited

Loading

Didainius Nov 17, 2021 •

edited

Loading

Didainius Nov 17, 2021 •

edited

Loading

Didainius Nov 17, 2021 •

edited

Loading

vbauzys Nov 16, 2021 •

edited

Loading

Didainius Nov 17, 2021 •

edited

Loading

Didainius Nov 17, 2021 •

edited

Loading