Add debugging support for .NET Core in Docker containers #500

philliphoff · 2018-09-20T19:44:50Z

Adds support for debugging .NET Core applications within Docker containers, in a manner similar to that performed by Visual Studio's Docker-related tooling.

The basic feature set is:

Adds a palette command to scaffold a “Docker .NET Core” debug launch configuration.
- The intent of this configuration is to minimize input/configuration from/by the user; properties will all be inferred
- Optional properties can be used to override image tags, names, arguments, variables, etc.
Adds a debug configuration provider that:
- Pulls down the latest .NET Core debugger from the Cloud, as needed
- Generates a debuggable Docker image for the application (e.g. building just the first stage of the multi-stage Dockerfile)
- Runs the Docker image in a container, applying shared volumes as necessary to expose debugger/application to the container
- Dynamically generates a .NET Core debug launch configuration allowing VS Code to initiate and attach the debugger within the container
- Launches the browser to the running application via the container’s mapped ports
- Cleans up containers when debugging is done

NOTE: Only debugging of Linux containers is supported today (in VS Code running on either Mac or Windows). We cannot debug Windows containers because the .NET Core debugger (vsdbg) is not supported on Windows containers, and VS Code cannot use msvsmon (used by Visual Studio in that scenario).

The feature is disabled by default, using the new docker.debugging.enabled setting.

Merge from Microsoft/vscode-docker:master

Merge from Microsoft/vscode-docker

ejizba

Adding some initial comments, but still in the process of reviewing.

Also I got this error when I F5'ed:

Building Docker image...
> Sending build context to Docker daemon  2.504MB
> 
> 
> Step 1/16 : FROM microsoft/aspnetcore:2.0 AS base
> 2.0: Pulling from microsoft/aspnetcore
> 55cbf04beb70: Pulling fs layer
> 2ad9fb8b9d3d: Pulling fs layer
> 7debd121b442: Pulling fs layer
> 76fbad01d0bd: Pulling fs layer
> 1cba46fa0c00: Pulling fs layer
> 76fbad01d0bd: Waiting
> 1cba46fa0c00: Waiting
> 7debd121b442: Download complete
> 76fbad01d0bd: Verifying Checksum
> 76fbad01d0bd: Download complete
> 55cbf04beb70: Verifying Checksum
> 55cbf04beb70: Download complete
> 1cba46fa0c00: Verifying Checksum
> 1cba46fa0c00: Download complete
> 2ad9fb8b9d3d: Verifying Checksum
> 2ad9fb8b9d3d: Download complete
> 55cbf04beb70: Pull complete
> 2ad9fb8b9d3d: Pull complete
> 7debd121b442: Pull complete
> 76fbad01d0bd: Pull complete
> 1cba46fa0c00: Pull complete
> Digest: sha256:8bd7808981568a92b7f1d0aa4bab07ef811e6417898847d9caf698a4a788ab11
> Status: Downloaded newer image for microsoft/aspnetcore:2.0
>  ---> db030c19e94b
> Step 2/16 : WORKDIR /app
>  ---> 78f936151ff7
> Removing intermediate container 62a22a04608f
> Step 3/16 : EXPOSE 80
>  ---> Running in 721a55e2bb1e
>  ---> cab04bc7c223
> Removing intermediate container 721a55e2bb1e
> Successfully built cab04bc7c223
> Successfully tagged hellodotnetcore.csproj:dev
Docker image cab04bc7c223 built.
Acquiring the latest .NET Core debugger...
Debugger acquired.
Starting container...
Unable to start container: Error: Command failed: docker run -dt -P --name hellodotnetcore.csproj-dev -v "/Users/ericjizba/TestRepos/hellodotnetcore:/app:rw" -v "/Users/ericjizba/.vsdbg/debian.8-x64/vs2017u5:/remote_debugger:ro" -v "/Users/ericjizba/.nuget/packages:/root/.nuget/packages:ro" -v "/usr/local/share/dotnet/sdk/NuGetFallbackFolder:/root/.nuget/fallbackpackages:ro" --entrypoint tail cab04bc7c223 -f /dev/null
docker: Error response from daemon: Mounts denied: 
The path /usr/local/share/dotnet/sdk/NuGetFallbackFolder
is not shared from OS X and is not known to Docker.
You can configure shared paths from Docker -> Preferences... -> File Sharing.
See https://docs.docker.com/docker-for-mac/osxfs/#namespaces for more info.
.

Does that mean anything to you? I admittedly haven't investigated much and plan to look into it further.

CHANGELOG.md

dockerExtension.ts

