Merge pull request #12585 from jim-minter/issue12559-fix

Merged by openshift-bot

Author: OpenShift Bot

pkg/cmd/cli/cmd/debug.go

@@ -29,6 +29,7 @@ import (
 	"github.com/openshift/origin/pkg/cmd/templates"
 	cmdutil "github.com/openshift/origin/pkg/cmd/util"
 	"github.com/openshift/origin/pkg/cmd/util/clientcmd"
+	deployapi "github.com/openshift/origin/pkg/deploy/api"
 	generateapp "github.com/openshift/origin/pkg/generate/app"
 	imageapi "github.com/openshift/origin/pkg/image/api"
 )
@@ -394,18 +395,107 @@ func (o *DebugOptions) Debug() error {
 	})
 }
 
-func (o *DebugOptions) getContainerImageCommand(container *kapi.Container) ([]string, error) {
-	image := container.Image[strings.LastIndex(container.Image, "/")+1:]
-	name, id, ok := imageapi.SplitImageStreamImage(image)
-	if !ok {
-		return nil, errors.New("container image did not contain an id")
+// getContainerImageViaDeploymentConfig attempts to return an Image for a given
+// Container.  It tries to walk from the Container's Pod to its DeploymentConfig
+// (via the "openshift.io/deployment-config.name" annotation), then tries to
+// find the ImageStream from which the DeploymentConfig is deploying, then tries
+// to find a match for the Container's image in the ImageStream's Images.
+func (o *DebugOptions) getContainerImageViaDeploymentConfig(pod *kapi.Pod, container *kapi.Container) (*imageapi.Image, error) {
+	ref, err := imageapi.ParseDockerImageReference(container.Image)
+	if err != nil {
+		return nil, err
+	}
+
+	if ref.ID == "" {
+		return nil, nil // ID is needed for later lookup
+	}
+
+	dcname := pod.Annotations[deployapi.DeploymentConfigAnnotation]
+	if dcname == "" {
+		return nil, nil // Pod doesn't appear to have been created by a DeploymentConfig
+	}
+
+	dc, err := o.Client.DeploymentConfigs(o.Attach.Pod.Namespace).Get(dcname)
+	if err != nil {
+		return nil, err
+	}
+
+	for _, trigger := range dc.Spec.Triggers {
+		if trigger.Type == deployapi.DeploymentTriggerOnImageChange &&
+			trigger.ImageChangeParams != nil &&
+			trigger.ImageChangeParams.From.Kind == "ImageStreamTag" {
+
+			isname, _, err := imageapi.ParseImageStreamTagName(trigger.ImageChangeParams.From.Name)
+			if err != nil {
+				return nil, err
+			}
+
+			namespace := trigger.ImageChangeParams.From.Namespace
+			if len(namespace) == 0 {
+				namespace = o.Attach.Pod.Namespace
+			}
+
+			isi, err := o.Client.ImageStreamImages(namespace).Get(isname, ref.ID)
+			if err != nil {
+				return nil, err
+			}
+
+			return &isi.Image, nil
+		}
+	}
+
+	return nil, nil // DeploymentConfig doesn't have an ImageChange Trigger
+}
+
+// getContainerImageViaImageStreamImport attempts to return an Image for a given
+// Container.  It does this by submiting a ImageStreamImport request with Import
+// set to false.  The request will not succeed if the backing repository
+// requires Insecure to be set to true, which cannot be hard-coded for security
+// reasons.
+func (o *DebugOptions) getContainerImageViaImageStreamImport(container *kapi.Container) (*imageapi.Image, error) {
+	isi := &imageapi.ImageStreamImport{
+		ObjectMeta: kapi.ObjectMeta{
+			Name: "oc-debug",
+		},
+		Spec: imageapi.ImageStreamImportSpec{
+			Images: []imageapi.ImageImportSpec{
+				{
+					From: kapi.ObjectReference{
+						Kind: "DockerImage",
+						Name: container.Image,
+					},
+				},
+			},
+		},
 	}
-	isimage, err := o.Client.ImageStreamImages(o.Attach.Pod.Namespace).Get(name, id)
+
+	isi, err := o.Client.ImageStreams(o.Attach.Pod.Namespace).Import(isi)
 	if err != nil {
 		return nil, err
 	}
 
-	config := isimage.Image.DockerImageMetadata.Config
+	if len(isi.Status.Images) > 0 {
+		return isi.Status.Images[0].Image, nil
+	}
+
+	return nil, nil
+}
+
+func (o *DebugOptions) getContainerImageCommand(pod *kapi.Pod, container *kapi.Container) ([]string, error) {
+	if len(container.Command) > 0 {
+		return container.Command, nil
+	}
+
+	image, _ := o.getContainerImageViaDeploymentConfig(pod, container)
+	if image == nil {
+		image, _ = o.getContainerImageViaImageStreamImport(container)
+	}
+
+	if image == nil || image.DockerImageMetadata.Config == nil {
+		return nil, errors.New("error: no usable image found")
+	}
+
+	config := image.DockerImageMetadata.Config
 	return append(config.Entrypoint, config.Cmd...), nil
 }
 
@@ -421,13 +511,13 @@ func (o *DebugOptions) transformPodForDebug(annotations map[string]string) (*kap
 	container := containerForName(pod, o.Attach.ContainerName)
 
 	// identify the command to be run
-	originalCommand := append(container.Command, container.Args...)
-	container.Command = o.Command
-	if len(originalCommand) == 0 {
-		originalCommand, _ = o.getContainerImageCommand(container)
+	originalCommand, _ := o.getContainerImageCommand(pod, container)
+	if len(originalCommand) > 0 {
+		originalCommand = append(originalCommand, container.Args...)
 	}
-	container.Args = nil
 
+	container.Command = o.Command
+	container.Args = nil
 	container.TTY = o.Attach.Stdin && o.Attach.TTY
 	container.Stdin = o.Attach.Stdin
 	container.StdinOnce = o.Attach.Stdin

test/extended/builds/new_app.go

@@ -10,8 +10,8 @@ import (
 var _ = g.Describe("[builds][Conformance] oc new-app", func() {
 	// Previously, the maximum length of app names creatable by new-app has
 	// inadvertently been decreased, e.g. by creating an annotation somewhere
-	// whose name itself includes the app name.  Ensure we can create and deploy
-	// an app with a 58 character name [63 maximum - len('-9999' suffix)].
+	// whose name itself includes the app name.  Ensure we can create and fully
+	// deploy an app with a 58 character name [63 maximum - len('-9999' suffix)].
 
 	oc := exutil.NewCLI("new-app", exutil.KubeConfigPath())
 

test/extended/cli/debug.go

@@ -7,7 +7,7 @@ import (
 	exutil "github.com/openshift/origin/test/extended/util"
 )
 
-var _ = g.Describe("[cli] oc debug", func() {
+var _ = g.Describe("[cli][Slow] oc debug", func() {
 	oc := exutil.NewCLI("oc-debug", exutil.KubeConfigPath())
 	templatePath := exutil.FixturePath("testdata", "test-cli-debug.yaml")
 
@@ -20,17 +20,29 @@ var _ = g.Describe("[cli] oc debug", func() {
 		err = oc.Run("create").Args("-f", templatePath).Execute()
 		o.Expect(err).NotTo(o.HaveOccurred())
 
-		exutil.WaitForAnImageStreamTag(oc, oc.Namespace(), "busybox", "latest")
+		exutil.WaitForAnImageStreamTag(oc, oc.Namespace(), "local-busybox", "latest")
 		o.Expect(err).NotTo(o.HaveOccurred())
 	})
 
-	g.It("should print the container entrypoint/command", func() {
+	g.It("should print the imagestream-based container entrypoint/command", func() {
+		out, err := oc.Run("debug").Args("dc/local-busybox1").Output()
+		o.Expect(err).NotTo(o.HaveOccurred())
+		o.Expect(out).To(o.ContainSubstring("Debugging with pod/local-busybox1-debug, original command: sh\n"))
+	})
+
+	g.It("should print the overridden imagestream-based container entrypoint/command", func() {
+		out, err := oc.Run("debug").Args("dc/local-busybox2").Output()
+		o.Expect(err).NotTo(o.HaveOccurred())
+		o.Expect(out).To(o.ContainSubstring("Debugging with pod/local-busybox2-debug, original command: foo bar baz qux\n"))
+	})
+
+	g.It("should print the docker image-based container entrypoint/command", func() {
 		out, err := oc.Run("debug").Args("dc/busybox1").Output()
 		o.Expect(err).NotTo(o.HaveOccurred())
 		o.Expect(out).To(o.ContainSubstring("Debugging with pod/busybox1-debug, original command: sh\n"))
 	})
 
-	g.It("should print the overridden container entrypoint/command", func() {
+	g.It("should print the overridden docker image-based container entrypoint/command", func() {
 		out, err := oc.Run("debug").Args("dc/busybox2").Output()
 		o.Expect(err).NotTo(o.HaveOccurred())
 		o.Expect(out).To(o.ContainSubstring("Debugging with pod/busybox2-debug, original command: foo bar baz qux\n"))

test/extended/testdata/test-cli-debug.yaml

@@ -4,54 +4,65 @@ items:
 - kind: ImageStream
   apiVersion: v1
   metadata:
-    name: busybox
+    name: local-busybox
+
+- kind: BuildConfig
+  apiVersion: v1
+  metadata:
+    name: local-busybox
   spec:
-    tags:
-    - name: latest
-      from:
-        name: busybox
-        kind: DockerImage
+    strategy:
+      type: Docker
+    source:
+      type: Git
+      dockerfile: "FROM busybox:latest\n"
+    output:
+      to:
+        kind: ImageStreamTag
+        name: local-busybox:latest
+    triggers:
+    - type: ConfigChange
 
 - kind: DeploymentConfig
   apiVersion: v1
   metadata:
-    name: busybox1
+    name: local-busybox1
   spec:
     replicas: 0
     selector:
-      deploymentconfig: busybox1
+      deploymentconfig: local-busybox1
     template:
       metadata:
         labels:
-          deploymentconfig: busybox1
+          deploymentconfig: local-busybox1
       spec:
         containers:
-        - name: busybox
+        - name: local-busybox
     triggers:
     - type: ImageChange
       imageChangeParams:
         automatic: true
         containerNames:
-        - busybox
+        - local-busybox
         from:
-          name: busybox:latest
           kind: ImageStreamTag
+          name: local-busybox:latest
 
 - kind: DeploymentConfig
   apiVersion: v1
   metadata:
-    name: busybox2
+    name: local-busybox2
   spec:
     replicas: 0
     selector:
-      deploymentconfig: busybox2
+      deploymentconfig: local-busybox2
     template:
       metadata:
         labels:
-          deploymentconfig: busybox2
+          deploymentconfig: local-busybox2
       spec:
         containers:
-        - name: busybox
+        - name: local-busybox
           command:
           - foo
           - bar
@@ -63,7 +74,47 @@ items:
       imageChangeParams:
         automatic: true
         containerNames:
-        - busybox
+        - local-busybox
         from:
-          name: busybox:latest
           kind: ImageStreamTag
+          name: local-busybox:latest
+
+- kind: DeploymentConfig
+  apiVersion: v1
+  metadata:
+    name: busybox1
+  spec:
+    replicas: 0
+    selector:
+      deploymentconfig: busybox1
+    template:
+      metadata:
+        labels:
+          deploymentconfig: busybox1
+      spec:
+        containers:
+        - name: busybox
+          image: busybox
+
+- kind: DeploymentConfig
+  apiVersion: v1
+  metadata:
+    name: busybox2
+  spec:
+    replicas: 0
+    selector:
+      deploymentconfig: busybox2
+    template:
+      metadata:
+        labels:
+          deploymentconfig: busybox2
+      spec:
+        containers:
+        - name: busybox
+          image: busybox
+          command:
+          - foo
+          - bar
+          args:
+          - baz
+          - qux