Merge pull request #2205 from briandealwis/dbg-python

Add Python debugging support

docs/content/en/docs/how-tos/debug/_index.md

@@ -14,23 +14,24 @@ This functionality is in an alpha state and may change without warning.
 ## Debugging with Skaffold
 
 `skaffold debug` acts like `skaffold dev`, but it configures containers in pods
- for debugging as required for each container's runtime technology.
+for debugging as required for each container's runtime technology.
 The associated debugging ports are exposed and labelled and port-forwarded to the
 local machine.  Helper metadata is also added to allow IDEs to detect the debugging
 configuration parameters.
 
 ## How it works
 
 `skaffold debug` examines the built artifacts to determine the underlying runtime technology
-(currently supported: Java and NodeJS).  Any Kubernetes manifest that references these
+(currently supported: Java, NodeJS, and Python).  Any Kubernetes manifest that references these
 artifacts are transformed to enable the runtime technology's debugging functions:
 
   - a JDWP agent is configured for Java applications,
-  - the Chrome DevTools inspector is configured for NodeJS applications.
-
-`skaffold debug` uses a set of heuristics to identify the runtime technology.
-The Kubernetes manifests are transformed on-the-fly such that the on-disk
-representations are untouched.
+  - the Chrome DevTools inspector is configured for NodeJS applications,
+  - Python applications are configured to use [`ptvsd`](https://github.com/microsoft/ptvsd/).
+
+`skaffold debug` uses a set of heuristics to identify the runtime
+technology.  The Kubernetes manifests are transformed on-the-fly
+such that the on-disk representations are untouched.
 
 {{< alert title="Caution" >}}
 `skaffold debug` does not support deprecated versions of Workload API objects such as `apps/v1beta1`.
@@ -43,13 +44,19 @@ representations are untouched.
 
   - Only the `kubectl` and `kustomize` deployers are supported at the moment: support for
     the Helm deployer is not yet available.
-  - Only JVM and NodeJS applications are supported:
+  - File sync is disabled for all artifacts.
+  - Only JVM, NodeJS, and Python applications are supported:
       - JVM applications are configured using the `JAVA_TOOL_OPTIONS` environment variable
         which causes extra debugging output on launch.
       - NodeJS applications must be launched using `node` or `nodemon`, or `npm`
           - `npm` scripts shouldn't then invoke `nodemon` as the DevTools inspector
-            configuration will be picked up by `nodemon` 
-  - File watching is disabled for all artifacts, regardless of whether
-    the artifact could be configured for debugging.
+            configuration will be picked up by `nodemon` rather than the actual application
+      - Python applications are configured to use [`ptvsd`](https://github.com/microsoft/ptvsd/),
+        a wrapper around `pydevd` that uses the
+        [_debug adapter protocol_ (DAP)](https://microsoft.github.io/debug-adapter-protocol/). 
+        The DAP is supported by Visual Studio Code,
+        [Eclipse LSP4e](https://projects.eclipse.org/projects/technology.lsp4e),
+        [and other editors and IDEs](https://microsoft.github.io/debug-adapter-protocol/implementors/tools/).
+        DAP is not yet supported by JetBrains IDEs like PyCharm.
 
- Support for additional language runtimes will be forthcoming.
+ Support for additional language runtimes will be forthcoming.

integration/debug_test.go

@@ -43,14 +43,14 @@ func TestDebug(t *testing.T) {
 			description: "kubectl",
 			dir:         "testdata/debug",
 			deployments: []string{"jib"},
-			pods:        []string{"nodejs", "npm"},
+			pods:        []string{"nodejs", "npm", "python3"},
 		},
 		{
 			description: "kustomize",
 			args:        []string{"--profile", "kustomize"},
 			dir:         "testdata/debug",
 			deployments: []string{"jib"},
-			pods:        []string{"nodejs", "npm"},
+			pods:        []string{"nodejs", "npm", "python3"},
 		},
 	}
 	for _, test := range tests {

integration/testdata/debug/kustomization.yaml

@@ -2,3 +2,4 @@ resources:
   - jib/k8s/web.yaml
   - nodejs/k8s/pod.yaml
   - npm/k8s/pod.yaml
+  - python3/k8s/pod.yaml

integration/testdata/debug/nodejs/package.json

@@ -5,8 +5,5 @@
   "main": "index.js",
   "dependencies": {
     "express": "^4.16.4"
-  },
-  "devDependencies": {
-    "nodemon": "^1.18.4"
   }
 }

integration/testdata/debug/npm/package.json

@@ -8,8 +8,5 @@
   },
   "dependencies": {
     "express": "^4.16.4"
-  },
-  "devDependencies": {
-    "nodemon": "^1.18.4"
   }
 }

integration/testdata/debug/python3/Dockerfile

@@ -0,0 +1,8 @@
+FROM python:3
+
+EXPOSE 5000
+CMD ["python3", "app.py"]
+
+WORKDIR /app
+COPY . /app
+RUN pip3 install -r requirements.txt

integration/testdata/debug/python3/app.py

@@ -0,0 +1,10 @@
+from flask import Flask
+app = Flask(__name__)
+
+
+@app.route('/')
+def hello():
+    return "Hello World!"
+
+if __name__ == '__main__':
+    app.run()

integration/testdata/debug/python3/k8s/pod.yaml

@@ -0,0 +1,10 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: python3
+spec:
+  containers:
+  - name: web
+    image: gcr.io/k8s-skaffold/skaffold-debug-python3
+    ports:
+    - containerPort: 5000

integration/testdata/debug/python3/requirements.txt

@@ -0,0 +1 @@
+Flask==0.10.1

integration/testdata/debug/skaffold.yaml

@@ -11,12 +11,15 @@ build:
     context: npm
   - image: gcr.io/k8s-skaffold/skaffold-debug-nodejs
     context: nodejs
+  - image: gcr.io/k8s-skaffold/skaffold-debug-python3
+    context: python3
 deploy:
   kubectl:
     manifests:
     - jib/k8s/web.yaml
     - nodejs/k8s/pod.yaml
     - npm/k8s/pod.yaml
+    - python3/k8s/pod.yaml
 profiles:
   - name: kustomize
     deploy:

pkg/skaffold/debug/debug.go

@@ -57,7 +57,7 @@ func ApplyDebuggingTransforms(l kubectl.ManifestList, builds []build.Artifact, i
 		if artifact := findArtifact(image, builds); artifact != nil {
 			return retrieveImageConfiguration(ctx, artifact, insecureRegistries)
 		}
-		return imageConfiguration{}, errors.Errorf("no build artifact for [%q]", image)
+		return imageConfiguration{}, errors.Errorf("no build artifact for %q", image)
 	}
 	return applyDebuggingTransforms(l, retriever)
 }
@@ -89,7 +89,7 @@ func applyDebuggingTransforms(l kubectl.ManifestList, retriever configurationRet
 func findArtifact(image string, builds []build.Artifact) *build.Artifact {
 	for _, artifact := range builds {
 		if image == artifact.ImageName || image == artifact.Tag {
-			logrus.Debugf("Found artifact for image [%s]", image)
+			logrus.Debugf("Found artifact for image %q", image)
 			return &artifact
 		}
 	}

pkg/skaffold/debug/debug_test.go

@@ -110,6 +110,10 @@ func (t testTransformer) IsApplicable(config imageConfiguration) bool {
 	return true
 }
 
+func (t testTransformer) RuntimeSupportImage() string {
+	return ""
+}
+
 func (t testTransformer) Apply(container *v1.Container, config imageConfiguration, portAlloc portAllocator) map[string]interface{} {
 	port := portAlloc(9999)
 	container.Ports = append(container.Ports, v1.ContainerPort{Name: "test", ContainerPort: port})

pkg/skaffold/debug/transform.go

@@ -14,6 +14,39 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
+/*
+The `debug` package transforms Kubernetes pod-bearing resources so as to configure containers
+for remote debugging as suited for a container's runtime technology.  This package defines
+a _container transformer_ interface. Each transformer implementation should do the following:
+
+1. The transformer should modify the container's entrypoint, command arguments, and environment to enable debugging for the appropriate language runtime.
+2. The transformer should expose the port(s) required to connect remote debuggers.
+3. The transformer should identify any additional support files required to enable debugging (e.g., the `ptvsd` debugger for Python).
+4. The transform should return metadata to describe the remote connection information.
+
+Certain language runtimes require additional support files to enable remote debugging.
+These support files are provided through a set of support images defined at `gcr.io/gcp-dev-tools/duct-tape/`
+and defined at https://github.com/GoogleContainerTools/container-debug-support.
+The appropriate image ID is returned by the language transformer.  These support images
+are configured as initContainers on the pod and are expected to copy the debugging support
+files into a support volume mounted at `/dbg`.  The expected convention is that each runtime's
+files are placed in `/dbg/<runtimeId>`.  This same volume is then mounted into the
+actual containers at `/dbg`.
+
+As Kubernetes container objects don't actually carry metadata, we place this metadata on
+the container's parent as an _annotation_; as a pod/podspec can have multiple containers, each of which may
+be debuggable, we record this metadata using as a JSON object keyed by the container name.
+Kubernetes requires that containers within a podspec are uniquely named.
+For example, a pod with two containers named `microservice` and `adapter` may be:
+
+  debug.cloud.google.com/config: '{
+    "microservice":{"devtools":9229,"runtime":"nodejs"},
+    "adapter":{"jdwp":5005,"runtime":"jvm"}
+  }'
+
+Each configuration is itself a JSON object with a `runtime` field identifying the
+language runtime, and a set of runtime-specific fields describing connection information.
+*/
 package debug
 
 import (
@@ -52,10 +85,16 @@ type containerTransformer interface {
 	// IsApplicable determines if this container is suitable to be transformed.
 	IsApplicable(config imageConfiguration) bool
 
+	// RuntimeSupportImage returns the associated duct-tape helper image required or empty string
+	RuntimeSupportImage() string
+
 	// Apply configures a container definition for debugging, returning a simple map describing the debug configuration details or `nil` if it could not be done
 	Apply(container *v1.Container, config imageConfiguration, portAlloc portAllocator) map[string]interface{}
 }
 
+// debuggingSupportVolume is the name of the volume used to hold language runtime debugging support files
+const debuggingSupportFilesVolume = "debugging-support-files"
+
 var containerTransforms []containerTransformer
 
 // transformManifest attempts to configure a manifest for debugging.
@@ -120,18 +159,58 @@ func transformPodSpec(metadata *metav1.ObjectMeta, podSpec *v1.PodSpec, retrieve
 	portAlloc := func(desiredPort int32) int32 {
 		return allocatePort(podSpec, desiredPort)
 	}
-	// containers are required to have unique name within a pod
+	// map of containers -> debugging configuration maps; k8s ensures that a pod's containers are uniquely named
 	configurations := make(map[string]map[string]interface{})
+	// the container images that require debugging support files
+	var containersRequiringSupport []*v1.Container
+	// the set of image IDs required to provide debugging support files
+	requiredSupportImages := make(map[string]bool)
 	for i := range podSpec.Containers {
 		container := &podSpec.Containers[i]
-		// we only reconfigure build artifacts
-		if configuration, err := transformContainer(container, retrieveImageConfiguration, portAlloc); err == nil {
+		// the usual retriever returns an error for non-build artifacts
+		imageConfig, err := retrieveImageConfiguration(container.Image)
+		if err != nil {
+			continue
+		}
+		// requiredImage, if not empty, is the image ID providing the debugging support files
+		if configuration, requiredImage, err := transformContainer(container, imageConfig, portAlloc); err == nil {
 			configurations[container.Name] = configuration
+			if len(requiredImage) > 0 {
+				logrus.Infof("%q requires debugging support image %q", container.Name, requiredImage)
+				containersRequiringSupport = append(containersRequiringSupport, container)
+				requiredSupportImages[requiredImage] = true
+			}
 			// todo: add this artifact to the watch list?
 		} else {
-			logrus.Infof("Image [%s] not configured for debugging: %v", container.Image, err)
+			logrus.Infof("Image %q not configured for debugging: %v", container.Name, err)
 		}
 	}
+
+	// check if we have any images requiring additional debugging support files
+	if len(containersRequiringSupport) > 0 {
+		logrus.Infof("Configuring installation of debugging support files")
+		// we create the volume that will hold the debugging support files
+		supportVolume := v1.Volume{Name: debuggingSupportFilesVolume, VolumeSource: v1.VolumeSource{EmptyDir: &v1.EmptyDirVolumeSource{}}}
+		podSpec.Volumes = append(podSpec.Volumes, supportVolume)
+
+		// this volume is mounted in the containers at `/dbg`
+		supportVolumeMount := v1.VolumeMount{Name: debuggingSupportFilesVolume, MountPath: "/dbg"}
+		// the initContainers are responsible for populating the contents of `/dbg`
+		// TODO make this pluggable for airgapped clusters? or is making container `imagePullPolicy:IfNotPresent` sufficient?
+		for imageID := range requiredSupportImages {
+			supportFilesInitContainer := v1.Container{
+				Name:         fmt.Sprintf("install-%s-support", imageID),
+				Image:        fmt.Sprintf("gcr.io/gcp-dev-tools/duct-tape/%s", imageID),
+				VolumeMounts: []v1.VolumeMount{supportVolumeMount},
+			}
+			podSpec.InitContainers = append(podSpec.InitContainers, supportFilesInitContainer)
+		}
+		// the populated volume is then mounted in the containers at `/dbg` too
+		for _, container := range containersRequiringSupport {
+			container.VolumeMounts = append(container.VolumeMounts, supportVolumeMount)
+		}
+	}
+
 	if len(configurations) > 0 {
 		if metadata.Annotations == nil {
 			metadata.Annotations = make(map[string]string)
@@ -177,17 +256,15 @@ func isPortAvailable(podSpec *v1.PodSpec, port int32) bool {
 }
 
 // transformContainer rewrites the container definition to enable debugging.
-// Returns a debugging configuration description or an error if the rewrite was unsuccessful.
-func transformContainer(container *v1.Container, retrieveImageConfiguration configurationRetriever, portAlloc portAllocator) (map[string]interface{}, error) {
-	var config imageConfiguration
-	config, err := retrieveImageConfiguration(container.Image)
-	if err != nil {
-		return nil, err
-	}
-
+// Returns a debugging configuration description with associated language runtime support
+// container image, or an error if the rewrite was unsuccessful.
+func transformContainer(container *v1.Container, config imageConfiguration, portAlloc portAllocator) (map[string]interface{}, string, error) {
 	// update image configuration values with those set in the k8s manifest
 	for _, envVar := range container.Env {
 		// FIXME handle ValueFrom?
+		if config.env == nil {
+			config.env = make(map[string]string)
+		}
 		config.env[envVar.Name] = envVar.Value
 	}
 
@@ -200,10 +277,10 @@ func transformContainer(container *v1.Container, retrieveImageConfiguration conf
 
 	for _, transform := range containerTransforms {
 		if transform.IsApplicable(config) {
-			return transform.Apply(container, config, portAlloc), nil
+			return transform.Apply(container, config, portAlloc), transform.RuntimeSupportImage(), nil
 		}
 	}
-	return nil, errors.Errorf("unable to determine runtime for [%s]", container.Name)
+	return nil, "", errors.Errorf("unable to determine runtime for %q", container.Name)
 }
 
 func encodeConfigurations(configurations map[string]map[string]interface{}) string {

pkg/skaffold/debug/transform_jvm.go

@@ -63,10 +63,15 @@ type jdwpSpec struct {
 	server  bool
 }
 
+func (t jdwpTransformer) RuntimeSupportImage() string {
+	// no additional support required
+	return ""
+}
+
 // Apply configures a container definition for JVM debugging.
 // Returns a simple map describing the debug configuration details.
 func (t jdwpTransformer) Apply(container *v1.Container, config imageConfiguration, portAlloc portAllocator) map[string]interface{} {
-	logrus.Infof("Configuring [%s] for JVM debugging", container.Name)
+	logrus.Infof("Configuring %q for JVM debugging", container.Name)
 	// try to find existing JAVA_TOOL_OPTIONS or jdwp command argument
 	// todo: find existing containerPort "jdwp" and use port. But what if it conflicts with jdwp spec?
 	spec := retrieveJdwpSpec(config)

pkg/skaffold/debug/transform_jvm_test.go

@@ -29,6 +29,10 @@ import (
 	"github.com/google/go-cmp/cmp"
 )
 
+func TestJdwpTransformer_RuntimeSupportImage(t *testing.T) {
+	testutil.CheckDeepEqual(t, "", jdwpTransformer{}.RuntimeSupportImage())
+}
+
 func TestJdwpTransformer_IsApplicable(t *testing.T) {
 	tests := []struct {
 		description string