-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Add instructions for developing in a container #420
Conversation
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 - Very nice addition!
|
||
You can [build container images to create your environment](README.md), as described in a Dockerfile. That's straightforward and the common use case with Docker. This document describes a much more iterative and dynamic use of Docker, defining the container environment primarily via the commandline. .NET Core includes a command called `dotnet watch` that can rerun your application or your tests on each code change. This document describes how to use the Docker CLI and `dotnet watch` to develop applications in a container. | ||
|
||
This approach relies on [volume mounting](https://docs.docker.com/engine/admin/volumes/volumes/) (that's the `-v` argument in the following commands) to mount the app into the container (without using a Dockerfile). You may need to [Enable shared drives (Windows)](https://docs.docker.com/docker-for-windows/#shared-drives) or [file sharing (macOS)](https://docs.docker.com/docker-for-mac/#file-sharing) first. |
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.
Nit - I'm not crazy about the wording mount the app into the container
. This doesn't accurately describe what you are mounting to me as the "app" isn't created yet. How about something like app source
, source code
, or project
?
`dotnet watch run` is not working correctly in containers at this time. The instructions are still documented while we work on enabling this scenario. | ||
|
||
```console | ||
docker run --rm -p 8000:80 --name aspnetappsample -e ASPNETCORE_URLS=http://+:80 -e DOTNET_USE_POLLING_FILE_WATCHER=1 -v c:\git\dotnet-docker\samples\aspnetapp:c:\app\ microsoft/dotnet:2.0-sdk cmd /c "cd app/aspnetapp && dotnet restore && dotnet watch run" |
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 think using the workdir
option would be a more standard Docker approach over specifying cd
in the docker run command.
`dotnet watch run` is not working correctly in containers at this time. The instructions are still documented while we work on enabling this scenario. | ||
|
||
```console | ||
docker run --rm -p 8000:80 --name aspnetappsample -e ASPNETCORE_URLS=http://+:80 -e DOTNET_USE_POLLING_FILE_WATCHER=1 -v c:\git\dotnet-docker\samples\aspnetapp:c:\app\ microsoft/dotnet:2.0-sdk cmd /c "cd app/aspnetapp && dotnet restore && dotnet watch run" |
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.
Is dotnet restore
necessary with SDK > 2.1 for this scenario? It would be nice to get to a point where only a single command instruction is needed.
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 also was thinking about that. The first-order reason (as you touch on) is restore the dotnet watch
entrypoint. After 2.1, that's not needed. Still, it's very likely that an application will have dependencies outside of the SDK, which need to restored for this scenario to work.
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 understand the first-order reason for pre-2.1. Outside of that though, I wouldn't think the restore is necessary for >= 2.1 versions because dotnet run
will perform an implicit 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.
Does this command work for a normal ASP.NET Core app now? Given that the aspnetcore shared framework isn't in the dotnet SDK image? Or is that included now? I thought we had moved the DOTNET_USE_POLLING_FILE_WATCHER var into one of the base images, but checking proves that not to be true. Should we do that now? There is no scenario I know of where you can rely on non-polling to have filesystem watcher work with volume mounts. It is this Docker scenario that made us add the switch in the first place.
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.
DOTNET_USE_POLLING_FILE_WATCHER is not set in our images at the moment. IMO we should add to the 2.1.300 sdk images: aspnet/aspnet-docker#366
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 vote for adding DOTNET_USE_POLLING_FILE_WATCHER to the microsoft/dotnet sdk images.
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 we just add ASPNETCORE_URLS as well? It is less obvious than the polling watcher. But if we want people to do this sort of thing then it makes sense.
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.
Ya, let's add both. Seems like ASPNETCORE_URLS should go to the dotnet runtime image.
Given that the aspnetcore shared framework isn't in the dotnet SDK image?
Yes it is. That's why I picked the SDK image.
|
||
**Windows** using **Windows containers** | ||
|
||
`dotnet watch run` is not working correctly in containers at this time. The instructions are still documented while we work on enabling this scenario. |
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.
What is not working correctly? Can you link to an issue?
{ | ||
message = String.Join(" ",args); | ||
var message = $" {GetMessage(args)}{GetBot()}"; |
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.
Experience isn't great when you just pass in "--with-color". It would be nice to use the defaultMessage in this scenario.
|
||
WriteLine("**Environment**"); | ||
WriteLine($"Platform: .NET Core"); | ||
WriteLine($"OS: {RuntimeInformation.OSDescription}"); | ||
WriteLine(); | ||
} | ||
|
||
public static string GetBot() | ||
private static string GetMessage(string[] message) |
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.
Indent styling of this method is different. Should use 4 space indenting.
We should probably add instructions for how to change the bin/ and obj/ folder. Unless users are developing with Notepad or Vim, they are likely to have issues with volume mounting their source code folder into a Docker container. The SDK writes temp files into obj/ that are machine specific, so either dotnet-watch/dotnet-run will fail inside the container, or the intellisense in VS/VS Code will be broken. Or worse, it will alternate between those two broken states. To workaround this, I set <Project>
<PropertyGroup Condition="'$(RunningInsideDocker)' == 'true'">
<BaseIntermediateOutputPath>$(MSBuildProjectDirectory)/obj/docker/</BaseIntermediateOutputPath>
<BaseOutputPath>$(MSBuildProjectDirectory)/bin/docker/</BaseOutputPath>
</PropertyGroup>
<PropertyGroup Condition="'$(RunningInsideDocker)' != 'true'">
<BaseIntermediateOutputPath>$(MSBuildProjectDirectory)/obj/host/</BaseIntermediateOutputPath>
<BaseOutputPath>$(MSBuildProjectDirectory)/bin/host/</BaseOutputPath>
</PropertyGroup>
</Project> Unfortunately, this doesn't work with |
Any thoughts on the order of the Linux container options? Same q for the unit testing doc. I'd like the first option to be the one that most people are using. We should decide what that is. |
@natemcmaster Yes! I was running into this problem (a lot) and going to ask how to do this. Why is the second case needed? |
You could probably do without the second case...haven't tested though. I did this b/c I wanted the output directories to be side-by-side to avoid potential overlap. In dotnet-watch, we fixed the "obj/" folder issue. There was a hidden workaround in dotnet-watch 2.0.0 for it: https://github.com/aspnet/DotNetTools/blob/617499cd7cc448028303a8c66e54fef6ba8253e9/src/Microsoft.DotNet.Watcher.Tools/CommandLineOptions.cs#L69-L73, but this hidden switch doesn't exist in 2.1.0 since we fixed aspnet/DotNetTools#244 |
Sorry @natemcmaster . I get the need for the two cases now. I looked at it again. |
@natemcmaster I have tried this with 2.0.x and 2.1.4. and get the following error. If I delete d.b.p, then it works. I deleted all obj/bin folders first. C:\git\dotnet-docker\samples\dotnetapp\dotnetapp>dotnet watch run
watch : Error(s) finding watch items project file 'dotnetapp.csproj'
watch : MSBuild output from target 'GenerateWatchList':
watch :
watch : Build started 3/12/2018 7:49:04 PM.
watch : 1>Project "C:\git\dotnet-docker\samples\dotnetapp\dotnetapp\dotnetapp.csproj" on node 1 (GenerateWatchList target(s)).
watch : 1>C:\git\dotnet-docker\samples\dotnetapp\dotnetapp\dotnetapp.csproj : error MSB4057: The target "GenerateWatchList" does not exist in the project.
watch : 1>Done Building Project "C:\git\dotnet-docker\samples\dotnetapp\dotnetapp\dotnetapp.csproj" (GenerateWatchList target(s)) -- FAILED.
watch :
watch : Build FAILED. |
Yeah, this is the error I was talking about aspnet/DotNetTools#244. There are a bunch of workarounds, so pick your poison:
|
Ah, I thought you were saying this was fixed in 2.1.0 and that the hidden switch was removed. Did you mean 2.1.300? |
It will be fixed in 2.1.0, but by "2.1.0" I meant dotnet-watch v2.1.0, which ships in SDK v2.1.300. Yeah, that's confusing. |
Ah, that explains why I was confused. I saw your earlier directions but didn't follow them because of the confusion. Will try your instructions now. |
@richlander @natemcmaster FYI, we have dotnet/sdk#867 tracking making it easier to redirect the |
I've given up on making this work with 2.0. I'm going to switch to 2.1 Preview 2+ and assume that the environment variables I need are all there. This will make the experience a lot better. |
This change is now done (AFAIK). Waiting on a fix for #426. |
I did some experiments using the alpine image (nightly build) running aspnetcore (angular template). I was successful but the 'lesson learned' can be useful in the doc.
I can share the multi-stage dockerfile, if you believe it can be helpful. |
Yes, please do share your Dockerfile @raffaeler. @glennc is working on something similar, so it would be good to compare notes. There might be some confusion on the goals of this particular sample. The goal is that no Dockerfile is needed. |
@natemcmaster With the latest commit, the samples are finally working for me, as expected! Thanks for your offline feedback. CTRL-C of the aspnetapp samples doesn't work well for me on macOS. Works on Windows. |
First step: create an angular app from a developer command with 2.1 installed
In order to make it work in a container, the following property group must be added to the csproj file
The following multi-stage dockerfile provides the instruction to build the container with an Alpine Linux distribution and Net Core 2.1 (currently in preview)
Now the container can be built using the docker CLI
If everything worked correctly the Now the container can be run, specifying the port mapping and the url to listen inside the container itself.
It is also possible to open an interactive bash inside the container, for troubleshooting purposes
Please feel free to suggest/improve/modify. |
Last remaining issue for this PR is #431 |
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.
Looking good. I have included some suggestions.
{ | ||
var defaultMessage = "Hello from .NET Core!"; | ||
var bot = GetBot(); | ||
var (message, withColor) = GetMessage(args); |
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 me it feels like GetMessage should be renamed to ParseArgs. If you disagree, I would suggest that GetMessage should be refactored to contain the logic to default the message when one isn't specified. In other-words GetMessage should encapsulate all of the logic around getting the message to display.
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 agree. I refactored the code and didn't rename the method. My goal was to add more functionality while keeping the implementation simple. This was the simplest approach I could find.
samples/aspnetapp/README.md
Outdated
The [.NET Core Docker Sample](../dotnetapp/README.md) demonstrates more functionality, including unit testing, publishing self-contained applications and using the Alpine base image. The same techniques can be applied to ASP.NET applications. | ||
Looking for other samples: | ||
|
||
* [Develop .NET Core Applications in a container](../dotnetapp/dotnet-docker-dev-in-container.md) |
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 is Develop .NET Core Applications in a container
called out explicitly but not the other .NET Core app variations (e.g. arm, self-contained, unit testing, etc). To me, it feels like either all of other samples should be listed or all of the other ASP.NET samples plus a single reference to the .NET Core samples readme.
The same comment applies to the mirrored change in the dotnetapp readme.
@@ -0,0 +1,65 @@ | |||
# Develop ASP.NET Core Applications in a Container | |||
|
|||
Docker containers can provide a local .NET Core development environment without having to install anything on your machine. You can also use containers to augment a locally installed development environment with one that matches production. This scenario is useful if you develop on an operating system that is different than production. If you support multiple operating systems, then this approach can be even more useful. |
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.
The wording "without having to install anything" sounds like a line a used car salesman would say...lol. You need to install Docker and it would be nice to have a code editor.
|
||
Docker containers can provide a local .NET Core development environment without having to install anything on your machine. You can also use containers to augment a locally installed development environment with one that matches production. This scenario is useful if you develop on an operating system that is different than production. If you support multiple operating systems, then this approach can be even more useful. | ||
|
||
You can [build container images to create your environment](README.md), as described in a Dockerfile. That's straightforward and the common use case with Docker. This document describes a much more iterative and dynamic use of Docker, defining the container environment primarily via the commandline. .NET Core includes a command called `dotnet watch` that can rerun your application or your tests on each code change. This document describes how to use the Docker CLI and `dotnet watch` to develop applications in a container. |
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.
"container images" - I'm not crazy about this wording. It is confusing because it is mixing two distinctly different Docker concepts. How about just "You can build images"?
"create your environment" - I'm not sure what this is trying to convey. Perhaps describing this in more detail. Is this better?
A common use case of Docker is to containerize an application. You can define the environment necessary to run the application and even build the application itself within a Dockerfile. This document describes...
`dotnet watch run` is not working correctly in containers at this time. The instructions are still documented while we work on enabling this scenario. | ||
|
||
```console | ||
docker run --rm --name aspnetappsample -it -p 8000:80 -v c:\git\dotnet-docker\samples\aspnetapp:c:\app\ -w \app\aspnetapp microsoft/dotnet-nightly:2.1-sdk dotnet watch run |
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.
The order of the args are different for the Windows containers
cmds. It would be helpful to use the same order to make it easier for the readers to pick apart the differences across the container environments.
|
||
## Test your application in a container while you develop | ||
|
||
You can retest your application in a container with every local code change. This works for both console applications and websites. The syntax differs a bit for Windows and Linux containers. You can see this demonstrated in [Develop .NET Core Applications in a Container](../dotnetapp/dotnet-docker-dev-in-container.md). |
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.
What do you think about removing the This works for both console applications and websites. The syntax differs a bit for Windows and Linux containers.
statement here? Let the reference go into the details.
You could provide a direct link to the testing section.
06b288f
to
475c1d5
Compare
They were conflicts with master. Rebased master content into this branch and pushed -f. The content is now ready to go. |
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 good. I just have one comment.
`dotnet watch run` is not working correctly in containers at this time. The instructions are still documented while we work on enabling this scenario. | ||
|
||
```console | ||
docker run --name aspnetappsample --rm -it -p 8000:80 -v c:\git\dotnet-docker\samples\aspnetapp:c:\app\ -w \app\aspnetapp microsoft/dotnet-nightly:2.1-sdk dotnet watch run |
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 think a previous comment I made may have been overlooked.
The order of the args are different for the Windows containers cmd. It would be helpful to use the same order to make it easier for the readers to see the differences across the container environments.
Added instructions for establishing a more dynamic container environment.