Container group cannot be updated

roncsak commented 3 years ago

Hello!

I wanted to to use this action for the following use-case:

Build docker image with GH Actions
Push docker image to Azure Container Registry with GH Actions
Deploy the pushed image to Container Instances with GH Actions (this action does it actually)

Description

For some reason it failed and GH Actions log said that if I wish to update memory, cpu, os, restart strategy (and whatnot) I should delete and create a new ACI.

After some thinking I checked the actions.yml and I found these optional inputs with default values:

cpu (default is: 1)
memory (default is: 1.5)
restart-policy (default is: Always)

However I created my ACI with 2 cpus, 7GB memory, and OnFailure restart policy.

After I set these optional inputs to actual values in my GitHub Action deployment works.

Why it is a problem?

I simple want to deploy a new image from the container registry to the container instance. I create my ACI with arm template or Bicep and I don't want to define cpu, memory, os (etc) specific metadata during this action.

How would be better?

If someone does not defines these inputs, don't set a default value to it. If the action cannot update ACI without them, make these inputs required without default values.

Regards, roncsak

HonzaV commented 3 years ago

I had the same problem. The cause was, that when creting container instances I used non-default values for RAM nad the aci-deploy action has default values on these params and these values were different, therefore Azure thinks "Oh hey, you are trying add more memory? No way without delete." So the solution was to specify these optional params in pipeline and set them to the same values as in Container Instances.

It also updated my secure env variables. I have some secrets there, so I can't specify it in pipeline. It could be resolved adding them to Gihtub Secrets probably, but I'd prefer not to have specify them at all.

github-actions[bot] commented 3 years ago

This issue is idle because it has been open for 14 days with no activity.

kanika1894 commented 3 years ago

Thanks for your insights @HonzaV @roncsak Were you able to make progress?

roncsak commented 3 years ago

Hello @kanika1894, if you read carefully, @HonzaV confirms the issue and the workaround I had, too. I see only two options viable:

Make cpu, memory, restart-policy mandatory
Let cpu, memory, restart-policy optional and aci-deploy should know that it is currently updating an ACI or creating a new one. Upon creating a new one, use default values (in case not provided) to create ACI but check set values of created ACI upon updating.

The latter one would be the ultimate solution.

github-actions[bot] commented 3 years ago

This issue is idle because it has been open for 14 days with no activity.

kanika1894 commented 2 years ago

Hey @roncsak,

The default values for cpu, memory and restart-policy are given for the scenario when the user deploys a new container from the workflow itself. The error The updates on container group 'mycontainer' are invalid. If you are going to update the os type, restart policy, network profile, CPU, memory, or GPU resources for a container group, you must delete it first and then create a new one. comes when a user is trying to update an existing container with a new image(having different values for the specified parameters). Also, if the update is except these image parameters, say, while updating dns-name-label, this won't come.

This is in sync with the fact that changes in certain properties require deletion and redeployment. Please refer this doc for more information.

roncsak commented 2 years ago

Hey @kanika1894,
that's the very thing that I don't want to!

Just look at the example code written here.

        - name: 'Deploy to Azure Container Instances'
          uses: 'azure/aci-deploy@v1'
          with:
            resource-group: contoso
            dns-name-label: url-for-container
            image: contoso.azurecr.io/nodejssampleapp:${{ github.sha }}
            registry-username: ${{ secrets.REGISTRY_USERNAME }}
            registry-password: ${{ secrets.REGISTRY_PASSWORD }}
            name: contoso-container
            location: 'west us'

Do you see? Even the example code omits the usage of cpu, memory and restart-plocy.

So what happens if an infastructure guy creates a Container Instance beforehand with 2 vcpu and 2 GB memory? This example code will fail because this GH Action will pass default values of those attributes! The problem is, that a regular developer shouldn't care about how much cpu and memory this resource has. If developers want to change the image only, they shouldn't care providing cpu and memory.
Operations will monitor the resource and they will act if needed.

The very simple problem with this action that it provides default values for some attributes (e.g. cpu and memory) even if the Container Instance is already created! It could be easily fixed as I wrote in the original post.

kanika1894 commented 2 years ago

I understand your concern @roncsak . Thanks a lot for explaining it with all details. As of now, the requirement of these parameters is mandatory by design. To address your concern regarding ease of usage, we'll consider this enhancement. Can't promise a date yet, will have to see when can this be prioritized. Meanwhile, feel free to contribute and raise a PR. Contributions are most welcome!

github-actions[bot] commented 2 years ago

This issue is idle because it has been open for 14 days with no activity.

Azure / aci-deploy