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

Add .NET Core container debugging support #4699

Merged
merged 12 commits into from
Aug 25, 2020

Conversation

MrLuje
Copy link
Contributor

@MrLuje MrLuje commented Aug 19, 2020

Fixes: #2175

Description
This patch adds support to skaffold for .NET Core debugging.
.NET Core requires additional debugging runtime support files, which are fetched and installed using an initContainer base.

I used the already existing debugging runtime support at https://github.com/GoogleContainerTools/container-debug-support/tree/duct-tape/netcore that makes vsdbg available.

The debugger must be started by kubectl "execing" vsdbg on the container (usually, from your IDE)

No transformation is needed for the container command & args
vsdbg doesn't expose any debugging port to the existing container.

This can be tested using Visual Studio Code, I hope the doc changes are clear enough for the test steps.

FYI, I never used buildpacks before this PR, so this part may needs extra attention.

@codecov
Copy link

codecov bot commented Aug 19, 2020

Codecov Report

Merging #4699 into master will increase coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4699      +/-   ##
==========================================
+ Coverage   73.38%   73.42%   +0.04%     
==========================================
  Files         341      342       +1     
  Lines       13505    13529      +24     
==========================================
+ Hits         9910     9934      +24     
  Misses       2976     2976              
  Partials      619      619              
Impacted Files Coverage Δ
pkg/skaffold/debug/transform.go 91.24% <ø> (ø)
pkg/skaffold/debug/transform_netcore.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bad4b31...9469eb3. Read the comment docs.

@briandealwis briandealwis self-requested a review August 19, 2020 18:27
@briandealwis briandealwis self-assigned this Aug 19, 2020
@briandealwis
Copy link
Member

Very cool @MrLuje! I'll get to reviewing this today.

Copy link
Member

@briandealwis briandealwis left a comment

Choose a reason for hiding this comment

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

This looks great! Just a few nits to take care of and we can commit it.

docs/content/en/docs/workflows/debug.md Outdated Show resolved Hide resolved
"type": "coreclr",
"request": "attach",
"processId" : "${command:pickRemoteProcess}", // if your docker image doesn't contain `ps`, this command will fail... usually processId is 1
"justMyCode": true, //dotnet debug=true,dotner release=false
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what this comment means.

Suggested change
"justMyCode": true, //dotnet debug=true,dotner release=false
"justMyCode": true, // for dotnet debug set to `true`, for release set to `false`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is a well-known debugging option in .net world

The debugger displays and steps into user code ("My Code") only, ignoring system code and other code that is optimized or that does not have debugging symbols. (from msdn)

What about :
"justMyCode": true, // set to true in debug and false in release configuration

"name": "Skaffold Debug",
"type": "coreclr",
"request": "attach",
"processId" : "${command:pickRemoteProcess}", // if your docker image doesn't contain `ps`, this command will fail... usually processId is 1
Copy link
Member

Choose a reason for hiding this comment

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

Might be better to use "1" and add some discussion above or below about use of command:pickRemoteProcess.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done & added the discussion to the text above

integration/testdata/debug/skaffold.yaml Outdated Show resolved Hide resolved
pkg/skaffold/debug/transform_netcore.go Outdated Show resolved Hide resolved
integration/testdata/debug/netcore/Dockerfile Outdated Show resolved Hide resolved
integration/testdata/debug/netcore/Dockerfile Outdated Show resolved Hide resolved
integration/testdata/debug/skaffold.yaml Outdated Show resolved Hide resolved
pkg/skaffold/debug/transform_netcore_test.go Outdated Show resolved Hide resolved
@briandealwis
Copy link
Member

@MrLuje will you have a chance to make these minor fixes?

@MrLuje
Copy link
Contributor Author

MrLuje commented Aug 24, 2020

Done, you can check the fixes
Thanks for the review

@briandealwis
Copy link
Member

Just pushed some tiny tweaks (minor edits and line-wrap in docs, and changed copyright year to 2020).

@briandealwis briandealwis added the kokoro:run runs the kokoro jobs on a PR label Aug 25, 2020
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Aug 25, 2020
@briandealwis briandealwis added the kokoro:run runs the kokoro jobs on a PR label Aug 25, 2020
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Aug 25, 2020
Copy link
Member

@briandealwis briandealwis left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!

@briandealwis briandealwis merged commit 241e202 into GoogleContainerTools:master Aug 25, 2020
@MrLuje MrLuje deleted the netcore branch August 26, 2020 15:15
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.

Add support for C#/.NET for skaffold debug
4 participants