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

Update samples to .NET 8 #4742

Merged
merged 54 commits into from
Sep 14, 2023
Merged

Update samples to .NET 8 #4742

merged 54 commits into from
Sep 14, 2023

Conversation

richlander
Copy link
Member

@richlander richlander commented Jul 11, 2023

The samples need to be updated to .NET 8. We intend to publish these changes with .NET 8 RC1.

Intended changes:

  • Switch to port 8080
  • Switch to non-root
  • Produce Windows-specific tags
  • Push Chiseled images
  • Push composite images, for ASP.NET Core
  • Adopt new SDK changes (like -a)
  • Adopt new pattern for multi-platform images (like --platform $BUILDPLATFORM)
  • Remove most/all msbuild properties from Dockerfiles. Properties -- like PublishTrimmed -- should be in project files so that developers get good feedback from analyzers. Our previous use of msbuild properties in Dockerfiles is an anti-pattern.
  • Removed the slim versions because they rely on project properties in Dockerfiles and due to MVC apps no longer work with PublishTrimmed aspnetcore#49360.
  • I added new "releases*" samples. They produce this output: https://gist.github.com/richlander/6c6eefaf47e7d65c42822d8c00c2620a

TODO:

  • Document and test docker buildx build

Reported two bugs while doing this work:

Related:

@richlander richlander changed the title Update sample to .NET 8 Update samples to .NET 8 Jul 11, 2023
@lbussell
Copy link
Contributor

Publishing these samples will be blocked on dotnet/docker-tools#1159

@richlander
Copy link
Member Author

Same thing for #4743

manifest.samples.json Outdated Show resolved Hide resolved
@lbussell
Copy link
Contributor

@richlander, I'll be trying a few things here to get these samples building.

@jetersen
Copy link

jetersen commented Sep 8, 2023

I ran into this issue trying to compile with the nightly sdk aot rc mcr.microsoft.com/dotnet/nightly/sdk:8.0-jammy-aot@sha256:c477e92a600d5a8aa052c93503a6d65c3

0.560 MSBuild version 17.7.0+5785ed5c2 for .NET
0.959   Determining projects to restore...
3.030 /tmp/project/project.csproj : warning NU1603: BitwardenDirectorySync depends on Microsoft.NET.ILLink.Tasks (>= 8.0.0-rc.1.23414.4) but Microsoft.NET.ILLink.Tasks 8.0.0-rc.1.23414.4 was not found. An approximate best match of Microsoft.NET.ILLink.Tasks 8.0.100-1.23067.1 was resolved.
3.036 /tmp/project/project.csproj : error NU1102: Unable to find package Microsoft.DotNet.ILCompiler with version (>= 8.0.0-rc.1.23414.4)
3.036 /tmp/project/project.csproj : error NU1102:   - Found 25 version(s) in nuget.org [ Nearest version: 8.0.0-preview.7.23375.6 ]
3.037 /tmp/project/project.csproj : error NU1102: Unable to find package Microsoft.NETCore.App.Runtime.linux-x64 with version (= 8.0.0-rc.1.23414.4)
3.037 /tmp/project/project.csproj : error NU1102:   - Found 130 version(s) in nuget.org [ Nearest version: 8.0.0-preview.7.23375.6 ]
3.037 /tmp/project/project.csproj : error NU1102: Unable to find package runtime.linux-x64.Microsoft.DotNet.ILCompiler with version (= 8.0.0-rc.1.23414.4)
3.037 /tmp/project/project.csproj : error NU1102:   - Found 25 version(s) in nuget.org [ Nearest version: 8.0.0-preview.7.23375.6 ]
3.037 /tmp/project/project.csproj : error NU1102: Unable to find package Microsoft.AspNetCore.App.Runtime.linux-x64 with version (= 8.0.0-rc.1.23414.10)
3.037 /tmp/project/project.csproj : error NU1102:   - Found 130 version(s) in nuget.org [ Nearest version: 8.0.0-preview.7.23375.9 ]
3.171   Failed to restore /tmp/project/project.csproj (in 1.53 sec).

@@ -36,7 +36,7 @@ steps:
condition: and(succeeded(), ${{ parameters.condition }})
- script: >
echo "##vso[task.setvariable variable=runImageBuilderCmd]
docker run --rm
\$env:DOCKER_BUILDKIT=1; docker run --rm
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be needed anymore.

