[nightly] Build images for 2.1.0-preview1 #361
Conversation
| Required microsoft/dotnet-nightly changes are in dotnet/dotnet-docker-nightly#546 |
2.1/jessie/amd64/sdk/Dockerfile Outdated
| @@ -0,0 +1,17 @@ | |||
| FROM microsoft/dotnet-nightly:2.1.300-preview1-jessie | |||
There was a problem hiding this comment.
Base SDK tag used across platforms isn't consistent. This particular usage isn't using the explicit SDK tag while others are.
| Remove-Item -Force node.zip; ` | ||
| Remove-Item -Force nodejs-tmp | ||
| | ||
| ## https://github.com/git-for-windows/git/releases |
There was a problem hiding this comment.
I would like to know more on the motivation for adding git. Have you been getting requests for this? I haven't heard of any which is why I ask. Should this be pushed to the base image?
There was a problem hiding this comment.
Now that I think about it, we can probably remove this. I believe installing it was a requirement to make bower work. We're not installing bower anymore.
| | ||
| # set up environment | ||
| ENV ASPNETCORE_URLS http://+:80 | ||
| ENV NODE_VERSION 8.9.4 |
There was a problem hiding this comment.
Looks to be defined multiple times.
2.1/jessie/amd64/sdk/Dockerfile Outdated
| ENV ASPNETCORE_URLS http://+:80 | ||
| | ||
| # Ensure packages required for standalone apps are pre-fetched | ||
| RUN mkdir /tmp/warmup && cd /tmp/warmup \ |
There was a problem hiding this comment.
I find the mixture of multi-command lines (e.g. && cd / && rm -rf /tmp/warmup) and single lines to be a little confusing.
| ENV ASPNETCORE_URLS http://+:80 | ||
| | ||
| COPY --from=installer-env ["nodejs", "C:\\Program Files\\nodejs"] | ||
| USER ContainerAdministrator |
There was a problem hiding this comment.
Nit - inconsistent use of comments across Dockerfiles explaining the use of ContainerAdministrator
| Remove-Item -Force nodejs-tmp | ||
| | ||
| # Build image | ||
| FROM microsoft/dotnet-nightly:2.1.300-preview1-sdk-nanoserver-1709 |
There was a problem hiding this comment.
Curious why the difference in the order the components are installed between Linux and windows.
There was a problem hiding this comment.
No specific reason. I'll update to be consistent.
| "platforms": [ | ||
| { | ||
| "dockerfile": "2.1/stretch/runtime", | ||
| "dockerfile": "2.1/stretch/amd64/runtime", |
There was a problem hiding this comment.
The readme should also get updated to correctly reference the new Dockerfile locations.
| @MichaelSimons updated with cleanup according to your feedback. |
test/Dockerfile.testrunner.linux Outdated
| # docker run --rm -v /var/run/docker.sock:/var/run/docker.sock testrunner test/test.ps1 <test.ps1 args> | ||
| | ||
| FROM ubuntu:16.04 | ||
| FROM microsoft/powershell:ubuntu16.04 |
There was a problem hiding this comment.
You may consider using a prebuilt image here - debian-stretch-docker-testrunner-63f2145-20184325094343. The source for it is at https://github.com/dotnet/dotnet-buildtools-prereqs-docker/blob/master/src/debian/stretch/docker-testrunner/Dockerfile. Would save some time not having to build this entire thing every time. This is being used in dotnet-docker-nightly - dotnet/dotnet-docker-nightly#538
There was a problem hiding this comment.
I'll give that a shot
There was a problem hiding this comment.
Works great! Thanks for pointing that out @MichaelSimons
2 participants
Fixes #279
Changes:
microsoft/aspnetcore-nightlymicrosoft/dotnet-nightly:2.0-runtime-depsinstead of2.0-runtime. ASP.NET Core installers include the Microsoft.NETCore.App shared runtimemicrosoft/aspnetcore-build-nightly-
Install Git 2.16.1 (NanoServer only)Removed git from the images.WORKDIR /root/code. Helps with the common pitfall of trying to build the app in/, which fails b/c MSBuild tries to overwrite/bin.microsoft/dotnet-nightly:2.1.300-preview1-sdkNote: CI is expected to fail until microsoft/dotnet-nightly:2.1.300-preview1-sdk images are available.
cc @MichaelSimons @glennc