Add support for composite run/debug commands

pkg/component/delete/delete.go

@@ -181,7 +181,7 @@ func (do *DeleteComponentClient) ExecutePreStopEvents(devfileObj parser.DevfileO
 
 	klog.V(4).Infof("Executing %q event commands for component %q", libdevfile.PreStop, componentName)
 	// ignore the failures if any; delete should not fail because preStop events failed to execute
-	err = libdevfile.ExecPreStopEvents(devfileObj, component.NewExecHandler(do.kubeClient, pod.Name, "", false))
+	err = libdevfile.ExecPreStopEvents(devfileObj, component.NewExecHandler(do.kubeClient, appName, componentName, pod.Name, "", false))
 	if err != nil {
 		klog.V(4).Infof("Failed to execute %q event commands for component %q, cause: %v", libdevfile.PreStop, componentName, err.Error())
 	}

pkg/component/delete/delete_test.go

@@ -560,7 +560,7 @@ func TestDeleteComponentClient_ExecutePreStopEvents(t *testing.T) {
 					selector := odolabels.GetSelector(componentName, "app", odolabels.ComponentDevMode)
 					client.EXPECT().GetOnePodFromSelector(selector).Return(odoTestingUtil.CreateFakePod(componentName, "runtime"), nil)
 
-					cmd := []string{"/bin/sh", "-c", "cd /projects/nodejs-starter && echo \"Hello World!\""}
+					cmd := []string{"/bin/sh", "-c", "cd /projects/nodejs-starter && (echo \"Hello World!\") 1>>/proc/1/fd/1 2>>/proc/1/fd/2"}
 					client.EXPECT().ExecCMDInContainer("runtime", "runtime", cmd, gomock.Any(), gomock.Any(), nil, false).Return(nil)
 
 					return client
@@ -598,9 +598,13 @@ func TestDeleteComponentClient_ExecutePreStopEvents(t *testing.T) {
 					client := kclient.NewMockClientInterface(ctrl)
 
 					selector := odolabels.GetSelector(componentName, "app", odolabels.ComponentDevMode)
-					client.EXPECT().GetOnePodFromSelector(selector).Return(odoTestingUtil.CreateFakePod(componentName, "runtime"), nil)
+					fakePod := odoTestingUtil.CreateFakePod(componentName, "runtime")
+					//Expecting this method to be called twice because if the command execution fails, we try to get the pod logs by calling GetOnePodFromSelector again.
+					client.EXPECT().GetOnePodFromSelector(selector).Return(fakePod, nil).Times(2)
+
+					client.EXPECT().GetPodLogs(fakePod.Name, gomock.Any(), gomock.Any()).Return(nil, errors.New("an error"))
 
-					cmd := []string{"/bin/sh", "-c", "cd /projects/nodejs-starter && echo \"Hello World!\""}
+					cmd := []string{"/bin/sh", "-c", "cd /projects/nodejs-starter && (echo \"Hello World!\") 1>>/proc/1/fd/1 2>>/proc/1/fd/2"}
 					client.EXPECT().ExecCMDInContainer("runtime", "runtime", cmd, gomock.Any(), gomock.Any(), nil, false).Return(errors.New("some error"))
 
 					return client

pkg/component/exec_handler.go

@@ -14,20 +14,24 @@ import (
 )
 
 type execHandler struct {
-	kubeClient kclient.ClientInterface
-	podName    string
-	msg        string
-	show       bool
+	kubeClient    kclient.ClientInterface
+	appName       string
+	componentName string
+	podName       string
+	msg           string
+	show          bool
 }
 
 const ShellExecutable string = "/bin/sh"
 
-func NewExecHandler(kubeClient kclient.ClientInterface, podName string, msg string, show bool) *execHandler {
+func NewExecHandler(kubeClient kclient.ClientInterface, appName, cmpName, podName, msg string, show bool) *execHandler {
 	return &execHandler{
-		kubeClient: kubeClient,
-		podName:    podName,
-		msg:        msg,
-		show:       show,
+		kubeClient:    kubeClient,
+		appName:       appName,
+		componentName: cmpName,
+		podName:       podName,
+		msg:           msg,
+		show:          show,
 	}
 }
 
@@ -43,6 +47,8 @@ func (o *execHandler) Execute(command v1alpha2.Command) error {
 	msg := o.msg
 	if msg == "" {
 		msg = fmt.Sprintf("Executing %s command %q on container %q", command.Id, command.Exec.CommandLine, command.Exec.Component)
+	} else {
+		msg += " (command: " + command.Id + ")"
 	}
 	spinner := log.Spinner(msg)
 	defer spinner.End(false)
@@ -56,6 +62,19 @@ func (o *execHandler) Execute(command v1alpha2.Command) error {
 	closeWriterAndWaitForAck(stdoutWriter, stdoutChannel, stderrWriter, stderrChannel)
 
 	spinner.End(err == nil)
+	if err != nil {
+		rd, errLog := Log(o.kubeClient, o.componentName, o.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(), o.componentName, -1)
+		if errLog != nil {
+			return fmt.Errorf("unable to log error %v: %w", err, errLog)
+		}
+	}
 	return err
 }
 
@@ -71,12 +90,16 @@ func getCmdline(command v1alpha2.Command) []string {
 	}
 
 	// Change to the workdir and execute the command
+	// 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"
 	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"
-		cmd = []string{ShellExecutable, "-c", "cd " + command.Exec.WorkingDir + " && " + cmdLine}
+		cmd = []string{ShellExecutable, "-c", "cd " + command.Exec.WorkingDir + " && (" + cmdLine + ") " + redirectString}
 	} else {
-		cmd = []string{ShellExecutable, "-c", cmdLine}
+		cmd = []string{ShellExecutable, "-c", "(" + cmdLine + ") " + redirectString}
 	}
 	return cmd
 }

pkg/devfile/adapters/kubernetes/component/adapter.go

@@ -299,7 +299,8 @@ func (a Adapter) Push(parameters common.PushParameters) (err error) {
 	// PostStart events from the devfile will only be executed when the component
 	// didn't previously exist
 	if !componentExists && libdevfile.HasPostStartEvents(a.Devfile) {
-		err = libdevfile.ExecPostStartEvents(a.Devfile, component.NewExecHandler(a.kubeClient, a.pod.Name, "", parameters.Show))
+		err = libdevfile.ExecPostStartEvents(a.Devfile,
+			component.NewExecHandler(a.kubeClient, a.AppName, a.ComponentName, a.pod.Name, "", parameters.Show))
 		if err != nil {
 			return err
 		}
@@ -348,7 +349,8 @@ func (a Adapter) Push(parameters common.PushParameters) (err error) {
 		// Invoke the build command once (before calling libdevfile.ExecuteCommandByKind), as, if cmd is a composite command,
 		// the handler we pass will be called for each command in that composite command.
 		doExecuteBuildCommand := func() error {
-			execHandler := component.NewExecHandler(a.kubeClient, a.pod.Name, "Building your application in container on cluster", parameters.Show)
+			execHandler := component.NewExecHandler(a.kubeClient, a.AppName, a.ComponentName, a.pod.Name,
+				"Building your application in container on cluster", parameters.Show)
 			return libdevfile.Build(a.Devfile, execHandler, true)
 		}
 		if componentExists {
@@ -414,7 +416,7 @@ func (a *Adapter) createOrUpdateComponent(componentExists bool, ei envinfo.EnvSp
 		return err
 	}
 
-	containers, err = utils.UpdateContainersEntrypointsIfNeeded(a.Devfile, containers, a.devfileRunCmd, a.devfileDebugCmd)
+	containers, err = utils.UpdateContainersEntrypointsIfNeeded(a.Devfile, containers, a.devfileBuildCmd, a.devfileRunCmd, a.devfileDebugCmd)
 	if err != nil {
 		return err
 	}

pkg/devfile/adapters/kubernetes/component/commandhandler.go

@@ -44,7 +44,7 @@ func (a *adapterHandler) Execute(devfileCmd devfilev1.Command) error {
 		switch status {
 		case remotecmd.Starting:
 			_ = log.SpinnerNoSpin(fmt.Sprintf("Executing the application (command: %s)", devfileCmd.Id))
-		case remotecmd.Stopped:
+		case remotecmd.Stopped, remotecmd.Errored:
 			if err != nil {
 				klog.V(2).Infof("error while running background command: %v", err)
 			}

pkg/devfile/adapters/kubernetes/utils/utils.go

@@ -3,6 +3,7 @@ package utils
 import (
 	"strconv"
 
+	"github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
 	devfileParser "github.com/devfile/library/pkg/devfile/parser"
 
 	corev1 "k8s.io/api/core/v1"
@@ -94,28 +95,32 @@ func AddOdoMandatoryVolume(containers *[]corev1.Container) {
 func UpdateContainersEntrypointsIfNeeded(
 	devfileObj devfileParser.DevfileObj,
 	containers []corev1.Container,
+	devfileBuildCmd string,
 	devfileRunCmd string,
 	devfileDebugCmd string,
 ) ([]corev1.Container, error) {
-	runCommand, err := libdevfile.GetRunCommand(devfileObj.Data, devfileRunCmd)
-	if err != nil {
-		return nil, err
-	}
-	debugCommand, err := libdevfile.GetDebugCommand(devfileObj.Data, devfileDebugCmd)
+	buildCmd, err := libdevfile.GetBuildCommand(devfileObj.Data, devfileBuildCmd)
 	if err != nil {
 		return nil, err
 	}
-
-	runContainers, err := libdevfile.GetContainerComponentsForCommand(devfileObj, runCommand)
+	runCommand, err := libdevfile.GetRunCommand(devfileObj.Data, devfileRunCmd)
 	if err != nil {
 		return nil, err
 	}
-	debugContainers, err := libdevfile.GetContainerComponentsForCommand(devfileObj, debugCommand)
+	debugCommand, err := libdevfile.GetDebugCommand(devfileObj.Data, devfileDebugCmd)
 	if err != nil {
 		return nil, err
 	}
 
-	components := append(runContainers, debugContainers...)
+	var components []string
+	var containerComps []string
+	for _, cmd := range []v1alpha2.Command{buildCmd, runCommand, debugCommand} {
+		containerComps, err = libdevfile.GetContainerComponentsForCommand(devfileObj, cmd)
+		if err != nil {
+			return nil, err
+		}
+		components = append(components, containerComps...)
+	}
 
 	for i := range containers {
 		container := &containers[i]

pkg/devfile/adapters/kubernetes/utils/utils_test.go

@@ -883,6 +883,11 @@ func TestUpdateContainerEnvVars(t *testing.T) {
 }
 
 func TestUpdateContainersEntrypointsIfNeeded(t *testing.T) {
+	const (
+		buildCommand            = "my-build"
+		buildCmdLine            = "echo my-build-command-line"
+		buildContainerComponent = "build-container-component"
+	)
 	const (
 		runCommand            = "my-run"
 		runCmdLine            = "echo my-run-command-line"
@@ -894,6 +899,10 @@ func TestUpdateContainersEntrypointsIfNeeded(t *testing.T) {
 		debugContainerComponent = "debug-container-component"
 	)
 
+	execBuildGroup := devfilev1.CommandGroup{
+		IsDefault: util.GetBoolPtr(true),
+		Kind:      devfilev1.BuildCommandGroupKind,
+	}
 	execRunGroup := devfilev1.CommandGroup{
 		IsDefault: util.GetBoolPtr(true),
 		Kind:      devfilev1.RunCommandGroupKind,
@@ -907,10 +916,13 @@ func TestUpdateContainersEntrypointsIfNeeded(t *testing.T) {
 		name                  string
 		commands              []devfilev1.Command
 		components            []devfilev1.Component
+		buildCommand          string
 		runCommand            string
 		debugCommand          string
+		buildContainerCommand []string
 		runContainerCommand   []string
 		debugContainerCommand []string
+		buildContainerArgs    []string
 		runContainerArgs      []string
 		debugContainerArgs    []string
 		wantErr               bool
@@ -930,50 +942,88 @@ func TestUpdateContainersEntrypointsIfNeeded(t *testing.T) {
 			debugCommand: "a-non-existing-debug-name",
 			wantErr:      true,
 		},
+		{
+			name:         "missing build command specified by name",
+			buildCommand: buildCommand + "-not-found",
+			runCommand:   runCommand,
+			debugCommand: debugCommand,
+			wantErr:      true,
+		},
 		{
 			name:         "containers without any command or args => must be overridden with 'tail -f /dev/null'",
+			buildCommand: buildCommand,
 			runCommand:   runCommand,
 			debugCommand: debugCommand,
 			wantErr:      false,
 			expectedContainerCommand: map[string][]string{
+				buildContainerComponent: {"tail"},
 				runContainerComponent:   {"tail"},
 				debugContainerComponent: {"tail"},
 			},
 			expectedContainerArgs: map[string][]string{
+				buildContainerComponent: {"-f", "/dev/null"},
 				runContainerComponent:   {"-f", "/dev/null"},
 				debugContainerComponent: {"-f", "/dev/null"},
 			},
 		},
 		{
-			name:                "containers with one without any command or args => must be overridden with 'tail -f /dev/null'",
+			name:                  "containers with one without any command or args => must be overridden with 'tail -f /dev/null'",
+			buildCommand:          buildCommand,
+			runCommand:            runCommand,
+			debugCommand:          debugCommand,
+			wantErr:               false,
+			buildContainerCommand: []string{"npm"},
+			buildContainerArgs:    []string{"install"},
+			runContainerCommand:   []string{"printenv"},
+			runContainerArgs:      []string{"HOSTNAME"},
+			expectedContainerCommand: map[string][]string{
+				buildContainerComponent: {"npm"},
+				runContainerComponent:   {"printenv"},
+				debugContainerComponent: {"tail"},
+			},
+			expectedContainerArgs: map[string][]string{
+				buildContainerComponent: {"install"},
+				runContainerComponent:   {"HOSTNAME"},
+				debugContainerComponent: {"-f", "/dev/null"},
+			},
+		},
+		{
+			name:                "default build command, containers with one without any command or args => must be overridden with 'tail -f /dev/null'",
 			runCommand:          runCommand,
 			debugCommand:        debugCommand,
 			wantErr:             false,
 			runContainerCommand: []string{"printenv"},
 			runContainerArgs:    []string{"HOSTNAME"},
 			expectedContainerCommand: map[string][]string{
+				buildContainerComponent: {"tail"},
 				runContainerComponent:   {"printenv"},
 				debugContainerComponent: {"tail"},
 			},
 			expectedContainerArgs: map[string][]string{
+				buildContainerComponent: {"-f", "/dev/null"},
 				runContainerComponent:   {"HOSTNAME"},
 				debugContainerComponent: {"-f", "/dev/null"},
 			},
 		},
 		{
 			name:                  "containers with explicit command or args",
+			buildCommand:          buildCommand,
 			runCommand:            runCommand,
 			debugCommand:          debugCommand,
 			wantErr:               false,
+			buildContainerCommand: []string{"npm"},
+			buildContainerArgs:    []string{"install"},
 			runContainerCommand:   []string{"printenv"},
 			runContainerArgs:      []string{"HOSTNAME"},
 			debugContainerCommand: []string{"tail"},
 			debugContainerArgs:    []string{"-f", "/path/to/my/custom/log/file"},
 			expectedContainerCommand: map[string][]string{
+				buildContainerComponent: {"npm"},
 				runContainerComponent:   {"printenv"},
 				debugContainerComponent: {"tail"},
 			},
 			expectedContainerArgs: map[string][]string{
+				buildContainerComponent: {"install"},
 				runContainerComponent:   {"HOSTNAME"},
 				debugContainerComponent: {"-f", "/path/to/my/custom/log/file"},
 			},
@@ -985,6 +1035,14 @@ func TestUpdateContainersEntrypointsIfNeeded(t *testing.T) {
 				t.Error(err)
 			}
 			err = devfileData.AddComponents([]devfilev1.Component{
+				{
+					Name: buildContainerComponent,
+					ComponentUnion: devfilev1.ComponentUnion{
+						Container: &devfilev1.ContainerComponent{
+							Container: devfilev1.Container{},
+						},
+					},
+				},
 				{
 					Name: runContainerComponent,
 					ComponentUnion: devfilev1.ComponentUnion{
@@ -1006,6 +1064,20 @@ func TestUpdateContainersEntrypointsIfNeeded(t *testing.T) {
 				t.Error(err)
 			}
 			err = devfileData.AddCommands([]devfilev1.Command{
+				{
+					Id: buildCommand,
+					CommandUnion: devfilev1.CommandUnion{
+						Exec: &devfilev1.ExecCommand{
+							CommandLine: buildCmdLine,
+							Component:   buildContainerComponent,
+							LabeledCommand: devfilev1.LabeledCommand{
+								BaseCommand: devfilev1.BaseCommand{
+									Group: &execBuildGroup,
+								},
+							},
+						},
+					},
+				},
 				{
 					Id: runCommand,
 					CommandUnion: devfilev1.CommandUnion{
@@ -1043,6 +1115,11 @@ func TestUpdateContainersEntrypointsIfNeeded(t *testing.T) {
 			}
 
 			containerForComponents := []corev1.Container{
+				{
+					Name:    buildContainerComponent,
+					Command: tt.buildContainerCommand,
+					Args:    tt.buildContainerArgs,
+				},
 				{
 					Name:    runContainerComponent,
 					Command: tt.runContainerCommand,
@@ -1055,9 +1132,10 @@ func TestUpdateContainersEntrypointsIfNeeded(t *testing.T) {
 				},
 			}
 
-			containers, err := UpdateContainersEntrypointsIfNeeded(devfileObj, containerForComponents, tt.runCommand, tt.debugCommand)
+			containers, err := UpdateContainersEntrypointsIfNeeded(devfileObj, containerForComponents, tt.buildCommand, tt.runCommand, tt.debugCommand)
 			if tt.wantErr != (err != nil) {
 				t.Errorf("error = %v, wantErr %v", err, tt.wantErr)
+				return
 			}
 
 			if len(containers) != len(tt.expectedContainerCommand) {