@@ -1,5 +1,5 @@
variables:
imageNames.imageBuilderName: mcr.microsoft.com/dotnet-buildtools/image-builder:2230260
imageNames.imageBuilderName: mcr.microsoft.com/dotnet-buildtools/image-builder:2233114
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
imageNames.imageBuilderName: mcr.microsoft.com/dotnet-buildtools/image-builder:2233114
imageNames.imageBuilderName: mcr.microsoft.com/dotnet-buildtools/image-builder:2240150

@@ -1,19 +1,21 @@
# Learn about building .NET container images:
# https://github.com/dotnet/dotnet-docker/blob/main/samples/README.md
FROM mcr.microsoft.com/dotnet/sdk:7.0 AS build
FROM --platform=$BUILDPLATFORM mcr.microsoft.com/dotnet/sdk:8.0-preview AS build
Copy link
Member

Choose a reason for hiding this comment

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

All 8.0-preview tags in this PR will need to be updated to just 8.0.


# copy and publish app and libraries
COPY . .
RUN dotnet publish -a $TARGETARCH --self-contained --no-restore -o /app releasesapi.csproj
Copy link
Member

Choose a reason for hiding this comment

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

If we're going to use --ucr instead of --use-current-runtime then we should use --sc instead of --self-contained. Applies to other Dockerfiles too.

## Supported Linux distros

The .NET Team publishes images for [multiple distros](../../documentation/supported-platforms.md).

Samples are provided for:

- [Alpine](Dockerfile.alpine)
- [Alpine with ICU installed](Dockerfile.alpine-icu)
- [Debian](Dockerfile.debian)
- [Ubuntu](Dockerfile.ubuntu)
- [Ubuntu Chiseled](Dockerfile.chiseled)
Copy link
Member

Choose a reason for hiding this comment

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

Should we also have Dockerfile.chiseled-icu that uses the extra image?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea. Let's have a conversation on how to handle ICU with these samples. It's a question of whether we want to favor a little extra ceremony or push people to SCD (which the extra images would required). I'm open to either, but its a policy topic for us to discuss.

{
// Only show releases in support or < 1 year EOL
if (DateOnly.TryParse(releaseSummary.EolDate, out DateOnly eolDate)
&& DateOnly.FromDateTime(DateTime.UtcNow.AddYears(-1)).DayNumber > eolDate.DayNumber)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
&& DateOnly.FromDateTime(DateTime.UtcNow.AddYears(-1)).DayNumber > eolDate.DayNumber)
&& DateOnly.FromDateTime(DateTime.UtcNow.AddYears(-1)).DayNumber > eolDate.DayNumber)

{
HttpClient httpClient = new();
var loadError = "Failed to load release information.";
var releases = await httpClient.GetFromJsonAsync<ReleaseIndex>(ReleaseValues.RELEASE_INDEX_URL, ReleaseJsonSerializerContext.Default.ReleaseIndex) ?? throw new Exception(loadError);
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason that the library from https://www.nuget.org/packages/Microsoft.Deployment.DotNet.Releases isn't used instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

I wrote this sample for another reason, to demonstrate System.Text.Json (for a blog post). We can certainly update this code to use that library.

Copy link
Member

Choose a reason for hiding this comment

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

Consider deleting this. There's only one project here and this doesn't add any value.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's devkit. It generates these all the time so I decided to check it in.

Comment on lines +22 to +25
// if (os == Tests.OS.Alpine)
// {
// os += "-slim";
// }
Copy link
Member

Choose a reason for hiding this comment

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

Delete

Comment on lines +102 to +104
// new SampleImageData { OS = OS.Alpine, Arch = Arch.Amd64, DockerfileSuffix = "alpine-slim", IsPublished = true },
// new SampleImageData { OS = OS.Alpine, Arch = Arch.Arm, DockerfileSuffix = "alpine-slim", IsPublished = true },
// new SampleImageData { OS = OS.Alpine, Arch = Arch.Arm64, DockerfileSuffix = "alpine-slim", IsPublished = true },
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this need to be replaced with the jammy-chiseled Dockerfiles?

@mthalman
Copy link
Member

mthalman commented Sep 8, 2023

I can into this issue trying to compile with the nightly sdk aot rc mcr.microsoft.com/dotnet/nightly/sdk:8.0-jammy-aot@sha256:c477e92a600d5a8aa052c93503a6d65c3

0.560 MSBuild version 17.7.0+5785ed5c2 for .NET
0.959   Determining projects to restore...
3.030 /tmp/project/project.csproj : warning NU1603: BitwardenDirectorySync depends on Microsoft.NET.ILLink.Tasks (>= 8.0.0-rc.1.23414.4) but Microsoft.NET.ILLink.Tasks 8.0.0-rc.1.23414.4 was not found. An approximate best match of Microsoft.NET.ILLink.Tasks 8.0.100-1.23067.1 was resolved.
3.036 /tmp/project/project.csproj : error NU1102: Unable to find package Microsoft.DotNet.ILCompiler with version (>= 8.0.0-rc.1.23414.4)
3.036 /tmp/project/project.csproj : error NU1102:   - Found 25 version(s) in nuget.org [ Nearest version: 8.0.0-preview.7.23375.6 ]
3.037 /tmp/project/project.csproj : error NU1102: Unable to find package Microsoft.NETCore.App.Runtime.linux-x64 with version (= 8.0.0-rc.1.23414.4)
3.037 /tmp/project/project.csproj : error NU1102:   - Found 130 version(s) in nuget.org [ Nearest version: 8.0.0-preview.7.23375.6 ]
3.037 /tmp/project/project.csproj : error NU1102: Unable to find package runtime.linux-x64.Microsoft.DotNet.ILCompiler with version (= 8.0.0-rc.1.23414.4)
3.037 /tmp/project/project.csproj : error NU1102:   - Found 25 version(s) in nuget.org [ Nearest version: 8.0.0-preview.7.23375.6 ]
3.037 /tmp/project/project.csproj : error NU1102: Unable to find package Microsoft.AspNetCore.App.Runtime.linux-x64 with version (= 8.0.0-rc.1.23414.10)
3.037 /tmp/project/project.csproj : error NU1102:   - Found 130 version(s) in nuget.org [ Nearest version: 8.0.0-preview.7.23375.9 ]
3.171   Failed to restore /tmp/project/project.csproj (in 1.53 sec).

The nightly images contain non-official builds of .NET, currently on 8.0 RC1. When consuming them, you'll need to setup your NuGet config to target the feed which contains these packages:

<add key="dotnet8" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet8/nuget/v3/index.json" />

@lbussell
Copy link
Contributor

Looks like there is a NanoServer issue with virtualization/Docker...

@richlander
Copy link
Member Author

What should we do about the Nanoserver issue? I assume that doesn't block merging then?

@lbussell
Copy link
Contributor

You'll notice the build was only failing due to the ComplexAppTest on NanoServer legs.. A few notes on that:

  1. We don't publish complexapp (it's not in manifest.samples.json)
  2. complexapp did not (until I just updated it) support Windows on .NET 8 due to Switch multi-platform tags to Linux only #4492
  3. complexapp is only built in the VerifyComplexAppSample test. This means it only runs on one OS on all legs.. kind of weird. I feel like it should run across all of the images. I defaulted it to nanoserver-1809. We should open an issue to make this test a [Theory]

Hopefully the changes make it work, and the strange error message was just a side effect of things being different between Docker Desktop/the build agents with Docker CE.

@@ -92,28 +92,22 @@ public async Task VerifyAspnetSample(SampleImageData imageData)
[Fact]
public void VerifyComplexAppSample()
{
// complexapp sample doesn't currently support building on Windows.
Copy link
Member Author

Choose a reason for hiding this comment

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

I think this is wrong. The app works fine on Windows, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

The app works fine but the Dockerfile is multi-platform so it won't work on Windows. I can amend the comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's merge first and then deal with that later.

Copy link
Contributor

Choose a reason for hiding this comment

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

Filed #4873 for this.

@richlander richlander merged commit c09ce8b into dotnet:main Sep 14, 2023
75 checks passed
@richlander richlander deleted the dotnet-8-samples branch September 14, 2023 16:07
lbussell added a commit to lbussell/dotnet-docker that referenced this pull request Sep 18, 2023
Co-authored-by: Justin Anderson <jander-msft@users.noreply.github.com>
Co-authored-by: Logan Bussell <loganbussell@microsoft.com>
Co-authored-by: Damian Edwards <damian@damianedwards.com>
Co-authored-by: Matt Thalman <mthalman@microsoft.com>
@lbussell lbussell mentioned this pull request Sep 16, 2024
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants