-
Notifications
You must be signed in to change notification settings - Fork 2k
[WIP] Updates based on review of docs #508
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
Conversation
| @@ -1,4 +1,4 @@ | |||
| FROM microsoft/dotnet:2.0-sdk AS build | |||
| FROM microsoft/dotnet-nightly:2.1-sdk AS build | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was it intentional to use a mismatch of multi-arch tags in this Dockerfile?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I'm aware of that. It seems like this is setup is the best case. This model enables the Dockerfile to work on X64 and ARM32. Isn't that good?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This model enables the Dockerfile to work on X64 and ARM32.
When you say to work, are you referring to building, running, or both?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
build. After build, the difference no longer matters. Right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Complication ... the 2.0 Multi-Arch DFs don't work for ARM32 since there is no matching manifest for linux/arm.
| @@ -1,4 +1,4 @@ | |||
| FROM microsoft/dotnet:2.1-sdk-alpine AS build | |||
| FROM microsoft/dotnet-nightly:2.1-sdk-alpine AS build | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the switch to nightly throughout the dotnetapp samples?
|
OK. I made a bunch more changes based on feedback from @MichaelSimons. I cannot build Dockerfile.latest on ARM32. I'll need some help with that. Everything else looks fine, I think. I had a bunch of small commits because I was using GitHub as a transport mechanism between my PC and my Pi for testing various things. |
| > Note: The instructions use example values that need to be changed to for your environment, specifically the password location, and the user account. More simply, make sure to change "rich" and "richlander" to something else. | ||
| ### Using Azure Container Registry (ACR) | ||
| You can build almost the same [.NET Core console samples](README.md) and [ASP.NET Core sample](../aspnetapp/README.md) on ARM devices as you can on other architectures. At present, the primary difference is that most .NET Core Docker file samples use .NET Core 2.0 multi-arch tags, and those don't yet offer `linux/arm` manifests. Starting with .NET Core 2.1, multi-arch tags support Linux ARM32 and are usable on ARM32 devices. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be precise, the runtime images support arm, it is the SDK that didn't support arm in 2.0.
samples/dotnetapp/README.md
Outdated
| dotnet dotnetapp.dll | ||
| ``` | ||
|
|
||
| Note: The `-c release` argument builds the application in release mode (the default is debug mode). See the [dotnet run reference](https://docs.microsoft.com/dotnet/core/tools/dotnet-run) for more information on commandline parameters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this reference the publish reference doc instead of run?
samples/dotnetapp/Dockerfile.latest
Outdated
| @@ -0,0 +1,20 @@ | |||
| FROM microsoft/dotnet-nightly:2.1-sdk AS build | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if "Dockerfile.latest" is a good name? Given the meaning of "latest" in Docker I have some concerns it may cause confusion and attract people to use it as the defacto one to copy. This is not the Dockerfile that the latest tag is in microsoft/dotnet-samples would be produced from.
|
I believe I'm done. I'm not intending on making any additional changes modulo additional feedback. |
MichaelSimons
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - I jut have two minor comments.
|
|
||
| * [Push Docker Images to Azure Container Registry](push-image-to-acr.md) | ||
| * [Push Docker Images to DockerHub](push-image-to-dockerhub.md) | ||
| You can build almost the same [.NET Core console samples](README.md) and [ASP.NET Core sample](../aspnetapp/README.md) on ARM devices as you can on other architectures. At present, the primary difference is that most .NET Core Docker file samples use the .NET Core 2.0 SDK multi-arch tags, and those don't offer `linux/arm` manifests. Starting with .NET Core 2.1, both .NET Core Runtime and SDK multi-arch tags support Linux ARM32 and are usable on ARM32 devices. [Dockerfile.preview](Dockerfile.preview) and [Dockerfile.preview](Dockerfile.basic-preview) have been added to work around this issue. They use .NET Core 2.1 instead of 2.0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
those don't offer
linux/armmanifests
That wording seems a little strange to me. Is the following any better?
those don't support
linux/arm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Text of error message:
pi@pi-black:~/dotnet-docker/samples/aspnetapp/aspnetapp $ docker pull microsoft/dotnet:2.0-sdk
2.0-sdk: Pulling from microsoft/dotnet
no matching manifest for linux/arm in the manifest list entries
samples/dotnetapp/Dockerfile.basic
Outdated
| COPY tests/*.csproj ./tests/ | ||
| RUN dotnet restore | ||
| WORKDIR /app/dotnetapp | ||
| RUN dotnet restore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like there is trailing WS.
Making updates to these docs as I work through microsoft/dotnet-framework-docker#122