Skip to content

Commit

Permalink
Add support for composite run/debug commands (redhat-developer#5841)
Browse files Browse the repository at this point in the history 
* Change integration test expectations regarding composite run commands

* Add integration test supporting composite debug commands

* Rename in-container pid file from '.odo_devfile_cmd_<name>.pid' into '.odo_cmd_<name>.pid'

The implementation in kubeexec.go is indeed able to run any kind of commands,
not only Devfile-related.

* Pave the way to supporting composite run/debug commands

Drop validation rules that prevented from going further
with composite run or debug commands.

* Update logic for determining which containers should have their entrypoint overridden or env vars updated

The previous implementation was limited to Exec commands solely.
This now makes it easier to support other kind of commands.
For example, when handling a composite command, we are now recursively
determining the containers (a.k.a components) that would get used by this composite command.

* Store the process exit code in the pid file handled by kubeexec.go

This allows to more accurately determine how the process could be reported.

* Fix issue with util#DisplayLogs potentially not picking the last element of lines

* Make sure to execute the Build command only once when running a composite command

* Test that the Build command is executed once running a composite command

* Make sure not to retrieve the parent default command uselessly

* Log the command Id when it is about to be restarted

* Redirect build command logs to PID 1 process

This way, users would be able to display them (and follow them) with `odo logs` or `kubectl logs`

* Handle Errored case when logging information about the process that exited

* Make sure to override container entrypoint if needed for Build commands

Build commands (which could also be composite ones)
might be executed in a container different from the run or debug ones.

* Redirect output from commands handled by component.exec_handler to PID 1 process streams

This way, we can also have them displayed with `odo logs` or `kubectl logs`.
This includes Build and Devfile events commands.

* Make remotecmd#ExecuteCommand log stdout or stderr content only if there not empty

This reduces the clutter in case of an error with the command execution.

* Add integration test with more complex Devfile (composite build/run/debug commands running in different containers and with a shared volume)

* Update the 'Executing the application' spinner accordingly when the related command is done
  • Loading branch information
@rm3l @cdrage
rm3l authored and cdrage committed Aug 31, 2022
1 parent 948819c commit 4e2bc73
Show file tree
Hide file tree
Showing 22 changed files with 1,247 additions and 288 deletions.
2 changes: 1 addition & 1 deletion pkg/component/delete/delete.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down
10 changes: 7 additions & 3 deletions pkg/component/delete/delete_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
45 changes: 34 additions & 11 deletions pkg/component/exec_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand All @@ -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)
Expand All @@ -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
}

Expand All @@ -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
}
Expand Down
44 changes: 39 additions & 5 deletions pkg/devfile/adapters/kubernetes/component/adapter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"k8s.io/utils/pointer"

"github.com/devfile/library/pkg/devfile/generator"
devfileCommon "github.com/devfile/library/pkg/devfile/parser/data/v2/common"

"github.com/redhat-developer/odo/pkg/component"
"github.com/redhat-developer/odo/pkg/devfile"
Expand Down Expand Up @@ -297,7 +298,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
}
Expand All @@ -315,20 +317,52 @@ func (a Adapter) Push(parameters common.PushParameters) (err error) {

cmdHandler := adapterHandler{
Adapter: a,
cmdKind: cmdKind,
parameters: parameters,
componentExists: componentExists,
}

running, err := cmdHandler.isRemoteProcessForCommandRunning(cmd)
commandType, err := devfileCommon.GetCommandType(cmd)
if err != nil {
return err
}
var running bool
var isComposite bool
if commandType == devfilev1.ExecCommandType {
running, err = cmdHandler.isRemoteProcessForCommandRunning(cmd)
if err != nil {
return err
}
} else if commandType == devfilev1.CompositeCommandType {
//this handler will run each command in this composite command individually,
//and will determine whether each command is running or not.
isComposite = true
} else {
return fmt.Errorf("unsupported type %q for Devfile command %s, only exec and composite are handled",
commandType, cmd.Id)
}

klog.V(4).Infof("running=%v, execRequired=%v, parameters.RunModeChanged=%v",
running, execRequired, parameters.RunModeChanged)

if !running || execRequired || parameters.RunModeChanged {
if isComposite || !running || execRequired || parameters.RunModeChanged {
// 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.AppName, a.ComponentName, a.pod.Name,
"Building your application in container on cluster", parameters.Show)
return libdevfile.Build(a.Devfile, execHandler, true)
}
if componentExists {
if parameters.RunModeChanged || cmd.Exec == nil || !util.SafeGetBool(cmd.Exec.HotReloadCapable) {
if err = doExecuteBuildCommand(); err != nil {
return err
}
}
} else {
if err = doExecuteBuildCommand(); err != nil {
return err
}
}
err = libdevfile.ExecuteCommandByKind(a.Devfile, cmdKind, &cmdHandler, false)
}

Expand Down Expand Up @@ -381,7 +415,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
}
Expand Down
72 changes: 32 additions & 40 deletions pkg/devfile/adapters/kubernetes/component/commandhandler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const numberOfLinesToOutputLog = 100

type adapterHandler struct {
Adapter
cmdKind devfilev1.CommandGroupKind
parameters common.PushParameters
componentExists bool
}
Expand All @@ -39,71 +38,56 @@ func (a *adapterHandler) ApplyKubernetes(_ devfilev1.Component) error {
}

