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

[Containers] Enable multi-RID and multi-project containerization #33166

Closed
wants to merge 3 commits into from

Conversation

baronfel
Copy link
Member

@baronfel baronfel commented Jun 10, 2023

Closes part of dotnet/sdk-container-builds#87

This surfaces the containerization targets in another target that orchestrates the creation of zero or more containers.

When called at the solution level, so-called 'traversal' targets are created for each project, and each project's version of this new target is called in parallel.

For each project, there are a few multi-targeting scenarios that have to be addressed:

How do you containerize a project with a single TFM and no RIDs (but potentially a single RID)?

This is the situation we have today - you just call the existing target.

How do you containerize with multiple TFMs?

Easy, you ask each TFM to create its own container by calling the existing target once for each TFM.

How do you containerize with multiple RIDs (and a single TFM)?

This is the novel thing, since we don't have a concept of multi-RID publishing buttoned down. Based on following patterns from MAUI, we can in parallel build the project with a different set of properties, and have those separate builds create containers for each architecture-specific variant of the container. We take the opportunity here to uniquely tag the containers with architecture-specific information, or else the container image names might overlap. Doing this tagging correctly required mapping .NET RIDs to Docker platform monikers, which was pulled out into a new Task for reuse, and also served to remove the need to rely on the RID graph from the SDK.

Future work

This doesn't create manifest lists, which would 'wrap' each of the architecture-specific images in a wrapper that to external users looks like the same image regardless of architecture used.

Use Cases

Microservices - now you can containerize an entire solution at once. Using dotnet build /t:Containerize && docker-compose up you can build new latest images of each of the projects, and if your Compose file is set up to use image: instead of build: then you are all ready to go!

TODOs

  • re-introduce (limited) RID graph based selection based on constrained RID graphs (not the open-ended model of today) to enable compatibility with source-built SDKs
  • consider removing the multi-RID/multi-TFM publishing for now, and instead focus on the core scenario of enabling solution-level publish for all compatible projects
    • I don't think we can remove multi-TFM and multi-RID publishing if we want to support solution-level publish, because at a solution level you can't really control the TFM/RID selection for each and every project in the solution. So it'll have to stay in some fashion, and we'll just have to evolve it responsibly.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Containers Related to dotnet SDK containers functionality untriaged Request triage from a team member labels Jun 10, 2023
@baronfel baronfel force-pushed the containers-multi-publishing branch from 2684d9b to beae715 Compare June 10, 2023 21:01
<_TFMItems Include="$(TargetFrameworks)" />
<_SingleContainerPublish Include="$(MSBuildProjectFullPath)"
AdditionalProperties="TargetFramework=%(_TFMItems.Identity);
ContainerImageTag=$([MSBuild]::GetTargetFrameworkVersion('%(_TFMItems.Identity)', 2))" />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussion: we need to disambiguate tags here because version/latest doesn't make unique container/tag pairs. I've gone with TFMs here. Is this useful, or should we sidestep/ignore/error the multi-TFM containerization case?

<_SingleContainerPublish Include="$(MSBuildProjectFullPath)"
AdditionalProperties="ContainerRuntimeIdentifier=%(_RIDItemsWithDockerPlatformTag.Identity);
RuntimeIdentifier=%(_RIDItemsWithDockerPlatformTag.Identity);
ContainerImageTag=$(Version)-%(_RIDItemsWithDockerPlatformTag.DockerPlatformTag);" />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's an interesting consequence of moving to latest for image tag inference as a default. Before, $(Version) was used as the default tag, which meant we had something useful to hang a platform-specific bit off of. For example, 1.0.0-amd64 would make sense as a container image tag.

Now that we default to latest, you would see things like latest-amd64, which I have never seen as a pattern or practice before. What I would like to do is just set something like ContainerImageTagSuffix here and flow Version along, but I think there's even a bug with that - $(Version) at the point that this target is run may not be calculated yet. For example my OSS projects generate Version in a target that's hooked up to PrepareForBuild as a dependency. To me, this means we need to find a way to inject this on a per-image basis. I'm not sure how to tackle that one. Love to have any ideas.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rainersigwald hate to ping, but I had a design question that I'd love your input on if you have a moment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know the answer to the implementation question.

My UX 2 cents:

The default publish doesn't include a version number (latest) so when containerizing with multiple rids, I think it's appropriate to not include a version number, like: amd64.

In case the user changes the configuration to include a version number, probably that version number will be the same for all images, and in the multi-rid case it then is combined with the platform suffix, like 1.0.0-amd64.

Comment on lines +16 to +48
runtimeIdentifier = golangPlatform.Split('/') switch {
["linux", "amd64"] when isMuslBased => "linux-musl-x64",
["linux", "amd64"] => "linux-x64",

["linux", "amd64", var _amd64Version] when isMuslBased => "linux-musl-x64",
["linux", "amd64", var _amd64Version] => "linux-x64",

["linux", "arm64"] when isMuslBased => "linux-musl-arm64",
["linux", "arm64"] => "linux-arm64",

["linux", "arm64", var _arm64Version ] when isMuslBased => "linux-musl-arm64",
["linux", "arm64", var _arm64Version ] => "linux-arm64",

["linux", "arm" ] or ["linux", "arm", "v7" ] when isMuslBased => "linux-musl-arm",
["linux", "arm" ] or ["linux", "arm", "v7" ] => "linux-arm",

["linux", "arm", "v6" ] when isMuslBased => "linux-musl-armv6",
["linux", "arm", "v6" ] => "linux-armv6",

["linux", "riscv64" ] when isMuslBased => "linux-musl-riscv64",
["linux", "riscv64" ] => "linux-riscv64",

["linux", "ppc64le" ] when isMuslBased => "linux-musl-ppc64le",
["linux", "ppc64le" ] => "linux-ppc64le",

["linux", "s390x" ] when isMuslBased => "linux-musl-s390x",
["linux", "s390x" ] => "linux-s390x",

["linux", "386" ] when isMuslBased => "linux-musl-x86",
["linux", "386" ] => "linux-x86",

["windows", "amd64"] => "win-x64",
["windows", "arm64"] => "win-arm64",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the overlap between our rid graph (which may change but is generally very static for these 'core' architectures) and the golang platform/arch combinations.

golang has a number of these that .NET doesn't support - but since we're focused on building .NET applications we don't need complete support here.

Comment on lines +76 to +93
public static bool TryGetDockerImageTagForRid(string runtimeIdentifier, [NotNullWhen(true)] out string? dockerPlatformTag) {
dockerPlatformTag = null;

runtimeIdentifier = runtimeIdentifier.Replace("-musl-", "-"); // we lose musl information in the docker platform name

dockerPlatformTag = runtimeIdentifier.Split('-') switch {
["linux", "x64"] => "amd64",
["linux", "arm64"] => "arm64v8",
["linux", "arm"] => "arm32v7",
["linux", "armv6"] => "arm32v6",
["linux", "riscv64"] => "riscv64",
["linux", "ppc64le"] => "ppc64le",
["linux", "s390x"] => "s390x",
["linux", "x86"] => "386",
_ => null // deliberately not trying to make tag names for windows containers
};
return dockerPlatformTag != null;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a slight different in format for the tags you'll see in the wild for docker architectures - there's an assumption of linux, first of all, so you drop that portion and then your just take the right hand side, with small differences for explicitly specifying versions in a couple places.

src/Containers/Microsoft.NET.Build.Containers/Registry.cs Outdated Show resolved Hide resolved
}

private static string? CreateRidForPlatform(PlatformInformation platform)
private static string? CreateRidForPlatform(PlatformInformation platform, bool isMuslBased = false)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this isMuslBased can also be removed when the rid graph is used again?
(The graph maps linux-musl-x64 to linux-x64 which corresponds to an image platform.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is worth digging into a bit more - this is about synthesizing a RID for a docker platform for 'best fit' purposes, and so we need some kind of flag/status from outside the Image Manifest/Image Index to know if the platforms should be interpreted as musl RIDs. This is because the platform info says nothing about musl/libc :( So I put this in here to do the plumbing for that decision, but haven't connected it to the 'outer' image inference code, where I'd expect to check things like "is ContainerImageFamily 'alpine'", or "did the user specify a musl rid", etc. to determine this boolean.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use the NETCoreSdkPortableRuntimeIdentifier property and check if it starts with linux-musl-.

Copy link
Member

@tmds tmds Jun 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignore that. This is the target rid (not the sdk rid).

Either it's linux-musl-{arch} as the portable rid.
Or if it's not portable, you can run it through the runtime graph and see if matches linux-musl.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you can defer linux-musl to another PR. I assume it's mostly about picking an appropriate default base image when the user targets the portable linux-musl rid?

Copy link
Member

@tmds tmds Jun 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the Registry needs to know about isMuslBased.

Like a non-portable rid, the runtime graph will map linux-musl-{arch} to the most compatible rid provided by a container image (which is: linux-{arch}).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh interesting - I missed that completely. I'll have to think about the UX for that. What I'd like to be able to say in the error message is "Image BLAH supports RIDs X, Y, and Z" and have those RIDs be the actual musl-based RIDs if appropriate. Because I generate the error in this call I figured this call would need to have a concept of musl-ness.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that images don't describe if they are musl based.

The sdk could know for some images (in particular: the Microsoft ones) that they are musl-based based on their name.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's the kind of determination I was thinking about hooking up later. For the MS images we know a family called alpine exists so if the user asked for a musl RID we could auto-insert that. Anything else we probably can't rely on - we just have to accept that a user knows what they are doing if the eject out of inference.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. That could be handled by the ComputeDotnetBaseImageTag Task if we pass it the RuntimeIdentifier.

@baronfel baronfel force-pushed the containers-multi-publishing branch from 14962bd to ef6ae36 Compare July 9, 2023 19:45
@baronfel baronfel changed the base branch from release/7.0.4xx to main July 9, 2023 19:45
@baronfel
Copy link
Member Author

baronfel commented Jul 9, 2023

Re-targeted to main for 8.0.100

@baronfel baronfel marked this pull request as ready for review July 9, 2023 20:24
@baronfel baronfel requested a review from a team as a code owner July 9, 2023 20:24
if (osPart is null || platformPart is null) return null;
return $"{osPart}{versionPart ?? ""}-{platformPart}";
var dockerPlatformString = $"{platform.os}/{platform.architecture}{(platform.variant is null ? "" : $"/{platform.variant}")}";
if (PlatformMapping.TryGetRidForDockerPlatform(dockerPlatformString, isMuslBased: isMuslBased, out string? rid)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The result of this method is used to pick the proper base image for the target rid.
This base image is the same one for musl-based and glibc-based image rids.

We can remove the isMuslBased argument here because it doesn't affect the result.

return runtimeIdentifier != null;
}

public static bool TryGetDockerPlatformForRid(string runtimeIdentifier, [NotNullWhen(true)] out string? dockerPlatform) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method isn't used.

["linux", "ppc64le"] => "ppc64le",
["linux", "s390x"] => "s390x",
["linux", "x86"] => "386",
_ => null // deliberately not trying to make tag names for windows containers
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add support for non-portable rids to this method?
It could be to return null for anything that starts with win, and return the currently mapped values based on ending with the arch suffix.

An alternative to map from rids could be to map from docker platforms.

@baronfel
Copy link
Member Author

Closing as this has drifted quite a bit.

@baronfel baronfel closed this Dec 15, 2023
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 untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants