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
182 stars 39 forks source link

Provide better guardrails for .NET Framework TFMs #525

Open baronfel opened 1 year ago

baronfel commented 1 year ago

@wasabiii has reported on the MSBuild channel of the DotNetEvolution discord that we have a few issues with our .NET Framework TFM support. At minimum we set an entrypoint of dotnet.exe path/to.dll for these scenarios, which 100% will not work.

We should instead detect this case and fail fast. We don't support packaging .NET Framework apps in containers, because they do not target Linux.

wasabii commented 7 months ago

It can be fixed for Framework, as well as Windows images, pretty easily, as well, if that's a choice.

baronfel commented 1 month ago

Windows images are no longer supported implicitly in the manifest-list images created by the .NET team, so this will be a very low priority.

wasabii commented 1 month ago

Can you explain a bit more what that means?

baronfel commented 1 month ago

I got fixated on the

as well as Windows images

part of your previous context and forgot about the Framework part. Broadly, the tooling requires anyone wanting to target Windows to explicitly specify base image information because Windows containers do not provide enough information to make an informed decision. Basically we don't think the juice is worth the squeeze for this repo. Of course we'd accept PRs making the entrypoints work for .NET Framework apps but any Framework support is incidental as opposed to designed.

wasabii commented 1 month ago

Ahh. Gotcha.

wasabii commented 1 month ago

So, for knowledge, at this point I've run into 5 different clients that I've done work for to get their Framework apps running on Windows images using the SDK container support. Requirements range from things like old WCF services, to old WebAPI (actually needing IIS), to needing to access esoteric Windows APIs: for instance, the first client I had which needed this is a service that autogenerates PPKG files using Windows ICD APIs. Clearly that's never going to run on Linux, Core or not (but right now MS only distributes Framework APIs for it).

These apps sit in solutions along side new .NET core apps. Most of the time they're just standalone Framework executables which need to run on Windows for things like ICD. So we build both types of images from the same solution. Which is something Docker itself cannot do, since the system daemon has to switch between Windows and Linux mode. Hence why the SDK container support is so important here: it is the only way to take a single VS Solution and build container images for both Linux and Windows at the same time on the same machine as part of your MSBuild. And it's super clean: you just run the MSBuild and get a bunch of .tar.gz files for all of your projects, both Windows and Linux.

And it's great because you can throw the results all together into AKS, by just adding some Windows nodes into your cluster and setting up tolerations.

Like, it's perfect.

So please don't break it. :)

wasabii commented 1 month ago

It's pretty close to working right out of the box on SDK 8, as well. There are only a few adjustments I've needed to make:

ContainerRuntimeIdentifier needs to be set by hand to win10-x64, ContainerBaseImage needs to be set by hand to the correct Windows image, and the ContainerAppCommand ItemGroup needs to be adjusted a bit, else it tries to launch the executable incorrectly:

    <Target Name="BeforePublishContainer" BeforeTargets="PublishContainer">
        <ItemGroup>
            <ContainerAppCommand Remove="@(ContainerAppCommand)" />
            <ContainerAppCommand Include="$(AssemblyName)$(_NativeExecutableExtension)" />
        </ItemGroup>
    </Target>

If I were to fix anything, it would just be to spit out an error for Framework TFMs, that you need to manually specify a ContainerBaseImage, and for it to figure out the appcommand on it's own.

baronfel commented 1 month ago

If you want to PR any/all of that we'd take it - the ContainerBaseImage we cannot infer correctly because of the technical limitations I alluded to above, but the AppCommand and Framework Error message would be welcome improvements.