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

debug should make suspending on start to be configurable #4870

Open
briandealwis opened this issue Oct 7, 2020 · 16 comments
Open

debug should make suspending on start to be configurable #4870

briandealwis opened this issue Oct 7, 2020 · 16 comments
Labels

Comments

@briandealwis
Copy link
Member

debug currently configures containers to be in a continue mode where the containers run without suspending. It can be useful for users to be able to connect to their containers immediately on execution, such as to debug a main() or init() method.

Proposal: debug should look at the container and/or pod for a suspend option and if present, then configure the pod to suspend on start (where possible). Suggestions are:

  • SKAFFOLD_DEBUG_SUSPEND environment variable
  • pod annotation for a skaffold.dev/debug/suspend
@briandealwis
Copy link
Member Author

Request on StackOverflow

@relloyd
Copy link

relloyd commented Mar 3, 2021

Thanks for raising this @briandealwis. I agree the feature is needed; I need a streamlined way to debug main().

@aaron-prindle aaron-prindle added this to the v1.23.0 milestone Apr 21, 2021
@aaron-prindle aaron-prindle self-assigned this Apr 21, 2021
@aaron-prindle
Copy link
Contributor

aaron-prindle commented Apr 26, 2021

For the skaffold debug suspend on start configuration @briandealwis referenced that for setting the feature:

Suggestions are:

SKAFFOLD_DEBUG_SUSPEND environment variable
pod annotation for a skaffold.dev/debug/suspend

From poking around the code I have few questions regarding these suggestions:

  • Currently debug does not seem to pass a runcontext into it's code path. Is this something worth plumbing through for other user settable debug options (or is this a guaranteed one off?). In doing so I believe this could also be a flag option (which automatically adds a corresponding env var, etc.). For example --suspend. Am I missing something and it is currently plumbed through?
  • (related to above questions) For the SKAFFOLD_DEBUG_SUSPEND env var, does this make sense to do ad-hoc to avoid plumbing args through the debug code path? (os.GetEnv for this specifically and treat it as a one-off, etc. ?)
  • Could you clarify the pod annotation suggestion? I am not sure I understand the suggestion. Would this be annotating the pod after deployment to label the pod w/ this configuration option or would this be setting a pod with a specific annotation and that alerts skaffold to use this functionality?

@aaron-prindle
Copy link
Contributor

aaron-prindle commented Apr 27, 2021

Talked offline, question answers are inline:

  • skaffold debug is not plumbed through for RunContext. This will be plumbed through for —protocols flag: skaffold debug may need to apply IDE-specific language runtime bindings #2202
  • Skaffold already loads flags from corresponding env vars automatically. I don’t think we then reset the environment with the resolved values (since command-line flags override envvars). A global would be less terrible (IMHO)
    (And I loathe globals :-))
  • Debug sets a debug.cloud.google.com/config annotation with the settings derived, and also ignores pods that already have such an annotation. My suggestion is to look for a different annotation to provide a configuration value
    So if a pod has skaffold.dev/debug/suspend set them configure it as suspend-on-start

@aaron-prindle aaron-prindle modified the milestones: v1.23.0, v1.24.0 Apr 28, 2021
@tejal29 tejal29 removed this from the v1.24.0 milestone Apr 29, 2021
@aaron-prindle
Copy link
Contributor

aaron-prindle commented Apr 29, 2021

Currently I have mapped the bug to the following required work:

  • go

    • Modify dlv entrypoint go transform to make --continuous flag parameterized (false for suspend=true). Code to to touch here
  • netcore

    • Didn't make progress here
  • nodejs

    • Need to add option - NODE_OPTIONS=--inspect-brk for skaffold option - suspend=true
    • Modify both transform_nodejs.go
    • and Node debug container
    • Needs to be updated in go in both places. The container-debug-support nodejs wrapper propagates the node debug options to the "right" nodejs instance. This is necessary as so many node tools are themselves written in node and will intercept the debug options.
  • python

    • need to modify each debuggers suspend/wait option
      • example of what to parameterize for ptvsd - code here
  • java/jvm

    • Need to make suspend option configurable - code here

@aaron-prindle
Copy link
Contributor

Unassigning myself to focus on performance work, tried to add some notes above for next person

@briandealwis
Copy link
Member Author

For NodeJS, the node wrapper already handles --inspect-brk. It's just a matter of adjusting the transformer.

@jaekook
Copy link

jaekook commented Sep 21, 2021

any update?

@tejal29
Copy link
Member

tejal29 commented Jan 10, 2022

@jaekook We have added --continue support for pydevd. Does that help you or you are using NodeJS?

@briandealwis
Copy link
Member Author

@tejal29 this request to is provide some kind of indication to Skaffold as to whether --continue (or the equivalents elsewhere) should be used

@anthonyalayo
Copy link

This would be a great feature to have. Is it possible to get it re-prioritized? The lack of this feature creates downstream issues for Cloud Code.

@aaron-prindle aaron-prindle added this to the debug milestone Nov 2, 2022
@gsquared94 gsquared94 removed their assignment Apr 1, 2023
@anthonyalayo
Copy link

@aaron-prindle how is this feature development going?

@renzodavid9
Copy link
Contributor

Hey @anthonyalayo, thanks for the ping here. Currently the team had to shift gears to tackle other work, so we don't have bandwidth for this. However, if anyone from the community is willing to take a look to this and work on it will be great. Thanks a lot!

@pkoutsovasilis
Copy link

Hey @renzodavid9 I have opened a PR about this one

@anthonyalayo
Copy link

@pkoutsovasilis fantastic news!! @renzodavid9 could the team take a look? It will be a huge win for skaffold.

@pkoutsovasilis
Copy link

@renzodavid9 any news? (not that I got a reply in my previous message).. When I opened this PR back in September, I never imagined that we would reach almost the beginning of 2024 without a single comment from the skaffold team 🥲

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

9 participants