Skip to content

Commit

Permalink
Implement odo dev --no-commands (#6855)
Browse files Browse the repository at this point in the history 
* Document the '--no-commands' flag

Co-authored-by: Philippe Martin <phmartin@redhat.com>

* Add integration tests highlighting the expectations

* Add '--no-commands' flag to the 'dev' sub-command

* Implement '--no-command' on Podman

* Implement '--no-command' on cluster

* Validate the '--no-commands' flag in the CLI

For example, it should not be possible to call it alongside '--build-command' or '--run-command',
because we won't be able to know what to do.

* Fix 'odo dev' tests when the run/debug does not exist

For consistency with how e.g., the build command works,
instead of erroring out, `odo dev` will
now simply log the error message and wait for new input.

* Check for command existence when we want to run them

As suggested in review, this makes it more adaptable.
As a consequence, if the default (or single) build/run command does not exist,
"odo dev" will not error out immediately; instead, it will behave a bit like
"odo dev --no-commands" (by starting, but printing a warning about the
missing command).
The difference here is that if a run command is added later on
during the dev session, "odo dev" will pick it up and run it,
but "odo dev --no-commands" will continue to purposely ignore it.

The existence of the optional default Build is already checked
by the 'libdevfile.Build' command. It does not error out if there is
no default (or single) Build command. It errors out only if the specified
'--build-command' does not exist in the Devfile.

Co-authored-by: Philippe Martin <phmartin@redhat.com>

* Check for application ports only if the Devfile has run or debug command and if there are ports to forward

This prevents:
i) displaying the "Waiting for the application to be ready" spinner for nothing,
  if there are no ports to forward
ii) waiting until the timeout of 1m has expired if there was no run/debug command executed,
  in which case, it is less unlikely that the application ports defined in the Devfile
  will ever be reachable.

* Test "odo run" command against a Dev Session started with and without "--no-commands"

---------

Co-authored-by: Philippe Martin <phmartin@redhat.com>
  • Loading branch information
