dotnet / sdk-container-builds

Libraries and build tooling to create container images from .NET projects using MSBuild
https://learn.microsoft.com/en-us/dotnet/core/docker/publish-as-container
MIT License
179 stars 34 forks source link

Request to add option to disable default pushing to remote registry when specifying ContainerRegistry #509

Open vyruz1986 opened 1 year ago

vyruz1986 commented 1 year ago

When adding the ContainerRegistry property to a project, and specifying a remote registry, it seems the default behaviour is that the built image will also be immediately pushed to that remote registry. While this is exactly what I want in a build pipeline environment, I would like to request an option to disable this when building locally.

Often times I find myself running the dotnet publish command locally, to debug a specific image build, these builds should not be published to the remote registry.

I would suggest adding a parameter like PushToRemoteRegistry with a default value of true, so that we can turn this off in our project configs and then pass /p:PushToRemoteRegistry=true in our build pipelines.

The work around I am using now is to not specify ContainerRegistry at all, then in my build pipeline add some docker tag test-image:1.2.3 myregistry.azurecr.io/test-image:1.2.3 commands followed by a docker push myregistry.azurecr.io/test-image:1.2.3 command.

baronfel commented 1 year ago

This is a topic that @Danielku15 has asked about before - I agree it makes sense. Can you see what happens when you set <LocalRegistry>Docker</LocalRegistry> in your project file or via a command-line MSBuild property when you do the publish?

We clearly need more thought around the interactions between:

We've been laser focused on 'put container for project in another registry', but different use cases may require different capabilities, like just tagging without pushing as you describe here.

vyruz1986 commented 1 year ago

Can you see what happens when you set <LocalRegistry>Docker</LocalRegistry> in your project file or via a command-line MSBuild property when you do the publish?

This does not seem to make any difference

mu88 commented 8 months ago

I'd love to see this, too. In my case, it would simplify building/releasing containers within a GitHub Action.

Please see the following part of a GitHub Action (complete file is here):

    - name: Build Docker
      if: ${{ env.IS_RELEASE != 'true' }}
      run: |
        dotnet publish RaspiFanController/RaspiFanController.csproj --os linux --arch arm64 /t:PublishContainer '-p:ContainerImageTags="${{ env.VERSION }};latest"'
        dotnet publish RaspiFanController/RaspiFanController.csproj --os linux --arch arm64 /t:PublishContainer '-p:ContainerImageTags="${{ env.VERSION }}-chiseled;latest-chiseled"' -p:ContainerFamily=jammy-chiseled
    - name: Push Docker
      if: ${{ env.IS_RELEASE == 'true' }}
      run: |
        dotnet publish RaspiFanController/RaspiFanController.csproj --os linux --arch arm64 /t:PublishContainer '-p:ContainerImageTags="${{ env.VERSION }};latest"' -p:ContainerRegistry=registry.hub.docker.com
        dotnet publish RaspiFanController/RaspiFanController.csproj --os linux --arch arm64 /t:PublishContainer '-p:ContainerImageTags="${{ env.VERSION }}-chiseled;latest-chiseled"' -p:ContainerRegistry=registry.hub.docker.com -p:ContainerFamily=jammy-chiseled

Having a boolean MSBuild property like <PushImageToRegistry> would shorten my GitHub Action to:

    - name: Build and Push Docker
      run: |
        dotnet publish RaspiFanController/RaspiFanController.csproj --os linux --arch arm64 /t:PublishContainer '-p:ContainerImageTags="${{ env.VERSION }};latest"' -p:ContainerRegistry=registry.hub.docker.com -p:PushImageToRegistry=${{ env.IS_RELEASE }}
        dotnet publish RaspiFanController/RaspiFanController.csproj --os linux --arch arm64 /t:PublishContainer '-p:ContainerImageTags="${{ env.VERSION }}-chiseled;latest-chiseled"' -p:ContainerRegistry=registry.hub.docker.com -p:ContainerFamily=jammy-chiseled -p:PushImageToRegistry=${{ env.IS_RELEASE }}
mu88 commented 7 months ago

@baronfel: Out of curiosity, I checked the sources and came up with the following approach/idea:

However, this approach would break the current behavior as solely specifying <ContainerRegistry> would no longer push images as <PushToRemoteRegistry> would be false by default.

What do you think? If this looks okay to you, I could give it a try...