azurerm_container_group - set storage_account_key in the update operation
#26640
Conversation
There was a problem hiding this comment.
Thanks @neil-yechenwei.
Unfortunately all the tests for container group have been failing since the 2nd of July with the same error
=== CONT TestAccContainerGroup_AssignedIdentityUpdate
testcase.go:121: Step 1/3 error: Error running apply: exit status 1
Error: creating Container Group (Subscription: "*******"
Resource Group Name: "acctestRG-240730232202481222"
Container Group Name: "acctestcontainergroup-240730232202481222"): performing ContainerGroupsCreateOrUpdate: unexpected status 409 (409 Conflict) with error: RegistryErrorResponse: An error response is received from the docker registry 'index.docker.io'. Please retry later.
So I'm unable to run the tests for this. Would you be able to reach out to the service team about this?
|
@stephybun , sure. I will check with service team about the failure. |
|
@stephybun , service team said that this is a known issue due to docker imposing stricter rate limits on docker public registry : DockerHub Rate limit with Azure Container Instance and AKS | by alaa barqawi | Jul, 2024 | Medium. When a rate limit is hit, ACI is unable to pull images from docker, and deployment fails.
|
There was a problem hiding this comment.
Thanks for fixing the tests @neil-yechenwei. I left one more suggestions in-line, once that's resolved this should be good to go.
| // As API doesn't return the value of StorageAccountKey, so it has to get the value from tf config and set it to request payload. Otherwise, the Update API call would fail | ||
| addedEmptyDirs := map[string]bool{} | ||
| _, initContainerVolumes, err := expandContainerGroupInitContainers(d, addedEmptyDirs) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| _, _, containerVolumes, err := expandContainerGroupContainers(d, addedEmptyDirs) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| var containerGroupVolumes []containerinstance.Volume | ||
| if initContainerVolumes != nil { | ||
| containerGroupVolumes = initContainerVolumes | ||
| } | ||
| if containerGroupVolumes != nil { | ||
| containerGroupVolumes = append(containerGroupVolumes, containerVolumes...) | ||
| } | ||
| model.Properties.Volumes = pointer.To(containerGroupVolumes) | ||
|
|
||
| // As Update API doesn't support to update identity, so it has to use CreateOrUpdate API to update identity | ||
| if err := client.ContainerGroupsCreateOrUpdateThenPoll(ctx, *id, model); err != nil { | ||
| return fmt.Errorf("updating %s: %+v", *id, err) | ||
| } |
There was a problem hiding this comment.
If only the tags are updated then we would be making an unnecessary update call. I think this whole block should be moved into
if d.HasChange("identity") {
...
}
|
@stephybun , thanks for the comments. I updated PR. Please take another look. Below is the latest test result I just now triggered and all test cases passed. |
|
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. |
Community Note
Description
API needs storage account key while updating the container group with storage account. Otherwise, the update API call would fail.
PR Checklist
For example: “
resource_name_here- description of change e.g. adding propertynew_property_name_here”Changes to existing Resource / Data Source
Testing
Below failed test cases are also failed with same error on Teamcity Daily Run. So they are not related with this PR.
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_container_group- setstorage_account_keyin the update operationThis is a (please select all that apply):
Related Issue(s)
Fixes #26631
Note
If this PR changes meaningfully during the course of review please update the title and description as required.