From ba7c8d98ccedf4ef9774c86eb5c07415308d0ad7 Mon Sep 17 00:00:00 2001 From: Matthew Frahry Date: Tue, 21 May 2019 23:18:27 -0600 Subject: [PATCH 1/2] Fixing express route circuit --- azurerm/resource_arm_express_route_circuit.go | 52 ++++++++++--- ...arm_express_route_circuit_authorization.go | 6 +- ...ource_arm_express_route_circuit_peering.go | 6 +- ..._arm_express_route_circuit_peering_test.go | 75 +++++++++++++++++++ ...resource_arm_express_route_circuit_test.go | 5 +- 5 files changed, 126 insertions(+), 18 deletions(-) diff --git a/azurerm/resource_arm_express_route_circuit.go b/azurerm/resource_arm_express_route_circuit.go index b617e61c93d9..078223cd9a79 100644 --- a/azurerm/resource_arm_express_route_circuit.go +++ b/azurerm/resource_arm_express_route_circuit.go @@ -113,6 +113,9 @@ func resourceArmExpressRouteCircuitCreateUpdate(d *schema.ResourceData, meta int name := d.Get("name").(string) resGroup := d.Get("resource_group_name").(string) + azureRMLockByName(name, expressRouteCircuitResourceName) + defer azureRMUnlockByName(name, expressRouteCircuitResourceName) + if requireResourcesToBeImported && d.IsNewResource() { existing, err := client.Get(ctx, resGroup, name) if err != nil { @@ -135,24 +138,53 @@ func resourceArmExpressRouteCircuitCreateUpdate(d *schema.ResourceData, meta int tags := d.Get("tags").(map[string]interface{}) expandedTags := expandTags(tags) - erc := network.ExpressRouteCircuit{ - Name: &name, - Location: &location, - Sku: sku, - ExpressRouteCircuitPropertiesFormat: &network.ExpressRouteCircuitPropertiesFormat{ + // There is the potential for the express to get out of sync when the service provider updates + // the express route circuit. We'll get and update the resource in place as per https://aka.ms/erRefresh + // We also want to keep track of the resource obtained from the api and pass down any attributes not + // managed by Terraform. + erc := network.ExpressRouteCircuit{} + if !d.IsNewResource() { + existing, err := client.Get(ctx, resGroup, name) + if err != nil { + if !utils.ResponseWasNotFound(erc.Response) { + return fmt.Errorf("Error checking for presence of existing ExpressRoute Circuit %q (Resource Group %q): %s", name, resGroup, err) + } + } + + future, err := client.CreateOrUpdate(ctx, resGroup, name, existing) + if err != nil { + return fmt.Errorf("Error Creating/Updating ExpressRouteCircuit %q (Resource Group %q): %+v", name, resGroup, err) + } + + if err = future.WaitForCompletionRef(ctx, client.Client); err != nil { + return fmt.Errorf("Error Creating/Updating ExpressRouteCircuit %q (Resource Group %q): %+v", name, resGroup, err) + } + erc = existing + } + + erc.Name = &name + erc.Location = &location + erc.Sku = sku + erc.Tags = expandedTags + + if erc.ExpressRouteCircuitPropertiesFormat != nil { + erc.ExpressRouteCircuitPropertiesFormat.AllowClassicOperations = &allowRdfeOps + if erc.ExpressRouteCircuitPropertiesFormat.ServiceProviderProperties != nil { + erc.ExpressRouteCircuitPropertiesFormat.ServiceProviderProperties.ServiceProviderName = &serviceProviderName + erc.ExpressRouteCircuitPropertiesFormat.ServiceProviderProperties.PeeringLocation = &peeringLocation + erc.ExpressRouteCircuitPropertiesFormat.ServiceProviderProperties.BandwidthInMbps = &bandwidthInMbps + } + } else { + erc.ExpressRouteCircuitPropertiesFormat = &network.ExpressRouteCircuitPropertiesFormat{ AllowClassicOperations: &allowRdfeOps, ServiceProviderProperties: &network.ExpressRouteCircuitServiceProviderProperties{ ServiceProviderName: &serviceProviderName, PeeringLocation: &peeringLocation, BandwidthInMbps: &bandwidthInMbps, }, - }, - Tags: expandedTags, + } } - azureRMLockByName(name, expressRouteCircuitResourceName) - defer azureRMUnlockByName(name, expressRouteCircuitResourceName) - future, err := client.CreateOrUpdate(ctx, resGroup, name, erc) if err != nil { return fmt.Errorf("Error Creating/Updating ExpressRouteCircuit %q (Resource Group %q): %+v", name, resGroup, err) diff --git a/azurerm/resource_arm_express_route_circuit_authorization.go b/azurerm/resource_arm_express_route_circuit_authorization.go index 498cdcbeb849..5e6703d252a4 100644 --- a/azurerm/resource_arm_express_route_circuit_authorization.go +++ b/azurerm/resource_arm_express_route_circuit_authorization.go @@ -56,6 +56,9 @@ func resourceArmExpressRouteCircuitAuthorizationCreate(d *schema.ResourceData, m resourceGroup := d.Get("resource_group_name").(string) circuitName := d.Get("express_route_circuit_name").(string) + azureRMLockByName(circuitName, expressRouteCircuitResourceName) + defer azureRMUnlockByName(circuitName, expressRouteCircuitResourceName) + if requireResourcesToBeImported && d.IsNewResource() { existing, err := client.Get(ctx, resourceGroup, circuitName, name) if err != nil { @@ -73,9 +76,6 @@ func resourceArmExpressRouteCircuitAuthorizationCreate(d *schema.ResourceData, m AuthorizationPropertiesFormat: &network.AuthorizationPropertiesFormat{}, } - azureRMLockByName(circuitName, expressRouteCircuitResourceName) - defer azureRMUnlockByName(circuitName, expressRouteCircuitResourceName) - future, err := client.CreateOrUpdate(ctx, resourceGroup, circuitName, name, properties) if err != nil { return fmt.Errorf("Error Creating/Updating Express Route Circuit Authorization %q (Circuit %q / Resource Group %q): %+v", name, circuitName, resourceGroup, err) diff --git a/azurerm/resource_arm_express_route_circuit_peering.go b/azurerm/resource_arm_express_route_circuit_peering.go index ade6965c10c4..7c6c7e3f4235 100644 --- a/azurerm/resource_arm_express_route_circuit_peering.go +++ b/azurerm/resource_arm_express_route_circuit_peering.go @@ -114,6 +114,9 @@ func resourceArmExpressRouteCircuitPeeringCreateUpdate(d *schema.ResourceData, m circuitName := d.Get("express_route_circuit_name").(string) resourceGroup := d.Get("resource_group_name").(string) + azureRMLockByName(circuitName, expressRouteCircuitResourceName) + defer azureRMUnlockByName(circuitName, expressRouteCircuitResourceName) + if requireResourcesToBeImported && d.IsNewResource() { existing, err := client.Get(ctx, resourceGroup, circuitName, peeringType) if err != nil { @@ -156,9 +159,6 @@ func resourceArmExpressRouteCircuitPeeringCreateUpdate(d *schema.ResourceData, m parameters.ExpressRouteCircuitPeeringPropertiesFormat.MicrosoftPeeringConfig = peeringConfig } - azureRMLockByName(circuitName, expressRouteCircuitResourceName) - defer azureRMUnlockByName(circuitName, expressRouteCircuitResourceName) - future, err := client.CreateOrUpdate(ctx, resourceGroup, circuitName, peeringType, parameters) if err != nil { return err diff --git a/azurerm/resource_arm_express_route_circuit_peering_test.go b/azurerm/resource_arm_express_route_circuit_peering_test.go index ef062978e741..b18e00cee9db 100644 --- a/azurerm/resource_arm_express_route_circuit_peering_test.go +++ b/azurerm/resource_arm_express_route_circuit_peering_test.go @@ -96,6 +96,42 @@ func testAccAzureRMExpressRouteCircuitPeering_microsoftPeering(t *testing.T) { }) } +func testAccAzureRMExpressRouteCircuitPeering_azurePrivatePeeringWithCircuitUpdate(t *testing.T) { + resourceName := "azurerm_express_route_circuit_peering.test" + ri := tf.AccRandTimeInt() + location := testLocation() + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testCheckAzureRMExpressRouteCircuitPeeringDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMExpressRouteCircuitPeering_privatePeering(ri, location), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMExpressRouteCircuitPeeringExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "peering_type", "AzurePrivatePeering"), + resource.TestCheckResourceAttr(resourceName, "microsoft_peering_config.#", "0"), + ), + }, + { + Config: testAccAzureRMExpressRouteCircuitPeering_privatePeeringWithCircuitUpdate(ri, location), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMExpressRouteCircuitPeeringExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "peering_type", "AzurePrivatePeering"), + resource.TestCheckResourceAttr(resourceName, "microsoft_peering_config.#", "0"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"shared_key"}, //is not returned by the API + }, + }, + }) +} + func testCheckAzureRMExpressRouteCircuitPeeringExists(resourceName string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[resourceName] @@ -251,3 +287,42 @@ resource "azurerm_express_route_circuit_peering" "test" { } `, rInt, location, rInt) } + +func testAccAzureRMExpressRouteCircuitPeering_privatePeeringWithCircuitUpdate(rInt int, location string) string { + return fmt.Sprintf(` +resource "azurerm_resource_group" "test" { + name = "acctestRG-%d" + location = "%s" +} + +resource "azurerm_express_route_circuit" "test" { + name = "acctest-erc-%d" + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + service_provider_name = "Equinix" + peering_location = "Silicon Valley" + bandwidth_in_mbps = 50 + + sku { + tier = "Standard" + family = "MeteredData" + } + + tags = { + Environment = "prod" + Purpose = "AcceptanceTests" + } +} + +resource "azurerm_express_route_circuit_peering" "test" { + peering_type = "AzurePrivatePeering" + express_route_circuit_name = "${azurerm_express_route_circuit.test.name}" + resource_group_name = "${azurerm_resource_group.test.name}" + shared_key = "SSSSsssssshhhhhItsASecret" + peer_asn = 100 + primary_peer_address_prefix = "192.168.1.0/30" + secondary_peer_address_prefix = "192.168.2.0/30" + vlan_id = 100 +} +`, rInt, location, rInt) +} diff --git a/azurerm/resource_arm_express_route_circuit_test.go b/azurerm/resource_arm_express_route_circuit_test.go index 0e3a61596352..bc235d05e777 100644 --- a/azurerm/resource_arm_express_route_circuit_test.go +++ b/azurerm/resource_arm_express_route_circuit_test.go @@ -28,8 +28,9 @@ func TestAccAzureRMExpressRouteCircuit(t *testing.T) { "data_basic": testAccDataSourceAzureRMExpressRoute_basicMetered, }, "PrivatePeering": { - "azurePrivatePeering": testAccAzureRMExpressRouteCircuitPeering_azurePrivatePeering, - "requiresImport": testAccAzureRMExpressRouteCircuitPeering_requiresImport, + "azurePrivatePeering": testAccAzureRMExpressRouteCircuitPeering_azurePrivatePeering, + "azurePrivatePeeringWithUpdate": testAccAzureRMExpressRouteCircuitPeering_azurePrivatePeeringWithCircuitUpdate, + "requiresImport": testAccAzureRMExpressRouteCircuitPeering_requiresImport, }, "MicrosoftPeering": { "microsoftPeering": testAccAzureRMExpressRouteCircuitPeering_microsoftPeering, From e40f9dacb38a181ecd781822eb5bbda21e46b8d2 Mon Sep 17 00:00:00 2001 From: Matthew Frahry Date: Wed, 22 May 2019 08:46:58 -0600 Subject: [PATCH 2/2] UPpdate comment --- azurerm/resource_arm_express_route_circuit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azurerm/resource_arm_express_route_circuit.go b/azurerm/resource_arm_express_route_circuit.go index 078223cd9a79..348b92961482 100644 --- a/azurerm/resource_arm_express_route_circuit.go +++ b/azurerm/resource_arm_express_route_circuit.go @@ -138,7 +138,7 @@ func resourceArmExpressRouteCircuitCreateUpdate(d *schema.ResourceData, meta int tags := d.Get("tags").(map[string]interface{}) expandedTags := expandTags(tags) - // There is the potential for the express to get out of sync when the service provider updates + // There is the potential for the express route circuit to become out of sync when the service provider updates // the express route circuit. We'll get and update the resource in place as per https://aka.ms/erRefresh // We also want to keep track of the resource obtained from the api and pass down any attributes not // managed by Terraform.