From b869e1682aaa0aa37190e727ef69ac94d20e99ff Mon Sep 17 00:00:00 2001 From: Peter McAtominey Date: Thu, 12 Jan 2017 13:57:07 +0000 Subject: [PATCH] provider/azurerm: make lb sub resources idempotent (#11128) If an error occurred which prevented the lb sub resources being written to state then the next apply would fail as the resources would already exist in the API. go test -c ./builtin/providers/azurerm -o ./builtin/providers/azurerm/test-azurerm TestAccAzureRMLoadBalancerBackEndAddressPool_reapply TestAccAzureRMLoadBalancerBackEndAddressPool_removal TestAccAzureRMLoadBalancerNatPool_basic TestAccAzureRMLoadBalancerBackEndAddressPool_basic TestAccAzureRMLoadBalancerNatRule_basic TestAccAzureRMLoadBalancerNatPool_reapply TestAccAzureRMLoadBalancerNatPool_removal TestAccAzureRMLoadBalancerNatPool_update TestAccAzureRMLoadBalancerProbe_basic TestAccAzureRMLoadBalancerNatRule_removal TestAccAzureRMLoadBalancerNatRule_update TestAccAzureRMLoadBalancerNatRule_reapply TestAccAzureRMLoadBalancerProbe_removal TestAccAzureRMLoadBalancerProbe_reapply TestAccAzureRMLoadBalancerProbe_update TestAccAzureRMLoadBalancerRule_basic TestAccAzureRMLoadBalancerRule_inconsistentReads TestAccAzureRMLoadBalancerRule_removal TestAccAzureRMLoadBalancerProbe_updateProtocol TestAccAzureRMLoadBalancer_basic TestAccAzureRMLoadBalancerRule_update TestAccAzureRMLoadBalancerRule_reapply TestAccAzureRMLoadBalancer_frontEndConfig TestAccAzureRMLoadBalancer_tags --- ...e_arm_loadbalancer_backend_address_pool.go | 9 +++-- ..._loadbalancer_backend_address_pool_test.go | 34 +++++++++++++++++++ .../resource_arm_loadbalancer_nat_pool.go | 6 ++-- ...resource_arm_loadbalancer_nat_pool_test.go | 20 ++++++++--- .../resource_arm_loadbalancer_nat_rule.go | 6 ++-- ...resource_arm_loadbalancer_nat_rule_test.go | 20 ++++++++--- .../resource_arm_loadbalancer_probe.go | 6 ++-- .../resource_arm_loadbalancer_probe_test.go | 30 ++++++++++------ .../azurerm/resource_arm_loadbalancer_rule.go | 6 ++-- .../resource_arm_loadbalancer_rule_test.go | 24 +++++++------ 10 files changed, 112 insertions(+), 49 deletions(-) diff --git a/builtin/providers/azurerm/resource_arm_loadbalancer_backend_address_pool.go b/builtin/providers/azurerm/resource_arm_loadbalancer_backend_address_pool.go index b85acfdcef..143a797d53 100644 --- a/builtin/providers/azurerm/resource_arm_loadbalancer_backend_address_pool.go +++ b/builtin/providers/azurerm/resource_arm_loadbalancer_backend_address_pool.go @@ -74,12 +74,15 @@ func resourceArmLoadBalancerBackendAddressPoolCreate(d *schema.ResourceData, met return nil } - _, _, exists = findLoadBalancerBackEndAddressPoolByName(loadBalancer, d.Get("name").(string)) + backendAddressPools := append(*loadBalancer.LoadBalancerPropertiesFormat.BackendAddressPools, expandAzureRmLoadBalancerBackendAddressPools(d)) + existingPool, existingPoolIndex, exists := findLoadBalancerBackEndAddressPoolByName(loadBalancer, d.Get("name").(string)) if exists { - return fmt.Errorf("A BackEnd Address Pool with name %q already exists.", d.Get("name").(string)) + if d.Get("name").(string) == *existingPool.Name { + // this pool is being updated/reapplied remove old copy from the slice + backendAddressPools = append(backendAddressPools[:existingPoolIndex], backendAddressPools[existingPoolIndex+1:]...) + } } - backendAddressPools := append(*loadBalancer.LoadBalancerPropertiesFormat.BackendAddressPools, expandAzureRmLoadBalancerBackendAddressPools(d)) loadBalancer.LoadBalancerPropertiesFormat.BackendAddressPools = &backendAddressPools resGroup, loadBalancerName, err := resourceGroupAndLBNameFromId(d.Get("loadbalancer_id").(string)) if err != nil { diff --git a/builtin/providers/azurerm/resource_arm_loadbalancer_backend_address_pool_test.go b/builtin/providers/azurerm/resource_arm_loadbalancer_backend_address_pool_test.go index 367f15b204..d612476bde 100644 --- a/builtin/providers/azurerm/resource_arm_loadbalancer_backend_address_pool_test.go +++ b/builtin/providers/azurerm/resource_arm_loadbalancer_backend_address_pool_test.go @@ -60,6 +60,40 @@ func TestAccAzureRMLoadBalancerBackEndAddressPool_removal(t *testing.T) { }) } +func TestAccAzureRMLoadBalancerBackEndAddressPool_reapply(t *testing.T) { + var lb network.LoadBalancer + ri := acctest.RandInt() + addressPoolName := fmt.Sprintf("%d-address-pool", ri) + + deleteAddressPoolState := func(s *terraform.State) error { + return s.Remove("azurerm_lb_backend_address_pool.test") + } + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testCheckAzureRMLoadBalancerDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMLoadBalancerBackEndAddressPool_basic(ri, addressPoolName), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMLoadBalancerExists("azurerm_lb.test", &lb), + testCheckAzureRMLoadBalancerBackEndAddressPoolExists(addressPoolName, &lb), + deleteAddressPoolState, + ), + ExpectNonEmptyPlan: true, + }, + { + Config: testAccAzureRMLoadBalancerBackEndAddressPool_basic(ri, addressPoolName), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMLoadBalancerExists("azurerm_lb.test", &lb), + testCheckAzureRMLoadBalancerBackEndAddressPoolExists(addressPoolName, &lb), + ), + }, + }, + }) +} + func testCheckAzureRMLoadBalancerBackEndAddressPoolExists(addressPoolName string, lb *network.LoadBalancer) resource.TestCheckFunc { return func(s *terraform.State) error { _, _, exists := findLoadBalancerBackEndAddressPoolByName(lb, addressPoolName) diff --git a/builtin/providers/azurerm/resource_arm_loadbalancer_nat_pool.go b/builtin/providers/azurerm/resource_arm_loadbalancer_nat_pool.go index a869e0a5e4..aa797b635a 100644 --- a/builtin/providers/azurerm/resource_arm_loadbalancer_nat_pool.go +++ b/builtin/providers/azurerm/resource_arm_loadbalancer_nat_pool.go @@ -100,11 +100,9 @@ func resourceArmLoadBalancerNatPoolCreate(d *schema.ResourceData, meta interface existingNatPool, existingNatPoolIndex, exists := findLoadBalancerNatPoolByName(loadBalancer, d.Get("name").(string)) if exists { - if d.Id() == *existingNatPool.ID { - // this probe is being updated remove old copy from the slice + if d.Get("name").(string) == *existingNatPool.Name { + // this probe is being updated/reapplied remove old copy from the slice natPools = append(natPools[:existingNatPoolIndex], natPools[existingNatPoolIndex+1:]...) - } else { - return fmt.Errorf("A NAT Pool with name %q already exists.", d.Get("name").(string)) } } diff --git a/builtin/providers/azurerm/resource_arm_loadbalancer_nat_pool_test.go b/builtin/providers/azurerm/resource_arm_loadbalancer_nat_pool_test.go index 128fac93d9..ac001cb790 100644 --- a/builtin/providers/azurerm/resource_arm_loadbalancer_nat_pool_test.go +++ b/builtin/providers/azurerm/resource_arm_loadbalancer_nat_pool_test.go @@ -5,8 +5,6 @@ import ( "os" "testing" - "regexp" - "github.com/Azure/azure-sdk-for-go/arm/network" "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" @@ -102,23 +100,35 @@ func TestAccAzureRMLoadBalancerNatPool_update(t *testing.T) { }) } -func TestAccAzureRMLoadBalancerNatPool_duplicate(t *testing.T) { +func TestAccAzureRMLoadBalancerNatPool_reapply(t *testing.T) { var lb network.LoadBalancer ri := acctest.RandInt() natPoolName := fmt.Sprintf("NatPool-%d", ri) + deleteNatPoolState := func(s *terraform.State) error { + return s.Remove("azurerm_lb_nat_pool.test") + } + resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testCheckAzureRMLoadBalancerDestroy, Steps: []resource.TestStep{ { - Config: testAccAzureRMLoadBalancerNatPool_multiplePools(ri, natPoolName, natPoolName), + Config: testAccAzureRMLoadBalancerNatPool_basic(ri, natPoolName), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMLoadBalancerExists("azurerm_lb.test", &lb), + testCheckAzureRMLoadBalancerNatPoolExists(natPoolName, &lb), + deleteNatPoolState, + ), + ExpectNonEmptyPlan: true, + }, + { + Config: testAccAzureRMLoadBalancerNatPool_basic(ri, natPoolName), Check: resource.ComposeTestCheckFunc( testCheckAzureRMLoadBalancerExists("azurerm_lb.test", &lb), testCheckAzureRMLoadBalancerNatPoolExists(natPoolName, &lb), ), - ExpectError: regexp.MustCompile(fmt.Sprintf("A NAT Pool with name %q already exists.", natPoolName)), }, }, }) diff --git a/builtin/providers/azurerm/resource_arm_loadbalancer_nat_rule.go b/builtin/providers/azurerm/resource_arm_loadbalancer_nat_rule.go index ea4d6a7355..ac32208115 100644 --- a/builtin/providers/azurerm/resource_arm_loadbalancer_nat_rule.go +++ b/builtin/providers/azurerm/resource_arm_loadbalancer_nat_rule.go @@ -100,11 +100,9 @@ func resourceArmLoadBalancerNatRuleCreate(d *schema.ResourceData, meta interface existingNatRule, existingNatRuleIndex, exists := findLoadBalancerNatRuleByName(loadBalancer, d.Get("name").(string)) if exists { - if d.Id() == *existingNatRule.ID { - // this probe is being updated remove old copy from the slice + if d.Get("name").(string) == *existingNatRule.Name { + // this probe is being updated/reapplied remove old copy from the slice natRules = append(natRules[:existingNatRuleIndex], natRules[existingNatRuleIndex+1:]...) - } else { - return fmt.Errorf("A NAT Rule with name %q already exists.", d.Get("name").(string)) } } diff --git a/builtin/providers/azurerm/resource_arm_loadbalancer_nat_rule_test.go b/builtin/providers/azurerm/resource_arm_loadbalancer_nat_rule_test.go index adcf4f56fa..b4ca2d23d1 100644 --- a/builtin/providers/azurerm/resource_arm_loadbalancer_nat_rule_test.go +++ b/builtin/providers/azurerm/resource_arm_loadbalancer_nat_rule_test.go @@ -5,8 +5,6 @@ import ( "os" "testing" - "regexp" - "github.com/Azure/azure-sdk-for-go/arm/network" "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" @@ -104,23 +102,35 @@ func TestAccAzureRMLoadBalancerNatRule_update(t *testing.T) { }) } -func TestAccAzureRMLoadBalancerNatRule_duplicate(t *testing.T) { +func TestAccAzureRMLoadBalancerNatRule_reapply(t *testing.T) { var lb network.LoadBalancer ri := acctest.RandInt() natRuleName := fmt.Sprintf("NatRule-%d", ri) + deleteNatRuleState := func(s *terraform.State) error { + return s.Remove("azurerm_lb_nat_rule.test") + } + resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testCheckAzureRMLoadBalancerDestroy, Steps: []resource.TestStep{ { - Config: testAccAzureRMLoadBalancerNatRule_multipleRules(ri, natRuleName, natRuleName), + Config: testAccAzureRMLoadBalancerNatRule_basic(ri, natRuleName), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMLoadBalancerExists("azurerm_lb.test", &lb), + testCheckAzureRMLoadBalancerNatRuleExists(natRuleName, &lb), + deleteNatRuleState, + ), + ExpectNonEmptyPlan: true, + }, + { + Config: testAccAzureRMLoadBalancerNatRule_basic(ri, natRuleName), Check: resource.ComposeTestCheckFunc( testCheckAzureRMLoadBalancerExists("azurerm_lb.test", &lb), testCheckAzureRMLoadBalancerNatRuleExists(natRuleName, &lb), ), - ExpectError: regexp.MustCompile(fmt.Sprintf("A NAT Rule with name %q already exists.", natRuleName)), }, }, }) diff --git a/builtin/providers/azurerm/resource_arm_loadbalancer_probe.go b/builtin/providers/azurerm/resource_arm_loadbalancer_probe.go index 9edfb5df23..0658a354de 100644 --- a/builtin/providers/azurerm/resource_arm_loadbalancer_probe.go +++ b/builtin/providers/azurerm/resource_arm_loadbalancer_probe.go @@ -105,11 +105,9 @@ func resourceArmLoadBalancerProbeCreate(d *schema.ResourceData, meta interface{} existingProbe, existingProbeIndex, exists := findLoadBalancerProbeByName(loadBalancer, d.Get("name").(string)) if exists { - if d.Id() == *existingProbe.ID { - // this probe is being updated remove old copy from the slice + if d.Get("name").(string) == *existingProbe.Name { + // this probe is being updated/reapplied remove old copy from the slice probes = append(probes[:existingProbeIndex], probes[existingProbeIndex+1:]...) - } else { - return fmt.Errorf("A Probe with name %q already exists.", d.Get("name").(string)) } } diff --git a/builtin/providers/azurerm/resource_arm_loadbalancer_probe_test.go b/builtin/providers/azurerm/resource_arm_loadbalancer_probe_test.go index 1cfb5b23d0..dc30ba7413 100644 --- a/builtin/providers/azurerm/resource_arm_loadbalancer_probe_test.go +++ b/builtin/providers/azurerm/resource_arm_loadbalancer_probe_test.go @@ -5,8 +5,6 @@ import ( "os" "testing" - "regexp" - "github.com/Azure/azure-sdk-for-go/arm/network" "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" @@ -102,7 +100,7 @@ func TestAccAzureRMLoadBalancerProbe_update(t *testing.T) { }) } -func TestAccAzureRMLoadBalancerProbe_duplicate(t *testing.T) { +func TestAccAzureRMLoadBalancerProbe_updateProtocol(t *testing.T) { var lb network.LoadBalancer ri := acctest.RandInt() probeName := fmt.Sprintf("probe-%d", ri) @@ -113,41 +111,53 @@ func TestAccAzureRMLoadBalancerProbe_duplicate(t *testing.T) { CheckDestroy: testCheckAzureRMLoadBalancerDestroy, Steps: []resource.TestStep{ { - Config: testAccAzureRMLoadBalancerProbe_multipleProbes(ri, probeName, probeName), + Config: testAccAzureRMLoadBalancerProbe_updateProtocolBefore(ri, probeName), Check: resource.ComposeTestCheckFunc( testCheckAzureRMLoadBalancerExists("azurerm_lb.test", &lb), testCheckAzureRMLoadBalancerProbeExists(probeName, &lb), + resource.TestCheckResourceAttr("azurerm_lb_probe.test", "protocol", "Http"), + ), + }, + { + Config: testAccAzureRMLoadBalancerProbe_updateProtocolAfter(ri, probeName), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMLoadBalancerExists("azurerm_lb.test", &lb), + testCheckAzureRMLoadBalancerProbeExists(probeName, &lb), + resource.TestCheckResourceAttr("azurerm_lb_probe.test", "protocol", "Tcp"), ), - ExpectError: regexp.MustCompile(fmt.Sprintf("A Probe with name %q already exists.", probeName)), }, }, }) } -func TestAccAzureRMLoadBalancerProbe_updateProtocol(t *testing.T) { +func TestAccAzureRMLoadBalancerProbe_reapply(t *testing.T) { var lb network.LoadBalancer ri := acctest.RandInt() probeName := fmt.Sprintf("probe-%d", ri) + deleteProbeState := func(s *terraform.State) error { + return s.Remove("azurerm_lb_probe.test") + } + resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testCheckAzureRMLoadBalancerDestroy, Steps: []resource.TestStep{ { - Config: testAccAzureRMLoadBalancerProbe_updateProtocolBefore(ri, probeName), + Config: testAccAzureRMLoadBalancerProbe_basic(ri, probeName), Check: resource.ComposeTestCheckFunc( testCheckAzureRMLoadBalancerExists("azurerm_lb.test", &lb), testCheckAzureRMLoadBalancerProbeExists(probeName, &lb), - resource.TestCheckResourceAttr("azurerm_lb_probe.test", "protocol", "Http"), + deleteProbeState, ), + ExpectNonEmptyPlan: true, }, { - Config: testAccAzureRMLoadBalancerProbe_updateProtocolAfter(ri, probeName), + Config: testAccAzureRMLoadBalancerProbe_basic(ri, probeName), Check: resource.ComposeTestCheckFunc( testCheckAzureRMLoadBalancerExists("azurerm_lb.test", &lb), testCheckAzureRMLoadBalancerProbeExists(probeName, &lb), - resource.TestCheckResourceAttr("azurerm_lb_probe.test", "protocol", "Tcp"), ), }, }, diff --git a/builtin/providers/azurerm/resource_arm_loadbalancer_rule.go b/builtin/providers/azurerm/resource_arm_loadbalancer_rule.go index 23b6022680..25a71f61e9 100644 --- a/builtin/providers/azurerm/resource_arm_loadbalancer_rule.go +++ b/builtin/providers/azurerm/resource_arm_loadbalancer_rule.go @@ -127,11 +127,9 @@ func resourceArmLoadBalancerRuleCreate(d *schema.ResourceData, meta interface{}) existingRule, existingRuleIndex, exists := findLoadBalancerRuleByName(loadBalancer, d.Get("name").(string)) if exists { - if d.Id() == *existingRule.ID { - // this rule is being updated remove old copy from the slice + if d.Get("name").(string) == *existingRule.Name { + // this rule is being updated/reapplied remove old copy from the slice lbRules = append(lbRules[:existingRuleIndex], lbRules[existingRuleIndex+1:]...) - } else { - return fmt.Errorf("A LoadBalancer Rule with name %q already exists.", d.Get("name").(string)) } } diff --git a/builtin/providers/azurerm/resource_arm_loadbalancer_rule_test.go b/builtin/providers/azurerm/resource_arm_loadbalancer_rule_test.go index 43892e6a26..aa15a66e5a 100644 --- a/builtin/providers/azurerm/resource_arm_loadbalancer_rule_test.go +++ b/builtin/providers/azurerm/resource_arm_loadbalancer_rule_test.go @@ -5,8 +5,6 @@ import ( "os" "testing" - "regexp" - "github.com/Azure/azure-sdk-for-go/arm/network" "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" @@ -199,15 +197,14 @@ func TestAccAzureRMLoadBalancerRule_update(t *testing.T) { }) } -func TestAccAzureRMLoadBalancerRule_duplicateRules(t *testing.T) { +func TestAccAzureRMLoadBalancerRule_reapply(t *testing.T) { var lb network.LoadBalancer ri := acctest.RandInt() lbRuleName := fmt.Sprintf("LbRule-%s", acctest.RandStringFromCharSet(8, acctest.CharSetAlpha)) - subscriptionID := os.Getenv("ARM_SUBSCRIPTION_ID") - lbRuleID := fmt.Sprintf( - "/subscriptions/%s/resourceGroups/acctestrg-%d/providers/Microsoft.Network/loadBalancers/arm-test-loadbalancer-%d/loadBalancingRules/%s", - subscriptionID, ri, ri, lbRuleName) + deleteRuleState := func(s *terraform.State) error { + return s.Remove("azurerm_lb_rule.test") + } resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -215,13 +212,20 @@ func TestAccAzureRMLoadBalancerRule_duplicateRules(t *testing.T) { CheckDestroy: testCheckAzureRMLoadBalancerDestroy, Steps: []resource.TestStep{ { - Config: testAccAzureRMLoadBalancerRule_multipleRules(ri, lbRuleName, lbRuleName), + Config: testAccAzureRMLoadBalancerRule_basic(ri, lbRuleName), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMLoadBalancerExists("azurerm_lb.test", &lb), + testCheckAzureRMLoadBalancerRuleExists(lbRuleName, &lb), + deleteRuleState, + ), + ExpectNonEmptyPlan: true, + }, + { + Config: testAccAzureRMLoadBalancerRule_basic(ri, lbRuleName), Check: resource.ComposeTestCheckFunc( testCheckAzureRMLoadBalancerExists("azurerm_lb.test", &lb), testCheckAzureRMLoadBalancerRuleExists(lbRuleName, &lb), - resource.TestCheckResourceAttr("azurerm_lb_rule.test", "id", lbRuleID), ), - ExpectError: regexp.MustCompile(fmt.Sprintf("A LoadBalancer Rule with name %q already exists.", lbRuleName)), }, }, })