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 Build: multiple registry push, single archive for multiple tags, tar.gz file writing. #32132

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Danielku15
Copy link
Contributor

This PR relates to following issues:

The issues somehow closely relate to the workflow around registries so I decided to rather tackle them all together. The PR has the following adaptions:

  • The single OutputRegistry is converted to an array OutputRegistries to support pushing to multiple registries with following syntax:
    • Not providing any registries to push means a load to local container daemon (e.g. docker)
    • Providing an empty item in the OutputRegistries (or null) also means push to local container daemon. (see open design decision 1)
    • Providing absolute file paths, relative file paths or file URLs means writing a tar.gz archive
    • Providing any other string means a remote registry to push to.
  • Instead of generating one image per tag we now
    • Write one tar.gz with all tags in the RepoTags key (for local tar.gz and daemon load)
    • Pushing the same manifest to multiple tags on the API (for remote registries)

Open Design Decisions:

  1. Shall we better use an explicit string for the local daemon load? Empty strings are often filtered or ignored. Having something more explicit might be better.
    Proposal: Use a string like local-container-daemon as registry to trigger docker load flow.

  2. On all registries the image will be named the same. Maybe the image should be named differently on different registries due to naming policies (public vs internal).
    Proposal: Use the URL fragment syntax to define a different name:

<Project>
  <PropertyGroup>
	<ContainerImageName>my-app</ContainerImageName>
	<!-- Local Docker will have my-app -->
	<ContainerRegistries>local-container-daemon</ContainerRegistries>
	<!-- Internal JFrog Artifactory instance will have team-my-app in the my-docker-repo 'artifactory repository' -->
	<ContainerRegistries>artifactory.my-company.com#my-docker-repo/team-my-app</ContainerRegistries> 
	<!-- Public Docker Hub will  JFrog Artifactory instance will have my-app in the respective user -->
	<ContainerRegistries>artifactory.my-company.com#my-username/my-app</ContainerRegistries> 	
  </PropertyGroup>
</Project>
 

Taken Design Decisions:

  1. We do not allow different tags on different registries within one single dotnet publish command for now. A user would call dotnet publish multiple times with different tags and repositories to do that individualization.
    Pros:
    - We keep the configuration syntax simple for now. The feature can still be added with a special syntax or an additional configuration file.
    Cons:
    - Multiple publish commands will result in different layer hashes for the app due to non-deterministic gzip compression and timestamp differences.

Ping @baronfel for further discussion here.

@Danielku15
Copy link
Contributor Author

@baronfel The PR is at this point still in a draft state to discuss the general design of the different features. Will add the final implementations and all the unit/integration tests once I have the approval that the general approach and extensions are fine like this 😁

@vlada-shubina vlada-shubina added Area-Containers Related to dotnet SDK containers functionality and removed Area-Infrastructure untriaged Request triage from a team member labels May 10, 2023
@menasheh
Copy link

  1. Would it make sense to support multiple ContainerRegistry tags instead of a separate ContainerRegistries tag? Since each line only defines one ContainerRegistry
  2. Side question, Is that # syntax the way artifactory is meant to be used? If so that might explain why my artifactory based docker images are always failing their integrity checks and nonfunctional

@Danielku15
Copy link
Contributor Author

@menasheh

  1. Yes it would be also possible to use an approach like docker push works by defining tags including the registry. This would give docker users a similar experience.
    e.g. we could define a list like the following to define all the registries with respective names and tags. Then this framework would need to do the respective grouping, multit-tag image generation and the push accordingly. Should be farily straight forward but a bit effort.
<ItemGroup>
	<!-- Multiple Tags on company artifactory (name team-my-app in the my-docker-repo folder) -->
	<ContainerImageTags Include="artifactory.my-company.com/my-docker-repo/team-my-app:latest" />
	<ContainerImageTags Include="artifactory.my-company.com/my-docker-repo/team-my-app:v1.0.1" />
	<ContainerImageTags Include="artifactory.my-company.com/my-docker-repo/team-my-app:v1.0" />
	<ContainerImageTags Include="artifactory.my-company.com/my-docker-repo/team-my-app:v1" />
	<!-- Only latest and main release version on docker hub (name my-app under my-username) ->
	<ContainerImageTags Include="registry.hub.docker.com/my-username/my-app:latest" />
	<ContainerImageTags Include="registry.hub.docker.com/my-username/my-app:v1" />
	<!-- Only latest on Azure Container Registry (only my-app, no folder) ->
	<ContainerImageTags Include="myregistry.azurecr.io/my-app:latest" />
	<!-- Proposal: Local daemon push with latest -->
	<ContainerImageTags Include="local-daemon/my-app:latest" />
	<!-- Proposal: Local tar.gz writing via file URLs? -->
	<ContainerImageTags Include="file:///C:/containers/my-app.tar.gz:latest" />
	<ContainerImageTags Include="file:///C:/containers/my-app.tar.gz:v1" />
</ItemGroup>

@baronfel What do you think about this proposal?

@baronfel
Copy link
Member

baronfel commented Aug 2, 2023

Hi folks, very sorry for the delay getting back to this - it's been a very packed July.

I very much think that we should keep these features separate. I agree that there is some similarity between the use cases, but I would rather make sure that individual pushes (to any location) are as solid and well-thought-out as they can be before we get into the world of trying to create a unified interface for multiple-destination publishing. I do agree that @Danielku15's con of - Multiple publish commands will result in different layer hashes for the app due to non-deterministic gzip compression and timestamp differences. is completely correct - and is something that we should work to correct. Once correct, multiple scripted outputs become more trivial to accomplish with traditional MSBuild mechanisms rather than pushing that logic into the Task.

I do really like the insight that different fully-qualified tags should be able to be provided by the user - that is a natural progression of our existing ContainerImageTags mechanism and is something that we should definitely do!

In summary, I'd rather focus on individual publishes being precise and semantically solid (i.e. introducing new flags/properties/etc for publishing to local daemons vs remote registries vs exporting to a local tar.gz), and then as a separate design discussions tackle

  • identify the sources of nondeterminism and stamp them out
  • come up with ways that aren't string-y for requesting multiple pushes to different destinations

As an aside, one thing I worry about with multiple pushes is how the current environment-variable-based username/password last-chance mechanism would work with multiple registries - we'd need to redesign that mechanism for a world with multiple registries I believe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Containers Related to dotnet SDK containers functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants