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

Container group cannot be updated #56

Open
roncsak opened this issue Jul 22, 2021 · 9 comments
Open

Container group cannot be updated #56

roncsak opened this issue Jul 22, 2021 · 9 comments
Labels
enhancement New feature or request idle

Comments

@roncsak
Copy link

roncsak commented Jul 22, 2021

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
Copy link

HonzaV commented Jul 25, 2021

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
Copy link

github-actions bot commented Aug 8, 2021

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

@github-actions github-actions bot added the idle label Aug 8, 2021
@kanika1894
Copy link
Contributor

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

@kanika1894 kanika1894 added question Further information is requested and removed need-to-triage labels Aug 24, 2021
@roncsak
Copy link
Author

roncsak commented Aug 29, 2021

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 github-actions bot removed the idle label Aug 29, 2021
@github-actions
Copy link

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

@github-actions github-actions bot added the idle label Sep 12, 2021
@kanika1894
Copy link
Contributor

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
Copy link
Author

roncsak commented Sep 24, 2021

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.

@github-actions github-actions bot removed the idle label Sep 24, 2021
@kanika1894
Copy link
Contributor

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!

@kanika1894 kanika1894 added enhancement New feature or request and removed question Further information is requested labels Sep 27, 2021
@github-actions
Copy link

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

@github-actions github-actions bot added the idle label Oct 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request idle
Projects
None yet
Development

No branches or pull requests

3 participants