-
Notifications
You must be signed in to change notification settings - Fork 30
Add node wrapper and nodejs debug helper image
#34
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 node wrapper and nodejs debug helper image
#34
Conversation
|
I think it's hard to understand what you mean in the description. Can you include like a before and after transformation (or something in the description)? |
If argv[0] does not exist then the first reference in the PATH should be this wrapper.
|
Rewrote description and made some other fixes. |
|
|
||
| // use an absolute path in case we're being run within a node_modules directory | ||
| script := findScript(nc.args) | ||
| if abs, err := filepath.Abs(script); err == nil { |
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 about when there's an error?
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.
Hmm, I was thinking of just treating it as a file name but maybe it's better to skip any processing and just immediately hand off to the unwrapped executable.
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 filepath.Abs() only errors if it can't determine the current directory
|
PTAL @dgageot |
This PR:
--inspect-style arguments are only used when running application scripts.nodejsdebug helper image that installs thisnodewrapper.Towards fixing GoogleContainerTools/skaffold#2170. With this
nodejsdebug helper image in place, we can then changeskaffold debugto 1) use the debug helper image, and 2) set PATH to cause this node-wrapper to be invoked.NodeJS's
nodesupports a set of--inspectarguments to activate debugging mode using the Chrome DevTools protocol. There are a few variants:--inspector--inspect=<port>: launches the app with devtools listening on the given port (or default 9229)--inspect-brkor--inspect-brk=<port>: launches but immediately stops the appMany NodeJS applications use NodeJS-based tools like
npmandnodemonto launch the app rather than runningnodedirectly, and often use several layered together. For example, Skaffold's ownnodejsexample uses npm invoking nodemon, which then invokes node. Further complicating the issue is that thenodeimages on Docker Hub usedocker-entrypoint.shas a helper script for launching apps. So this example might cause the following sequence:docker-entrypoint.sh sh -c 'npm run production'npm run productionnode /path/to/npm run productionnode src/index.jsThis sequence makes it very difficult for
skaffold debugto start launch the application for debugging as--inspects are usually intercepted by one of the launch tools.This PR provides a node wrapper that adds special handling for
--inspect-style arguments on the command-line andNODE_OPTIONSenvironment variable. When executing anode_modulesscript, any--inspect-like arguments are stripped and put inNODE_DEBUG. When executing an application script (i.e., does not live innode_modules), theNODE_DEBUGvalue is inlined.For example,
skaffold debug -p devonnodejsexample would do the following:docker-entrypoint.sh sh -c 'npm run production'withNODE_OPTIONS=--inspect=9229but withPATH=/path/to/this/wrapper:$PATHnpm run production(withNODE_OPTIONS=--inspect=9229)/path/to/this/wrapper/node /path/to/npm run production—npmis recognized as a non-application script and thus the--inspect=9229is stripped from theNODE_OPTIONSand placed it inNODE_DEBUG/path/to/actual/node /path/to/npm run production/path/to/this/wrapper/node src/index.js—src/index.jsis recognized as an application script and soNODE_DEBUGis inlined to thenodecommand-line/path/to/actual/node --inspect=9229 src/index.jsIf you have npm/node on your system, you can check it out with something as simple as:
Whereas with the node-wrapper in place:
To see it in action, we use Skaffold's simple NodeJS project with
npmand thenodeimage: