diff --git a/nsxt/gateway_common.go b/nsxt/gateway_common.go index 2dd896d87..efefd3455 100644 --- a/nsxt/gateway_common.go +++ b/nsxt/gateway_common.go @@ -90,6 +90,7 @@ func getPolicyLocaleServiceSchema(isTier1 bool) *schema.Schema { } elemSchema := map[string]*schema.Schema{ + "nsx_id": getNsxIDSchema(), "edge_cluster_path": { Type: schema.TypeString, Description: "The path of the edge cluster connected to this gateway", @@ -297,15 +298,25 @@ func initGatewayLocaleServices(context utl.SessionContext, d *schema.ResourceDat } } lsType := "LocaleServices" + idMap := make(map[string]bool) for _, service := range services { cfg := service.(map[string]interface{}) edgeClusterPath := cfg["edge_cluster_path"].(string) edgeNodes := interfaceListToStringList(cfg["preferred_edge_paths"].([]interface{})) path := cfg["path"].(string) + nsxID := cfg["nsx_id"].(string) + // validate unique ids are provided for services + if len(nsxID) > 0 { + if _, ok := idMap[nsxID]; ok { + return nil, fmt.Errorf("Duplicate nsx_id for locale_service %s - please specify unique id", nsxID) + } + idMap[nsxID] = true + } var serviceID string - if path != "" { - serviceID = getPolicyIDFromPath(path) + if nsxID != "" { + log.Printf("[DEBUG] Updating locale service %s", path) + serviceID = nsxID } else { serviceID = newUUID() log.Printf("[DEBUG] Preparing to create locale service %s for gateway %s", serviceID, d.Id()) diff --git a/nsxt/policy_common.go b/nsxt/policy_common.go index 4e22f113b..1912631c7 100644 --- a/nsxt/policy_common.go +++ b/nsxt/policy_common.go @@ -36,6 +36,15 @@ func getNsxIDSchema() *schema.Schema { } } +func getNestedNsxIDSchema() *schema.Schema { + return &schema.Schema{ + Type: schema.TypeString, + Description: "NSX ID for this resource", + Optional: true, + ForceNew: true, + } +} + func getFlexNsxIDSchema(readOnly bool) *schema.Schema { return &schema.Schema{ Type: schema.TypeString, diff --git a/nsxt/resource_nsxt_policy_tier0_gateway.go b/nsxt/resource_nsxt_policy_tier0_gateway.go index 568750a19..60340deb5 100644 --- a/nsxt/resource_nsxt_policy_tier0_gateway.go +++ b/nsxt/resource_nsxt_policy_tier0_gateway.go @@ -931,12 +931,14 @@ func resourceNsxtPolicyTier0GatewayRead(d *schema.ResourceData, m interface{}) e return handleReadError(d, "Locale Service for T0", id, err) } var services []map[string]interface{} - _, shouldSetLS := d.GetOk("locale_service") + intentServices, shouldSetLS := d.GetOk("locale_service") // decide if we should set locale_service or edge_cluser_path // for GM, it is always locale_service; for LM, config dependent if isGlobalManager { shouldSetLS = true } + // map of nsx IDs that was provided in locale_services in intent + nsxIDMap := getAttrKeyMapFromSchemaSet(intentServices, "nsx_id") if len(localeServices) > 0 { for i, service := range localeServices { if shouldSetLS { @@ -946,6 +948,14 @@ func resourceNsxtPolicyTier0GatewayRead(d *schema.ResourceData, m interface{}) e cfgMap["preferred_edge_paths"] = service.PreferredEdgePaths cfgMap["revision"] = service.Revision cfgMap["display_name"] = service.DisplayName + // to avoid diff and recreation of locale service, we set nsx_id only + // if user specified it in the intent. + // this workaround is necessary due to lack of proper support for computed + // values in TypeSet + // TODO: refactor this post upgrade to plugin framework + if _, ok := nsxIDMap[*service.Id]; ok { + cfgMap["nsx_id"] = service.Id + } redistributionConfigs := getLocaleServiceRedistributionConfig(&localeServices[i]) if d.Get("redistribution_set").(bool) { // redistribution_config is deprecated and should be diff --git a/nsxt/resource_nsxt_policy_tier0_gateway_test.go b/nsxt/resource_nsxt_policy_tier0_gateway_test.go index e4f14b68c..f200b696e 100644 --- a/nsxt/resource_nsxt_policy_tier0_gateway_test.go +++ b/nsxt/resource_nsxt_policy_tier0_gateway_test.go @@ -5,6 +5,7 @@ package nsxt import ( "fmt" + "regexp" "testing" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" @@ -65,6 +66,52 @@ func TestAccResourceNsxtPolicyTier0Gateway_basic(t *testing.T) { }) } +func TestAccResourceNsxtPolicyTier0Gateway_localeService(t *testing.T) { + name := getAccTestResourceName() + testResourceName := "nsxt_policy_tier0_gateway.test" + regexpPath, err := regexp.Compile("/.*/" + name) + if err != nil { + t.Errorf("Error while compiling regexp: %v", err) + return + } + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t); testAccNSXVersion(t, "3.0.0") }, + Providers: testAccProviders, + CheckDestroy: func(state *terraform.State) error { + return testAccNsxtPolicyTier0CheckDestroy(state, name) + }, + Steps: []resource.TestStep{ + { + Config: testAccNsxtPolicyTier0CreateWithLocaleTemplate(name), + Check: resource.ComposeTestCheckFunc( + testAccNsxtPolicyTier0Exists(testResourceName), + resource.TestCheckResourceAttr(testResourceName, "display_name", name), + resource.TestCheckResourceAttr(testResourceName, "ha_mode", "ACTIVE_STANDBY"), + resource.TestCheckResourceAttr(testResourceName, "locale_service.#", "1"), + resource.TestCheckResourceAttr(testResourceName, "locale_service.0.nsx_id", name), + resource.TestMatchResourceAttr(testResourceName, "locale_service.0.path", regexpPath), + resource.TestCheckResourceAttrSet(testResourceName, "path"), + resource.TestCheckResourceAttrSet(testResourceName, "revision"), + ), + }, + { + Config: testAccNsxtPolicyTier0UpdateWithLocaleTemplate(name), + Check: resource.ComposeTestCheckFunc( + testAccNsxtPolicyTier0Exists(testResourceName), + resource.TestCheckResourceAttr(testResourceName, "display_name", name), + resource.TestCheckResourceAttr(testResourceName, "ha_mode", "ACTIVE_STANDBY"), + resource.TestCheckResourceAttr(testResourceName, "locale_service.#", "1"), + resource.TestCheckResourceAttr(testResourceName, "locale_service.0.nsx_id", name), + resource.TestMatchResourceAttr(testResourceName, "locale_service.0.path", regexpPath), + resource.TestCheckResourceAttrSet(testResourceName, "path"), + resource.TestCheckResourceAttrSet(testResourceName, "revision"), + ), + }, + }, + }) +} + func TestAccResourceNsxtPolicyTier0Gateway_withId(t *testing.T) { name := getAccTestResourceName() id := "test-id" @@ -664,6 +711,50 @@ resource "nsxt_policy_tier0_gateway" "test" { }`, id, name, testAccNsxtPolicyTier0EdgeClusterTemplate()) } +func testAccNsxtPolicyTier0CreateWithLocaleTemplate(name string) string { + config := testAccNsxtPolicyGatewayFabricDeps(true) + fmt.Sprintf(` +data "nsxt_policy_edge_node" "node1" { + edge_cluster_path = data.nsxt_policy_edge_cluster.EC.path + member_index = 0 +} + +resource "nsxt_policy_tier0_gateway" "test" { + display_name = "%s" + failover_mode = "PREEMPTIVE" + ha_mode = "ACTIVE_STANDBY" + + locale_service { + nsx_id = "%s" + edge_cluster_path = data.nsxt_policy_edge_cluster.EC.path + preferred_edge_paths = [data.nsxt_policy_edge_node.node1.path] + } +}`, name, name) + + return testAccAdjustPolicyInfraConfig(config) +} + +func testAccNsxtPolicyTier0UpdateWithLocaleTemplate(name string) string { + config := testAccNsxtPolicyGatewayFabricDeps(true) + fmt.Sprintf(` +data "nsxt_policy_edge_node" "node1" { + edge_cluster_path = data.nsxt_policy_edge_cluster.EC.path + member_index = 1 +} + +resource "nsxt_policy_tier0_gateway" "test" { + display_name = "%s" + failover_mode = "PREEMPTIVE" + ha_mode = "ACTIVE_STANDBY" + + locale_service { + nsx_id = "%s" + edge_cluster_path = data.nsxt_policy_edge_cluster.EC.path + preferred_edge_paths = [data.nsxt_policy_edge_node.node1.path] + } +}`, name, name) + + return testAccAdjustPolicyInfraConfig(config) +} + func testAccNsxtPolicyTier0SubnetsTemplate(name string) string { return fmt.Sprintf(` resource "nsxt_policy_tier0_gateway" "test" { diff --git a/nsxt/resource_nsxt_policy_tier1_gateway.go b/nsxt/resource_nsxt_policy_tier1_gateway.go index 033a256ca..3d4beaa65 100644 --- a/nsxt/resource_nsxt_policy_tier1_gateway.go +++ b/nsxt/resource_nsxt_policy_tier1_gateway.go @@ -582,10 +582,12 @@ func resourceNsxtPolicyTier1GatewayRead(d *schema.ResourceData, m interface{}) e return handleReadError(d, "Locale Service for T1", id, err) } var services []map[string]interface{} - _, shouldSetLS := d.GetOk("locale_service") + intentServices, shouldSetLS := d.GetOk("locale_service") if context.ClientType == utl.Global { shouldSetLS = true } + // map of nsx IDs that was provided in locale_services in intent + nsxIDMap := getAttrKeyMapFromSchemaSet(intentServices, "nsx_id") if len(localeServices) > 0 { for _, service := range localeServices { @@ -596,6 +598,14 @@ func resourceNsxtPolicyTier1GatewayRead(d *schema.ResourceData, m interface{}) e cfgMap["preferred_edge_paths"] = service.PreferredEdgePaths cfgMap["revision"] = service.Revision cfgMap["display_name"] = service.DisplayName + // to avoid diff and recreation of locale service, we set nsx_id only + // if user specified it in the intent. + // this workaround is necessary due to lack of proper support for computed + // values in TypeSet + // TODO: refactor this post upgrade to plugin framework + if _, ok := nsxIDMap[*service.Id]; ok { + cfgMap["nsx_id"] = service.Id + } services = append(services, cfgMap) } else { diff --git a/nsxt/utils.go b/nsxt/utils.go index 146a7e551..2cbadbc06 100644 --- a/nsxt/utils.go +++ b/nsxt/utils.go @@ -70,6 +70,22 @@ func getStringListFromSchemaList(d *schema.ResourceData, schemaAttrName string) return interface2StringList(d.Get(schemaAttrName).([]interface{})) } +// helper to construct a map based on curtain attribute in schema list +// this is useful when Read function needs to make a decision based on intent provided +// by user in a nested schema +func getAttrKeyMapFromSchemaSet(schemaSet interface{}, attrName string) map[string]bool { + + keyMap := make(map[string]bool) + for _, item := range schemaSet.(*schema.Set).List() { + mapItem := item.(map[string]interface{}) + if value, ok := mapItem[attrName]; ok { + keyMap[value.(string)] = true + } + } + + return keyMap +} + func intList2int64List(configured []interface{}) []int64 { vs := make([]int64, 0, len(configured)) for _, v := range configured { diff --git a/website/docs/r/policy_tier0_gateway.html.markdown b/website/docs/r/policy_tier0_gateway.html.markdown index caa9cfd48..eb75139c6 100644 --- a/website/docs/r/policy_tier0_gateway.html.markdown +++ b/website/docs/r/policy_tier0_gateway.html.markdown @@ -121,6 +121,7 @@ The following arguments are supported: * `nsx_id` - (Optional) The NSX ID of this resource. If set, this ID will be used to create the policy resource. * `edge_cluster_path` - (Optional) The path of the edge cluster where the Tier-0 is placed.For advanced configuration and on Global Manager, use `locale_service` clause instead. Note that for some configurations (such as BGP) setting edge cluster is required. * `locale_service` - (Optional) This is required on NSX Global Manager. Multiple locale services can be specified for multiple locations. + * `nsx_id` - (Optional) NSX id for the locale service. It is recommended to specify this attribute in order to avoid unnecessary recreation of this object. Should be unique within the gateway. * `edge_cluster_path` - (Required) The path of the edge cluster where the Tier-0 is placed. * `preferred_edge_paths` - (Optional) Policy paths to edge nodes. Specified edge is used as preferred edge cluster member when failover mode is set to `PREEMPTIVE`. * `display_name` - (Optional) Display name for the locale service. diff --git a/website/docs/r/policy_tier1_gateway.html.markdown b/website/docs/r/policy_tier1_gateway.html.markdown index 839613512..8be05f920 100644 --- a/website/docs/r/policy_tier1_gateway.html.markdown +++ b/website/docs/r/policy_tier1_gateway.html.markdown @@ -119,6 +119,7 @@ The following arguments are supported: * `project_id` - (Required) The ID of the project which the object belongs to * `edge_cluster_path` - (Optional) The path of the edge cluster where the Tier-1 is placed.For advanced configuration, use `locale_service` clause instead. * `locale_service` - (Optional) This argument is required on NSX Global Manager. Multiple locale services can be specified for multiple locations. + * `nsx_id` - (Optional) NSX id for the locale service. It is recommended to specify this attribute in order to avoid unnecessary recreation of this object. Should be unique within the gateway. * `edge_cluster_path` - (Required) The path of the edge cluster where the Tier-0 is placed. * `preferred_edge_paths` - (Optional) Policy paths to edge nodes. Specified edge is used as preferred edge cluster member when failover mode is set to `PREEMPTIVE`. * `display_name` - (Optional) Display name for the locale service.