Skip to content

Commit 85f1e8e

Browse files
briandealwisbriandealwis
authored
Launch original app command-line should debugging configuration fail (#84)
Separate the debug-launching into a preparation phase and launching phase. If the preparation phase fails, then just launch the original application command-line. - Treat failure to determine Python version as a preparation error. - Propagate exit code Document the environment variables to tweak the launcher behaviour.
1 parent 84e9fcd commit 85f1e8e

File tree

2 files changed

+120
-49
lines changed

2 files changed

+120
-49
lines changed

‎python/helper-image/launcher/launcher.go‎

Lines changed: 83 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ Licensed under the Apache License, Version 2.0 (the "License");
55
you may not use this file except in compliance with the License.
66
You may obtain a copy of the License at
77
8-
http://www.apache.org/licenses/LICENSE-2.0
8+
http://www.apache.org/licenses/LICENSE-2.0
99
1010
Unless required by applicable law or agreed to in writing, software
1111
distributed under the License is distributed on an "AS IS" BASIS,
@@ -16,15 +16,15 @@ limitations under the License.
1616

1717
// A `skaffold debug` launcher for Python.
1818
//
19-
// Python introduces some quirks. There are now three
20-
// methods for hooking up a debugging backend:
19+
// Configuring a Python app for debugging is quirky. There are
20+
// four debugging backends:
2121
//
2222
// - pydevd: the stock Python debugging backend
2323
// - pydevd-pycharm: PyDev with modifications for IntelliJ/PyCharm
2424
// - ptvsd: wraps pydevd with the debug-adapter protocol (obsolete)
2525
// - debugpy: new and improved ptvsd
2626
//
27-
// pydevd has pyx libraries which are specific to particular versions of Python.
27+
// Each has pyx libraries which are specific to particular versions of Python.
2828
//
2929
// Further complicating matters is that a number of Python packages
3030
// use launcher scripts (e.g., gunicorn), and so we can't simply run
@@ -37,14 +37,18 @@ limitations under the License.
3737
// not that unusual to have a `python`, `python3`, and `python2`
3838
// scripts that invoke different python installations.
3939
//
40+
// And hence the introduction of this debug launcher.
41+
//
4042
// This launcher is expected to be invoked as follows:
4143
//
4244
// launcher --mode <pydevd|pydevd-pycharm|debugpy|ptvsd> \
4345
// --port p [--wait] -- original-command-line ...
4446
//
45-
// This launcher will determine the python executable based on the `original-command-line`.
46-
// The launcher will configure the PYTHONPATH to point to the appropriate installation
47-
// of pydevd/debugpy/ptvsd for the corresponding python binary.
47+
// This launcher determines the python executable based on
48+
// `original-command-line`, unwrapping any python scripts, and
49+
// configures the debugging back-end.
50+
// The launcher configures the PYTHONPATH to point to the appropriate
51+
// installation pydevd/debugpy/ptvsd for the corresponding python binary.
4852
//
4953
// debugpy and ptvsd are pretty straightforward translations of the
5054
// launcher command-line `python -m debugpy`.
@@ -64,10 +68,24 @@ limitations under the License.
6468
// python -m pydevd --server --port 5678 --DEBUG --continue \
6569
// --file /tmp/pydevd716531212/skaffold_pydevd_launch.py
6670
// ```
71+
//
72+
// The launcher can be configured through several environment
73+
// variables:
74+
//
75+
// - Set `WRAPPER_ENABLED=false` to disable the launcher: the
76+
// launcher will execute the original-command-line as-is.
77+
// - Set `WRAPPER_SKIP_ENV=true` to avoid setting PYTHONPATH
78+
// to point to bundled debugging backends: this is useful if
79+
// your app already includes `debugpy`.
80+
// - Set `WRAPPER_PYTHON_VERSION=3.9` to avoid trying to determine
81+
// the python version by executing `python -V`
82+
// - Set `WRAPPER_VERBOSE` to one of `error`, `warn`, `info`, `debug`,
83+
// or `trace` to reduce or increase the verbosity
6784
package main
6885

6986
import (
7087
"context"
88+
"errors"
7189
"flag"
7290
"fmt"
7391
"io"
@@ -103,13 +121,15 @@ type pythonContext struct {
103121

104122
args []string
105123
env env
124+
125+
major, minor int // python version
106126
}
107127

108128
func main() {
109129
ctx := context.Background()
110130
env := EnvFromPairs(os.Environ())
111131
logrus.SetLevel(logrusLevel(env))
112-
logrus.Debug("launcher args:", os.Args[1:])
132+
logrus.Trace("launcher args:", os.Args[1:])
113133

114134
pc := pythonContext{env: env}
115135
flag.StringVar(&dbgRoot, "helpers", "/dbg", "base location for skaffold-debug helpers")
@@ -126,11 +146,16 @@ func main() {
126146
logrus.Fatal("expected python command-line args")
127147
}
128148
pc.args = flag.Args()
129-
logrus.Tracef("command-line: %v", pc.args)
149+
logrus.Debug("app command-line: ", pc.args)
130150

131-
if err := pc.launch(ctx); err != nil {
132-
logrus.Fatalf("error launching python debugging: %v", err)
151+
if !pc.prepare(ctx) {
152+
logrus.Info("launching original command: ", flag.Args())
153+
cmd := newConsoleCommand(ctx, flag.Args(), env)
154+
run(cmd)
155+
} else {
156+
pc.launch(ctx)
133157
}
158+
// NOTREACHED
134159
}
135160

136161
// validateDebugMode ensures the provided mode is a supported mode.
@@ -143,40 +168,61 @@ func validateDebugMode(mode string) error {
143168
}
144169
}
145170

146-
func (pc *pythonContext) launch(ctx context.Context) error {
171+
func run(cmd commander) {
172+
if err := cmd.Run(); err != nil {
173+
var ee exec.ExitError
174+
if errors.Is(err, &ee) {
175+
os.Exit(ee.ExitCode())
176+
}
177+
logrus.Fatal("error launching python debugging: ", err)
178+
}
179+
os.Exit(0)
180+
// NOTREACHED
181+
}
182+
183+
// prepare sets up the debugging command line. Return true if successful or false if setup could not be completed.
184+
func (pc *pythonContext) prepare(ctx context.Context) bool {
147185
if !isEnabled(pc.env) {
148-
logrus.Infof("wrapper disabled, launching %v", pc.args)
149-
cmd := newConsoleCommand(ctx, pc.args, pc.env)
150-
return cmd.Run()
186+
logrus.Infof("wrapper disabled")
187+
return false
151188
}
152189
if pc.alreadyConfigured() {
153190
logrus.Infof("already configured for debugging")
154-
cmd := newConsoleCommand(ctx, pc.args, pc.env)
155-
return cmd.Run()
191+
return false
156192
}
157193

158194
// rewrite the command-line by expanding script shebangs to run python and launch the app
159195
if err := pc.unwrapLauncher(ctx); err != nil {
160-
return err
196+
logrus.Warn("unable to determine launcher: ", err)
197+
return false
198+
}
199+
if err := pc.isPythonLauncher(ctx); err != nil {
200+
logrus.Warn("not a python launcher: ", err)
201+
return false
161202
}
162203

163204
// set PYTHONPATH to point to the appropriate library for the given python version.
164205
if err := pc.updateEnv(ctx); err != nil {
165-
return err
206+
logrus.Warn("unable to configure environment: ", err)
207+
return false
166208
}
167209
// so pc.args[0] should be the python interpreter
168210

169211
if err := pc.updateCommandLine(ctx); err != nil {
170-
return err
212+
logrus.Warn("unable to setup launcher: ", err)
213+
return false
171214
}
215+
return true
216+
}
172217

218+
func (pc *pythonContext) launch(ctx context.Context) {
173219
cmd := newConsoleCommand(ctx, pc.args, pc.env)
174-
return cmd.Run()
220+
run(cmd)
221+
// NOTREACHED
175222
}
176223

177224
// alreadyConfigured tries to determine if the python command-line is already configured
178-
// for debugging. Only handles simple command-lines; users should set `WRAPPER_ENABLED=false`
179-
// for more complicated situations.
225+
// for debugging.
180226
func (pc *pythonContext) alreadyConfigured() bool {
181227
// TODO: consider handling `#!/usr/bin/env python` too, though `pip install` seems
182228
// to hard-code the python location instead.
@@ -252,6 +298,13 @@ func (pc *pythonContext) unwrapLauncher(_ context.Context) error {
252298
return nil
253299
}
254300

301+
func (pc *pythonContext) isPythonLauncher(ctx context.Context) error {
302+
major, minor, err := determinePythonMajorMinor(ctx, pc.args[0], pc.env)
303+
pc.major = major
304+
pc.minor = minor
305+
return err
306+
}
307+
255308
func (pc *pythonContext) updateEnv(ctx context.Context) error {
256309
// Perhaps we should check PYTHONPATH or ~/.local to see if the user has already
257310
// installed one of our supported debug libraries
@@ -269,13 +322,6 @@ func (pc *pythonContext) updateEnv(ctx context.Context) error {
269322
return fmt.Errorf("skaffold-debug helpers are inaccessible at %q: %w", dbgRoot, err)
270323
}
271324

272-
major, minor, err := determinePythonMajorMinor(ctx, pc.args[0], pc.env)
273-
if err != nil {
274-
// We could skip setting PYTHONPATH in the hopes that the appropaite
275-
// debugger library was installed explicitly by the user?
276-
return fmt.Errorf("unable to determine python version from %q: %w", pc.args[0], err)
277-
}
278-
279325
if pc.env == nil {
280326
pc.env = env{}
281327
}
@@ -284,18 +330,18 @@ func (pc *pythonContext) updateEnv(ctx context.Context) error {
284330
var libraryPath string
285331
switch pc.debugMode {
286332
case ModePtvsd, ModeDebugpy:
287-
libraryPath = fmt.Sprintf(dbgRoot+"/python/lib/python%d.%d/site-packages", major, minor)
333+
libraryPath = fmt.Sprintf(dbgRoot+"/python/lib/python%d.%d/site-packages", pc.major, pc.minor)
288334

289335
case ModePydevd:
290-
libraryPath = fmt.Sprintf(dbgRoot+"/python/pydevd/python%d.%d/lib/python%d.%d/site-packages", major, minor, major, minor)
336+
libraryPath = fmt.Sprintf(dbgRoot+"/python/pydevd/python%d.%d/lib/python%d.%d/site-packages", pc.major, pc.minor, pc.major, pc.minor)
291337

292338
case ModePydevdPycharm:
293-
libraryPath = fmt.Sprintf(dbgRoot+"/python/pydevd-pycharm/python%d.%d/lib/python%d.%d/site-packages", major, minor, major, minor)
339+
libraryPath = fmt.Sprintf(dbgRoot+"/python/pydevd-pycharm/python%d.%d/lib/python%d.%d/site-packages", pc.major, pc.minor, pc.major, pc.minor)
294340
}
295341
if libraryPath != "" {
296342
if !pathExists(libraryPath) {
297343
// Warn as the user may have installed debugpy themselves
298-
logrus.Warnf("Debugging support for Python %d.%d not found: may require manually installing %q", major, minor, pc.debugMode)
344+
logrus.Warnf("Debugging support for Python %d.%d not found: may require manually installing %q", pc.major, pc.minor, pc.debugMode)
299345
}
300346
// Append to ensure user-configured values are found first.
301347
pc.env.AppendFilepath("PYTHONPATH", libraryPath)
@@ -353,12 +399,13 @@ func determinePythonMajorMinor(ctx context.Context, launcherBin string, env env)
353399
var versionString string
354400
if env["WRAPPER_PYTHON_VERSION"] != "" {
355401
versionString = env["WRAPPER_PYTHON_VERSION"]
402+
logrus.Debugf("Python version from WRAPPER_PYTHON_VERSION=%q", versionString)
356403
} else {
404+
logrus.Debugf("trying to determine python version from %q", launcherBin)
357405
cmd := newCommand(ctx, []string{launcherBin, "-V"}, env)
358406
out, err := cmd.CombinedOutput()
359407
if err != nil {
360-
logrus.Warnf("'%s -V' errored: %v", launcherBin, err)
361-
return -1, -1, err
408+
return -1, -1, fmt.Errorf("unable to determine python version from %q: %w", launcherBin, err)
362409
}
363410
versionString = string(out)
364411
logrus.Debugf("'%s -V' = %q", launcherBin, versionString)

‎python/helper-image/launcher/launcher_test.go‎

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -235,70 +235,94 @@ func TestDeterminePythonMajorMinor(t *testing.T) {
235235
}
236236
}
237237

238-
func TestLaunch(t *testing.T) {
238+
func TestPrepare(t *testing.T) {
239239
dbgRoot = t.TempDir()
240240

241241
tests := []struct {
242242
description string
243243
pc pythonContext
244244
commands commands
245-
shouldErr bool
245+
shouldFail bool
246246
expected pythonContext
247247
}{
248248
{
249249
description: "debugpy",
250250
pc: pythonContext{debugMode: "debugpy", port: 2345, wait: false, args: []string{"python", "app.py"}, env: nil},
251251
commands: RunCmdOut([]string{"python", "-V"}, "Python 3.7.4\n").
252252
AndRunCmd([]string{"python", "-m", "debugpy", "--listen", "2345", "app.py"}),
253-
expected: pythonContext{debugMode: "debugpy", port: 2345, wait: false, args: []string{"python", "-m", "debugpy", "--listen", "2345", "app.py"}, env: env{"PYTHONPATH": dbgRoot + "/python/lib/python3.7/site-packages"}},
253+
expected: pythonContext{debugMode: "debugpy", port: 2345, wait: false, major: 3, minor: 7, args: []string{"python", "-m", "debugpy", "--listen", "2345", "app.py"}, env: env{"PYTHONPATH": dbgRoot + "/python/lib/python3.7/site-packages"}},
254254
},
255255
{
256256
description: "debugpy with wait",
257257
pc: pythonContext{debugMode: "debugpy", port: 2345, wait: true, args: []string{"python", "app.py"}, env: nil},
258258
commands: RunCmdOut([]string{"python", "-V"}, "Python 3.7.4\n").
259259
AndRunCmd([]string{"python", "-m", "debugpy", "--listen", "2345", "--wait-for-client", "app.py"}),
260-
expected: pythonContext{debugMode: "debugpy", port: 2345, wait: true, args: []string{"python", "-m", "debugpy", "--listen", "2345", "--wait-for-client", "app.py"}, env: env{"PYTHONPATH": dbgRoot + "/python/lib/python3.7/site-packages"}},
260+
expected: pythonContext{debugMode: "debugpy", port: 2345, wait: true, major: 3, minor: 7, args: []string{"python", "-m", "debugpy", "--listen", "2345", "--wait-for-client", "app.py"}, env: env{"PYTHONPATH": dbgRoot + "/python/lib/python3.7/site-packages"}},
261261
},
262262
{
263263
description: "ptvsd",
264264
pc: pythonContext{debugMode: "ptvsd", port: 2345, wait: false, args: []string{"python", "app.py"}, env: nil},
265265
commands: RunCmdOut([]string{"python", "-V"}, "Python 3.7.4\n").
266266
AndRunCmd([]string{"python", "-m", "ptvsd", "--host", "localhost", "--port", "2345", "app.py"}),
267-
expected: pythonContext{debugMode: "ptvsd", port: 2345, wait: false, args: []string{"python", "-m", "ptvsd", "--host", "localhost", "--port", "2345", "app.py"}, env: env{"PYTHONPATH": dbgRoot + "/python/lib/python3.7/site-packages"}},
267+
expected: pythonContext{debugMode: "ptvsd", port: 2345, wait: false, major: 3, minor: 7, args: []string{"python", "-m", "ptvsd", "--host", "localhost", "--port", "2345", "app.py"}, env: env{"PYTHONPATH": dbgRoot + "/python/lib/python3.7/site-packages"}},
268268
},
269269
{
270270
description: "ptvsd with wait",
271271
pc: pythonContext{debugMode: "ptvsd", port: 2345, wait: true, args: []string{"python", "app.py"}, env: nil},
272272
commands: RunCmdOut([]string{"python", "-V"}, "Python 3.7.4\n").
273273
AndRunCmd([]string{"python", "-m", "ptvsd", "--host", "localhost", "--port", "2345", "--wait", "app.py"}),
274-
expected: pythonContext{debugMode: "ptvsd", port: 2345, wait: true, args: []string{"python", "-m", "ptvsd", "--host", "localhost", "--port", "2345", "--wait", "app.py"}, env: env{"PYTHONPATH": dbgRoot + "/python/lib/python3.7/site-packages"}},
274+
expected: pythonContext{debugMode: "ptvsd", port: 2345, wait: true, major: 3, minor: 7, args: []string{"python", "-m", "ptvsd", "--host", "localhost", "--port", "2345", "--wait", "app.py"}, env: env{"PYTHONPATH": dbgRoot + "/python/lib/python3.7/site-packages"}},
275275
},
276276
{
277277
description: "pydevd",
278278
pc: pythonContext{debugMode: "pydevd", port: 2345, wait: false, args: []string{"python", "app.py"}, env: nil},
279279
commands: RunCmdOut([]string{"python", "-V"}, "Python 3.7.4\n").
280280
AndRunCmd([]string{"python", "-m", "pydevd", "--server", "--port", "2345", "--continue", "--file", "app.py"}),
281-
expected: pythonContext{debugMode: "pydevd", port: 2345, wait: false, args: []string{"python", "-m", "pydevd", "--server", "--port", "2345", "--continue", "--file", "app.py"}, env: env{"PYTHONPATH": dbgRoot + "/python/pydevd/python3.7/lib/python3.7/site-packages"}},
281+
expected: pythonContext{debugMode: "pydevd", port: 2345, wait: false, major: 3, minor: 7, args: []string{"python", "-m", "pydevd", "--server", "--port", "2345", "--continue", "--file", "app.py"}, env: env{"PYTHONPATH": dbgRoot + "/python/pydevd/python3.7/lib/python3.7/site-packages"}},
282282
},
283283
{
284284
description: "pydevd with wait",
285285
pc: pythonContext{debugMode: "pydevd", port: 2345, wait: true, args: []string{"python", "app.py"}, env: nil},
286286
commands: RunCmdOut([]string{"python", "-V"}, "Python 3.7.4\n").
287287
AndRunCmd([]string{"python", "-m", "pydevd", "--server", "--port", "2345", "--file", "app.py"}),
288-
expected: pythonContext{debugMode: "pydevd", port: 2345, wait: true, args: []string{"python", "-m", "pydevd", "--server", "--port", "2345", "--file", "app.py"}, env: env{"PYTHONPATH": dbgRoot + "/python/pydevd/python3.7/lib/python3.7/site-packages"}},
288+
expected: pythonContext{debugMode: "pydevd", port: 2345, wait: true, major: 3, minor: 7, args: []string{"python", "-m", "pydevd", "--server", "--port", "2345", "--file", "app.py"}, env: env{"PYTHONPATH": dbgRoot + "/python/pydevd/python3.7/lib/python3.7/site-packages"}},
289+
},
290+
{
291+
description: "WRAPPER_ENABLED=false",
292+
pc: pythonContext{debugMode: "pydevd", port: 2345, wait: true, args: []string{"python", "app.py"}, env: map[string]string{"WRAPPER_ENABLED": "false"}},
293+
shouldFail: true,
294+
expected: pythonContext{debugMode: "pydevd", port: 2345, wait: true, args: []string{"python", "app.py"}, env: map[string]string{"WRAPPER_ENABLED": "false"}},
295+
},
296+
{
297+
description: "already configured with debugpy",
298+
pc: pythonContext{debugMode: "debugpy", port: 2345, wait: false, args: []string{"python", "-m", "debugpy", "--listen", "2345", "app.py"}},
299+
shouldFail: true,
300+
expected: pythonContext{debugMode: "debugpy", port: 2345, wait: false, args: []string{"python", "-m", "debugpy", "--listen", "2345", "app.py"}},
301+
},
302+
{
303+
description: "already configured with ptvsd",
304+
pc: pythonContext{debugMode: "ptvsd", port: 2345, wait: true, args: []string{"python", "-m", "ptvsd", "--host", "localhost", "--port", "2345", "--wait", "app.py"}},
305+
shouldFail: true,
306+
expected: pythonContext{debugMode: "ptvsd", port: 2345, wait: true, args: []string{"python", "-m", "ptvsd", "--host", "localhost", "--port", "2345", "--wait", "app.py"}},
307+
},
308+
{
309+
description: "already configured with pydevd",
310+
pc: pythonContext{debugMode: "pydevd", port: 2345, wait: true, args: []string{"python", "-m", "pydevd", "--server", "--port", "2345", "--file", "app.py"}},
311+
shouldFail: true,
312+
expected: pythonContext{debugMode: "pydevd", port: 2345, wait: true, args: []string{"python", "-m", "pydevd", "--server", "--port", "2345", "--file", "app.py"}},
289313
},
290314
}
291315

292316
for _, test := range tests {
293317
t.Run(test.description, func(t *testing.T) {
294318
test.commands.Setup(t)
295319
pc := test.pc
296-
err := pc.launch(context.TODO())
320+
result := pc.prepare(context.TODO())
297321

298-
if test.shouldErr && err == nil {
299-
t.Error("expected error")
300-
} else if !test.shouldErr && err != nil {
301-
t.Error("unexpected error:", err)
322+
if test.shouldFail && result == true {
323+
t.Error("prepare() should have failed")
324+
} else if !test.shouldFail && !result {
325+
t.Error("prepare() should have succeeded")
302326
} else if diff := cmp.Diff(test.expected, pc, cmp.AllowUnexported(test.expected)); diff != "" {
303327
_t.Errorf("%T differ (-got, +want): %s", pc, diff)
304328
}

0 commit comments

Comments
 (0)