From d9e63e015382cfb238b0f12d99f3daf696748b5e Mon Sep 17 00:00:00 2001 From: Jim Minter Date: Tue, 10 Jan 2017 10:58:28 +0000 Subject: [PATCH] use ImageStreamImport for container command lookup in oc debug --- pkg/cmd/cli/cmd/debug.go | 114 ++++++++++++++++++--- test/extended/builds/new_app.go | 4 +- test/extended/cli/debug.go | 20 +++- test/extended/testdata/test-cli-debug.yaml | 87 ++++++++++++---- 4 files changed, 189 insertions(+), 36 deletions(-) diff --git a/pkg/cmd/cli/cmd/debug.go b/pkg/cmd/cli/cmd/debug.go index 590c777aeabd..229e293aa00f 100644 --- a/pkg/cmd/cli/cmd/debug.go +++ b/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 diff --git a/test/extended/builds/new_app.go b/test/extended/builds/new_app.go index 82551dcdda3a..32131c15a6ed 100644 --- a/test/extended/builds/new_app.go +++ b/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()) diff --git a/test/extended/cli/debug.go b/test/extended/cli/debug.go index d2450ed5e856..9384209ddf5d 100644 --- a/test/extended/cli/debug.go +++ b/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")) diff --git a/test/extended/testdata/test-cli-debug.yaml b/test/extended/testdata/test-cli-debug.yaml index 993d185e1d1e..86a5b35f10ec 100644 --- a/test/extended/testdata/test-cli-debug.yaml +++ b/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