Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

azurerm_lb - Fix adding/removing a frontend configuration forces recreation of loadbalancer issue #19548

Merged
merged 2 commits into from
Dec 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 30 additions & 1 deletion internal/services/loadbalancer/loadbalancer_resource.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
package loadbalancer

import (
"context"
"fmt"
"log"
"reflect"
"strconv"
"strings"
"time"

"github.com/hashicorp/go-azure-helpers/resourcemanager/commonschema"
"github.com/hashicorp/go-azure-helpers/resourcemanager/edgezones"
"github.com/hashicorp/go-azure-helpers/resourcemanager/location"
"github.com/hashicorp/go-azure-helpers/resourcemanager/zones"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-provider-azurerm/helpers/azure"
"github.com/hashicorp/terraform-provider-azurerm/helpers/tf"
"github.com/hashicorp/terraform-provider-azurerm/internal/clients"
Expand Down Expand Up @@ -45,6 +49,30 @@ func resourceArmLoadBalancer() *pluginsdk.Resource {
},

Schema: resourceArmLoadBalancerSchema(),

CustomizeDiff: pluginsdk.CustomDiffWithAll(
pluginsdk.ForceNewIf("frontend_ip_configuration", func(ctx context.Context, d *schema.ResourceDiff, meta interface{}) bool {
old, new := d.GetChange("frontend_ip_configuration")
if len(old.([]interface{})) == 0 && len(new.([]interface{})) > 0 || len(old.([]interface{})) > 0 && len(new.([]interface{})) == 0 {
return false
} else {
for i, nc := range new.([]interface{}) {
dataNew := nc.(map[string]interface{})
for _, oc := range old.([]interface{}) {
dataOld := oc.(map[string]interface{})
if dataOld["name"].(string) == dataNew["name"].(string) {
if !reflect.DeepEqual(dataOld["zones"].(*pluginsdk.Set).List(), dataNew["zones"].(*pluginsdk.Set).List()) {
// set ForceNew to true when the `frontend_ip_configuration.#.zones` is changed.
d.ForceNew("frontend_ip_configuration." + strconv.Itoa(i) + ".zones")
break
}
}
}
}
}
return false
}),
),
}
}

Expand All @@ -58,6 +86,7 @@ func resourceArmLoadBalancerCreateUpdate(d *pluginsdk.ResourceData, meta interfa

id := parse.NewLoadBalancerID(subscriptionId, d.Get("resource_group_name").(string), d.Get("name").(string))

log.Printf("elena: is new resource : %t", d.IsNewResource())
if d.IsNewResource() {
existing, err := client.Get(ctx, id.ResourceGroup, id.Name, "")
if err != nil {
Expand Down Expand Up @@ -495,7 +524,7 @@ func resourceArmLoadBalancerSchema() map[string]*pluginsdk.Schema {
Set: pluginsdk.HashString,
},

"zones": commonschema.ZonesMultipleOptionalForceNew(),
"zones": commonschema.ZonesMultipleOptional(),

"id": {
Type: pluginsdk.TypeString,
Expand Down
63 changes: 63 additions & 0 deletions internal/services/loadbalancer/loadbalancer_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,13 +169,34 @@ func TestAccAzureRMLoadBalancer_zonesSingle(t *testing.T) {
r := LoadBalancer{}

data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.standard(data),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep(),
{
Config: r.zonesSingle(data),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep(),
{
Config: r.zonesSingleUpdated(data),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep(),
{
Config: r.standard(data),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep(),
})
}

Expand Down Expand Up @@ -601,6 +622,48 @@ resource "azurerm_lb" "test" {
`, data.RandomInteger, data.Locations.Primary)
}

func (r LoadBalancer) zonesSingleUpdated(data acceptance.TestData) string {
return fmt.Sprintf(`
provider "azurerm" {
features {}
}

resource "azurerm_resource_group" "test" {
name = "acctestRG-lb-%[1]d"
location = "%[2]s"
}

resource "azurerm_virtual_network" "test" {
name = "acctvn-%[1]d"
address_space = ["10.0.0.0/16"]
location = azurerm_resource_group.test.location
resource_group_name = azurerm_resource_group.test.name
}

resource "azurerm_subnet" "test" {
name = "acctsub-%[1]d"
resource_group_name = azurerm_resource_group.test.name
virtual_network_name = azurerm_virtual_network.test.name
address_prefixes = ["10.0.2.0/24"]
}

resource "azurerm_lb" "test" {
name = "acctestlb-%[1]d"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
sku = "Standard"

frontend_ip_configuration {
name = "Internal"
private_ip_address_allocation = "Static"
private_ip_address_version = "IPv4"
private_ip_address = "10.0.2.7"
subnet_id = azurerm_subnet.test.id
}
}
`, data.RandomInteger, data.Locations.Primary)
}

func (r LoadBalancer) zonesMultiple(data acceptance.TestData) string {
return fmt.Sprintf(`
provider "azurerm" {
Expand Down
16 changes: 16 additions & 0 deletions internal/tf/pluginsdk/customize_diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
type (
CustomizeDiffFunc = func(context.Context, *ResourceDiff, interface{}) error
ValueChangeConditionFunc = func(ctx context.Context, old, new, meta interface{}) bool
ResourceConditionFunc = func(ctx context.Context, d *schema.ResourceDiff, meta interface{}) bool
)

// CustomDiffWithAll returns a CustomizeDiffFunc that runs all of the given
Expand Down Expand Up @@ -68,3 +69,18 @@ func ForceNewIfChange(key string, f ValueChangeConditionFunc) CustomizeDiffFunc
return nil
}
}

// ForceNewIf returns a CustomizeDiffFunc that flags the given key as
// requiring a new resource if the given condition function returns true.
//
// The return value of the condition function is ignored if the old and new
// values of the field compare equal, since no attribute diff is generated in
// that case.
func ForceNewIf(key string, f ResourceConditionFunc) CustomizeDiffFunc {
return func(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error {
if f(ctx, d, meta) {
d.ForceNew(key)
}
return nil
}
}