Skip to content

Commit

Permalink
update tests and fix a bug
Browse files Browse the repository at this point in the history
  • Loading branch information
ms-henglu committed Apr 14, 2021
1 parent e8823b5 commit c77fdad
Show file tree
Hide file tree
Showing 3 changed files with 138 additions and 75 deletions.
74 changes: 23 additions & 51 deletions azurerm/internal/services/containers/container_registry_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func resourceContainerRegistry() *schema.Resource {
Optional: true,
Computed: true, // TODO -- remove this when deprecation resolves
ConflictsWith: []string{"georeplication_locations"},
ConfigMode: schema.SchemaConfigModeAttr,
ConfigMode: schema.SchemaConfigModeAttr, // TODO -- remove in 3.0, because this property is optional and computed, it has to be declared as empty array to remove existed values
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"location": {
Expand Down Expand Up @@ -364,20 +364,16 @@ func resourceContainerRegistryCreate(d *schema.ResourceData, meta interface{}) e
return fmt.Errorf("Error waiting for creation of Container Registry %q (Resource Group %q): %+v", name, resourceGroup, err)
}

// locations have been specified for geo-replication
// the ACR is being created so no previous geo-replication locations
var oldGeoReplicationLocations, newGeoReplicationLocations []*containerregistry.Replication
if geoReplicationLocations != nil && geoReplicationLocations.Len() > 0 {
// the ACR is being created so no previous geo-replication locations
var oldGeoReplicationLocations []*containerregistry.Replication
err = applyGeoReplicationLocations(d, meta, resourceGroup, name, oldGeoReplicationLocations, expandReplicationsFromLocations(geoReplicationLocations.List()))
if err != nil {
return fmt.Errorf("Error applying geo replications for Container Registry %q (Resource Group %q): %+v", name, resourceGroup, err)
}
newGeoReplicationLocations = expandReplicationsFromLocations(geoReplicationLocations.List())
} else {
newGeoReplicationLocations = expandReplications(geoReplications)
}
// geo replications have been specified
if len(geoReplications) > 0 {
// the ACR is being created so no previous geo-replication locations
var oldGeoReplicationLocations []*containerregistry.Replication
err = applyGeoReplicationLocations(d, meta, resourceGroup, name, oldGeoReplicationLocations, expandReplications(geoReplications))
if len(newGeoReplicationLocations) > 0 {
err = applyGeoReplicationLocations(d, meta, resourceGroup, name, oldGeoReplicationLocations, newGeoReplicationLocations)
if err != nil {
return fmt.Errorf("Error applying geo replications for Container Registry %q (Resource Group %q): %+v", name, resourceGroup, err)
}
Expand Down Expand Up @@ -530,65 +526,41 @@ func applyContainerRegistrySku(d *schema.ResourceData, meta interface{}, sku str
return nil
}

func applyGeoReplicationLocations(d *schema.ResourceData, meta interface{}, resourceGroup string, name string, oldGeoReplicationLocations []*containerregistry.Replication, newGeoReplicationLocations []*containerregistry.Replication) error {
func applyGeoReplicationLocations(d *schema.ResourceData, meta interface{}, resourceGroup string, name string, oldGeoReplications []*containerregistry.Replication, newGeoReplications []*containerregistry.Replication) error {
replicationClient := meta.(*clients.Client).Containers.ReplicationsClient
ctx, cancel := timeouts.ForCreateUpdate(meta.(*clients.Client).StopContext, d)
defer cancel()
log.Printf("[INFO] preparing to apply geo-replications for Container Registry.")

createReplications := make(map[string]*containerregistry.Replication)

// loop on the new location values
for _, nl := range newGeoReplicationLocations {
if nl == nil {
// delete previously deployed locations
for _, replication := range oldGeoReplications {
if replication == nil {
continue
}
newLocation := azure.NormalizeLocation(*nl.Location)
createReplications[newLocation] = nl // the replication needs to be created
}
oldLocation := azure.NormalizeLocation(*replication.Location)

// loop on the old location values
for _, ol := range oldGeoReplicationLocations {
if ol == nil {
continue
}
// oldLocation was created from a previous deployment
oldLocation := azure.NormalizeLocation(*ol.Location)

delete(createReplications, oldLocation) // the location do not need to be created, it already exists
}

// create new geo-replication locations
for locationToCreate, replication := range createReplications {
future, err := replicationClient.Create(ctx, resourceGroup, name, locationToCreate, *replication)
future, err := replicationClient.Delete(ctx, resourceGroup, name, oldLocation)
if err != nil {
return fmt.Errorf("Error creating Container Registry Replication %q (Resource Group %q, Location %q): %+v", name, resourceGroup, locationToCreate, err)
return fmt.Errorf("Error deleting Container Registry Replication %q (Resource Group %q, Location %q): %+v", name, resourceGroup, oldLocation, err)
}

if err = future.WaitForCompletionRef(ctx, replicationClient.Client); err != nil {
return fmt.Errorf("Error waiting for creation of Container Registry Replication %q (Resource Group %q, Location %q): %+v", name, resourceGroup, locationToCreate, err)
return fmt.Errorf("Error waiting for deletion of Container Registry Replication %q (Resource Group %q, Location %q): %+v", name, resourceGroup, oldLocation, err)
}
}

// loop on the list of previously deployed locations
for _, ol := range oldGeoReplicationLocations {
if ol == nil {
continue
}
oldLocation := azure.NormalizeLocation(*ol.Location)
// if the old location is still in the list of locations, then continue
if _, ok := createReplications[oldLocation]; ok {
// create new geo-replication locations
for _, replication := range newGeoReplications {
if replication == nil {
continue
}

// the old location is not in the list of locations, delete it
future, err := replicationClient.Delete(ctx, resourceGroup, name, oldLocation)
locationToCreate := azure.NormalizeLocation(*replication.Location)
future, err := replicationClient.Create(ctx, resourceGroup, name, locationToCreate, *replication)
if err != nil {
return fmt.Errorf("Error deleting Container Registry Replication %q (Resource Group %q, Location %q): %+v", name, resourceGroup, oldLocation, err)
return fmt.Errorf("Error creating Container Registry Replication %q (Resource Group %q, Location %q): %+v", name, resourceGroup, locationToCreate, err)
}

if err = future.WaitForCompletionRef(ctx, replicationClient.Client); err != nil {
return fmt.Errorf("Error waiting for deletion of Container Registry Replication %q (Resource Group %q, Location %q): %+v", name, resourceGroup, oldLocation, err)
return fmt.Errorf("Error waiting for creation of Container Registry Replication %q (Resource Group %q, Location %q): %+v", name, resourceGroup, locationToCreate, err)
}
}

Expand Down
137 changes: 114 additions & 23 deletions azurerm/internal/services/containers/container_registry_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ func TestAccContainerRegistry_geoReplicationLocation(t *testing.T) {
data.ResourceTest(t, r, []resource.TestStep{
// first config creates an ACR with locations
{
Config: r.geoReplicationLocation(data, skuPremium, []string{secondaryLocation}),
Config: r.geoReplicationLocation(data, []string{secondaryLocation}),
Check: resource.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("sku").HasValue(skuPremium),
Expand All @@ -227,25 +227,35 @@ func TestAccContainerRegistry_geoReplicationLocation(t *testing.T) {
},
// second config updates the ACR with updated locations
{
Config: r.geoReplicationLocation(data, skuPremium, []string{ternaryLocation}),
Config: r.geoReplicationLocation(data, []string{ternaryLocation}),
Check: resource.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("sku").HasValue(skuPremium),
check.That(data.ResourceName).Key("georeplication_locations.#").HasValue("1"),
check.That(data.ResourceName).Key("georeplication_locations.0").HasValue(ternaryLocation),
),
},
// third config updates the ACR with updated locations
{
Config: r.geoReplicationLocation(data, []string{secondaryLocation, ternaryLocation}),
Check: resource.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("sku").HasValue(skuPremium),
check.That(data.ResourceName).Key("georeplication_locations.#").HasValue("2"),
),
},
data.ImportStep(),
// For compatibility, downgrade from Premium to Basic should remove all replications first, but it's unnecessary. Once georeplication_locations is deprecated, this can be done in single update.
// third config updates the ACR with no location.
// fourth config updates the ACR with no location.
{
Config: r.geoReplicationUpdateWithNoLocation(data, skuPremium),
Config: r.geoReplicationUpdateWithNoLocation(data),
Check: resource.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("sku").HasValue(skuPremium),
check.That(data.ResourceName).Key("georeplication_locations.#").HasValue("0"),
),
},
// fourth config updates the SKU to basic.
// fifth config updates the SKU to basic.
{
Config: r.geoReplicationUpdateWithNoLocation_basic(data),
Check: resource.ComposeTestCheckFunc(
Expand All @@ -270,38 +280,56 @@ func TestAccContainerRegistry_geoReplication(t *testing.T) {
data.ResourceTest(t, r, []resource.TestStep{
// first config creates an ACR with locations
{
Config: r.geoReplication(data, skuPremium, secondaryLocation),
Config: r.geoReplication(data, secondaryLocation),
Check: resource.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("sku").HasValue(skuPremium),
check.That(data.ResourceName).Key("georeplications.#").HasValue("1"),
check.That(data.ResourceName).Key("georeplications.0.location").HasValue(secondaryLocation),
check.That(data.ResourceName).Key("georeplications.0.tags.%").HasValue("1"),
check.That(data.ResourceName).Key("georeplications.0.tags.Environment").HasValue("Production"),
),
},
data.ImportStep(),
// second config updates the ACR with updated locations
{
Config: r.geoReplication(data, skuPremium, ternaryLocation),
Config: r.geoReplication(data, ternaryLocation),
Check: resource.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("sku").HasValue(skuPremium),
check.That(data.ResourceName).Key("georeplications.#").HasValue("1"),
check.That(data.ResourceName).Key("georeplications.0.location").HasValue(ternaryLocation),
check.That(data.ResourceName).Key("georeplications.0.tags.%").HasValue("1"),
check.That(data.ResourceName).Key("georeplications.0.tags.Environment").HasValue("Production"),
),
},
data.ImportStep(),
// third config updates the ACR with updated locations
{
Config: r.geoReplicationMultipleLocations(data, secondaryLocation, ternaryLocation),
Check: resource.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("sku").HasValue(skuPremium),
check.That(data.ResourceName).Key("georeplications.#").HasValue("2"),
check.That(data.ResourceName).Key("georeplications.0.location").HasValue(secondaryLocation),
check.That(data.ResourceName).Key("georeplications.0.tags.%").HasValue("0"),
check.That(data.ResourceName).Key("georeplications.1.location").HasValue(ternaryLocation),
check.That(data.ResourceName).Key("georeplications.1.tags.%").HasValue("0"),
),
},
data.ImportStep(),
// For compatibility, downgrade from Premium to Basic should remove all replications first, but it's unnecessary. Once georeplication_locations is deprecated, this can be done in single update.
// third config updates the ACR with no location
// fourth config updates the ACR with no location
{
Config: r.geoReplicationUpdateWithNoReplication(data, skuPremium),
Config: r.geoReplicationUpdateWithNoReplication(data),
Check: resource.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("sku").HasValue(skuPremium),
check.That(data.ResourceName).Key("georeplications.#").HasValue("0"),
),
},
data.ImportStep(),
// fourth config updates the SKU to basic.
// fifth config updates the SKU to basic.
{
Config: r.geoReplicationUpdateWithNoReplication_basic(data),
Check: resource.ComposeTestCheckFunc(
Expand All @@ -313,6 +341,43 @@ func TestAccContainerRegistry_geoReplication(t *testing.T) {
})
}

func TestAccContainerRegistry_geoReplicationSwitch(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_container_registry", "test")
r := ContainerRegistryResource{}

skuPremium := "Premium"

secondaryLocation := location.Normalize(data.Locations.Secondary)
ternaryLocation := location.Normalize(data.Locations.Ternary)

data.ResourceTest(t, r, []resource.TestStep{
// first config creates an ACR using georeplication_locations
{
Config: r.geoReplicationLocation(data, []string{secondaryLocation, ternaryLocation}),
Check: resource.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("sku").HasValue(skuPremium),
check.That(data.ResourceName).Key("georeplication_locations.#").HasValue("2"),
),
},
data.ImportStep(),
// second config updates the ACR using georeplications
{
Config: r.geoReplicationMultipleLocations(data, secondaryLocation, ternaryLocation),
Check: resource.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("sku").HasValue(skuPremium),
check.That(data.ResourceName).Key("georeplications.#").HasValue("2"),
check.That(data.ResourceName).Key("georeplications.0.location").HasValue(secondaryLocation),
check.That(data.ResourceName).Key("georeplications.0.tags.%").HasValue("0"),
check.That(data.ResourceName).Key("georeplications.1.location").HasValue(ternaryLocation),
check.That(data.ResourceName).Key("georeplications.1.tags.%").HasValue("0"),
),
},
data.ImportStep(),
})
}

func TestAccContainerRegistry_networkAccessProfileIp(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_container_registry", "test")
r := ContainerRegistryResource{}
Expand Down Expand Up @@ -552,7 +617,7 @@ resource "azurerm_container_registry" "test" {
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger)
}

func (ContainerRegistryResource) geoReplicationLocation(data acceptance.TestData, sku string, replicationRegions []string) string {
func (ContainerRegistryResource) geoReplicationLocation(data acceptance.TestData, replicationRegions []string) string {
regions := make([]string, 0)
for _, region := range replicationRegions {
// ensure they're quoted
Expand All @@ -572,13 +637,13 @@ resource "azurerm_container_registry" "test" {
name = "testacccr%d"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
sku = "%s"
sku = "Premium"
georeplication_locations = [%s]
}
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, sku, strings.Join(regions, ","))
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, strings.Join(regions, ","))
}

func (ContainerRegistryResource) geoReplication(data acceptance.TestData, sku string, replication string) string {
func (ContainerRegistryResource) geoReplication(data acceptance.TestData, replication string) string {
return fmt.Sprintf(`
provider "azurerm" {
features {}
Expand All @@ -593,18 +658,44 @@ resource "azurerm_container_registry" "test" {
name = "testacccr%d"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
sku = "%s"
sku = "Premium"
georeplications {
location = %s
location = "%s"
tags = {
Environment = "Production"
}
}
}
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, sku, fmt.Sprintf("%q", replication))
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, replication)
}

func (ContainerRegistryResource) geoReplicationMultipleLocations(data acceptance.TestData, primaryLocation string, secondaryLocation string) string {
return fmt.Sprintf(`
provider "azurerm" {
features {}
}
resource "azurerm_resource_group" "test" {
name = "acctestRG-aks-%d"
location = "%s"
}
resource "azurerm_container_registry" "test" {
name = "testacccr%d"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
sku = "Premium"
georeplications {
location = "%s"
}
georeplications {
location = "%s"
}
}
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, primaryLocation, secondaryLocation)
}

func (ContainerRegistryResource) geoReplicationUpdateWithNoLocation(data acceptance.TestData, sku string) string {
func (ContainerRegistryResource) geoReplicationUpdateWithNoLocation(data acceptance.TestData) string {
return fmt.Sprintf(`
provider "azurerm" {
features {}
Expand All @@ -619,13 +710,13 @@ resource "azurerm_container_registry" "test" {
name = "testacccr%d"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
sku = "%s"
sku = "Premium"
georeplication_locations = []
}
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, sku)
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger)
}

func (ContainerRegistryResource) geoReplicationUpdateWithNoReplication(data acceptance.TestData, sku string) string {
func (ContainerRegistryResource) geoReplicationUpdateWithNoReplication(data acceptance.TestData) string {
return fmt.Sprintf(`
provider "azurerm" {
features {}
Expand All @@ -640,10 +731,10 @@ resource "azurerm_container_registry" "test" {
name = "testacccr%d"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
sku = "%s"
sku = "Premium"
georeplications = []
}
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, sku)
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger)
}
func (ContainerRegistryResource) geoReplicationUpdateWithNoLocation_basic(data acceptance.TestData) string {
return fmt.Sprintf(`
Expand Down
2 changes: 1 addition & 1 deletion website/docs/r/container_registry.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ The following arguments are supported:

* `tags` - (Optional) A mapping of tags to assign to the resource.

* `georeplication_locations` - (Optional) A list of Azure locations where the container registry should be geo-replicated.
* `georeplication_locations` - (Optional / **Deprecated in favor of `georeplications`**) A list of Azure locations where the container registry should be geo-replicated.

~> **NOTE:** The `georeplication_locations` is only supported on new resources with the `Premium` SKU.

Expand Down

0 comments on commit c77fdad

Please sign in to comment.