Display outputs when executing odo run (#6865)

* Change NewRunHandler params with Options * Pass an options to RunHandler to show logs * Hide spinner and std output since outputs are displayed * Integration tests with failing command * Fix outputs * use raw terminal and local standard i/o streams * Fix podman i/o * Fix stdout/err * Test if in/out are terminal * command reference doc
redhat-developer · Jun 12, 2023 · 6bc3011 · 6bc3011
1 parent d892e8e
commit 6bc3011
Show file tree

Hide file tree

Showing 12 changed files with 248 additions and 155 deletions.
diff --git a/docs/website/docs/command-reference/run.md b/docs/website/docs/command-reference/run.md
@@ -0,0 +1,41 @@
+---
+title: odo run
+---
+
+`odo run` is used to manually execute commands defined in a Devfile.
+
+<details>
+<summary>Example</summary>
+
+A command `connect` is defined in the Devfile, executing the `bash` command in the `runtime` component.
+
+```yaml
+schemaVersion: 2.2.0
+[...]
+commands:
+  - id: connect
+    exec:
+      component: runtime
+      commandLine: bash
+  [...]
+
+```
+
+```shell
+$ odo run connect
+bash-4.4$ 
+```
+
+</details>
+
+
+For `Exec` commands, `odo dev` needs to be running, and `odo run` 
+will execute commands in the containers deployed by the `odo dev` command. 
+
+Standard input is redirected to the command running in the container, and the terminal is configured in Raw mode. For these reasons, any character will be redirected to the command in container, including the Ctrl-c character which can thus be used to interrupt the command in container.
+
+The `odo run` command terminates when the command in container terminates, and the exit status of `odo run` will reflect the exit status of the distant command: it will be `0` if the command in container terminates with status `0` and will be `1` if the command in container terminates with any other status.
+
+Resources deployed with `Apply` commands will be deployed in *Dev mode*, 
+and these resources will be deleted when `odo dev` terminates.
+
diff --git a/pkg/component/delete/delete.go b/pkg/component/delete/delete.go
@@ -219,13 +219,14 @@ func (do *DeleteComponentClient) ExecutePreStopEvents(ctx context.Context, devfi
 		do.kubeClient,
 		do.execClient,
 		do.configAutomountClient,
-		pod.Name,
-		false,
-		component.GetContainersNames(pod),
-		"Executing pre-stop command in container",
+		// TODO(feloy) set these values when we want to support Apply Image commands for PreStop events
+		nil, nil,
 
-		// TODO(feloy) set these values when we want to support Apply Image/Kubernetes/OpenShift commands for PreStop events
-		nil, nil, parser.DevfileObj{}, "",
+		component.HandlerOptions{
+			PodName:           pod.Name,
+			ContainersRunning: component.GetContainersNames(pod),
+			Msg:               "Executing pre-stop command in container",
+		},
 	)
 	err = libdevfile.ExecPreStopEvents(ctx, devfileObj, handler)
 	if err != nil {

diff --git a/pkg/component/execute_terminating.go b/pkg/component/execute_terminating.go
@@ -18,47 +18,67 @@ import (
 
 const ShellExecutable string = "/bin/sh"
 
-func ExecuteTerminatingCommand(ctx context.Context, execClient exec.Client, platformClient platform.Client, command devfilev1.Command, componentExists bool, podName string, appName string, componentName string, msg string, show bool) error {
+func ExecuteTerminatingCommand(
+	ctx context.Context,
+	execClient exec.Client,
+	platformClient platform.Client,
+	command devfilev1.Command,
+	componentExists bool,
+	podName string,
+	appName string,
+	componentName string,
+	msg string,
+	directRun bool,
+) error {
 
 	if componentExists && command.Exec != nil && pointer.BoolDeref(command.Exec.HotReloadCapable, false) {
 		klog.V(2).Infof("command is hot-reload capable, not executing %q again", command.Id)
 		return nil
 	}
 
-	if msg == "" {
-		msg = fmt.Sprintf("Executing %s command on container %q", command.Id, command.Exec.Component)
-	} else {
-		msg += " (command: " + command.Id + ")"
-	}
-	spinner := log.Spinner(msg)
-	defer spinner.End(false)
+	// Spinner is displayed only if no outputs are displayed
+	var spinner *log.Status
+	var stdoutWriter, stderrWriter *io.PipeWriter
+	var stdoutChannel, stderrChannel chan interface{}
 
-	logger := machineoutput.NewMachineEventLoggingClient()
-	stdoutWriter, stdoutChannel, stderrWriter, stderrChannel := logger.CreateContainerOutputWriter()
+	if !directRun {
+		if msg == "" {
+			msg = fmt.Sprintf("Executing %s command on container %q", command.Id, command.Exec.Component)
+		} else {
+			msg += " (command: " + command.Id + ")"
+		}
+		spinner = log.Spinner(msg)
+		defer spinner.End(false)
 
-	cmdline := getCmdline(command)
-	_, _, err := execClient.ExecuteCommand(ctx, cmdline, podName, command.Exec.Component, show, stdoutWriter, stderrWriter)
+		logger := machineoutput.NewMachineEventLoggingClient()
+		stdoutWriter, stdoutChannel, stderrWriter, stderrChannel = logger.CreateContainerOutputWriter()
+	}
 
-	closeWriterAndWaitForAck(stdoutWriter, stdoutChannel, stderrWriter, stderrChannel)
+	cmdline := getCmdline(command, !directRun)
+	_, _, err := execClient.ExecuteCommand(ctx, cmdline, podName, command.Exec.Component, directRun, stdoutWriter, stderrWriter)
 
-	spinner.End(err == nil)
-	if err != nil {
-		rd, errLog := Log(platformClient, componentName, appName, false, command)
-		if errLog != nil {
-			return fmt.Errorf("unable to log error %v: %w", err, errLog)
-		}
+	if !directRun {
+		closeWriterAndWaitForAck(stdoutWriter, stdoutChannel, stderrWriter, stderrChannel)
+		spinner.End(err == nil)
+
+		if err != nil {
+			rd, errLog := Log(platformClient, componentName, appName, false, command)
+			if errLog != nil {
+				return fmt.Errorf("unable to log error %v: %w", err, errLog)
+			}
 
-		// Use GetStderr in order to make sure that colour output is correct
-		// on non-TTY terminals
-		errLog = util.DisplayLog(false, rd, log.GetStderr(), componentName, -1)
-		if errLog != nil {
-			return fmt.Errorf("unable to log error %v: %w", err, errLog)
+			// Use GetStderr in order to make sure that colour output is correct
+			// on non-TTY terminals
+			errLog = util.DisplayLog(false, rd, log.GetStderr(), componentName, -1)
+			if errLog != nil {
+				return fmt.Errorf("unable to log error %v: %w", err, errLog)
+			}
 		}
 	}
 	return err
 }
 
-func getCmdline(command v1alpha2.Command) []string {
+func getCmdline(command v1alpha2.Command, redirectToPid1 bool) []string {
 	// deal with environment variables
 	var cmdLine string
 	setEnvVariable := util.GetCommandStringFromEnvs(command.Exec.Env)
@@ -73,7 +93,10 @@ func getCmdline(command v1alpha2.Command) []string {
 	// Redirecting to /proc/1/fd/* allows to redirect the process output to the output streams of PID 1 process inside the container.
 	// This way, returning the container logs with 'odo logs' or 'kubectl logs' would work seamlessly.
 	// See https://stackoverflow.com/questions/58716574/where-exactly-do-the-logs-of-kubernetes-pods-come-from-at-the-container-level
-	redirectString := "1>>/proc/1/fd/1 2>>/proc/1/fd/2"
+	redirectString := ""
+	if redirectToPid1 {
+		redirectString = "1>>/proc/1/fd/1 2>>/proc/1/fd/2"
+	}
 	var cmd []string
 	if command.Exec.WorkingDir != "" {
 		// since we are using /bin/sh -c, the command needs to be within a single double quote instance, for example "cd /tmp && pwd"

diff --git a/pkg/component/handler.go b/pkg/component/handler.go
@@ -31,6 +31,7 @@ type runHandler struct {
 	ComponentExists       bool
 	containersRunning     []string
 	msg                   string
+	directRun             bool
 
 	fs           filesystem.Filesystem
 	imageBackend image.Backend
@@ -41,40 +42,47 @@ type runHandler struct {
 
 var _ libdevfile.Handler = (*runHandler)(nil)
 
+type HandlerOptions struct {
+	PodName           string
+	ComponentExists   bool
+	ContainersRunning []string
+	Msg               string
+	DirectRun         bool
+
+	// For apply Kubernetes / Openshift
+	Devfile parser.DevfileObj
+	Path    string
+}
+
 func NewRunHandler(
 	ctx context.Context,
 	platformClient platform.Client,
 	execClient exec.Client,
 	configAutomountClient configAutomount.Client,
-	podName string,
-	componentExists bool,
-	containersRunning []string,
-	msg string,
 
 	// For building images
 	fs filesystem.Filesystem,
 	imageBackend image.Backend,
 
-	// For apply Kubernetes / Openshift
-	devfile parser.DevfileObj,
-	path string,
+	options HandlerOptions,
 
 ) *runHandler {
 	return &runHandler{
 		ctx:                   ctx,
 		platformClient:        platformClient,
 		execClient:            execClient,
 		configAutomountClient: configAutomountClient,
-		podName:               podName,
-		ComponentExists:       componentExists,
-		containersRunning:     containersRunning,
-		msg:                   msg,
+		podName:               options.PodName,
+		ComponentExists:       options.ComponentExists,
+		containersRunning:     options.ContainersRunning,
+		msg:                   options.Msg,
+		directRun:             options.DirectRun,
 
 		fs:           fs,
 		imageBackend: imageBackend,
 
-		devfile: devfile,
-		path:    path,
+		devfile: options.Devfile,
+		path:    options.Path,
 	}
 }
 
@@ -129,7 +137,7 @@ func (a *runHandler) ExecuteTerminatingCommand(ctx context.Context, command devf
 		appName       = odocontext.GetApplication(a.ctx)
 	)
 	if isContainerRunning(command.Exec.Component, a.containersRunning) {
-		return ExecuteTerminatingCommand(ctx, a.execClient, a.platformClient, command, a.ComponentExists, a.podName, appName, componentName, a.msg, false)
+		return ExecuteTerminatingCommand(ctx, a.execClient, a.platformClient, command, a.ComponentExists, a.podName, appName, componentName, a.msg, a.directRun)
 	}
 	switch platform := a.platformClient.(type) {
 	case kclient.ClientInterface:

diff --git a/pkg/deploy/deploy.go b/pkg/deploy/deploy.go
@@ -44,14 +44,12 @@ func (o *DeployClient) Deploy(ctx context.Context) error {
 		o.kubeClient,
 		nil,
 		o.configAutomountClient,
-		"",
-		false,
-		nil,
-		"",
 		o.fs,
 		image.SelectBackend(ctx),
-		*devfileObj,
-		path,
+		component.HandlerOptions{
+			Devfile: *devfileObj,
+			Path:    path,
+		},
 	)
 
 	err := o.buildPushAutoImageComponents(handler, *devfileObj)

diff --git a/pkg/dev/common/run.go b/pkg/dev/common/run.go
@@ -38,15 +38,16 @@ func Run(
 		platformClient,
 		execClient,
 		configAutomountClient,
-		pod.Name,
-		false,
-		component.GetContainersNames(pod),
-		"Executing command in container",
-
 		filesystem,
 		image.SelectBackend(ctx),
-		*devfileObj,
-		devfilePath,
+		component.HandlerOptions{
+			PodName:           pod.Name,
+			ContainersRunning: component.GetContainersNames(pod),
+			Msg:               "Executing command in container",
+			DirectRun:         true,
+			Devfile:           *devfileObj,
+			Path:              devfilePath,
+		},
 	)
 
 	return libdevfile.ExecuteCommandByName(ctx, *devfileObj, commandName, handler, false)

diff --git a/pkg/dev/kubedev/innerloop.go b/pkg/dev/kubedev/innerloop.go
@@ -7,7 +7,6 @@ import (
 	"time"
 
 	devfilev1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
-	"github.com/devfile/library/v2/pkg/devfile/parser"
 	parsercommon "github.com/devfile/library/v2/pkg/devfile/parser/data/v2/common"
 
 	"github.com/redhat-developer/odo/pkg/component"
@@ -95,13 +94,13 @@ func (o *DevClient) innerloop(ctx context.Context, parameters common.PushParamet
 			o.kubernetesClient,
 			o.execClient,
 			o.configAutomountClient,
-			pod.Name,
-			false,
-			component.GetContainersNames(pod),
-			"Executing post-start command in container",
-
 			// TODO(feloy) set these values when we want to support Apply Image/Kubernetes/OpenShift commands for PostStart commands
-			nil, nil, parser.DevfileObj{}, "",
+			nil, nil,
+			component.HandlerOptions{
+				PodName:           pod.Name,
+				ContainersRunning: component.GetContainersNames(pod),
+				Msg:               "Executing post-start command in container",
+			},
 		)
 		err = libdevfile.ExecPostStartEvents(ctx, parameters.Devfile, handler)
 		if err != nil {
@@ -127,15 +126,14 @@ func (o *DevClient) innerloop(ctx context.Context, parameters common.PushParamet
 		o.kubernetesClient,
 		o.execClient,
 		o.configAutomountClient,
-		pod.GetName(),
-		false,
-		component.GetContainersNames(pod),
-		"",
-
 		o.filesystem,
 		image.SelectBackend(ctx),
-		parameters.Devfile,
-		path,
+		component.HandlerOptions{
+			PodName:           pod.GetName(),
+			ContainersRunning: component.GetContainersNames(pod),
+			Devfile:           parameters.Devfile,
+			Path:              path,
+		},
 	)
 
 	if commandType == devfilev1.ExecCommandType {
@@ -166,13 +164,14 @@ func (o *DevClient) innerloop(ctx context.Context, parameters common.PushParamet
 				o.kubernetesClient,
 				o.execClient,
 				o.configAutomountClient,
-				pod.Name,
-				running,
-				component.GetContainersNames(pod),
-				"Building your application in container",
-
 				// TODO(feloy) set these values when we want to support Apply Image/Kubernetes/OpenShift commands for PostStart commands
-				nil, nil, parser.DevfileObj{}, "",
+				nil, nil,
+				component.HandlerOptions{
+					PodName:           pod.Name,
+					ComponentExists:   running,
+					ContainersRunning: component.GetContainersNames(pod),
+					Msg:               "Building your application in container",
+				},
 			)
 			return libdevfile.Build(ctx, parameters.Devfile, parameters.StartOptions.BuildCommand, execHandler)
 		}