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_container_registry - support update replications on demand #16678

Merged
merged 3 commits into from May 18, 2022

Conversation

magodo
Copy link
Collaborator

@magodo magodo commented May 6, 2022

This PR adds more finer grained control on how to update the replications for azurerm_container_registry.

Previously, any change in the georeplications will cause all replications to be recreated. This causes unexpected downtime for replications that are not affected (see #16634). This PR fixes it in way that:

  • Only delete replications that are deleted from the config
  • Only create replications that are newly added into the config
  • Only Update replications that are updated in the config
  • Otherwise, do nothing to the replication

Specially for update replications, API currently doesn't support updating the zoneRedundancy in place, which makes us have to delete then create it. Also, API has a bug when create a just deleted replication, see: Azure/azure-rest-api-specs#18934. Therefore, I have added a explicit wait in this PR to avoid that.

Additionally, during the test I noticed the LIST api of the replication will return the replications in arbitrary order. I have sorted it for the georeplications alphabetically during flattening. So that make the plan diff more readable for users (assuming users order their config alphabetically as well). Ideally, it should use Set instead, but it can't.

Fixes #16634.

Test

💢  TF_ACC=1 go test -v -timeout=20h ./internal/services/containers -run='TestAccContainerRegistry_geoReplication|TestAccContainerRegistry_geoReplicationLocation'
=== RUN   TestAccContainerRegistry_geoReplicationLocation
=== PAUSE TestAccContainerRegistry_geoReplicationLocation
=== RUN   TestAccContainerRegistry_geoReplication
=== PAUSE TestAccContainerRegistry_geoReplication
=== RUN   TestAccContainerRegistry_geoReplicationZoneRedundancy
=== PAUSE TestAccContainerRegistry_geoReplicationZoneRedundancy
=== RUN   TestAccContainerRegistry_geoReplicationRegionEndpoint
=== PAUSE TestAccContainerRegistry_geoReplicationRegionEndpoint
=== CONT  TestAccContainerRegistry_geoReplicationLocation
=== CONT  TestAccContainerRegistry_geoReplicationZoneRedundancy
=== CONT  TestAccContainerRegistry_geoReplicationRegionEndpoint
=== CONT  TestAccContainerRegistry_geoReplication
--- PASS: TestAccContainerRegistry_geoReplicationRegionEndpoint (259.38s)
--- PASS: TestAccContainerRegistry_geoReplicationZoneRedundancy (350.45s)
--- PASS: TestAccContainerRegistry_geoReplicationLocation (1235.67s)
--- PASS: TestAccContainerRegistry_geoReplication (1965.56s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/containers    1965.580s

Additionally, I've locally inspected the API sequence and confirmed it worked as expected.

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @magodo - LGTM 🐍

website/docs/r/container_registry.html.markdown Outdated Show resolved Hide resolved
@katbyte
Copy link
Collaborator

katbyte commented May 12, 2022

@magodo - could we fix up the merge conflics to get this merged? thanks

@magodo
Copy link
Collaborator Author

magodo commented May 13, 2022

@katbyte Sure, I've resolved it. Thank you for the review!

@katbyte katbyte merged commit 7630e49 into hashicorp:main May 18, 2022
@github-actions github-actions bot added this to the v3.7.0 milestone May 18, 2022
katbyte added a commit that referenced this pull request May 18, 2022
@github-actions
Copy link

This functionality has been released in v3.7.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Geo-replication regions deleted/recreated when modified
2 participants