From e84a55d5f8e4bc8f5efe68470744bcaf4ba8365d Mon Sep 17 00:00:00 2001 From: Anna Khmelnitsky Date: Thu, 4 Feb 2021 11:58:54 -0800 Subject: [PATCH] Support bgp config resource for Local Manager This resource should not be used together with bgp_config clause in T0 gateway. A corresponding note was added to the doc. --- nsxt/gateway_common.go | 16 ++ nsxt/policy_utils.go | 17 --- nsxt/resource_nsxt_policy_bgp_config.go | 137 +++++++++++------- nsxt/resource_nsxt_policy_bgp_config_test.go | 44 +++--- nsxt/resource_nsxt_policy_ospf_config.go | 11 +- .../docs/r/policy_bgp_config.html.markdown | 24 ++- 6 files changed, 143 insertions(+), 106 deletions(-) diff --git a/nsxt/gateway_common.go b/nsxt/gateway_common.go index 9b3ab4268..ffc8c37f0 100644 --- a/nsxt/gateway_common.go +++ b/nsxt/gateway_common.go @@ -554,3 +554,19 @@ func parseGatewayInterfacePolicyPath(path string) (bool, string, string, string) return isT0, gwID, localeServiceID, interfaceID } + +func getComputedLocaleServiceIDSchema() *schema.Schema { + return &schema.Schema{ + Type: schema.TypeString, + Description: "NSX ID of associated Gateway Locale Service", + Computed: true, + } +} + +func getComputedGatewayIDSchema() *schema.Schema { + return &schema.Schema{ + Type: schema.TypeString, + Description: "NSX ID of associated Tier0 Gateway", + Computed: true, + } +} diff --git a/nsxt/policy_utils.go b/nsxt/policy_utils.go index 55a936c0d..7f901542a 100644 --- a/nsxt/policy_utils.go +++ b/nsxt/policy_utils.go @@ -14,7 +14,6 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/vmware/vsphere-automation-sdk-go/runtime/bindings" "github.com/vmware/vsphere-automation-sdk-go/runtime/protocol/client" - gm_model "github.com/vmware/vsphere-automation-sdk-go/services/nsxt-gm/model" "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/infra/realized_state" "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" ) @@ -110,22 +109,6 @@ func setPolicyTagsInSchema(d *schema.ResourceData, tags []model.Tag) { setCustomizedPolicyTagsInSchema(d, tags, "tag") } -func getPolicyGlobalManagerTagsFromSchema(d *schema.ResourceData) []gm_model.Tag { - tags := d.Get("tag").(*schema.Set).List() - var tagList []gm_model.Tag - for _, tag := range tags { - data := tag.(map[string]interface{}) - tagScope := data["scope"].(string) - tagTag := data["tag"].(string) - elem := gm_model.Tag{ - Scope: &tagScope, - Tag: &tagTag} - - tagList = append(tagList, elem) - } - return tagList -} - func getPathListFromMap(data map[string]interface{}, attrName string) []string { pathList := interface2StringList(data[attrName].(*schema.Set).List()) if len(pathList) == 0 { diff --git a/nsxt/resource_nsxt_policy_bgp_config.go b/nsxt/resource_nsxt_policy_bgp_config.go index e85f79e0f..bf277cbe3 100644 --- a/nsxt/resource_nsxt_policy_bgp_config.go +++ b/nsxt/resource_nsxt_policy_bgp_config.go @@ -9,15 +9,16 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" gm_locale_services "github.com/vmware/vsphere-automation-sdk-go/services/nsxt-gm/global_infra/tier_0s/locale_services" gm_model "github.com/vmware/vsphere-automation-sdk-go/services/nsxt-gm/model" + "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/infra/tier_0s/locale_services" "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" ) -// Note: this resource is supported for global manager only -// TODO: consider supporting it for local manager and deprecating bgp config on T0 gateway func resourceNsxtPolicyBgpConfig() *schema.Resource { bgpSchema := getPolicyBGPConfigSchema() bgpSchema["gateway_path"] = getPolicyPathSchema(true, true, "Gateway for this BGP config") - bgpSchema["site_path"] = getPolicyPathSchema(true, true, "Site Path for this BGP config") + bgpSchema["site_path"] = getPolicyPathSchema(false, true, "Site Path for this BGP config") + bgpSchema["gateway_id"] = getComputedGatewayIDSchema() + bgpSchema["locale_service_id"] = getComputedLocaleServiceIDSchema() return &schema.Resource{ Create: resourceNsxtPolicyBgpConfigCreate, @@ -30,32 +31,35 @@ func resourceNsxtPolicyBgpConfig() *schema.Resource { } func resourceNsxtPolicyBgpConfigRead(d *schema.ResourceData, m interface{}) error { - if !isPolicyGlobalManager(m) { - return globalManagerOnlyError() - } connector := getPolicyConnector(m) gwPath := d.Get("gateway_path").(string) - gwID := getPolicyIDFromPath(gwPath) - sitePath := d.Get("site_path").(string) - if !isPolicyGlobalManager(m) { - return fmt.Errorf("This resource is not supported for local manager") - } - - serviceID, err := findTier0LocaleServiceForSite(connector, gwID, sitePath) - if err != nil { - return err - } - client := gm_locale_services.NewDefaultBgpClient(connector) - gmObj, err := client.Get(gwID, serviceID) - if err != nil { - return handleReadError(d, "BGP Config", serviceID, err) - } - lmObj, convErr := convertModelBindingType(gmObj, gm_model.BgpRoutingConfigBindingType(), model.BgpRoutingConfigBindingType()) - if convErr != nil { - return convErr + isT0, gwID := parseGatewayPolicyPath(gwPath) + if !isT0 { + return fmt.Errorf("Tier0 Gateway path expected, got %s", gwPath) + } + serviceID := d.Get("locale_service_id").(string) + var lmRoutingConfig model.BgpRoutingConfig + if isPolicyGlobalManager(m) { + + client := gm_locale_services.NewDefaultBgpClient(connector) + gmObj, err := client.Get(gwID, serviceID) + if err != nil { + return handleReadError(d, "BGP Config", serviceID, err) + } + lmObj, convErr := convertModelBindingType(gmObj, gm_model.BgpRoutingConfigBindingType(), model.BgpRoutingConfigBindingType()) + if convErr != nil { + return convErr + } + lmRoutingConfig = lmObj.(model.BgpRoutingConfig) + } else { + var err error + client := locale_services.NewDefaultBgpClient(connector) + lmRoutingConfig, err = client.Get(gwID, serviceID) + if err != nil { + return handleReadError(d, "BGP Config", serviceID, err) + } } - lmRoutingConfig := lmObj.(model.BgpRoutingConfig) data := initPolicyTier0BGPConfigMap(&lmRoutingConfig) @@ -66,7 +70,7 @@ func resourceNsxtPolicyBgpConfigRead(d *schema.ResourceData, m interface{}) erro return nil } -func resourceNsxtPolicyBgpConfigToStruct(d *schema.ResourceData) (*gm_model.BgpRoutingConfig, error) { +func resourceNsxtPolicyBgpConfigToStruct(d *schema.ResourceData) (*model.BgpRoutingConfig, error) { ecmp := d.Get("ecmp").(bool) enabled := d.Get("enabled").(bool) interSrIbgp := d.Get("inter_sr_ibgp").(bool) @@ -75,16 +79,16 @@ func resourceNsxtPolicyBgpConfigToStruct(d *schema.ResourceData) (*gm_model.BgpR restartMode := d.Get("graceful_restart_mode").(string) restartTimer := int64(d.Get("graceful_restart_timer").(int)) staleTimer := int64(d.Get("graceful_restart_stale_route_timer").(int)) - tags := getPolicyGlobalManagerTagsFromSchema(d) + tags := getPolicyTagsFromSchema(d) - var aggregationStructs []gm_model.RouteAggregationEntry + var aggregationStructs []model.RouteAggregationEntry routeAggregations := d.Get("route_aggregation").([]interface{}) if len(routeAggregations) > 0 { for _, agg := range routeAggregations { data := agg.(map[string]interface{}) prefix := data["prefix"].(string) summary := data["summary_only"].(bool) - elem := gm_model.RouteAggregationEntry{ + elem := model.RouteAggregationEntry{ Prefix: &prefix, SummaryOnly: &summary, } @@ -93,17 +97,17 @@ func resourceNsxtPolicyBgpConfigToStruct(d *schema.ResourceData) (*gm_model.BgpR } } - restartTimerStruct := gm_model.BgpGracefulRestartTimer{ + restartTimerStruct := model.BgpGracefulRestartTimer{ RestartTimer: &restartTimer, StaleRouteTimer: &staleTimer, } - restartConfigStruct := gm_model.BgpGracefulRestartConfig{ + restartConfigStruct := model.BgpGracefulRestartConfig{ Mode: &restartMode, Timer: &restartTimerStruct, } - routeStruct := gm_model.BgpRoutingConfig{ + routeStruct := model.BgpRoutingConfig{ Ecmp: &ecmp, Enabled: &enabled, RouteAggregations: aggregationStructs, @@ -118,14 +122,14 @@ func resourceNsxtPolicyBgpConfigToStruct(d *schema.ResourceData) (*gm_model.BgpR } func resourceNsxtPolicyBgpConfigCreate(d *schema.ResourceData, m interface{}) error { - if !isPolicyGlobalManager(m) { - return globalManagerOnlyError() - } // This is not a create operation on NSX, since BGP config us auto created connector := getPolicyConnector(m) gwPath := d.Get("gateway_path").(string) - gwID := getPolicyIDFromPath(gwPath) + isT0, gwID := parseGatewayPolicyPath(gwPath) + if !isT0 { + return fmt.Errorf("Tier0 Gateway path expected, got %s", gwPath) + } sitePath := d.Get("site_path").(string) obj, err := resourceNsxtPolicyBgpConfigToStruct(d) @@ -133,17 +137,40 @@ func resourceNsxtPolicyBgpConfigCreate(d *schema.ResourceData, m interface{}) er return handleCreateError("BgpRoutingConfig", gwID, err) } - serviceID, err := findTier0LocaleServiceForSite(connector, gwID, sitePath) - if err != nil { - return handleCreateError("BgpRoutingConfig", gwID, err) - } + var localeServiceID string + if isPolicyGlobalManager(m) { + serviceID, err1 := findTier0LocaleServiceForSite(connector, gwID, sitePath) + if err1 != nil { + return handleCreateError("BgpRoutingConfig", gwID, err1) + } - client := gm_locale_services.NewDefaultBgpClient(connector) - err = client.Patch(gwID, serviceID, *obj, nil) + localeServiceID = serviceID + + gmObj, convErr := convertModelBindingType(obj, model.BgpRoutingConfigBindingType(), gm_model.BgpRoutingConfigBindingType()) + if convErr != nil { + return convErr + } + gmRoutingConfig := gmObj.(gm_model.BgpRoutingConfig) + + client := gm_locale_services.NewDefaultBgpClient(connector) + err = client.Patch(gwID, serviceID, gmRoutingConfig, nil) + } else { + localeService, err1 := getPolicyTier0GatewayLocaleServiceWithEdgeCluster(gwID, connector) + if err1 != nil { + return fmt.Errorf("Tier0 Gateway path with configured edge cluster expected, got %s", gwPath) + } + localeServiceID = *localeService.Id + + client := locale_services.NewDefaultBgpClient(connector) + err = client.Patch(gwID, localeServiceID, *obj, nil) + } if err != nil { return handleCreateError("BgpRoutingConfig", gwID, err) } + d.SetId(newUUID()) + d.Set("gateway_id", gwID) + d.Set("locale_service_id", localeServiceID) return resourceNsxtPolicyBgpConfigRead(d, m) } @@ -151,25 +178,31 @@ func resourceNsxtPolicyBgpConfigCreate(d *schema.ResourceData, m interface{}) er func resourceNsxtPolicyBgpConfigUpdate(d *schema.ResourceData, m interface{}) error { connector := getPolicyConnector(m) - gwPath := d.Get("gateway_path").(string) - gwID := getPolicyIDFromPath(gwPath) - sitePath := d.Get("site_path").(string) revision := int64(d.Get("revision").(int)) + gwPath := d.Get("gateway_path").(string) + _, gwID := parseGatewayPolicyPath(gwPath) + serviceID := d.Get("locale_service_id").(string) obj, err := resourceNsxtPolicyBgpConfigToStruct(d) if err != nil { return handleUpdateError("BgpRoutingConfig", gwID, err) } - serviceID, err := findTier0LocaleServiceForSite(connector, gwID, sitePath) - if err != nil { - return handleUpdateError("BgpRoutingConfig", gwID, err) - } - obj.Revision = &revision + if isPolicyGlobalManager(m) { + gmObj, convErr := convertModelBindingType(obj, model.BgpRoutingConfigBindingType(), gm_model.BgpRoutingConfigBindingType()) + if convErr != nil { + return convErr + } + gmRoutingConfig := gmObj.(gm_model.BgpRoutingConfig) + + client := gm_locale_services.NewDefaultBgpClient(connector) + _, err = client.Update(gwID, serviceID, gmRoutingConfig, nil) + } else { + client := locale_services.NewDefaultBgpClient(connector) + _, err = client.Update(gwID, serviceID, *obj, nil) + } - client := gm_locale_services.NewDefaultBgpClient(connector) - _, err = client.Update(gwID, serviceID, *obj, nil) if err != nil { return handleUpdateError("BgpRoutingConfig", gwID, err) } diff --git a/nsxt/resource_nsxt_policy_bgp_config_test.go b/nsxt/resource_nsxt_policy_bgp_config_test.go index 865939de2..0bfacb25d 100644 --- a/nsxt/resource_nsxt_policy_bgp_config_test.go +++ b/nsxt/resource_nsxt_policy_bgp_config_test.go @@ -38,7 +38,7 @@ func TestAccResourceNsxtPolicyBgpConfig_basic(t *testing.T) { testResourceName := "nsxt_policy_bgp_config.test" resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccOnlyGlobalManager(t); testAccPreCheck(t) }, + PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, Steps: []resource.TestStep{ { @@ -85,7 +85,7 @@ func TestAccResourceNsxtPolicyBgpConfig_minimalistic(t *testing.T) { testResourceName := "nsxt_policy_bgp_config.test" resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccOnlyGlobalManager(t); testAccPreCheck(t) }, + PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, Steps: []resource.TestStep{ { @@ -107,15 +107,11 @@ func testAccNsxtPolicyBgpConfigTemplate(createFlow bool) string { } else { attrMap = accTestPolicyBgpConfigUpdateAttributes } - return testAccNsxtPolicyGMGatewayDeps() + fmt.Sprintf(` - -resource "nsxt_policy_tier0_gateway" "test" { - display_name = "terraform-bgp-test" - locale_service { - edge_cluster_path = data.nsxt_policy_edge_cluster.ec_site1.path - } -} - + extraConfig := "" + if testAccIsGlobalManager() { + extraConfig = `site_path = data.nsxt_policy_site.test.path` + } + return testAccNsxtPolicyEdgeClusterReadTemplate(getEdgeClusterName()) + testAccNsxtPolicyTier0WithEdgeClusterTemplate("test", false) + fmt.Sprintf(` resource "nsxt_policy_bgp_config" "test" { enabled = %s inter_sr_ibgp = %s @@ -132,7 +128,7 @@ resource "nsxt_policy_bgp_config" "test" { graceful_restart_stale_route_timer = %s gateway_path = nsxt_policy_tier0_gateway.test.path - site_path = data.nsxt_policy_site.site1.path + %s tag { scope = "scope1" tag = "tag1" @@ -142,28 +138,24 @@ resource "nsxt_policy_bgp_config" "test" { data "nsxt_policy_realization_info" "bgp_realization_info" { path = nsxt_policy_bgp_config.test.path - site_path = data.nsxt_policy_site.site1.path -}`, attrMap["enabled"], attrMap["inter_sr_ibgp"], attrMap["local_as_num"], attrMap["multipath_relax"], attrMap["prefix"], attrMap["summary_only"], attrMap["graceful_restart_mode"], attrMap["graceful_restart_timer"], attrMap["graceful_restart_stale_route_timer"]) + %s +}`, attrMap["enabled"], attrMap["inter_sr_ibgp"], attrMap["local_as_num"], attrMap["multipath_relax"], attrMap["prefix"], attrMap["summary_only"], attrMap["graceful_restart_mode"], attrMap["graceful_restart_timer"], attrMap["graceful_restart_stale_route_timer"], extraConfig, extraConfig) } func testAccNsxtPolicyBgpConfigMinimalistic() string { - return testAccNsxtPolicyGMGatewayDeps() + ` - -resource "nsxt_policy_tier0_gateway" "test" { - display_name = "terraform-bgp-test" - locale_service { - edge_cluster_path = data.nsxt_policy_edge_cluster.ec_site1.path - } -} - + extraConfig := "" + if testAccIsGlobalManager() { + extraConfig = `site_path = data.nsxt_policy_site.test.path` + } + return testAccNsxtPolicyEdgeClusterReadTemplate(getEdgeClusterName()) + testAccNsxtPolicyTier0WithEdgeClusterTemplate("test", false) + fmt.Sprintf(` resource "nsxt_policy_bgp_config" "test" { gateway_path = nsxt_policy_tier0_gateway.test.path - site_path = data.nsxt_policy_site.site1.path + %s local_as_num = 65001 } data "nsxt_policy_realization_info" "realization_info" { path = nsxt_policy_bgp_config.test.path - site_path = data.nsxt_policy_site.site1.path -}` + %s +}`, extraConfig, extraConfig) } diff --git a/nsxt/resource_nsxt_policy_ospf_config.go b/nsxt/resource_nsxt_policy_ospf_config.go index b0082dffb..62670b701 100644 --- a/nsxt/resource_nsxt_policy_ospf_config.go +++ b/nsxt/resource_nsxt_policy_ospf_config.go @@ -83,15 +83,8 @@ func getPolicyOspfConfigSchema() map[string]*schema.Schema { }, }, }, - "locale_service_id": { - Type: schema.TypeString, - Description: "Id of associated Gateway Locale Service on NSX", - Computed: true, - }, - "gateway_id": { - Type: schema.TypeString, Description: "Id of associated Tier0 Gateway on NSX", - Computed: true, - }, + "locale_service_id": getComputedLocaleServiceIDSchema(), + "gateway_id": getComputedGatewayIDSchema(), } } diff --git a/website/docs/r/policy_bgp_config.html.markdown b/website/docs/r/policy_bgp_config.html.markdown index b711364ac..bb6652808 100644 --- a/website/docs/r/policy_bgp_config.html.markdown +++ b/website/docs/r/policy_bgp_config.html.markdown @@ -2,19 +2,38 @@ subcategory: "Policy - Gateways and Routing" layout: "nsxt" page_title: "NSXT: nsxt_policy_bgp_config" -description: A resource to configure BGP Settings of Tier0 Gateway on NSX Global Manager. +description: A resource to configure BGP Settings of Tier0 Gateway. --- # nsxt_policy_bgp_config This resource provides a method for the management of BGP for T0 Gateway on specific site. A single resource should be specified per T0 Gateway + Site. -This resource is applicable to NSX Global Manager only. For Local Manager, use `bgp_config` clause in gateway resource configuration. +~> **NOTE:** This resource should NOT be used together with `bgp_config` clause in gateway resource configuration - such usage may produce inconsistent experience. Please choose one way or the other to configure BGP. ~> **NOTE:** Since BGP config is auto-created on NSX, this resource will update NSX object, but never create or delete it. ## Example Usage +```hcl +resource "nsxt_policy_bgp_config" "gw1" { + gateway_path = nsxt_policy_tier0_gateway.gw1.path + + enabled = true + inter_sr_ibgp = true + local_as_num = 60001 + graceful_restart_mode = "HELPER_ONLY" + graceful_restart_timer = 2400 + + route_aggregation { + prefix = "20.1.0.0/24" + summary_only = false + } +} +``` + +## Global Manager Example Usage + ```hcl resource "nsxt_policy_bgp_config" "gw1-paris" { site_path = data.nsxt_policy_site.paris.path @@ -37,6 +56,7 @@ resource "nsxt_policy_bgp_config" "gw1-paris" { The following arguments are supported: +* `site_path` - (Optional) Path for policy site. This attribute is required for Global Manager and is not relevant for Local Manager. * `ecmp` - (Optional) A boolean flag to enable/disable ECMP. Default is `true`. * `enabled` - (Optional) A boolean flag to enable/disable BGP. Default is `true`. * `inter_sr_ibgp` - (Optional) A boolean flag to enable/disable inter SR IBGP configuration. Default is `true`.