From cb5196d9e7bd62a384b0d7e7544f507345f7bb84 Mon Sep 17 00:00:00 2001 From: Anna Khmelnitsky Date: Fri, 20 Nov 2020 17:11:12 -0800 Subject: [PATCH] Make TZ optional in segment for sake of GM On Global Manager, transport zone can be different for each site, and is auto-assigned on site level. In future, when latest SDK is available, the provider may be extended to specify segment TZ per site. --- nsxt/resource_nsxt_policy_segment_test.go | 70 +++++++++++++++++++ .../resource_nsxt_policy_vlan_segment_test.go | 62 ++++++++++++++++ nsxt/segment_common.go | 12 ++-- website/docs/r/policy_segment.html.markdown | 2 +- .../docs/r/policy_vlan_segment.html.markdown | 2 +- 5 files changed, 142 insertions(+), 6 deletions(-) diff --git a/nsxt/resource_nsxt_policy_segment_test.go b/nsxt/resource_nsxt_policy_segment_test.go index 441f1d306..a6e5ea9e6 100644 --- a/nsxt/resource_nsxt_policy_segment_test.go +++ b/nsxt/resource_nsxt_policy_segment_test.go @@ -172,6 +172,54 @@ func TestAccResourceNsxtPolicySegment_updateAdvConfig(t *testing.T) { }) } +func TestAccResourceNsxtPolicySegment_noTransportZone(t *testing.T) { + name := "test-nsx-policy-segment" + updatedName := fmt.Sprintf("%s-update", name) + testResourceName := "nsxt_policy_segment.test" + cidr := "4003::1/64" + updatedCidr := "4004::1/64" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t); testAccOnlyGlobalManager(t) }, + Providers: testAccProviders, + CheckDestroy: func(state *terraform.State) error { + return testAccNsxtPolicySegmentCheckDestroy(state, name) + }, + Steps: []resource.TestStep{ + { + Config: testAccNsxtPolicySegmentNoTransportZoneTemplate(name, cidr), + Check: resource.ComposeTestCheckFunc( + testAccNsxtPolicySegmentExists(testResourceName), + resource.TestCheckResourceAttr(testResourceName, "display_name", name), + resource.TestCheckResourceAttr(testResourceName, "description", "Acceptance Test"), + resource.TestCheckResourceAttr(testResourceName, "subnet.#", "1"), + resource.TestCheckResourceAttr(testResourceName, "subnet.0.cidr", cidr), + resource.TestCheckResourceAttr(testResourceName, "domain_name", "tftest.org"), + resource.TestCheckResourceAttr(testResourceName, "overlay_id", "1011"), + resource.TestCheckResourceAttr(testResourceName, "vlan_ids.#", "0"), + resource.TestCheckResourceAttr(testResourceName, "tag.#", "0"), + resource.TestCheckResourceAttr(testResourceName, "advanced_config.#", "0"), + ), + }, + { + Config: testAccNsxtPolicySegmentNoTransportZoneTemplate(updatedName, updatedCidr), + Check: resource.ComposeTestCheckFunc( + testAccNsxtPolicySegmentExists(testResourceName), + resource.TestCheckResourceAttr(testResourceName, "display_name", updatedName), + resource.TestCheckResourceAttr(testResourceName, "description", "Acceptance Test"), + resource.TestCheckResourceAttr(testResourceName, "subnet.#", "1"), + resource.TestCheckResourceAttr(testResourceName, "subnet.0.cidr", updatedCidr), + resource.TestCheckResourceAttr(testResourceName, "domain_name", "tftest.org"), + resource.TestCheckResourceAttr(testResourceName, "overlay_id", "1011"), + resource.TestCheckResourceAttr(testResourceName, "vlan_ids.#", "0"), + resource.TestCheckResourceAttr(testResourceName, "tag.#", "0"), + resource.TestCheckResourceAttr(testResourceName, "advanced_config.#", "0"), + ), + }, + }, + }) +} + // TODO: Rewrite this test based on profile resources when these are available. const testAccSegmentQosProfileName = "test-nsx-policy-segment-qos-profile" @@ -645,3 +693,25 @@ resource "nsxt_policy_segment" "test" { } `, name, lease, dnsServerV4, lease, preferred, dnsServerV6) } + +func testAccNsxtPolicySegmentNoTransportZoneTemplate(name string, cidr string) string { + return fmt.Sprintf(` + +resource "nsxt_policy_tier1_gateway" "tier1ForSegments" { + display_name = "terraform-segment-test" + failover_mode = "NON_PREEMPTIVE" +} + +resource "nsxt_policy_segment" "test" { + display_name = "%s" + description = "Acceptance Test" + domain_name = "tftest.org" + overlay_id = 1011 + connectivity_path = nsxt_policy_tier1_gateway.tier1ForSegments.path + + subnet { + cidr = "%s" + } +} +`, name, cidr) +} diff --git a/nsxt/resource_nsxt_policy_vlan_segment_test.go b/nsxt/resource_nsxt_policy_vlan_segment_test.go index 98e0ea463..c4a628849 100644 --- a/nsxt/resource_nsxt_policy_vlan_segment_test.go +++ b/nsxt/resource_nsxt_policy_vlan_segment_test.go @@ -109,6 +109,52 @@ func TestAccResourceNsxtPolicyVlanSegment_updateAdvConfig(t *testing.T) { }) } +func TestAccResourceNsxtPolicyiVlanSegment_noTransportZone(t *testing.T) { + name := "test-nsx-policy-segment" + updatedName := fmt.Sprintf("%s-update", name) + testResourceName := "nsxt_policy_vlan_segment.test" + cidr := "4003::1/64" + updatedCidr := "4004::1/64" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t); testAccOnlyGlobalManager(t) }, + Providers: testAccProviders, + CheckDestroy: func(state *terraform.State) error { + return testAccNsxtPolicySegmentCheckDestroy(state, name) + }, + Steps: []resource.TestStep{ + { + Config: testAccNsxtPolicyVlanSegmentNoTransportZoneTemplate(name, cidr), + Check: resource.ComposeTestCheckFunc( + testAccNsxtPolicySegmentExists(testResourceName), + resource.TestCheckResourceAttr(testResourceName, "display_name", name), + resource.TestCheckResourceAttr(testResourceName, "description", "Acceptance Test"), + resource.TestCheckResourceAttr(testResourceName, "subnet.#", "1"), + resource.TestCheckResourceAttr(testResourceName, "subnet.0.cidr", cidr), + resource.TestCheckResourceAttr(testResourceName, "domain_name", "tftest.org"), + resource.TestCheckResourceAttr(testResourceName, "vlan_ids.#", "1"), + resource.TestCheckResourceAttr(testResourceName, "tag.#", "0"), + resource.TestCheckResourceAttr(testResourceName, "advanced_config.#", "0"), + ), + }, + { + Config: testAccNsxtPolicyVlanSegmentNoTransportZoneTemplate(updatedName, updatedCidr), + Check: resource.ComposeTestCheckFunc( + testAccNsxtPolicySegmentExists(testResourceName), + resource.TestCheckResourceAttr(testResourceName, "display_name", updatedName), + resource.TestCheckResourceAttr(testResourceName, "description", "Acceptance Test"), + resource.TestCheckResourceAttr(testResourceName, "subnet.#", "1"), + resource.TestCheckResourceAttr(testResourceName, "subnet.0.cidr", updatedCidr), + resource.TestCheckResourceAttr(testResourceName, "domain_name", "tftest.org"), + resource.TestCheckResourceAttr(testResourceName, "vlan_ids.#", "1"), + resource.TestCheckResourceAttr(testResourceName, "tag.#", "0"), + resource.TestCheckResourceAttr(testResourceName, "advanced_config.#", "0"), + ), + }, + }, + }) +} + func TestAccResourceNsxtPolicyVlanSegment_withDhcp(t *testing.T) { name := "test-nsx-policy-vlan-segment" updatedName := fmt.Sprintf("%s-update", name) @@ -395,3 +441,19 @@ resource "nsxt_policy_vlan_segment" "test" { } `, getEdgeClusterName(), name, lease, dnsServerV4, lease, preferred, dnsServerV6) } + +func testAccNsxtPolicyVlanSegmentNoTransportZoneTemplate(name string, cidr string) string { + return fmt.Sprintf(` + +resource "nsxt_policy_segment" "test" { + display_name = "%s" + description = "Acceptance Test" + domain_name = "tftest.org" + vlan_ids = ["101"] + + subnet { + cidr = "%s" + } +} +`, name, cidr) +} diff --git a/nsxt/segment_common.go b/nsxt/segment_common.go index 95c4cb423..f30b9c565 100644 --- a/nsxt/segment_common.go +++ b/nsxt/segment_common.go @@ -281,7 +281,7 @@ func getPolicyCommonSegmentSchema(vlanRequired bool, isFixed bool) map[string]*s "transport_zone_path": { Type: schema.TypeString, Description: "Policy path to the transport zone", - Required: true, + Optional: true, ValidateFunc: validatePolicyPath(), }, "vlan_ids": { @@ -618,7 +618,7 @@ func nsxtPolicySegmentAddGatewayToInfraStruct(d *schema.ResourceData, dataValue return dataValue1.(*data.StructValue), nil } -func policySegmentResourceToInfraStruct(id string, d *schema.ResourceData, isVlan bool, isFixed bool) (model.Infra, error) { +func policySegmentResourceToInfraStruct(id string, d *schema.ResourceData, isVlan bool, isFixed bool, isGlobalManager bool) (model.Infra, error) { // Read the rest of the configured parameters var infraChildren []*data.StructValue @@ -631,6 +631,10 @@ func policySegmentResourceToInfraStruct(id string, d *schema.ResourceData, isVla revision := int64(d.Get("revision").(int)) resourceType := "Segment" + if (tzPath == "") && !isGlobalManager { + return model.Infra{}, fmt.Errorf("transport_zone_path needs to be specified for segment on local manager") + } + obj := model.Segment{ Id: &id, DisplayName: &displayName, @@ -1329,7 +1333,7 @@ func nsxtPolicySegmentCreate(d *schema.ResourceData, m interface{}, isVlan bool, return err } - obj, err := policySegmentResourceToInfraStruct(id, d, isVlan, isFixed) + obj, err := policySegmentResourceToInfraStruct(id, d, isVlan, isFixed, isPolicyGlobalManager(m)) if err != nil { return err } @@ -1352,7 +1356,7 @@ func nsxtPolicySegmentUpdate(d *schema.ResourceData, m interface{}, isVlan bool, return fmt.Errorf("Error obtaining Segment ID") } - obj, err := policySegmentResourceToInfraStruct(id, d, isVlan, isFixed) + obj, err := policySegmentResourceToInfraStruct(id, d, isVlan, isFixed, isPolicyGlobalManager(m)) if err != nil { return err } diff --git a/website/docs/r/policy_segment.html.markdown b/website/docs/r/policy_segment.html.markdown index 502672712..aa410207f 100644 --- a/website/docs/r/policy_segment.html.markdown +++ b/website/docs/r/policy_segment.html.markdown @@ -62,7 +62,7 @@ The following arguments are supported: * `domain_name`- (Optional) DNS domain names. * `overlay_id` - (Optional) Overlay connectivity ID for this Segment. * `vlan_ids` - (Optional) List of VLAN IDs or ranges. Specifying vlan ids can be useful for overlay segments, f.e. for EVPN. -* `transport_zone_path` - (Required) Policy path to the Overlay transport zone. This property is required if more than one overlay transport zone is defined, and none is marked as default. +* `transport_zone_path` - (Optional) Policy path to the Overlay transport zone. This property is required for NSX Local Manager, and should not be specified for NSX Global Manager, where NSX will automatically assign default transport zone on each site. * `dhcp_config_path` - (Optional) Policy path to DHCP server or relay configuration to use for subnets configured on this segment. This attribute is supported with NSX 3.0.0 onwards. * `subnet` - (Optional) Subnet configuration block. * `cidr` - (Required) Gateway IP address CIDR. This argument can not be changed if DHCP is enabled for the subnet. diff --git a/website/docs/r/policy_vlan_segment.html.markdown b/website/docs/r/policy_vlan_segment.html.markdown index 0ee012ee6..15eb8ab49 100644 --- a/website/docs/r/policy_vlan_segment.html.markdown +++ b/website/docs/r/policy_vlan_segment.html.markdown @@ -64,7 +64,7 @@ The following arguments are supported: * `tag` - (Optional) A list of scope + tag pairs to associate with this policy. * `nsx_id` - (Optional) The NSX ID of this resource. If set, this ID will be used to create the resource. * `domain_name`- (Optional) DNS domain names. -* `transport_zone_path` - (Required) Policy path to the VLAN backed transport zone. +* `transport_zone_path` - (Optional) Policy path to the VLAN backed transport zone. This property is required for NSX Local Manager, and should not be specified for NSX Global Manager, where NSX will automatically assign default transport zone on each site. * `vlan_ids` - (Optional) List of VLAN IDs or VLAN ranges. * `dhcp_config_path` - (Optional) Policy path to DHCP server or relay configuration to use for subnets configured on this segment. This attribute is supported with NSX 3.0.0 onwards. * `subnet` - (Optional) Subnet configuration block.