debugging/netcoreapp/dockerDebugConfigurationProvider.ts

dockerExtension.ts

ejizba · 2018-09-27T21:38:39Z

dockerExtension.ts

+        ctx.subscriptions.push(
+            vscode.debug.onDidChangeActiveDebugSession(
+                session => {
+                    if (session === undefined) {


I don't get if (session === undefined). How do you know the session was a docker debug session? Doesn't this event fire for any debug session?

The intent is to shutdown any containers used for Docker debugging still running when debugging ends in VS Code, whether from the ending session or a previous one. I'm not sure it matters which kind of session was ending, in that if there are no containers running it's a no-op and if there are, then we'd still want them shutdown.

(As far as I know, VS Code only supports one debugging session at a time.)

I'm just concerned because this code will be run for every user that has the docker extension installed - even if they never touch the 'docker-netcoreapp' experience (well - every user once we get rid of the feature flag). No matter how perfect the code is today (which could change in the future) I would prefer to have some checking here to prevent the docker extension from affecting users that are debugging entirely different things.

Can you use onDidTerminateDebugSession and check session.type === 'docker-netcoreapp'?

I ran into issues with onDidTerminateDebugSession when the VS Live Share extension was installed, as it may "wrap" other debug adaptors, even when not actively used. In that case, the returned type is the Live Share adaptor. It exposes the underlying adaptor type via a custom request, but when a session is terminated you cannot make custom requests.

Instead, I add a "session manager" that's informed by my debug adaptor when debugging is about to start, at which time it starts listening to the termination event. Then, on termination, it performs the necessary cleanup and stops listening for future events. This should limit it the cleanup to running only when the user has attempted to debug within a Docker container, but no more than once per attempt.

ejizba · 2018-09-27T21:42:24Z

debugging/netcoreapp/fsProvider.ts

+}
+
+export class LocalFileSystemProvider implements FileSystemProvider {
+    public async dirExists(path: string): Promise<boolean> {


Take a look at the 'fs-extra' npm module (already a dependency of this extension). It has promisified versions of all the 'fs' methods, plus some extra helper methods. Hopefully it's enough that you can get rid of LocalFileSystemProvider completely, but it should at least make this a lot smaller.

Ah, hadn't realized that was already in use. Updated to use that instead. (The use of the provider structure itself is an intentional abstraction of the file system for (future) testing purposes.)

I understand the desire to plan ahead, but I'm also not a fan of over-complicating code before it's actually needed. How soon do you plan to write the tests? Are you sure the tests need an abstraction and can't just run against the real thing?

Abstracting/mocking the file system tends to be handy, in that it can simplify/eliminate test initialization/cleanup code as well as avoids collisions if/when tests run in parallel. I've added some sample tests; I'd love to add more as time allows.

debugging/netcoreapp/osProvider.ts

ejizba · 2018-09-27T21:57:23Z

debugging/netcoreapp/appStorage.ts

+import * as path from 'path';
+import { FileSystemProvider } from "./fsProvider";
+
+export interface MementoAsync {


The extensionContext already has a memento - globalState or workspaceState depending on your needs

AppStorage represents a state at the "application" or "project" level, which may often be sub-workspace-state. (You might have several applications within the workspace folder, say, if using VS Code to open a solution created by VS comprised of several individual ASP.NET projects.) It's intended to be used for things that are specific to the application (e.g. cache settings used to build/run its Docker images/containers) and not dependent on which folder was opened in VS Code (e.g. the solution level or project level folder) to edit that project. This parallels how VS stores such data.

Okay I'll have to try out the scenario you described. I just want to make sure we're following VS Code conventions here, which may be different than VS. In general, VS Code has a much bigger reliance on the fact that the project is at the root of the workspace. They also have a concept of 'multi-root' workspaces, which might be how they handle the multi-project scenario you're describing (which is handled appropriately by workspaceState)

I think it would be good to have a meeting to discuss how we expect users to manage these types of projects/solutions in vscode. Do we have actual customer usage data?

I setup a time for us to meet and go through Stephen's questions.

philliphoff · 2018-09-27T23:34:41Z

Does that mean anything to you? I admittedly haven't investigated much and plan to look into it further.

I'll have to look into that too. Does that path exist on your machine? What folders are shared in your Docker preferences? (Mine does not explicitly state that /usr/... is shared, but it somehow still works.)

philliphoff · 2018-09-28T18:53:40Z

@EricJizbaMSFT Did you happen to recently install OS X Mojave (10.14)? I'm now seeing that same Docker mount error after doing so. I had to manually add /usr/local/share/dotnet as a shared folder in the Docker preferences; after that, it worked. I'll look into why that's happening and what the extension might be able to do about it.

ejizba · 2018-09-28T18:56:46Z

Uh nope not on Mojave

philliphoff · 2018-09-28T19:20:01Z

It looks like we ran into this issue with the VS for Mac Docker Tools and submitted a request to allow programmatic additions to the shared folder list. So strange that I hadn't run into this issue until now on my machine. I'll add instructions to the user to add the shared folder in the README.md.

philliphoff · 2018-09-28T23:12:50Z

I've added a "prerequisite check" that the shared folder exists in the user's Docker preferences.

ejizba

You've addressed my biggest blockers so I'll go ahead and approve. (Only thing that's debatably a blocker for me is telemetry - definitely need that before releasing)

I do want to say I'm still not a huge fan of the Provider pattern for the following reasons:

I think it makes the code unnecessarily complicated
I think tests are more effective if they're running against the 'real thing'
I think you might be using patterns that were helpful testing against VS but are less necessary when testing VS Code (which is a lot easier from my experience)

All that being said - I think it's a philosophical discussion that doesn't have a "correct" answer and thus isn't worth blocking a PR.

Thanks for your contribution!

…/philliphoff/vscode-docker into philliphoff-netcoreapp-debugging

Merge from vscode-docker master

philliphoff · 2018-10-16T23:19:42Z

Also still need CTI testcases

Requested access to the OneNote in order to add test cases.

Should do some basic testing on Linux.

Ran through the basic scenario on Ubuntu (and added prerequisite check for user inclusion in docker group).

philliphoff · 2018-10-17T18:12:28Z

Also still need CTI testcases

Added a CTI test page including a main end-to-end scenario test:

https://microsoft.sharepoint.com/teams/azcode/_layouts/OneNote.aspx?id=%2Fteams%2Fazcode%2FSiteAssets%2Fazcode%20Notebook&wd=target%28Testing.one%7CD12CBB51-9002-4371-B958-B88191DF5CF5%2F.NET%20Core%20Debugging%20%28Not%20yet%20checked%20in%5C%29%7C818BF24C-34CD-8940-A165-3407A13A5F9E%2F%29
onenote:https://microsoft.sharepoint.com/teams/azcode/SiteAssets/azcode%20Notebook/Testing.one#.NET%20Core%20Debugging%20(Not%20yet%20checked%20in)&section-id={D12CBB51-9002-4371-B958-B88191DF5CF5}&page-id={818BF24C-34CD-8940-A165-3407A13A5F9E}&end

philliphoff · 2018-10-17T18:15:15Z

Ok, so all comments should now be addressed one way or another.

@StephenWeatherford Let me know if there's anything else I can do.

StephenWeatherford · 2018-10-17T20:19:46Z

debugging/coreclr/dockerDebugConfigurationProvider.ts

-        return this.resolveDockerDebugConfiguration(folder, debugConfiguration);
+        return callWithTelemetryAndErrorHandling(
+            'debugCoreClr',
+            async () => await this.resolveDockerDebugConfiguration(folder, debugConfiguration));


nit: debugCoreClr doesn't seem to say what it's really doing. How about "resolveDebugConfiguration"?

I chose debugCoreClr as that was effectively what was being done (i.e. an attempt to initiate debugging of .NET Core in Docker) even though resolveDebugConfiguration is what VS Code happens to name the interface function (and PMs looking at the telemetry data typically care more about the intent than the implementation specifics). I'm amenable to other IDs or schemes; are there any conventions/restrictions in the naming (as the codebase uses a couple different styles)?

If that's what it's effectively doing, it's fine, thanks.

StephenWeatherford · 2018-10-17T21:00:47Z

Other than the suggested rename of telemetry, ship it! Thanks!

philliphoff · 2018-10-17T21:17:47Z

Many thanks to @EricJizbaMSFT and @StephenWeatherford for their very thorough review!

philliphoff and others added 21 commits July 16, 2018 15:20

Pull in debugging prototype.

57145f4

Support both launch configuration entrypoints.

9c729aa

Only show Docker output channel once per session.

aba9d4d

Sketch removal of containers when debugging session ends.

7010aae

Refactor launch cleanup.

bf8521d

Provide access to result/error in operation output.

b62d05c

Add nested output.

a9f4cef

Some type renaming.

b7a714a

Hide debugging behind "feature flag" and add minimal docs.

b4eb92b

Merge pull request #1 from Microsoft/master

c9c34e7

Merge from Microsoft/vscode-docker:master

Merge from master.

cb7924c

Fix issue with running container using app folder containing variables.

4edc06d

Add C# extension dependency check.

86473b2

Begin support for inferring app output.

8300353

Refactoring to please linter.

c4eb964

Properly deal with temp files.

08be5c9

Fix spelling error.

90c7e82

Switch to dotnet msbuild and normalize.

fea2151

Merge pull request #2 from Microsoft/master

61aa97b

Merge from Microsoft/vscode-docker

Merge from master.

a6c9280

Cleanup of linting issues.

2146a58

ejizba reviewed Sep 27, 2018

View reviewed changes

Updates per PR feedback.

2a38708

philliphoff added 2 commits September 28, 2018 15:05

Move dependency check into prereq infrastructure.

5f1a84d

Add prerequisite for Mac on shared folder.

7c83b8b

Add prerequisite check for a Linux Docker daemon.

c0a1bd9

ejizba approved these changes Oct 11, 2018

View reviewed changes

Phillip Hoff and others added 17 commits October 12, 2018 13:53

Minor test fixes.

28ec421

Add line-splitting utility.

5b98042

Refactored to support Windows.

a9f8182

Refactored and added new test.

92a0460

Use line splitter for output manager.

c9450d2

Use splitter in Docker client.

6ba6f92

Fix lint warnings.

66e25c3

Fix generated default tag.

55e94dc

Merge branch 'philliphoff-netcoreapp-debugging' of https://github.com…

0af366b

…/philliphoff/vscode-docker into philliphoff-netcoreapp-debugging

Some updates per PR feedback.

363a83f

Add telemetry to configuration resolution.

9a47f31

Merge branch 'philliphoff-netcoreapp-debugging' of https://github.com…

d476ba1

…/philliphoff/vscode-docker into philliphoff-netcoreapp-debugging

Merge pull request #4 from Microsoft/master

b5cbc3f

Merge from vscode-docker master

Merge from master.

0b7e5b2

Add prerequisite for the .NET Core SDK.

90234cc

Rename MSBuild client.

da87225

Minor tweaks for strictNullChecks.

2d2542b

StephenWeatherford reviewed Oct 17, 2018

View reviewed changes

StephenWeatherford approved these changes Oct 17, 2018

View reviewed changes

philliphoff merged commit ddb600a into microsoft:master Oct 17, 2018

philliphoff mentioned this pull request Oct 19, 2018

Debug Container #56

Closed

ejizba mentioned this pull request Aug 2, 2019

Supporting SSL in ASP.NET Core web apps #1173

Merged

microsoft locked and limited conversation to collaborators Oct 27, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add debugging support for .NET Core in Docker containers #500

Add debugging support for .NET Core in Docker containers #500

philliphoff commented Sep 20, 2018

ejizba left a comment

ejizba Sep 27, 2018

philliphoff Sep 27, 2018

ejizba Sep 27, 2018

philliphoff Oct 1, 2018

ejizba Sep 27, 2018

philliphoff Sep 27, 2018

ejizba Sep 27, 2018

philliphoff Oct 4, 2018

ejizba Sep 27, 2018

philliphoff Sep 27, 2018

ejizba Sep 28, 2018

StephenWeatherford Oct 2, 2018

philliphoff Oct 4, 2018

philliphoff commented Sep 27, 2018

philliphoff commented Sep 28, 2018

ejizba commented Sep 28, 2018

philliphoff commented Sep 28, 2018

philliphoff commented Sep 28, 2018

ejizba left a comment

philliphoff commented Oct 16, 2018 •

edited

philliphoff commented Oct 17, 2018

philliphoff commented Oct 17, 2018

StephenWeatherford Oct 17, 2018

philliphoff Oct 17, 2018

StephenWeatherford Oct 17, 2018

StephenWeatherford commented Oct 17, 2018

philliphoff commented Oct 17, 2018

Add debugging support for .NET Core in Docker containers #500

Add debugging support for .NET Core in Docker containers #500

Conversation

philliphoff commented Sep 20, 2018

ejizba left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philliphoff commented Sep 27, 2018

philliphoff commented Sep 28, 2018

ejizba commented Sep 28, 2018

philliphoff commented Sep 28, 2018

philliphoff commented Sep 28, 2018

ejizba left a comment

Choose a reason for hiding this comment

philliphoff commented Oct 16, 2018 • edited

philliphoff commented Oct 17, 2018

philliphoff commented Oct 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StephenWeatherford commented Oct 17, 2018

philliphoff commented Oct 17, 2018

philliphoff commented Oct 16, 2018 •

edited