func (a *adapterHandler) Execute(devfileCmd devfilev1.Command) error {
processName := "devrun"
if a.parameters.Debug {
processName = "debugrun"
}

doExecuteBuildCommand := func() error {
execHandler := component.NewExecHandler(a.kubeClient, a.pod.Name, "Building your application in container on cluster", a.parameters.Show)
return libdevfile.Build(a.Devfile, execHandler, true)
}

remoteProcessHandler := remotecmd.NewKubeExecProcessHandler()

startHandler := func(status remotecmd.RemoteProcessStatus, stdout []string, stderr []string, err error) {
switch status {
case remotecmd.Starting:
_ = log.SpinnerNoSpin("Executing the application")
case remotecmd.Stopped:
if err != nil {
klog.V(2).Infof("error while running background command: %v", err)
statusHandlerFunc := func(s *log.Status) remotecmd.CommandOutputHandler {
return func(status remotecmd.RemoteProcessStatus, stdout []string, stderr []string, err error) {
switch status {
case remotecmd.Starting:
// Creating with no spin because the command could be long-running, and we cannot determine when it will end.
s.Start(fmt.Sprintf("Executing the application (command: %s)", devfileCmd.Id), true)
case remotecmd.Stopped, remotecmd.Errored:
s.End(status == remotecmd.Stopped)
if err != nil {
klog.V(2).Infof("error while running background command: %v", err)
}
}
}
}

// Spinner created but not started yet.
// It will be displayed when the statusHandlerFunc function is called with the "Starting" state.
spinner := log.NewStatus(log.GetStdout())

// if we need to restart, issue the remote process handler command to stop all running commands first.
// We do not need to restart Hot reload capable commands.
if a.componentExists {
cmd, err := libdevfile.GetDefaultCommand(a.Devfile, a.cmdKind)
if err != nil {
return err
}

if a.parameters.RunModeChanged || cmd.Exec == nil || !util.SafeGetBool(cmd.Exec.HotReloadCapable) {
klog.V(2).Info("restart required for command")
if a.parameters.RunModeChanged || devfileCmd.Exec == nil || !util.SafeGetBool(devfileCmd.Exec.HotReloadCapable) {
klog.V(2).Infof("restart required for command %s", devfileCmd.Id)

cmdDef, err := devfileCommandToRemoteCmdDefinition(devfileCmd)
if err != nil {
return err
}

if err = doExecuteBuildCommand(); err != nil {
return err
}

err = remoteProcessHandler.StopProcessForCommand(cmdDef, a.kubeClient, a.pod.Name, devfileCmd.Exec.Component)
if err != nil {
return err
}

if err = remoteProcessHandler.StartProcessForCommand(cmdDef, a.kubeClient, a.pod.Name, devfileCmd.Exec.Component, startHandler); err != nil {
if err = remoteProcessHandler.StartProcessForCommand(cmdDef, a.kubeClient, a.pod.Name, devfileCmd.Exec.Component, statusHandlerFunc(spinner)); err != nil {
return err
}
} else {
klog.V(2).Infof("command is hot-reload capable, not restarting %s", processName)
klog.V(2).Infof("command is hot-reload capable, not restarting %s", devfileCmd.Id)
}
} else {
cmdDef, err := devfileCommandToRemoteCmdDefinition(devfileCmd)
if err != nil {
return err
}

if err := doExecuteBuildCommand(); err != nil {
return err
}

if err := remoteProcessHandler.StartProcessForCommand(cmdDef, a.kubeClient, a.pod.Name, devfileCmd.Exec.Component, startHandler); err != nil {
if err := remoteProcessHandler.StartProcessForCommand(cmdDef, a.kubeClient, a.pod.Name, devfileCmd.Exec.Component, statusHandlerFunc(spinner)); err != nil {
return err
}
}
Expand All @@ -120,8 +104,15 @@ func (a *adapterHandler) Execute(devfileCmd devfilev1.Command) error {

_, err := task.NewRetryable(fmt.Sprintf("process for command %q", devfileCmd.Id), func() (bool, interface{}, error) {
klog.V(4).Infof("checking if process for command %q is running", devfileCmd.Id)
isRunning, err := a.isRemoteProcessForCommandRunning(devfileCmd)
return err == nil && isRunning, isRunning, err
remoteProcess, err := remoteProcessHandler.GetProcessInfoForCommand(
remotecmd.CommandDefinition{Id: devfileCmd.Id}, a.kubeClient, a.pod.Name, devfileCmd.Exec.Component)
if err != nil {
return false, nil, err
}
isRunningOrDone := remoteProcess.Status == remotecmd.Running ||
remoteProcess.Status == remotecmd.Stopped ||
remoteProcess.Status == remotecmd.Errored
return isRunningOrDone, nil, err
}).RetryWithSchedule(retrySchedule, false)
if err != nil {
return err
Expand Down Expand Up @@ -165,12 +156,13 @@ func (a *adapterHandler) isRemoteProcessForCommandRunning(command devfilev1.Comm
// checkRemoteCommandStatus checks if the command is running .
// if the command is not in a running state, we fetch the last 20 lines of the component's log and display it
func (a *adapterHandler) checkRemoteCommandStatus(command devfilev1.Command, notRunningMessage string) error {
running, err := a.isRemoteProcessForCommandRunning(command)
remoteProcessHandler := remotecmd.NewKubeExecProcessHandler()
remoteProcess, err := remoteProcessHandler.GetProcessInfoForCommand(remotecmd.CommandDefinition{Id: command.Id}, a.kubeClient, a.pod.Name, command.Exec.Component)
if err != nil {
return err
}

if !running {
if remoteProcess.Status != remotecmd.Running && remoteProcess.Status != remotecmd.Stopped {
log.Warningf(notRunningMessage)
log.Warningf("Last %d lines of log:", numberOfLinesToOutputLog)

Expand Down
Loading

0 comments on commit 4e2bc73

Please sign in to comment.