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

Update deploy.sh #88

Merged
merged 3 commits into from
Jul 23, 2024
Merged

Update deploy.sh #88

merged 3 commits into from
Jul 23, 2024

Conversation

Dead-Stone
Copy link
Contributor

[BUG] ACR Autocreate may not actually test if ACR already exists #43

Issue

  • Problem: After a Bash session reset, CONTAINER_REGISTRY_SERVER might not be set, causing incorrect logging about creating a new ACR.
  • Steps to Reproduce:
    1. Run deploy.sh to create an ACR.
    2. Reset the Bash session or unset CONTAINER_REGISTRY_SERVER.
    3. Run deploy.sh again; it incorrectly logs creating a new ACR.

Expected Behavior

The script should check if an ACR exists and log correctly.

Fix

Updated createAcrIfNotExists() to always check for an existing ACR and log appropriately.

Updated Function

createAcrIfNotExists() {
    printf "Checking if container registry exists... "
    local existingRegistry
    existingRegistry=$(az acr show --name $CONTAINER_REGISTRY_SERVER --query loginServer -o tsv 2>/dev/null)
    if [ $? -eq 0 ]; then
        printf "Yes. Using existing registry '$existingRegistry'.\n"
        CONTAINER_REGISTRY_SERVER=$existingRegistry
        return 0
    fi

    printf "No. Creating container registry... "
    AZURE_ACR_DEPLOY_RESULT=$(az deployment group create --resource-group $RESOURCE_GROUP --name "acr-deployment" --template-file core/acr/acr.bicep --only-show-errors --no-prompt -o json \
        --parameters "name=$CONTAINER_REGISTRY_SERVER")
    exitIfCommandFailed $? "Error creating container registry, exiting..."
    CONTAINER_REGISTRY_SERVER=$(jq -r .properties.outputs.loginServer.value <<< $AZURE_ACR_DEPLOY_RESULT)
    exitIfValueEmpty "$CONTAINER_REGISTRY_SERVER" "Unable to parse container registry login server from deployment, exiting..."
    printf "Container registry '$CONTAINER_REGISTRY_SERVER' created.\n"
}

Summary

  • Always Checks: Now always checks if the ACR exists.
  • Correct Logging: Logs whether it found an existing ACR or created a new one.

This fix ensures accurate logging and proper handling of the ACR status across sessions. Thanks for your contribution!

## [BUG] ACR Autocreate may not actually test if ACR already exists Azure-Samples#43

### Issue
- **Problem**: After a Bash session reset, `CONTAINER_REGISTRY_SERVER` might not be set, causing incorrect logging about creating a new ACR.
- **Steps to Reproduce**:
  1. Run `deploy.sh` to create an ACR.
  2. Reset the Bash session or unset `CONTAINER_REGISTRY_SERVER`.
  3. Run `deploy.sh` again; it incorrectly logs creating a new ACR.

### Expected Behavior
The script should check if an ACR exists and log correctly.

### Fix
Updated `createAcrIfNotExists()` to always check for an existing ACR and log appropriately.

### Updated Function

```bash
createAcrIfNotExists() {
    printf "Checking if container registry exists... "
    local existingRegistry
    existingRegistry=$(az acr show --name $CONTAINER_REGISTRY_SERVER --query loginServer -o tsv 2>/dev/null)
    if [ $? -eq 0 ]; then
        printf "Yes. Using existing registry '$existingRegistry'.\n"
        CONTAINER_REGISTRY_SERVER=$existingRegistry
        return 0
    fi

    printf "No. Creating container registry... "
    AZURE_ACR_DEPLOY_RESULT=$(az deployment group create --resource-group $RESOURCE_GROUP --name "acr-deployment" --template-file core/acr/acr.bicep --only-show-errors --no-prompt -o json \
        --parameters "name=$CONTAINER_REGISTRY_SERVER")
    exitIfCommandFailed $? "Error creating container registry, exiting..."
    CONTAINER_REGISTRY_SERVER=$(jq -r .properties.outputs.loginServer.value <<< $AZURE_ACR_DEPLOY_RESULT)
    exitIfValueEmpty "$CONTAINER_REGISTRY_SERVER" "Unable to parse container registry login server from deployment, exiting..."
    printf "Container registry '$CONTAINER_REGISTRY_SERVER' created.\n"
}
```

### Summary
- **Always Checks**: Now always checks if the ACR exists.
- **Correct Logging**: Logs whether it found an existing ACR or created a new one.

This fix ensures accurate logging and proper handling of the ACR status across sessions. Thanks for your contribution!
@jgbradley1 jgbradley1 requested a review from a team as a code owner July 22, 2024 22:02
@jgbradley1 jgbradley1 merged commit 106f123 into Azure-Samples:main Jul 23, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants