feat: introduce DebuggerMetaData to control if process should wait fo…

…r debugger to be attached or not

pkg/skaffold/debug/container.go

@@ -50,14 +50,14 @@ type containerTransformer interface {
 	// and required initContainer (an empty string if not required), or return a non-nil error if
 	// the container could not be transformed.  The initContainer image is intended to install any
 	// required debug support tools.
-	Apply(adapter types.ContainerAdapter, config ImageConfiguration, portAlloc PortAllocator, overrideProtocols []string) (types.ContainerDebugConfiguration, string, error)
+	Apply(adapter types.ContainerAdapter, config ImageConfiguration, portAlloc PortAllocator, overrideProtocols []string, dmd *DebuggerMetaData) (types.ContainerDebugConfiguration, string, error)
 }
 
 // TransformContainer rewrites the container definition to enable debugging.
 // Returns a debugging configuration description with associated language runtime support
 // container image, or an error if the rewrite was unsuccessful.
-func TransformContainer(adapter types.ContainerAdapter, config ImageConfiguration, portAlloc PortAllocator) (types.ContainerDebugConfiguration, string, error) {
-	configuration, requiredImage, err := transformContainer(adapter, config, portAlloc)
+func TransformContainer(adapter types.ContainerAdapter, config ImageConfiguration, portAlloc PortAllocator, dmd *DebuggerMetaData) (types.ContainerDebugConfiguration, string, error) {
+	configuration, requiredImage, err := transformContainer(adapter, config, portAlloc, dmd)
 	if err == nil {
 		configuration.Artifact = config.Artifact
 		if configuration.WorkingDir == "" {
@@ -67,7 +67,7 @@ func TransformContainer(adapter types.ContainerAdapter, config ImageConfiguratio
 	return configuration, requiredImage, err
 }
 
-func transformContainer(adapter types.ContainerAdapter, config ImageConfiguration, portAlloc PortAllocator) (types.ContainerDebugConfiguration, string, error) {
+func transformContainer(adapter types.ContainerAdapter, config ImageConfiguration, portAlloc PortAllocator, dmd *DebuggerMetaData) (types.ContainerDebugConfiguration, string, error) {
 	// Update the image configuration's environment with those set in the k8s manifest.
 	// (Environment variables in the k8s container's `env` add to the image configuration's `env` settings rather than replace.)
 	container := adapter.GetContainer()
@@ -88,7 +88,7 @@ func transformContainer(adapter types.ContainerAdapter, config ImageConfiguratio
 
 	// Apply command-line unwrapping for buildpack images and images using `sh -c`-style command-lines
 	next := func(adapter types.ContainerAdapter, config ImageConfiguration) (types.ContainerDebugConfiguration, string, error) {
-		return performContainerTransform(adapter, config, portAlloc)
+		return performContainerTransform(adapter, config, portAlloc, dmd)
 	}
 	if isCNBImage(config) {
 		return updateForCNBImage(adapter, config, next)
@@ -148,11 +148,11 @@ func isShDashC(cmd, arg string) bool {
 	return (cmd == "/bin/sh" || cmd == "/bin/bash") && arg == "-c"
 }
 
-func performContainerTransform(adapter types.ContainerAdapter, config ImageConfiguration, portAlloc PortAllocator) (types.ContainerDebugConfiguration, string, error) {
+func performContainerTransform(adapter types.ContainerAdapter, config ImageConfiguration, portAlloc PortAllocator, dmd *DebuggerMetaData) (types.ContainerDebugConfiguration, string, error) {
 	log.Entry(context.TODO()).Tracef("Examining container %q with config %v", adapter.GetContainer().Name, config)
 	for transform := range containerTransforms {
 		if transform.IsApplicable(config) {
-			return transform.Apply(adapter, config, portAlloc, Protocols)
+			return transform.Apply(adapter, config, portAlloc, Protocols, dmd)
 		}
 	}
 	return types.ContainerDebugConfiguration{}, "", fmt.Errorf("unable to determine runtime for %q", adapter.GetContainer().Name)

pkg/skaffold/debug/transform.go

@@ -67,6 +67,14 @@ type PortAllocator func(int32) int32
 // configurationRetriever retrieves an container image configuration
 type ConfigurationRetriever func(string) (ImageConfiguration, error)
 
+// DebuggerMetaData captures info for debug tranformers to construct the cli command appropriately
+type DebuggerMetaData struct {
+	// ShouldWait hints that the process should start and wait for the debugger to be attached.
+	// For kubernetes this derives from an annotation at Pod-Level skaffold.dev/debug-suspend and
+	// for docker from an image label skaffold.dev/debug-suspend.
+	ShouldWait bool
+}
+
 // ImageConfiguration captures information from a docker/oci image configuration.
 // It also includes a "artifact", usually containing the corresponding artifact's' image name from `skaffold.yaml`.
 type ImageConfiguration struct {
@@ -92,6 +100,20 @@ var entrypointLaunchers []string
 
 var Protocols = []string{}
 
+// ExtractDebuggerMetaData extracts and returns the appropriate DebuggerMetaData based on the passed map[string]string
+// which contains the context-appropriate metadata, e.g. for Kubernetes deployment the annotations and for Docker the image
+// Labels.
+func ExtractDebuggerMetaData(md map[string]string) *DebuggerMetaData {
+	dmd := &DebuggerMetaData{}
+
+	if md == nil {
+		return dmd
+	}
+
+	_, dmd.ShouldWait = md["skaffold.dev/debug-suspend"]
+	return dmd
+}
+
 // isEntrypointLauncher checks if the given entrypoint is a known entrypoint launcher,
 // meaning an entrypoint that treats the image's CMD as a command-line.
 func isEntrypointLauncher(entrypoint []string) bool {

pkg/skaffold/debug/transform_go.go

@@ -51,11 +51,12 @@ type dlvSpec struct {
 	port       uint16
 	headless   bool
 	log        bool
+	wait       bool
 	apiVersion int
 }
 
 func newDlvSpec(port uint16) dlvSpec {
-	return dlvSpec{mode: "exec", port: port, apiVersion: defaultAPIVersion, headless: true}
+	return dlvSpec{mode: "exec", port: port, apiVersion: defaultAPIVersion, headless: true, wait: true}
 }
 
 // isLaunchingDlv determines if the arguments seems to be invoking Delve
@@ -106,7 +107,7 @@ func (t dlvTransformer) IsApplicable(config ImageConfiguration) bool {
 
 // Apply configures a container definition for Go with Delve.
 // Returns the debug configuration details, with the "go" support image
-func (t dlvTransformer) Apply(adapter types.ContainerAdapter, config ImageConfiguration, portAlloc PortAllocator, overrideProtocols []string) (types.ContainerDebugConfiguration, string, error) {
+func (t dlvTransformer) Apply(adapter types.ContainerAdapter, config ImageConfiguration, portAlloc PortAllocator, overrideProtocols []string, dmd *DebuggerMetaData) (types.ContainerDebugConfiguration, string, error) {
 	container := adapter.GetContainer()
 	log.Entry(context.TODO()).Infof("Configuring %q for Go/Delve debugging", container.Name)
 
@@ -116,6 +117,13 @@ func (t dlvTransformer) Apply(adapter types.ContainerAdapter, config ImageConfig
 	if spec == nil {
 		newSpec := newDlvSpec(uint16(portAlloc(defaultDlvPort)))
 		spec = &newSpec
+
+		if dmd != nil {
+			spec.wait = dmd.ShouldWait
+		} else {
+			spec.wait = false
+		}
+
 		switch {
 		case len(config.Entrypoint) > 0 && !isEntrypointLauncher(config.Entrypoint):
 			container.Command = rewriteDlvCommandLine(config.Entrypoint, *spec, container.Args)
@@ -151,7 +159,7 @@ func extractDlvSpec(args []string) *dlvSpec {
 		return nil
 	}
 	// delve's defaults
-	spec := dlvSpec{apiVersion: 2, log: false, headless: false}
+	spec := dlvSpec{apiVersion: 2, log: false, headless: false, wait: true}
 arguments:
 	for _, arg := range args {
 		switch {
@@ -163,6 +171,8 @@ arguments:
 			spec.headless = true
 		case arg == "--log":
 			spec.log = true
+		case arg == "--continue":
+			spec.wait = false
 		case strings.HasPrefix(arg, "--listen="):
 			address := strings.SplitN(arg, "=", 2)[1]
 			split := strings.SplitN(address, ":", 2)
@@ -203,7 +213,12 @@ func (spec dlvSpec) asArguments() []string {
 	if spec.headless {
 		args = append(args, "--headless")
 	}
-	args = append(args, "--continue", "--accept-multiclient")
+
+	if !spec.wait {
+		args = append(args, "--continue")
+	}
+
+	args = append(args, "--accept-multiclient")
 	if spec.port > 0 {
 		args = append(args, fmt.Sprintf("--listen=%s:%d", spec.host, spec.port))
 	} else {

pkg/skaffold/debug/transform_go_test.go

@@ -28,7 +28,7 @@ import (
 
 func TestNewDlvSpecDefaults(t *testing.T) {
 	spec := newDlvSpec(20)
-	expected := dlvSpec{mode: "exec", port: 20, apiVersion: 2, headless: true, log: false}
+	expected := dlvSpec{mode: "exec", port: 20, apiVersion: 2, headless: true, log: false, wait: true}
 	testutil.CheckDeepEqual(t, expected, spec, cmp.AllowUnexported(spec))
 }
 
@@ -40,14 +40,15 @@ func TestExtractDlvArg(t *testing.T) {
 		{nil, nil},
 		{[]string{"foo"}, nil},
 		{[]string{"foo", "--foo"}, nil},
-		{[]string{"dlv", "debug", "--headless"}, &dlvSpec{mode: "debug", headless: true, apiVersion: 2, log: false}},
-		{[]string{"dlv", "--headless", "exec"}, &dlvSpec{mode: "exec", headless: true, apiVersion: 2, log: false}},
-		{[]string{"dlv", "--headless", "exec", "--", "--listen=host:4345"}, &dlvSpec{mode: "exec", headless: true, apiVersion: 2, log: false}},
-		{[]string{"dlv", "test", "--headless", "--listen=host:4345"}, &dlvSpec{mode: "test", host: "host", port: 4345, headless: true, apiVersion: 2, log: false}},
-		{[]string{"dlv", "debug", "--headless", "--api-version=1"}, &dlvSpec{mode: "debug", headless: true, apiVersion: 1, log: false}},
-		{[]string{"dlv", "debug", "--listen=host:4345", "--headless", "--api-version=2", "--log"}, &dlvSpec{mode: "debug", host: "host", port: 4345, headless: true, apiVersion: 2, log: true}},
-		{[]string{"dlv", "debug", "--listen=:4345"}, &dlvSpec{mode: "debug", port: 4345, apiVersion: 2}},
-		{[]string{"dlv", "debug", "--listen=host:"}, &dlvSpec{mode: "debug", host: "host", apiVersion: 2}},
+		{[]string{"dlv", "debug", "--headless"}, &dlvSpec{mode: "debug", headless: true, apiVersion: 2, log: false, wait: true}},
+		{[]string{"dlv", "--headless", "exec"}, &dlvSpec{mode: "exec", headless: true, apiVersion: 2, log: false, wait: true}},
+		{[]string{"dlv", "--headless", "exec", "--", "--listen=host:4345"}, &dlvSpec{mode: "exec", headless: true, apiVersion: 2, log: false, wait: true}},
+		{[]string{"dlv", "test", "--headless", "--listen=host:4345"}, &dlvSpec{mode: "test", host: "host", port: 4345, headless: true, apiVersion: 2, log: false, wait: true}},
+		{[]string{"dlv", "debug", "--headless", "--api-version=1"}, &dlvSpec{mode: "debug", headless: true, apiVersion: 1, log: false, wait: true}},
+		{[]string{"dlv", "debug", "--listen=host:4345", "--headless", "--api-version=2", "--log"}, &dlvSpec{mode: "debug", host: "host", port: 4345, headless: true, apiVersion: 2, log: true, wait: true}},
+		{[]string{"dlv", "debug", "--listen=:4345"}, &dlvSpec{mode: "debug", port: 4345, apiVersion: 2, wait: true}},
+		{[]string{"dlv", "debug", "--listen=host:"}, &dlvSpec{mode: "debug", host: "host", apiVersion: 2, wait: true}},
+		{[]string{"dlv", "debug", "--listen=host:", "--continue"}, &dlvSpec{mode: "debug", host: "host", apiVersion: 2, wait: false}},
 	}
 	for _, test := range tests {
 		testutil.Run(t, strings.Join(test.in, " "), func(t *testutil.T) {

pkg/skaffold/debug/transform_jvm.go

@@ -73,7 +73,7 @@ type jdwpSpec struct {
 
 // Apply configures a container definition for JVM debugging.
 // Returns a simple map describing the debug configuration details.
-func (t jdwpTransformer) Apply(adapter types.ContainerAdapter, config ImageConfiguration, portAlloc PortAllocator, overrideProtocols []string) (types.ContainerDebugConfiguration, string, error) {
+func (t jdwpTransformer) Apply(adapter types.ContainerAdapter, config ImageConfiguration, portAlloc PortAllocator, overrideProtocols []string, dmd *DebuggerMetaData) (types.ContainerDebugConfiguration, string, error) {
 	container := adapter.GetContainer()
 	log.Entry(context.TODO()).Infof("Configuring %q for JVM debugging", container.Name)
 	// try to find existing JAVA_TOOL_OPTIONS or jdwp command argument
@@ -84,7 +84,15 @@ func (t jdwpTransformer) Apply(adapter types.ContainerAdapter, config ImageConfi
 		port = int32(spec.port)
 	} else {
 		port = portAlloc(defaultJdwpPort)
-		jto := fmt.Sprintf("-agentlib:jdwp=transport=dt_socket,server=y,address=%d,suspend=n,quiet=y", port)
+
+		var suspendParam string
+		if dmd != nil && dmd.ShouldWait {
+			suspendParam = "y"
+		} else {
+			suspendParam = "n"
+		}
+
+		jto := fmt.Sprintf("-agentlib:jdwp=transport=dt_socket,server=y,address=%d,suspend=%s,quiet=y", port, suspendParam)
 		if existing, found := config.Env["JAVA_TOOL_OPTIONS"]; found {
 			jto = existing + " " + jto
 		}

pkg/skaffold/debug/transform_netcore.go

@@ -77,7 +77,7 @@ func (t netcoreTransformer) IsApplicable(config ImageConfiguration) bool {
 
 // Apply configures a container definition for vsdbg.
 // Returns a simple map describing the debug configuration details.
-func (t netcoreTransformer) Apply(adapter types.ContainerAdapter, config ImageConfiguration, portAlloc PortAllocator, overrideProtocols []string) (types.ContainerDebugConfiguration, string, error) {
+func (t netcoreTransformer) Apply(adapter types.ContainerAdapter, config ImageConfiguration, portAlloc PortAllocator, overrideProtocols []string, dmd *DebuggerMetaData) (types.ContainerDebugConfiguration, string, error) {
 	container := adapter.GetContainer()
 	log.Entry(context.TODO()).Infof("Configuring %q for netcore debugging", container.Name)
 

pkg/skaffold/debug/transform_nodejs.go

@@ -85,14 +85,20 @@ func (t nodeTransformer) IsApplicable(config ImageConfiguration) bool {
 
 // Apply configures a container definition for NodeJS Chrome V8 Inspector.
 // Returns a simple map describing the debug configuration details.
-func (t nodeTransformer) Apply(adapter types.ContainerAdapter, config ImageConfiguration, portAlloc PortAllocator, overrideProtocols []string) (types.ContainerDebugConfiguration, string, error) {
+func (t nodeTransformer) Apply(adapter types.ContainerAdapter, config ImageConfiguration, portAlloc PortAllocator, overrideProtocols []string, dmd *DebuggerMetaData) (types.ContainerDebugConfiguration, string, error) {
 	container := adapter.GetContainer()
 	log.Entry(context.TODO()).Infof("Configuring %q for node.js debugging", container.Name)
 
 	// try to find existing `--inspect` command
 	spec := retrieveNodeInspectSpec(config)
 	if spec == nil {
 		spec = &inspectSpec{host: "0.0.0.0", port: portAlloc(defaultDevtoolsPort)}
+		if dmd != nil {
+			spec.brk = dmd.ShouldWait
+		} else {
+			spec.brk = false
+		}
+
 		switch {
 		case isLaunchingNode(config.Entrypoint):
 			container.Command = rewriteNodeCommandLine(config.Entrypoint, *spec)

pkg/skaffold/debug/transform_python.go

@@ -115,7 +115,7 @@ func (t pythonTransformer) IsApplicable(config ImageConfiguration) bool {
 
 // Apply configures a container definition for Python with ptvsd/debugpy/pydevd.
 // Returns a simple map describing the debug configuration details.
-func (t pythonTransformer) Apply(adapter types.ContainerAdapter, config ImageConfiguration, portAlloc PortAllocator, overrideProtocols []string) (types.ContainerDebugConfiguration, string, error) {
+func (t pythonTransformer) Apply(adapter types.ContainerAdapter, config ImageConfiguration, portAlloc PortAllocator, overrideProtocols []string, dmd *DebuggerMetaData) (types.ContainerDebugConfiguration, string, error) {
 	container := adapter.GetContainer()
 	log.Entry(context.TODO()).Infof("Configuring %q for python debugging", container.Name)
 
@@ -129,7 +129,12 @@ func (t pythonTransformer) Apply(adapter types.ContainerAdapter, config ImageCon
 		}, "", nil
 	}
 
-	spec := createPythonDebugSpec(overrideProtocols, portAlloc)
+	shouldWait := false
+	if dmd != nil {
+		shouldWait = dmd.ShouldWait
+	}
+
+	spec := createPythonDebugSpec(overrideProtocols, portAlloc, shouldWait)
 
 	switch {
 	case isLaunchingPython(config.Entrypoint):
@@ -174,17 +179,17 @@ func extractPythonDebugSpec(args []string) *pythonSpec {
 	return nil
 }
 
-func createPythonDebugSpec(overrideProtocols []string, portAlloc PortAllocator) *pythonSpec {
+func createPythonDebugSpec(overrideProtocols []string, portAlloc PortAllocator, shouldWait bool) *pythonSpec {
 	for _, p := range overrideProtocols {
 		switch p {
 		case pydevdProtocol:
-			return &pythonSpec{debugger: pydevd, port: portAlloc(defaultPydevdPort)}
+			return &pythonSpec{debugger: pydevd, port: portAlloc(defaultPydevdPort), wait: shouldWait}
 		case dapProtocol:
-			return &pythonSpec{debugger: debugpy, port: portAlloc(defaultDebugpyPort)}
+			return &pythonSpec{debugger: debugpy, port: portAlloc(defaultDebugpyPort), wait: shouldWait}
 		}
 	}
 
-	return &pythonSpec{debugger: debugpy, port: portAlloc(defaultDebugpyPort)}
+	return &pythonSpec{debugger: debugpy, port: portAlloc(defaultDebugpyPort), wait: shouldWait}
 }
 
 func extractPtvsdSpec(args []string) *pythonSpec {

pkg/skaffold/docker/debugger/transform.go

@@ -58,7 +58,9 @@ func transformImage(ctx context.Context, artifact graph.Artifact, cfg *container
 	// the set of image IDs required to provide debugging support files
 	requiredSupportImages := make(map[string]bool)
 
-	if configuration, requiredImage, err := debug.TransformContainer(adapter, imageConfig, portAlloc); err == nil {
+	dmd := debug.ExtractDebuggerMetaData(imageConfig.Labels)
+
+	if configuration, requiredImage, err := debug.TransformContainer(adapter, imageConfig, portAlloc, dmd); err == nil {
 		configurations[adapter.GetContainer().Name] = configuration
 		if len(requiredImage) > 0 {
 			log.Entry(ctx).Infof("%q requires debugging support image %q", cfg.Image, requiredImage)

pkg/skaffold/kubernetes/debugging/debug_test.go

@@ -37,7 +37,7 @@ func (t testTransformer) IsApplicable(config debug.ImageConfiguration) bool {
 	return true
 }
 
-func (t testTransformer) Apply(adapter types.ContainerAdapter, config debug.ImageConfiguration, portAlloc debug.PortAllocator, overrideProtocols []string) (types.ContainerDebugConfiguration, string, error) {
+func (t testTransformer) Apply(adapter types.ContainerAdapter, config debug.ImageConfiguration, portAlloc debug.PortAllocator, overrideProtocols []string, dmd *debug.DebuggerMetaData) (types.ContainerDebugConfiguration, string, error) {
 	port := portAlloc(9999)
 	container := adapter.GetContainer()
 	container.Ports = append(container.Ports, types.ContainerPort{Name: "test", ContainerPort: port})

pkg/skaffold/kubernetes/debugging/transform.go

@@ -267,7 +267,10 @@ func rewriteContainers(metadata *metav1.ObjectMeta, podSpec *v1.PodSpec, retriev
 		a := adapter.NewAdapter(&container)
 		// requiredImage, if not empty, is the image ID providing the debugging support files
 		// `err != nil` means that the container did not or could not be transformed
-		if configuration, requiredImage, err := debug.TransformContainer(a, imageConfig, portAlloc); err == nil {
+
+		dmd := debug.ExtractDebuggerMetaData(metadata.Annotations)
+
+		if configuration, requiredImage, err := debug.TransformContainer(a, imageConfig, portAlloc, dmd); err == nil {
 			configurations[container.Name] = configuration
 			podSpec.Containers[i] = container // apply any configuration changes
 			if len(requiredImage) > 0 {

pkg/skaffold/kubernetes/debugging/transform_go_test.go

@@ -40,6 +40,7 @@ func TestDlvTransformerApply(t *testing.T) {
 		result        v1.Container
 		debugConfig   types.ContainerDebugConfiguration
 		image         string
+		dmd           *debug.DebuggerMetaData
 	}{
 		{
 			description:   "empty",
@@ -58,6 +59,20 @@ func TestDlvTransformerApply(t *testing.T) {
 			debugConfig: types.ContainerDebugConfiguration{Runtime: "go", Ports: map[string]uint32{"dlv": 56268}},
 			image:       "go",
 		},
+		{
+			description:   "basic with wait debugger",
+			containerSpec: v1.Container{},
+			configuration: debug.ImageConfiguration{Entrypoint: []string{"app", "arg"}},
+			result: v1.Container{
+				Command: []string{"/dbg/go/bin/dlv", "exec", "--headless", "--accept-multiclient", "--listen=:56268", "--api-version=2", "app", "--", "arg"},
+				Ports:   []v1.ContainerPort{{Name: "dlv", ContainerPort: 56268}},
+			},
+			debugConfig: types.ContainerDebugConfiguration{Runtime: "go", Ports: map[string]uint32{"dlv": 56268}},
+			image:       "go",
+			dmd: &debug.DebuggerMetaData{
+				ShouldWait: true,
+			},
+		},
 		{
 			description: "existing port",
 			containerSpec: v1.Container{
@@ -116,7 +131,7 @@ func TestDlvTransformerApply(t *testing.T) {
 	for _, test := range tests {
 		testutil.Run(t, test.description, func(t *testutil.T) {
 			adapter := adapter.NewAdapter(&test.containerSpec)
-			config, image, err := debug.NewDlvTransformer().Apply(adapter, test.configuration, identity, nil)
+			config, image, err := debug.NewDlvTransformer().Apply(adapter, test.configuration, identity, nil, test.dmd)
 			adapter.Apply()
 
 			t.CheckError(test.shouldErr, err)