Skip to content

Commit b869e16

Browse files
pmcatomineystack72
authored andcommitted
provider/azurerm: make lb sub resources idempotent (hashicorp#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
1 parent 9e7ee8d commit b869e16

10 files changed

Lines changed: 112 additions & 49 deletions

builtin/providers/azurerm/resource_arm_loadbalancer_backend_address_pool.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,12 +74,15 @@ func resourceArmLoadBalancerBackendAddressPoolCreate(d *schema.ResourceData, met
7474
return nil
7575
}
7676

77-
_, _, exists = findLoadBalancerBackEndAddressPoolByName(loadBalancer, d.Get("name").(string))
77+
backendAddressPools := append(*loadBalancer.LoadBalancerPropertiesFormat.BackendAddressPools, expandAzureRmLoadBalancerBackendAddressPools(d))
78+
existingPool, existingPoolIndex, exists := findLoadBalancerBackEndAddressPoolByName(loadBalancer, d.Get("name").(string))
7879
if exists {
79-
return fmt.Errorf("A BackEnd Address Pool with name %q already exists.", d.Get("name").(string))
80+
if d.Get("name").(string) == *existingPool.Name {
81+
// this pool is being updated/reapplied remove old copy from the slice
82+
backendAddressPools = append(backendAddressPools[:existingPoolIndex], backendAddressPools[existingPoolIndex+1:]...)
83+
}
8084
}
8185

82-
backendAddressPools := append(*loadBalancer.LoadBalancerPropertiesFormat.BackendAddressPools, expandAzureRmLoadBalancerBackendAddressPools(d))
8386
loadBalancer.LoadBalancerPropertiesFormat.BackendAddressPools = &backendAddressPools
8487
resGroup, loadBalancerName, err := resourceGroupAndLBNameFromId(d.Get("loadbalancer_id").(string))
8588
if err != nil {

builtin/providers/azurerm/resource_arm_loadbalancer_backend_address_pool_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,40 @@ func TestAccAzureRMLoadBalancerBackEndAddressPool_removal(t *testing.T) {
6060
})
6161
}
6262

63+
func TestAccAzureRMLoadBalancerBackEndAddressPool_reapply(t *testing.T) {
64+
var lb network.LoadBalancer
65+
ri := acctest.RandInt()
66+
addressPoolName := fmt.Sprintf("%d-address-pool", ri)
67+
68+
deleteAddressPoolState := func(s *terraform.State) error {
69+
return s.Remove("azurerm_lb_backend_address_pool.test")
70+
}
71+
72+
resource.Test(t, resource.TestCase{
73+
PreCheck: func() { testAccPreCheck(t) },
74+
Providers: testAccProviders,
75+
CheckDestroy: testCheckAzureRMLoadBalancerDestroy,
76+
Steps: []resource.TestStep{
77+
{
78+
Config: testAccAzureRMLoadBalancerBackEndAddressPool_basic(ri, addressPoolName),
79+
Check: resource.ComposeTestCheckFunc(
80+
testCheckAzureRMLoadBalancerExists("azurerm_lb.test", &lb),
81+
testCheckAzureRMLoadBalancerBackEndAddressPoolExists(addressPoolName, &lb),
82+
deleteAddressPoolState,
83+
),
84+
ExpectNonEmptyPlan: true,
85+
},
86+
{
87+
Config: testAccAzureRMLoadBalancerBackEndAddressPool_basic(ri, addressPoolName),
88+
Check: resource.ComposeTestCheckFunc(
89+
testCheckAzureRMLoadBalancerExists("azurerm_lb.test", &lb),
90+
testCheckAzureRMLoadBalancerBackEndAddressPoolExists(addressPoolName, &lb),
91+
),
92+
},
93+
},
94+
})
95+
}
96+
6397
func testCheckAzureRMLoadBalancerBackEndAddressPoolExists(addressPoolName string, lb *network.LoadBalancer) resource.TestCheckFunc {
6498
return func(s *terraform.State) error {
6599
_, _, exists := findLoadBalancerBackEndAddressPoolByName(lb, addressPoolName)

builtin/providers/azurerm/resource_arm_loadbalancer_nat_pool.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,9 @@ func resourceArmLoadBalancerNatPoolCreate(d *schema.ResourceData, meta interface
100100

101101
existingNatPool, existingNatPoolIndex, exists := findLoadBalancerNatPoolByName(loadBalancer, d.Get("name").(string))
102102
if exists {
103-
if d.Id() == *existingNatPool.ID {
104-
// this probe is being updated remove old copy from the slice
103+
if d.Get("name").(string) == *existingNatPool.Name {
104+
// this probe is being updated/reapplied remove old copy from the slice
105105
natPools = append(natPools[:existingNatPoolIndex], natPools[existingNatPoolIndex+1:]...)
106-
} else {
107-
return fmt.Errorf("A NAT Pool with name %q already exists.", d.Get("name").(string))
108106
}
109107
}
110108

builtin/providers/azurerm/resource_arm_loadbalancer_nat_pool_test.go

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ import (
55
"os"
66
"testing"
77

8-
"regexp"
9-
108
"github.com/Azure/azure-sdk-for-go/arm/network"
119
"github.com/hashicorp/terraform/helper/acctest"
1210
"github.com/hashicorp/terraform/helper/resource"
@@ -102,23 +100,35 @@ func TestAccAzureRMLoadBalancerNatPool_update(t *testing.T) {
102100
})
103101
}
104102

105-
func TestAccAzureRMLoadBalancerNatPool_duplicate(t *testing.T) {
103+
func TestAccAzureRMLoadBalancerNatPool_reapply(t *testing.T) {
106104
var lb network.LoadBalancer
107105
ri := acctest.RandInt()
108106
natPoolName := fmt.Sprintf("NatPool-%d", ri)
109107

108+
deleteNatPoolState := func(s *terraform.State) error {
109+
return s.Remove("azurerm_lb_nat_pool.test")
110+
}
111+
110112
resource.Test(t, resource.TestCase{
111113
PreCheck: func() { testAccPreCheck(t) },
112114
Providers: testAccProviders,
113115
CheckDestroy: testCheckAzureRMLoadBalancerDestroy,
114116
Steps: []resource.TestStep{
115117
{
116-
Config: testAccAzureRMLoadBalancerNatPool_multiplePools(ri, natPoolName, natPoolName),
118+
Config: testAccAzureRMLoadBalancerNatPool_basic(ri, natPoolName),
119+
Check: resource.ComposeTestCheckFunc(
120+
testCheckAzureRMLoadBalancerExists("azurerm_lb.test", &lb),
121+
testCheckAzureRMLoadBalancerNatPoolExists(natPoolName, &lb),
122+
deleteNatPoolState,
123+
),
124+
ExpectNonEmptyPlan: true,
125+
},
126+
{
127+
Config: testAccAzureRMLoadBalancerNatPool_basic(ri, natPoolName),
117128
Check: resource.ComposeTestCheckFunc(
118129
testCheckAzureRMLoadBalancerExists("azurerm_lb.test", &lb),
119130
testCheckAzureRMLoadBalancerNatPoolExists(natPoolName, &lb),
120131
),
121-
ExpectError: regexp.MustCompile(fmt.Sprintf("A NAT Pool with name %q already exists.", natPoolName)),
122132
},
123133
},
124134
})

builtin/providers/azurerm/resource_arm_loadbalancer_nat_rule.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,9 @@ func resourceArmLoadBalancerNatRuleCreate(d *schema.ResourceData, meta interface
100100

101101
existingNatRule, existingNatRuleIndex, exists := findLoadBalancerNatRuleByName(loadBalancer, d.Get("name").(string))
102102
if exists {
103-
if d.Id() == *existingNatRule.ID {
104-
// this probe is being updated remove old copy from the slice
103+
if d.Get("name").(string) == *existingNatRule.Name {
104+
// this probe is being updated/reapplied remove old copy from the slice
105105
natRules = append(natRules[:existingNatRuleIndex], natRules[existingNatRuleIndex+1:]...)
106-
} else {
107-
return fmt.Errorf("A NAT Rule with name %q already exists.", d.Get("name").(string))
108106
}
109107
}
110108

builtin/providers/azurerm/resource_arm_loadbalancer_nat_rule_test.go

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ import (
55
"os"
66
"testing"
77

8-
"regexp"
9-
108
"github.com/Azure/azure-sdk-for-go/arm/network"
119
"github.com/hashicorp/terraform/helper/acctest"
1210
"github.com/hashicorp/terraform/helper/resource"
@@ -104,23 +102,35 @@ func TestAccAzureRMLoadBalancerNatRule_update(t *testing.T) {
104102
})
105103
}
106104

107-
func TestAccAzureRMLoadBalancerNatRule_duplicate(t *testing.T) {
105+
func TestAccAzureRMLoadBalancerNatRule_reapply(t *testing.T) {
108106
var lb network.LoadBalancer
109107
ri := acctest.RandInt()
110108
natRuleName := fmt.Sprintf("NatRule-%d", ri)
111109

110+
deleteNatRuleState := func(s *terraform.State) error {
111+
return s.Remove("azurerm_lb_nat_rule.test")
112+
}
113+
112114
resource.Test(t, resource.TestCase{
113115
PreCheck: func() { testAccPreCheck(t) },
114116
Providers: testAccProviders,
115117
CheckDestroy: testCheckAzureRMLoadBalancerDestroy,
116118
Steps: []resource.TestStep{
117119
{
118-
Config: testAccAzureRMLoadBalancerNatRule_multipleRules(ri, natRuleName, natRuleName),
120+
Config: testAccAzureRMLoadBalancerNatRule_basic(ri, natRuleName),
121+
Check: resource.ComposeTestCheckFunc(
122+
testCheckAzureRMLoadBalancerExists("azurerm_lb.test", &lb),
123+
testCheckAzureRMLoadBalancerNatRuleExists(natRuleName, &lb),
124+
deleteNatRuleState,
125+
),
126+
ExpectNonEmptyPlan: true,
127+
},
128+
{
129+
Config: testAccAzureRMLoadBalancerNatRule_basic(ri, natRuleName),
119130
Check: resource.ComposeTestCheckFunc(
120131
testCheckAzureRMLoadBalancerExists("azurerm_lb.test", &lb),
121132
testCheckAzureRMLoadBalancerNatRuleExists(natRuleName, &lb),
122133
),
123-
ExpectError: regexp.MustCompile(fmt.Sprintf("A NAT Rule with name %q already exists.", natRuleName)),
124134
},
125135
},
126136
})

builtin/providers/azurerm/resource_arm_loadbalancer_probe.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,11 +105,9 @@ func resourceArmLoadBalancerProbeCreate(d *schema.ResourceData, meta interface{}
105105

106106
existingProbe, existingProbeIndex, exists := findLoadBalancerProbeByName(loadBalancer, d.Get("name").(string))
107107
if exists {
108-
if d.Id() == *existingProbe.ID {
109-
// this probe is being updated remove old copy from the slice
108+
if d.Get("name").(string) == *existingProbe.Name {
109+
// this probe is being updated/reapplied remove old copy from the slice
110110
probes = append(probes[:existingProbeIndex], probes[existingProbeIndex+1:]...)
111-
} else {
112-
return fmt.Errorf("A Probe with name %q already exists.", d.Get("name").(string))
113111
}
114112
}
115113

builtin/providers/azurerm/resource_arm_loadbalancer_probe_test.go

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ import (
55
"os"
66
"testing"
77

8-
"regexp"
9-
108
"github.com/Azure/azure-sdk-for-go/arm/network"
119
"github.com/hashicorp/terraform/helper/acctest"
1210
"github.com/hashicorp/terraform/helper/resource"
@@ -102,7 +100,7 @@ func TestAccAzureRMLoadBalancerProbe_update(t *testing.T) {
102100
})
103101
}
104102

105-
func TestAccAzureRMLoadBalancerProbe_duplicate(t *testing.T) {
103+
func TestAccAzureRMLoadBalancerProbe_updateProtocol(t *testing.T) {
106104
var lb network.LoadBalancer
107105
ri := acctest.RandInt()
108106
probeName := fmt.Sprintf("probe-%d", ri)
@@ -113,41 +111,53 @@ func TestAccAzureRMLoadBalancerProbe_duplicate(t *testing.T) {
113111
CheckDestroy: testCheckAzureRMLoadBalancerDestroy,
114112
Steps: []resource.TestStep{
115113
{
116-
Config: testAccAzureRMLoadBalancerProbe_multipleProbes(ri, probeName, probeName),
114+
Config: testAccAzureRMLoadBalancerProbe_updateProtocolBefore(ri, probeName),
117115
Check: resource.ComposeTestCheckFunc(
118116
testCheckAzureRMLoadBalancerExists("azurerm_lb.test", &lb),
119117
testCheckAzureRMLoadBalancerProbeExists(probeName, &lb),
118+
resource.TestCheckResourceAttr("azurerm_lb_probe.test", "protocol", "Http"),
119+
),
120+
},
121+
{
122+
Config: testAccAzureRMLoadBalancerProbe_updateProtocolAfter(ri, probeName),
123+
Check: resource.ComposeTestCheckFunc(
124+
testCheckAzureRMLoadBalancerExists("azurerm_lb.test", &lb),
125+
testCheckAzureRMLoadBalancerProbeExists(probeName, &lb),
126+
resource.TestCheckResourceAttr("azurerm_lb_probe.test", "protocol", "Tcp"),
120127
),
121-
ExpectError: regexp.MustCompile(fmt.Sprintf("A Probe with name %q already exists.", probeName)),
122128
},
123129
},
124130
})
125131
}
126132

127-
func TestAccAzureRMLoadBalancerProbe_updateProtocol(t *testing.T) {
133+
func TestAccAzureRMLoadBalancerProbe_reapply(t *testing.T) {
128134
var lb network.LoadBalancer
129135
ri := acctest.RandInt()
130136
probeName := fmt.Sprintf("probe-%d", ri)
131137

138+
deleteProbeState := func(s *terraform.State) error {
139+
return s.Remove("azurerm_lb_probe.test")
140+
}
141+
132142
resource.Test(t, resource.TestCase{
133143
PreCheck: func() { testAccPreCheck(t) },
134144
Providers: testAccProviders,
135145
CheckDestroy: testCheckAzureRMLoadBalancerDestroy,
136146
Steps: []resource.TestStep{
137147
{
138-
Config: testAccAzureRMLoadBalancerProbe_updateProtocolBefore(ri, probeName),
148+
Config: testAccAzureRMLoadBalancerProbe_basic(ri, probeName),
139149
Check: resource.ComposeTestCheckFunc(
140150
testCheckAzureRMLoadBalancerExists("azurerm_lb.test", &lb),
141151
testCheckAzureRMLoadBalancerProbeExists(probeName, &lb),
142-
resource.TestCheckResourceAttr("azurerm_lb_probe.test", "protocol", "Http"),
152+
deleteProbeState,
143153
),
154+
ExpectNonEmptyPlan: true,
144155
},
145156
{
146-
Config: testAccAzureRMLoadBalancerProbe_updateProtocolAfter(ri, probeName),
157+
Config: testAccAzureRMLoadBalancerProbe_basic(ri, probeName),
147158
Check: resource.ComposeTestCheckFunc(
148159
testCheckAzureRMLoadBalancerExists("azurerm_lb.test", &lb),
149160
testCheckAzureRMLoadBalancerProbeExists(probeName, &lb),
150-
resource.TestCheckResourceAttr("azurerm_lb_probe.test", "protocol", "Tcp"),
151161
),
152162
},
153163
},

builtin/providers/azurerm/resource_arm_loadbalancer_rule.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,11 +127,9 @@ func resourceArmLoadBalancerRuleCreate(d *schema.ResourceData, meta interface{})
127127

128128
existingRule, existingRuleIndex, exists := findLoadBalancerRuleByName(loadBalancer, d.Get("name").(string))
129129
if exists {
130-
if d.Id() == *existingRule.ID {
131-
// this rule is being updated remove old copy from the slice
130+
if d.Get("name").(string) == *existingRule.Name {
131+
// this rule is being updated/reapplied remove old copy from the slice
132132
lbRules = append(lbRules[:existingRuleIndex], lbRules[existingRuleIndex+1:]...)
133-
} else {
134-
return fmt.Errorf("A LoadBalancer Rule with name %q already exists.", d.Get("name").(string))
135133
}
136134
}
137135

builtin/providers/azurerm/resource_arm_loadbalancer_rule_test.go

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ import (
55
"os"
66
"testing"
77

8-
"regexp"
9-
108
"github.com/Azure/azure-sdk-for-go/arm/network"
119
"github.com/hashicorp/terraform/helper/acctest"
1210
"github.com/hashicorp/terraform/helper/resource"
@@ -199,29 +197,35 @@ func TestAccAzureRMLoadBalancerRule_update(t *testing.T) {
199197
})
200198
}
201199

202-
func TestAccAzureRMLoadBalancerRule_duplicateRules(t *testing.T) {
200+
func TestAccAzureRMLoadBalancerRule_reapply(t *testing.T) {
203201
var lb network.LoadBalancer
204202
ri := acctest.RandInt()
205203
lbRuleName := fmt.Sprintf("LbRule-%s", acctest.RandStringFromCharSet(8, acctest.CharSetAlpha))
206204

207-
subscriptionID := os.Getenv("ARM_SUBSCRIPTION_ID")
208-
lbRuleID := fmt.Sprintf(
209-
"/subscriptions/%s/resourceGroups/acctestrg-%d/providers/Microsoft.Network/loadBalancers/arm-test-loadbalancer-%d/loadBalancingRules/%s",
210-
subscriptionID, ri, ri, lbRuleName)
205+
deleteRuleState := func(s *terraform.State) error {
206+
return s.Remove("azurerm_lb_rule.test")
207+
}
211208

212209
resource.Test(t, resource.TestCase{
213210
PreCheck: func() { testAccPreCheck(t) },
214211
Providers: testAccProviders,
215212
CheckDestroy: testCheckAzureRMLoadBalancerDestroy,
216213
Steps: []resource.TestStep{
217214
{
218-
Config: testAccAzureRMLoadBalancerRule_multipleRules(ri, lbRuleName, lbRuleName),
215+
Config: testAccAzureRMLoadBalancerRule_basic(ri, lbRuleName),
216+
Check: resource.ComposeTestCheckFunc(
217+
testCheckAzureRMLoadBalancerExists("azurerm_lb.test", &lb),
218+
testCheckAzureRMLoadBalancerRuleExists(lbRuleName, &lb),
219+
deleteRuleState,
220+
),
221+
ExpectNonEmptyPlan: true,
222+
},
223+
{
224+
Config: testAccAzureRMLoadBalancerRule_basic(ri, lbRuleName),
219225
Check: resource.ComposeTestCheckFunc(
220226
testCheckAzureRMLoadBalancerExists("azurerm_lb.test", &lb),
221227
testCheckAzureRMLoadBalancerRuleExists(lbRuleName, &lb),
222-
resource.TestCheckResourceAttr("azurerm_lb_rule.test", "id", lbRuleID),
223228
),
224-
ExpectError: regexp.MustCompile(fmt.Sprintf("A LoadBalancer Rule with name %q already exists.", lbRuleName)),
225229
},
226230
},
227231
})

0 commit comments

Comments
 (0)