dotnet/sdk-container-builds

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

Opened this issue · 4 comments

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.

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:

  • what full name to give to the container
  • where to push the container

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.

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

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

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

    internal CliOption<bool> PushToRemoteRegistry { get; } = new("--pushtoremoteregistry")
    {
        Description = "...",
        Required = false,
        DefaultValueFactory = (_) => false
    };
  • Within DestinationImageReference.CreateFromSettings, check for a valid combination of outputRegistry and pushToRemoteRegistry → ensures that one cannot set <PushToRemoteRegistry>true</PushToRemoteRegistry> without setting <ContainerRegistry>:
        else if (pushToRemoteRegistry && string.IsNullOrEmpty(outputRegistry))
        {
            throw new OutputRegistryMustBeSpecifiedWhenUsingPushToRegistryException();
        }
        if (!pushToRemoteRegistry) logger.LogInformation(...);

        try
        {
            await (destinationImageReference.RemoteRegistry!.PushAsync(
                builtImage,
                sourceImageReference,
                destinationImageReference,
                cancellationToken)).ConfigureAwait(false);
            logger.LogInformation(Strings.ContainerBuilder_ImageUploadedToRegistry, destinationImageReference, destinationImageReference.RemoteRegistry.RegistryName);
        }

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...