@rm3l @feloy
rm3l and feloy authored Jun 14, 2023
1 parent 218fd27 commit 63c8e27
Show file tree
Hide file tree
Showing 11 changed files with 593 additions and 290 deletions.
18 changes: 18 additions & 0 deletions docs/website/docs/command-reference/dev.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,24 @@ $ ODO_CONTAINER_BACKEND_GLOBAL_ARGS='--root=/tmp/podman/root;--storage-driver=ov
```
</details>
### Running with no commands
The `--no-commands` flag allows to start the Dev Session without implicitly executing any `build`, `run` or `debug` commands.
Specifying this flag will simply create all the resources necessary for the Dev Session, then set up file synchronization and port forwarding.
In detail, it will:
1. eventually build and push any `Image` Components defined in the Devfile, if they have the `autoBuild` flag set to `true`
2. eventually apply any `Kubernetes` or `OpenShift` Components defined in the Devfile, if they have the `deployByDefault` flag set to `true`
3. Start the Dev container(s), taking care of executing any commands that are bound to [Devfile lifecycle events](../user-guides/advanced/using-devfile-lifecycle-events.md), like [`postStart`](../user-guides/advanced/using-devfile-lifecycle-events.md#poststart)
4. Synchronize the local files to the Dev environment
5. Start the port forwarding logic if needed
This gives users complete control over the Dev session, for example by leveraging the [`odo run` command](./run.md) to manually run any command defined in the Devfile.
Note that this is the default behavior of `odo dev` if the Devfile does not define any Build, Run or Debug commands.
The difference is that if any of those commands is added during the Dev session, a Dev session started via `odo dev` will automatically pick them up and run them,
while a Dev session started via `odo dev --no-commands` will purposely not run them.
## Devfile (Advanced Usage)
Expand Down
3 changes: 3 additions & 0 deletions pkg/dev/interface.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ type StartOptions struct {
RunCommand string
// If DebugCommand is set, this will look up the specified debug command in the Devfile and execute it. Otherwise, it uses the default one.
DebugCommand string
// SkipCommands indicates if commands (either Build, Run or Debug) will be skipped when starting the Dev Session.
// If SkipCommands is true, then the specified (or default) Build, Run, or Debug commands will not be executed.
SkipCommands bool
// if RandomPorts is set, will port forward on random local ports, else uses ports starting at 20001
RandomPorts bool
// CustomForwardedPorts define custom ports for port forwarding
Expand Down
280 changes: 160 additions & 120 deletions pkg/dev/kubedev/innerloop.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

devfilev1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
parsercommon "github.com/devfile/library/v2/pkg/devfile/parser/data/v2/common"
corev1 "k8s.io/api/core/v1"

"github.com/redhat-developer/odo/pkg/component"
"github.com/redhat-developer/odo/pkg/dev/common"
Expand Down Expand Up @@ -35,56 +36,13 @@ func (o *DevClient) innerloop(ctx context.Context, parameters common.PushParamet
return fmt.Errorf("unable to get pod for component %s: %w", componentName, err)
}

// Find at least one pod with the source volume mounted, error out if none can be found
containerName, syncFolder, err := common.GetFirstContainerWithSourceVolume(pod.Spec.Containers)
if err != nil {
return fmt.Errorf("error while retrieving container from pod %s with a mounted project volume: %w", pod.GetName(), err)
}

s := log.Spinner("Syncing files into the container")
defer s.End(false)

// Get commands
pushDevfileCommands, err := o.getPushDevfileCommands(parameters)
if err != nil {
return fmt.Errorf("failed to validate devfile build and run commands: %w", err)
}

podChanged := componentStatus.GetState() == watch.StateWaitDeployment

// Get a sync adapter. Check if project files have changed and sync accordingly
compInfo := sync.ComponentInfo{
ComponentName: componentName,
ContainerName: containerName,
PodName: pod.GetName(),
SyncFolder: syncFolder,
}

cmdKind := devfilev1.RunCommandGroupKind
cmdName := parameters.StartOptions.RunCommand
if parameters.StartOptions.Debug {
cmdKind = devfilev1.DebugCommandGroupKind
cmdName = parameters.StartOptions.DebugCommand
}

syncParams := sync.SyncParameters{
Path: path,
WatchFiles: parameters.WatchFiles,
WatchDeletedFiles: parameters.WatchDeletedFiles,
IgnoredFiles: parameters.StartOptions.IgnorePaths,
DevfileScanIndexForWatch: parameters.DevfileScanIndexForWatch,

CompInfo: compInfo,
ForcePush: !o.deploymentExists || podChanged,
Files: common.GetSyncFilesFromAttributes(pushDevfileCommands[cmdKind]),
}

execRequired, err := o.syncClient.SyncFiles(ctx, syncParams)
execRequired, err := o.syncFiles(ctx, parameters, pod, podChanged)
if err != nil {
componentStatus.SetState(watch.StateReady)
return fmt.Errorf("failed to sync to component with name %s: %w", componentName, err)
}
s.End(true)

if !componentStatus.PostStartEventsDone && libdevfile.HasPostStartEvents(parameters.Devfile) {
// PostStart events from the devfile will only be executed when the component
Expand All @@ -109,95 +67,125 @@ func (o *DevClient) innerloop(ctx context.Context, parameters common.PushParamet
}
componentStatus.PostStartEventsDone = true

cmd, err := libdevfile.ValidateAndGetCommand(parameters.Devfile, cmdName, cmdKind)
if err != nil {
return err
}

commandType, err := parsercommon.GetCommandType(cmd)
if err != nil {
return err
}
var running bool
var isComposite bool

cmdHandler := component.NewRunHandler(
ctx,
o.kubernetesClient,
o.execClient,
o.configAutomountClient,
o.filesystem,
image.SelectBackend(ctx),
component.HandlerOptions{
PodName: pod.GetName(),
ContainersRunning: component.GetContainersNames(pod),
Devfile: parameters.Devfile,
Path: path,
},
)
var hasRunOrDebugCmd bool
innerLoopWithCommands := !parameters.StartOptions.SkipCommands
if innerLoopWithCommands {
var (
cmdKind = devfilev1.RunCommandGroupKind
cmdName = parameters.StartOptions.RunCommand
)
if parameters.StartOptions.Debug {
cmdKind = devfilev1.DebugCommandGroupKind
cmdName = parameters.StartOptions.DebugCommand
}

if commandType == devfilev1.ExecCommandType {
running, err = cmdHandler.IsRemoteProcessForCommandRunning(ctx, cmd, pod.Name)
var cmd devfilev1.Command
cmd, hasRunOrDebugCmd, err = libdevfile.GetCommand(parameters.Devfile, cmdName, cmdKind)
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)
}

cmdHandler.ComponentExists = running || isComposite

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

if isComposite || !running || execRequired {
// Invoke the build command once (before calling libdevfile.ExecuteCommandByNameAndKind), 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.NewRunHandler(
var running bool
var isComposite bool
var runHandler libdevfile.Handler
if hasRunOrDebugCmd {
var commandType devfilev1.CommandType
commandType, err = parsercommon.GetCommandType(cmd)
if err != nil {
return err
}

cmdHandler := component.NewRunHandler(
ctx,
o.kubernetesClient,
o.execClient,
o.configAutomountClient,
// TODO(feloy) set these values when we want to support Apply Image/Kubernetes/OpenShift commands for PostStart commands
nil, nil,
o.filesystem,
image.SelectBackend(ctx),
component.HandlerOptions{
PodName: pod.Name,
ComponentExists: running,
PodName: pod.GetName(),
ContainersRunning: component.GetContainersNames(pod),
Msg: "Building your application in container",
Devfile: parameters.Devfile,
Path: path,
},
)
return libdevfile.Build(ctx, parameters.Devfile, parameters.StartOptions.BuildCommand, execHandler)
}
if err = doExecuteBuildCommand(); err != nil {
componentStatus.SetState(watch.StateReady)
return err

if commandType == devfilev1.ExecCommandType {
running, err = cmdHandler.IsRemoteProcessForCommandRunning(ctx, cmd, pod.Name)
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)
}

cmdHandler.ComponentExists = running || isComposite
runHandler = cmdHandler
}

err = libdevfile.ExecuteCommandByNameAndKind(ctx, parameters.Devfile, cmdName, cmdKind, cmdHandler, false)
if err != nil {
return err
klog.V(4).Infof("running=%v, execRequired=%v",
running, execRequired)

if isComposite || !running || execRequired {
// Invoke the build command once (before calling libdevfile.ExecuteCommandByNameAndKind), 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.NewRunHandler(
ctx,
o.kubernetesClient,
o.execClient,
o.configAutomountClient,

// TODO(feloy) set these values when we want to support Apply Image/Kubernetes/OpenShift commands for PostStart commands
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)
}
if err = doExecuteBuildCommand(); err != nil {
componentStatus.SetState(watch.StateReady)
return err
}

if hasRunOrDebugCmd {
err = libdevfile.ExecuteCommandByNameAndKind(ctx, parameters.Devfile, cmdName, cmdKind, runHandler, false)
if err != nil {
return err
}
componentStatus.RunExecuted = true
} else {
msg := fmt.Sprintf("Missing default %v command", cmdKind)
if cmdName != "" {
msg = fmt.Sprintf("Missing %v command with name %q", cmdKind, cmdName)
}
log.Warning(msg)
}
}
}

if podChanged || o.portsChanged {
o.portForwardClient.StopPortForwarding(ctx, componentName)
}

// Check that the application is actually listening on the ports declared in the Devfile, so we are sure that port-forwarding will work
appReadySpinner := log.Spinner("Waiting for the application to be ready")
err = o.checkAppPorts(ctx, pod.Name, o.portsToForward)
appReadySpinner.End(err == nil)
if err != nil {
log.Warningf("Port forwarding might not work correctly: %v", err)
log.Warning("Running `odo logs --follow` might help in identifying the problem.")
fmt.Fprintln(log.GetStdout())
if innerLoopWithCommands && hasRunOrDebugCmd && len(o.portsToForward) != 0 {
// Check that the application is actually listening on the ports declared in the Devfile, so we are sure that port-forwarding will work
appReadySpinner := log.Spinner("Waiting for the application to be ready")
err = o.checkAppPorts(ctx, pod.Name, o.portsToForward)
appReadySpinner.End(err == nil)
if err != nil {
log.Warningf("Port forwarding might not work correctly: %v", err)
log.Warning("Running `odo logs --follow` might help in identifying the problem.")
fmt.Fprintln(log.GetStdout())
}
}

err = o.portForwardClient.StartPortForwarding(ctx, parameters.Devfile, componentName, parameters.StartOptions.Debug, parameters.StartOptions.RandomPorts, log.GetStdout(), parameters.StartOptions.ErrOut, parameters.StartOptions.CustomForwardedPorts, parameters.StartOptions.CustomAddress)
Expand All @@ -210,21 +198,73 @@ func (o *DevClient) innerloop(ctx context.Context, parameters common.PushParamet
return nil
}

func (o *DevClient) getPushDevfileCommands(parameters common.PushParameters) (map[devfilev1.CommandGroupKind]devfilev1.Command, error) {
pushDevfileCommands, err := libdevfile.ValidateAndGetPushCommands(parameters.Devfile, parameters.StartOptions.BuildCommand, parameters.StartOptions.RunCommand)
func (o *DevClient) syncFiles(ctx context.Context, parameters common.PushParameters, pod *corev1.Pod, podChanged bool) (bool, error) {
var (
devfileObj = odocontext.GetEffectiveDevfileObj(ctx)
componentName = odocontext.GetComponentName(ctx)
devfilePath = odocontext.GetDevfilePath(ctx)
path = filepath.Dir(devfilePath)
)

s := log.Spinner("Syncing files into the container")
defer s.End(false)

// Find at least one pod with the source volume mounted, error out if none can be found
containerName, syncFolder, err := common.GetFirstContainerWithSourceVolume(pod.Spec.Containers)
if err != nil {
return nil, fmt.Errorf("failed to validate devfile build and run commands: %w", err)
return false, fmt.Errorf("error while retrieving container from pod %s with a mounted project volume: %w", pod.GetName(), err)
}

if parameters.StartOptions.Debug {
pushDevfileDebugCommands, e := libdevfile.ValidateAndGetCommand(parameters.Devfile, parameters.StartOptions.DebugCommand, devfilev1.DebugCommandGroupKind)
if e != nil {
return nil, fmt.Errorf("debug command is not valid: %w", e)
syncFilesMap := make(map[string]string)
var devfileCmd devfilev1.Command
innerLoopWithCommands := !parameters.StartOptions.SkipCommands
if innerLoopWithCommands {
var (
cmdKind = devfilev1.RunCommandGroupKind
cmdName = parameters.StartOptions.RunCommand
)
if parameters.StartOptions.Debug {
cmdKind = devfilev1.DebugCommandGroupKind
cmdName = parameters.StartOptions.DebugCommand
}
var hasCmd bool
devfileCmd, hasCmd, err = libdevfile.GetCommand(*devfileObj, cmdName, cmdKind)
if err != nil {
return false, err
}
if hasCmd {
syncFilesMap = common.GetSyncFilesFromAttributes(devfileCmd)
} else {
klog.V(2).Infof("no command found with name %q and kind %v, syncing files without command attributes", cmdName, cmdKind)
}
pushDevfileCommands[devfilev1.DebugCommandGroupKind] = pushDevfileDebugCommands
}

return pushDevfileCommands, nil
// Get a sync adapter. Check if project files have changed and sync accordingly
compInfo := sync.ComponentInfo{
ComponentName: componentName,
ContainerName: containerName,
PodName: pod.GetName(),
SyncFolder: syncFolder,
}

syncParams := sync.SyncParameters{
Path: path,
WatchFiles: parameters.WatchFiles,
WatchDeletedFiles: parameters.WatchDeletedFiles,
IgnoredFiles: parameters.StartOptions.IgnorePaths,
DevfileScanIndexForWatch: parameters.DevfileScanIndexForWatch,

CompInfo: compInfo,
ForcePush: !o.deploymentExists || podChanged,
Files: syncFilesMap,
}

execRequired, err := o.syncClient.SyncFiles(ctx, syncParams)
if err != nil {
return false, err
}
s.End(true)
return execRequired, nil
}

func (o *DevClient) checkAppPorts(ctx context.Context, podName string, portsToFwd map[string][]devfilev1.Endpoint) error {
Expand Down
Loading

0 comments on commit 63c8e27

Please sign in to comment.