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

RFC: Extend .Net SDK #185

Merged
merged 9 commits into from
Jun 15, 2022
Merged

RFC: Extend .Net SDK #185

merged 9 commits into from
Jun 15, 2022

Conversation

ForestEckhardt
Copy link
Contributor

@ForestEckhardt ForestEckhardt commented Apr 21, 2022

@ForestEckhardt ForestEckhardt requested a review from a team as a code owner April 21, 2022 15:10
@ForestEckhardt ForestEckhardt added the team/dotnet-core .Net Core Subteam RFC label Apr 21, 2022
@sophiewigmore
Copy link
Member

Overall, I'm on board with this proposal. I think the 2.5 second download speed increase is OK with me, especially since this resolves a breaking issue.

@fg-j
Copy link

fg-j commented Apr 21, 2022

@ForestEckhardt Do you have a running proof-of-concept of this sitting somewhere? If possible, it'd be great to expose that so folks can play around with it.

@fg-j
Copy link

fg-j commented Apr 21, 2022

All in all I support this proposal because I think it aligns with Paketo's buildpacks philosophy around "Meet[ing] language-ecosystem expectations" rather than exploiting undocumented quirks (which it seems like the .NET buildpacks are currently doing).

Moreover, Docker's official documentation on how to containerize a .NET app recommends a multistage build that pulls the SDK image for build and aspnet image for run.

However, It seems that the official ASP.NET Core and .NET Runtime images' layers are subsets of the .NET SDK image. (To investigate this, I used container-diff.)This means there's no "double download" during a docker multistage build.

I wonder if there's a way that we can set up the dependencies so that we avoid double-downloading the .NET Runtime and ASP.NET parts of the SDK dependency.

@ForestEckhardt
Copy link
Contributor Author

@ForestEckhardt Do you have a running proof-of-concept of this sitting somewhere? If possible, it'd be great to expose that so folks can play around with it.

dotnet-core-runtime, dotnet-core-aspnet, dotnet-core-sdk, and dotnet-publish all have a publish-spike branch. If you build all of those buildpacks from that branch you can get an idea of what this new procedure would look like.

@sophiewigmore
Copy link
Member

I wonder if there's a way that we can set up the dependencies so that we avoid double-downloading the .NET Runtime and ASP.NET parts of the SDK dependency.

@fg-j not a bad idea- is this something that has been explored at all?

@fg-j
Copy link

fg-j commented Apr 26, 2022

@sophiewigmore My initial idea for this is to change the API of the .NET SDK buildpack and adjust how the .NET language family collaborates:

When building from source

  • The .NET SDK buildpack provides dotnet-core-sdk, dotnet-core-runtime, and dotnet-aspnetcore.
  • During its build phase, the .NET SDK buildpack moves the .NET runtime and ASP.NET subdirectories of the SDK installation into separate layers that will be present at launch time

When building an FDD/FDE

  • The .NET SDK buildpack provides dotnet-core-sdk at launch time
  • The .NET Runtime buildpack provides dotnet-core-runtime, ASP.NET buildpack provides dotnet-aspnetcore (as they do now)

Benefits:

  • For source code builds, runtime and ASP.NET assets are downloaded once
  • Does not change the proposed size of the offline .NET language family buildpack (since order groups would change, but individual buildpacks' dependencies wouldn't

Limitations/Risks:

  • If the .NET SDK buildpack only contains 1 version per major.minor version line, this approach will make exactly 1 version of the runtime/ASP.NET available at launch time. Users have less flexibility when it comes to .NET runtime version.
  • This involves "surgery" on the .NET SDK release artifact -- we lose the supportability benefit of shipping official distributions of the runtime and ASP.NET
  • Builds from source code and builds from FDD and FDEs install different .NET runtime and ASP.NET artifacts. Build behaviour for source code apps and FDEs (e.g. selected dependency versions) could diverge.

@ForestEckhardt
Copy link
Contributor Author

  • This involves "surgery" on the .NET SDK release artifact -- we lose the supportability benefit of shipping official distributions of the runtime and ASP.NET

I think that this is my biggest concern out of this request. The whole reason that I am purposing this RFC is to avoid having to do surgery in order to hopefully make the buildpack more predictable and in line with what Microsoft supports.

@fg-j
Copy link

fg-j commented Apr 27, 2022

The concern around double-downloading would be mitigated if CNB supported some sort of asset caching across builds. Today, if two apps are built on the same node and they both require the same dependency (e.g. the .NET runtime), each build will download the artifact separately, completely unaware that there are now two copies of an identical asset on the node.

If, instead, CNB platforms had some way of caching build assets across builds, only one app build would reach out to the internet for the dependency source. The other would simply use the cached version of the asset already present on the node. This draft RFC in the upstream CNB project outlines the problem and a proposed solution in more depth.

Perhaps the double-download issue is better thought of as a limitation of CNB platforms rather than the .NET buildpack. After all, Docker builds rely on cross-build layer caching to solve this performance issue.

@macsux
Copy link

macsux commented May 3, 2022

Just adding my 2c on this.

.NET SDK can compile ANY backwards compatible version of code as long as reference assemblies are included. Reference assemblies are "stubs" of real APIs that are being targeted by the code, but the are void of implementation. It's basically an interface.

These reference assemblies are distributed as nuget packages, and are placed in nuget cache (by default ~/.nuget folder but can be overridden via env var). If the SDK encounters code that requires a reference package (cuz it's older then current installed runtimes), it will look for it in nuget cache and try to restore it from internet if not present.

At runtime, those reference assemblies are substituted by real assemblies distributed as part of runtime package.

Each .NET SDK comes bundled with the same version runtime. This is needed because a lot of SDK tooling used during compilation is written in .NET and depends on the presence of the said runtime in order to execute.

We can confirm that this works by creating a simple .NET 3.1 app and using official .NET 6 SDK docker image to containerize:

docker run -it --rm -v C:\temp\netapp31\netapp31:/app -v nugetcache:/root/.nuget mcr.microsoft.com/dotnet/sdk:6.0
cd /app
dotnet build

this populates the nuget cache with the following packages

root@591bd7eb8337:~/.nuget/packages# tree -d
.
|-- microsoft.aspnetcore.app.ref
|   `-- 3.1.10
|       |-- data
|       `-- ref
|           `-- netcoreapp3.1
|-- microsoft.netcore.app.host.linux-x64
|   `-- 3.1.24
|       `-- runtimes
|           `-- linux-x64
|               `-- native
`-- microsoft.netcore.app.ref
    `-- 3.1.0
        |-- data
        `-- ref
            `-- netcoreapp3.1

We can confirm that the .NET 6 SDK is still able to build this app by remounting the cached nuget volume (but it will fail if you delete the ~/.nuget/packages/ folder and it can't redownload them).

docker run -it --rm -v C:\temp\netapp31\netapp31:/app -v nugetcache:/root/.nuget --network none mcr.microsoft.com/dotnet/sdk:6.0

Therefore it is my opinion and recommendation that the best approach is to include major versions of SDK in the buildpack, and pre-populate nuget cache with reference assemblies targeting old versions of runtimes, even those that are out of support. The reference assemblies for each .net runtime version in unpacked format in .nuget folder is ~18MB. However, this includes the original .nupkg file PLUS all the extracted content from it. If space is of concern, only the nuget packages can be included in the buildpack and used to repopulate the nuget cache folder by extracting them in the appropriate structure (they are just zip files with additional metadata, plus an additional metadata file that has sha256 of the original nuget it was extracted from. This would reduce size to about ~6mb for a given runtime.

@ForestEckhardt
Copy link
Contributor Author

@macsux I appreciate this deep dive into the inner workings of the SDK dependency. I think that my biggest hang up is that in this RFC we are trying to away from customizing the artifact and more towards consuming the artifacts that Microsoft uses directly. My hope is that by using the artifacts as is from Microsoft we will have make our buildpack more consistent as we will align with the intended Microsoft experience. We have struggled in the passed trying to make our buildpacks modular in ways the Microsoft does not expect/intended and it has made some of the behavior in our buildpacks flakey. This is a step towards trying to provide an expected .Net experience for developers with as little magic from the buildpacks as possible.

@macsux
Copy link

macsux commented Jun 1, 2022

What you're proposing will not work. You NEED reference packages in nuget cache to compile code of different version of SDK. It is possible to have a project that is targeting .net 6 reference one that is .net 3.1. You will not be able to compile such project, even both .NET 6.0 & .NET 3.1 SDKS/Runtimes are present. The only way to compile such project is via .NET 6.0 SDK and .net 3.1 reference assemblies present in nuget cache.

You can confirm this behavior easily by building a custom docker image that contains both SDKs:

FROM mcr.microsoft.com/dotnet/sdk:6.0 AS net60
FROM mcr.microsoft.com/dotnet/sdk:3.1 AS net31
COPY --from=net60 ["/usr/share/dotnet","/usr/share/dotnet"]
COPY --from=net60 ["/usr/bin/dotnet","/usr/bin/dotnet"]

Such image will have both SDKs and runtimes available, yet it will fail to build a .NET 3.1 app if access to internet is restricted preventing it from downloading reference assemblies. See output of such experiment

root@baa6cafd691c:/app# dotnet --list-sdks
3.1.419 [/usr/share/dotnet/sdk]
6.0.202 [/usr/share/dotnet/sdk]
root@baa6cafd691c:/app# dotnet --list-runtimes
Microsoft.AspNetCore.App 3.1.25 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 6.0.4 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.NETCore.App 3.1.25 [/usr/share/dotnet/shared/Microsoft.NETCore.App]
Microsoft.NETCore.App 6.0.4 [/usr/share/dotnet/shared/Microsoft.NETCore.App]
root@baa6cafd691c:/app# dotnet build
Microsoft (R) Build Engine version 17.1.1+a02f73656 for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

  Determining projects to restore...
/usr/share/dotnet/sdk/6.0.202/NuGet.targets(130,5): error : Unable to load the service index for source https://api.nuget.org/v3/index.json. [/app/netapp31.csproj]
/usr/share/dotnet/sdk/6.0.202/NuGet.targets(130,5): error :   Resource temporarily unavailable (api.nuget.org:443) [/app/netapp31.csproj]
/usr/share/dotnet/sdk/6.0.202/NuGet.targets(130,5): error :   Resource temporarily unavailable [/app/netapp31.csproj]

Build FAILED.

/usr/share/dotnet/sdk/6.0.202/NuGet.targets(130,5): error : Unable to load the service index for source https://api.nuget.org/v3/index.json. [/app/netapp31.csproj]
/usr/share/dotnet/sdk/6.0.202/NuGet.targets(130,5): error :   Resource temporarily unavailable (api.nuget.org:443) [/app/netapp31.csproj]
/usr/share/dotnet/sdk/6.0.202/NuGet.targets(130,5): error :   Resource temporarily unavailable [/app/netapp31.csproj]
    0 Warning(s)
    1 Error(s)

Time Elapsed 00:00:01.45

I highly recommend you come up with a number of these scenarios and test them in offline environment (docker is perfect for this) before committing to a solution strategy.

@macsux
Copy link

macsux commented Jun 1, 2022

Here's the repo that demonstrates this type of project https://github.com/macsux/multi-version-dotnet-project

@ForestEckhardt
Copy link
Contributor Author

@macsux

I have tested the following workflow:
Given a .Net project that require .Net version 3.1 I will download the latest .Net 3.1 SDK and its related runtimes. Using the SDK library I am able to create a framework dependent build. I am then able to run that framework dependent build of any 3.1 runtime not just the runtime included with the SDK.

If I am understanding what you are stating is that I could not use a 6.0 SDK and runtimes to compile a 3.1 project. Everything I have tested confirms that particular idea but if you have seen a situation where a 3.1 SDK cannot compile a 3.1 project offline then I would be interested in investigating further.

Because of the limitation that you have pointed out, we are planning on having a full SDK installation for every major.minor line that we support. Meaning that we should not have to do a cross version line compilation.

If I have gotten anything wrong about those steps I would be more than happy to take another look!

@macsux
Copy link

macsux commented Jun 1, 2022 via email

Comment on lines 71 to 72
The .Net SDK dependency referenced in the buildpack should be identical to the
one provided by Microsoft. This will allow us to install a fully realized .Net
Copy link
Member

Choose a reason for hiding this comment

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

For clarity with the dependency work happening: this means that the URI (and source URI) of the SDK dependency will be the Microsoft upstream URI, rather than the URI to a dependency that we host in the dep-server?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was trying to leave that implementation ambiguous for the time being cause I imagine it will start out as a reference to something the is hosted on dep-server and then move to the URI from Microsoft. I can state that it will still be on the dep-server.

Copy link

Choose a reason for hiding this comment

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

I think it's ok to leave this ambiguous. This RFC is about what the dependency is, not where it's hosted.

Copy link
Member

Choose a reason for hiding this comment

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

Yea, it was for my own understanding. The RFC can stay generic!

@fg-j fg-j merged commit 7ca86dd into main Jun 15, 2022
@fg-j fg-j deleted the extend-dotnet-sdk branch June 15, 2022 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team/dotnet-core .Net Core Subteam RFC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants