Skip to content

Commit

Permalink
azurerm_container_registry - Avoid unnecessary recreate when georep…
Browse files Browse the repository at this point in the history
…lication.tags update (#24994)

* Fix some issue introduced from #15340

* Check georeplication zone redundency by value

* pass ci

* Update test
  • Loading branch information
magodo authored Mar 6, 2024
1 parent cd6d8e7 commit 38d714e
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 30 deletions.
31 changes: 3 additions & 28 deletions internal/services/containers/container_registry_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,9 @@ func resourceContainerRegistry() *pluginsdk.Resource {
CustomizeDiff: pluginsdk.CustomizeDiffShim(func(ctx context.Context, d *pluginsdk.ResourceDiff, v interface{}) error {
sku := d.Get("sku").(string)

hasGeoReplications := false
geoReplications := d.Get("georeplications").([]interface{})
hasGeoReplicationsApplied := hasGeoReplications || len(geoReplications) > 0
// if locations have been specified for geo-replication then, the SKU has to be Premium
if hasGeoReplicationsApplied && !strings.EqualFold(sku, string(registries.SkuNamePremium)) {
if len(geoReplications) > 0 && !strings.EqualFold(sku, string(registries.SkuNamePremium)) {
return fmt.Errorf("ACR geo-replication can only be applied when using the Premium Sku.")
}

Expand Down Expand Up @@ -318,14 +316,8 @@ func resourceContainerRegistryUpdate(d *pluginsdk.ResourceData, meta interface{}
Tags: tags.Expand(d.Get("tags").(map[string]interface{})),
}

var hasGeoReplicationLocationsChanges bool
var hasNewGeoReplicationLocations bool
oldGeoReplicationLocations := make([]interface{}, 0)
newGeoReplicationLocations := make([]interface{}, 0)

// geo replication is only supported by Premium Sku
hasGeoReplicationsApplied := hasNewGeoReplicationLocations || len(newReplications) > 0
if hasGeoReplicationsApplied && !strings.EqualFold(sku, string(registries.SkuNamePremium)) {
if len(newReplications) > 0 && !strings.EqualFold(sku, string(registries.SkuNamePremium)) {
return fmt.Errorf("ACR geo-replication can only be applied when using the Premium Sku.")
}

Expand All @@ -334,11 +326,6 @@ func resourceContainerRegistryUpdate(d *pluginsdk.ResourceData, meta interface{}
if err != nil {
return fmt.Errorf("applying geo replications for %s: %+v", id, err)
}
} else if hasGeoReplicationLocationsChanges {
err := applyGeoReplicationLocations(ctx, meta, *id, expandReplicationsFromLocations(oldGeoReplicationLocations), expandReplicationsFromLocations(newGeoReplicationLocations))
if err != nil {
return fmt.Errorf("applying geo replications for %s: %+v", id, err)
}
}

if err := client.UpdateThenPoll(ctx, *id, parameters); err != nil {
Expand Down Expand Up @@ -432,7 +419,7 @@ func applyGeoReplicationLocations(ctx context.Context, meta interface{}, registr
// each properties are non-nil. Whilst we are still doing nil check here in case.
if oprop, nprop := oldRepl.Properties, newRepl.Properties; oprop != nil && nprop != nil {
// zoneRedundency can't be updated in place
if oprop.ZoneRedundancy != nprop.ZoneRedundancy {
if ov, nv := oprop.ZoneRedundancy, nprop.ZoneRedundancy; ov != nil && nv != nil && *ov != *nv {
needUpdate = true
needReplace = true
}
Expand Down Expand Up @@ -743,18 +730,6 @@ func expandExportPolicy(enabled bool) *registries.ExportPolicy {
return &exportPolicy
}

func expandReplicationsFromLocations(p []interface{}) []replications.Replication {
reps := make([]replications.Replication, 0)
for _, value := range p {
location := azure.NormalizeLocation(value)
reps = append(reps, replications.Replication{
Location: location,
Name: &location,
})
}
return reps
}

func expandReplications(p []interface{}) []replications.Replication {
reps := make([]replications.Replication, 0)
if p == nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package containers_test
import (
"context"
"fmt"
"slices"
"testing"

"github.com/hashicorp/go-azure-helpers/resourcemanager/location"
Expand Down Expand Up @@ -208,8 +209,11 @@ func TestAccContainerRegistry_geoReplicationLocation(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_container_registry", "test")
r := ContainerRegistryResource{}

secondaryLocation := location.Normalize(data.Locations.Secondary)
ternaryLocation := location.Normalize(data.Locations.Ternary)
locs := []string{location.Normalize(data.Locations.Secondary), location.Normalize(data.Locations.Ternary)}
// Sorting the secondary and ternary locations to ensure the order as is expected by this resource (see its Read() function)
slices.Sort(locs)
secondaryLocation := locs[0]
ternaryLocation := locs[1]

data.ResourceTest(t, r, []acceptance.TestStep{
// creates an ACR with locations
Expand Down

0 comments on commit 38d714e

Please sign in to comment.