From 3e30bf962514ab25ad633a8f5b7d844ccba81db7 Mon Sep 17 00:00:00 2001 From: Armel Soro Date: Mon, 12 Jun 2023 15:28:38 +0200 Subject: [PATCH 01/10] Document the '--no-commands' flag Co-authored-by: Philippe Martin --- docs/website/docs/command-reference/dev.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/docs/website/docs/command-reference/dev.md b/docs/website/docs/command-reference/dev.md index c390da2d4ff..dcc0da452a5 100644 --- a/docs/website/docs/command-reference/dev.md +++ b/docs/website/docs/command-reference/dev.md @@ -523,6 +523,24 @@ $ ODO_CONTAINER_BACKEND_GLOBAL_ARGS='--root=/tmp/podman/root;--storage-driver=ov ``` +### 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) From cc2ff1010c4e32c90b2c6a119af7b39721ca3cf0 Mon Sep 17 00:00:00 2001 From: Armel Soro Date: Mon, 12 Jun 2023 15:29:46 +0200 Subject: [PATCH 02/10] Add integration tests highlighting the expectations --- .../nodejs/devfile-without-commands.yaml | 34 +++ tests/helper/helper_dev.go | 4 + tests/integration/cmd_dev_test.go | 236 +++++++++++++++--- 3 files changed, 238 insertions(+), 36 deletions(-) create mode 100644 tests/examples/source/devfiles/nodejs/devfile-without-commands.yaml diff --git a/tests/examples/source/devfiles/nodejs/devfile-without-commands.yaml b/tests/examples/source/devfiles/nodejs/devfile-without-commands.yaml new file mode 100644 index 00000000000..758d8558dfd --- /dev/null +++ b/tests/examples/source/devfiles/nodejs/devfile-without-commands.yaml @@ -0,0 +1,34 @@ +schemaVersion: 2.1.0 +metadata: + name: nodejs + displayName: Node.js Runtime + description: Node.js 16 application + icon: https://nodejs.org/static/images/logos/nodejs-new-pantone-black.svg + tags: + - Node.js + - Express + - ubi8 + projectType: Node.js + language: JavaScript + version: 2.1.1 +starterProjects: + - name: nodejs-starter + git: + remotes: + origin: 'https://github.com/odo-devfiles/nodejs-ex.git' +components: + - name: runtime + container: + image: registry.access.redhat.com/ubi8/nodejs-16:latest + args: ['tail', '-f', '/dev/null'] + memoryLimit: 1024Mi + mountSources: true + env: + - name: DEBUG_PORT + value: '5858' + endpoints: + - name: http-node + targetPort: 3000 + - exposure: none + name: debug + targetPort: 5858 diff --git a/tests/helper/helper_dev.go b/tests/helper/helper_dev.go index 952b235587f..7a24be3b20b 100644 --- a/tests/helper/helper_dev.go +++ b/tests/helper/helper_dev.go @@ -123,6 +123,7 @@ type DevSessionOpts struct { TimeoutInSeconds int NoRandomPorts bool NoWatch bool + NoCommands bool CustomAddress string } @@ -140,6 +141,9 @@ func StartDevMode(options DevSessionOpts) (devSession DevSession, out []byte, er } args := []string{"dev"} + if options.NoCommands { + args = append(args, "--no-commands") + } if !options.NoRandomPorts { args = append(args, "--random-ports") } diff --git a/tests/integration/cmd_dev_test.go b/tests/integration/cmd_dev_test.go index 37224bf8686..5f46a9343b7 100644 --- a/tests/integration/cmd_dev_test.go +++ b/tests/integration/cmd_dev_test.go @@ -193,6 +193,88 @@ echo "$@" errOut := helper.Cmd("odo", args...).ShouldFail().Err() Expect(errOut).To(ContainSubstring("--random-ports and --port-forward cannot be used together")) })) + + for _, opts := range [][]string{ + {"--build-command", "my-build-cmd"}, + {"--run-command", "my-run-cmd"}, + {"--build-command", "my-build-cmd", "--run-command", "my-run-cmd"}, + } { + opts := opts + It("should fail when using --no-commands and --build-command and/or --run-command together", helper.LabelPodmanIf(podman, func() { + args := []string{"dev", "--no-commands"} + args = append(args, opts...) + if podman { + args = append(args, "--platform", "podman") + } + errOut := helper.Cmd("odo", args...).ShouldFail().Err() + Expect(errOut).To(ContainSubstring("--no-commands cannot be used with --build-command or --run-command")) + })) + } + + for _, debug := range []bool{false, true} { + debug := debug + When(fmt.Sprintf("running with --no-commands (debug=%v)", debug), helper.LabelPodmanIf(podman, func() { + var devSession helper.DevSession + var stdout, stderr string + var ports map[string]string + BeforeEach(func() { + args := []string{"--no-commands"} + if debug { + args = append(args, "--debug") + } + opts := helper.DevSessionOpts{ + RunOnPodman: podman, + CmdlineArgs: args, + } + + var outB, errB []byte + var err error + devSession, outB, errB, ports, err = helper.StartDevMode(opts) + Expect(err).ShouldNot(HaveOccurred()) + stdout = string(outB) + stderr = string(errB) + }) + + AfterEach(func() { + devSession.Stop() + devSession.WaitEnd() + }) + + It("should start the dev session", func() { + + By("not executing the build command", func() { + for _, out := range []string{stdout, stderr} { + Expect(out).ShouldNot(ContainSubstring("Building your application in container on cluster")) + } + }) + + By("not executing the run command", func() { + for _, out := range []string{stdout, stderr} { + Expect(out).ShouldNot(ContainSubstring("Executing the application")) + } + }) + + By("syncing the files", func() { + Expect(stdout).Should(ContainSubstring("Syncing files into the container")) + + component := helper.NewComponent(cmpName, "app", labels.ComponentDevMode, commonVar.Project, commonVar.CliRunner) + execResult, _ := component.Exec("runtime", []string{"ls", "-lai", "/projects"}, pointer.Bool(true)) + for _, fileName := range []string{"server.js", "package.json"} { + Expect(execResult).Should(ContainSubstring(fileName)) + } + + execResult, _ = component.Exec("runtime", []string{"cat", "/projects/server.js"}, pointer.Bool(true)) + Expect(execResult).Should(ContainSubstring("App started")) + }) + + By("setting up port forwarding", func() { + Expect(ports).ShouldNot(BeEmpty()) + _, ok := ports["3000"] + Expect(ok).To(BeTrue(), fmt.Sprintf("missing port forwarded for 3000: %v", ports)) + }) + }) + })) + } } It("ensure that index information is updated", func() { @@ -559,58 +641,70 @@ ComponentSettings: for _, podman := range []bool{true, false} { podman := podman - When("odo is executed with --no-watch flag", helper.LabelPodmanIf(podman, func() { - - var devSession helper.DevSession - - BeforeEach(func() { - var err error - devSession, _, _, _, err = helper.StartDevMode(helper.DevSessionOpts{ - CmdlineArgs: []string{"--no-watch"}, - RunOnPodman: podman, - }) - Expect(err).ToNot(HaveOccurred()) - }) + for _, noCommandsFlag := range []string{"", "--no-commands", "--no-commands=false"} { + noCommandsFlag := noCommandsFlag + suffix := "without --no-commands" + if noCommandsFlag != "" { + suffix = "with " + noCommandsFlag + } - AfterEach(func() { - devSession.Stop() - devSession.WaitEnd() - }) + When("odo is executed with --no-watch flag and "+suffix, helper.LabelPodmanIf(podman, func() { - When("a file in component directory is modified", func() { + var devSession helper.DevSession BeforeEach(func() { - helper.ReplaceString(filepath.Join(commonVar.Context, "server.js"), "App started", "App is super started") + var err error + args := []string{"--no-watch"} + if noCommandsFlag != "" { + args = append(args, noCommandsFlag) + } + devSession, _, _, _, err = helper.StartDevMode(helper.DevSessionOpts{ + CmdlineArgs: args, + RunOnPodman: podman, + }) + Expect(err).ToNot(HaveOccurred()) }) - It("should not trigger a push", func() { - component := helper.NewComponent(cmpName, "app", labels.ComponentDevMode, commonVar.Project, commonVar.CliRunner) - execResult, _ := component.Exec("runtime", []string{"cat", "/projects/server.js"}, pointer.Bool(true)) - Expect(execResult).To(ContainSubstring("App started")) - Expect(execResult).ToNot(ContainSubstring("App is super started")) - + AfterEach(func() { + devSession.Stop() + devSession.WaitEnd() }) - When("p is pressed", func() { + When("a file in component directory is modified", func() { BeforeEach(func() { - if os.Getenv("SKIP_KEY_PRESS") == "true" { - Skip("This is a unix-terminal specific scenario, skipping") - } - - devSession.PressKey('p') + helper.ReplaceString(filepath.Join(commonVar.Context, "server.js"), "App started", "App is super started") }) - It("should trigger a push", func() { - _, _, _, err := devSession.WaitSync() - Expect(err).ToNot(HaveOccurred()) + It("should not trigger a push", func() { component := helper.NewComponent(cmpName, "app", labels.ComponentDevMode, commonVar.Project, commonVar.CliRunner) execResult, _ := component.Exec("runtime", []string{"cat", "/projects/server.js"}, pointer.Bool(true)) - Expect(execResult).To(ContainSubstring("App is super started")) + Expect(execResult).To(ContainSubstring("App started")) + Expect(execResult).ToNot(ContainSubstring("App is super started")) + + }) + + When("p is pressed", func() { + + BeforeEach(func() { + if os.Getenv("SKIP_KEY_PRESS") == "true" { + Skip("This is a unix-terminal specific scenario, skipping") + } + + devSession.PressKey('p') + }) + + It("should trigger a push", func() { + _, _, _, err := devSession.WaitSync() + Expect(err).ToNot(HaveOccurred()) + component := helper.NewComponent(cmpName, "app", labels.ComponentDevMode, commonVar.Project, commonVar.CliRunner) + execResult, _ := component.Exec("runtime", []string{"cat", "/projects/server.js"}, pointer.Bool(true)) + Expect(execResult).To(ContainSubstring("App is super started")) + }) }) }) - }) - })) + })) + } } When("a delay is necessary for the component to start and running odo dev", func() { @@ -4806,4 +4900,74 @@ CMD ["npm", "start"] })) } + + for _, podman := range []bool{false, true} { + podman := podman + + When("using a Devfile with no commands", helper.LabelPodmanIf(podman, func() { + BeforeEach(func() { + helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project"), commonVar.Context) + helper.CopyExampleDevFile( + filepath.Join("source", "devfiles", "nodejs", "devfile-without-commands.yaml"), + filepath.Join(commonVar.Context, "devfile.yaml"), + cmpName) + }) + + for _, noCommandsFlag := range []string{"", "--no-commands", "--no-commands=false"} { + noCommandsFlag := noCommandsFlag + suffix := "without --no-commands" + if noCommandsFlag != "" { + suffix = "with " + noCommandsFlag + } + + It("should start the Dev Session "+suffix, func() { + var args []string + if noCommandsFlag != "" { + args = append(args, noCommandsFlag) + } + opts := helper.DevSessionOpts{ + RunOnPodman: podman, + CmdlineArgs: args, + } + devSession, stdout, stderr, ports, err := helper.StartDevMode(opts) + Expect(err).ShouldNot(HaveOccurred()) + defer func() { + devSession.Stop() + devSession.WaitEnd() + }() + + By("not executing any build command", func() { + for _, out := range [][]byte{stdout, stderr} { + Expect(string(out)).ShouldNot(ContainSubstring("Building your application in container on cluster")) + } + }) + + By("not executing any run command", func() { + for _, out := range [][]byte{stdout, stderr} { + Expect(out).ShouldNot(ContainSubstring("Executing the application")) + } + }) + + By("syncing the files", func() { + Expect(string(stdout)).Should(ContainSubstring("Syncing files into the container")) + + component := helper.NewComponent(cmpName, "app", labels.ComponentDevMode, commonVar.Project, commonVar.CliRunner) + execResult, _ := component.Exec("runtime", []string{"ls", "-lai", "/projects"}, pointer.Bool(true)) + for _, fileName := range []string{"server.js", "package.json"} { + Expect(execResult).Should(ContainSubstring(fileName)) + } + + execResult, _ = component.Exec("runtime", []string{"cat", "/projects/server.js"}, pointer.Bool(true)) + Expect(execResult).Should(ContainSubstring("App started")) + }) + + By("setting up port forwarding", func() { + Expect(ports).ShouldNot(BeEmpty()) + _, ok := ports["3000"] + Expect(ok).To(BeTrue(), fmt.Sprintf("missing port forwarded for 3000: %v", ports)) + }) + }) + } + })) + } }) From 8c9e4634beb3c72c50c6c6b5c09a5e5bfcd32d0e Mon Sep 17 00:00:00 2001 From: Armel Soro Date: Mon, 12 Jun 2023 15:31:45 +0200 Subject: [PATCH 03/10] Add '--no-commands' flag to the 'dev' sub-command --- pkg/dev/interface.go | 3 +++ pkg/odo/cli/dev/dev.go | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/pkg/dev/interface.go b/pkg/dev/interface.go index 02cde68a596..4776c26e4ca 100644 --- a/pkg/dev/interface.go +++ b/pkg/dev/interface.go @@ -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 diff --git a/pkg/odo/cli/dev/dev.go b/pkg/odo/cli/dev/dev.go index 829daded167..cd479506f6f 100644 --- a/pkg/odo/cli/dev/dev.go +++ b/pkg/odo/cli/dev/dev.go @@ -70,6 +70,7 @@ type DevOptions struct { forwardLocalhostFlag bool portForwardFlag []string addressFlag string + noCommandsFlag bool } var _ genericclioptions.Runnable = (*DevOptions)(nil) @@ -250,6 +251,7 @@ func (o *DevOptions) Run(ctx context.Context) (err error) { Debug: o.debugFlag, BuildCommand: o.buildCommandFlag, RunCommand: o.runCommandFlag, + SkipCommands: o.noCommandsFlag, RandomPorts: o.randomPortsFlag, WatchFiles: !o.noWatchFlag, IgnoreLocalhost: o.ignoreLocalhostFlag, @@ -309,6 +311,8 @@ It forwards endpoints with any exposure values ('public', 'internal' or 'none') devCmd.Flags().StringArrayVar(&o.portForwardFlag, "port-forward", nil, "Define custom port mapping for port forwarding. Acceptable formats: LOCAL_PORT:REMOTE_PORT, LOCAL_PORT:CONTAINER_NAME:REMOTE_PORT.") devCmd.Flags().StringVar(&o.addressFlag, "address", "127.0.0.1", "Define custom address for port forwarding.") + devCmd.Flags().BoolVar(&o.noCommandsFlag, "no-commands", false, "Do not run any commands; just start the development environment.") + clientset.Add(devCmd, clientset.BINDING, clientset.DEV, From e6caaf1988c470c1e8ff35adc4f8fa81349d22a9 Mon Sep 17 00:00:00 2001 From: Armel Soro Date: Mon, 12 Jun 2023 15:32:56 +0200 Subject: [PATCH 04/10] Implement '--no-command' on Podman --- pkg/dev/podmandev/podmandev.go | 33 ++++++++---- pkg/dev/podmandev/reconcile.go | 99 ++++++++++++++++++---------------- 2 files changed, 76 insertions(+), 56 deletions(-) diff --git a/pkg/dev/podmandev/podmandev.go b/pkg/dev/podmandev/podmandev.go index ad676e060fb..4178ecafdff 100644 --- a/pkg/dev/podmandev/podmandev.go +++ b/pkg/dev/podmandev/podmandev.go @@ -118,15 +118,28 @@ func (o *DevClient) syncFiles(ctx context.Context, options dev.StartOptions, pod s := log.Spinner("Syncing files into the container") defer s.End(false) - cmdKind := devfilev1.RunCommandGroupKind - cmdName := options.RunCommand - if options.Debug { - cmdKind = devfilev1.DebugCommandGroupKind - cmdName = options.DebugCommand - } - devfileCmd, err := libdevfile.ValidateAndGetCommand(*devfileObj, cmdName, cmdKind) - if err != nil { - return false, err + syncFilesMap := make(map[string]string) + var devfileCmd devfilev1.Command + innerLoopWithCommands := !options.SkipCommands + if innerLoopWithCommands { + var ( + cmdKind = devfilev1.RunCommandGroupKind + cmdName = options.RunCommand + ) + if options.Debug { + cmdKind = devfilev1.DebugCommandGroupKind + cmdName = options.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) + } } syncParams := sync.SyncParameters{ @@ -138,7 +151,7 @@ func (o *DevClient) syncFiles(ctx context.Context, options dev.StartOptions, pod CompInfo: compInfo, ForcePush: true, - Files: common.GetSyncFilesFromAttributes(devfileCmd), + Files: syncFilesMap, } execRequired, err := o.syncClient.SyncFiles(ctx, syncParams) if err != nil { diff --git a/pkg/dev/podmandev/reconcile.go b/pkg/dev/podmandev/reconcile.go index 2774865a63d..ccf2da2a5af 100644 --- a/pkg/dev/podmandev/reconcile.go +++ b/pkg/dev/podmandev/reconcile.go @@ -84,66 +84,73 @@ func (o *DevClient) reconcile( } componentStatus.PostStartEventsDone = true - if execRequired { - doExecuteBuildCommand := func() error { - execHandler := component.NewRunHandler( + innerLoopWithCommands := !parameters.StartOptions.SkipCommands + if innerLoopWithCommands { + if execRequired { + doExecuteBuildCommand := func() error { + execHandler := component.NewRunHandler( + ctx, + o.podmanClient, + o.execClient, + nil, // TODO(feloy) set this value when we want to support exec on new container on podman + + // TODO(feloy) set these values when we want to support Apply Image/Kubernetes/OpenShift commands for PreStop events + nil, nil, component.HandlerOptions{ + PodName: pod.Name, + ComponentExists: componentStatus.RunExecuted, + ContainersRunning: component.GetContainersNames(pod), + Msg: "Building your application in container", + }, + ) + return libdevfile.Build(ctx, devfileObj, options.BuildCommand, execHandler) + } + + err = doExecuteBuildCommand() + if err != nil { + return err + } + + cmdKind := devfilev1.RunCommandGroupKind + cmdName := options.RunCommand + if options.Debug { + cmdKind = devfilev1.DebugCommandGroupKind + cmdName = options.DebugCommand + } + + cmdHandler := component.NewRunHandler( ctx, o.podmanClient, o.execClient, nil, // TODO(feloy) set this value when we want to support exec on new container on podman - // TODO(feloy) set these values when we want to support Apply Image/Kubernetes/OpenShift commands for PreStop events - nil, nil, + + o.fs, + image.SelectBackend(ctx), + + // TODO(feloy) set to deploy Kubernetes/Openshift components component.HandlerOptions{ PodName: pod.Name, ComponentExists: componentStatus.RunExecuted, ContainersRunning: component.GetContainersNames(pod), - Msg: "Building your application in container", }, ) - return libdevfile.Build(ctx, devfileObj, options.BuildCommand, execHandler) - } - - err = doExecuteBuildCommand() - if err != nil { - return err - } - - cmdKind := devfilev1.RunCommandGroupKind - cmdName := options.RunCommand - if options.Debug { - cmdKind = devfilev1.DebugCommandGroupKind - cmdName = options.DebugCommand + err = libdevfile.ExecuteCommandByNameAndKind(ctx, devfileObj, cmdName, cmdKind, cmdHandler, false) + if err != nil { + return err + } + componentStatus.RunExecuted = true } + } - cmdHandler := component.NewRunHandler( - ctx, - o.podmanClient, - o.execClient, - nil, // TODO(feloy) set this value when we want to support exec on new container on podman - o.fs, - image.SelectBackend(ctx), - // TODO(feloy) set to deploy Kubernetes/Openshift components - component.HandlerOptions{ - PodName: pod.Name, - ComponentExists: componentStatus.RunExecuted, - ContainersRunning: component.GetContainersNames(pod), - }, - ) - err = libdevfile.ExecuteCommandByNameAndKind(ctx, devfileObj, cmdName, cmdKind, cmdHandler, false) + if innerLoopWithCommands { + // 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, fwPorts) + appReadySpinner.End(err == nil) if err != nil { - return err + log.Warningf("Port forwarding might not work correctly: %v", err) + log.Warning("Running `odo logs --follow --platform podman` might help in identifying the problem.") + fmt.Fprintln(options.Out) } - componentStatus.RunExecuted = true - } - - // 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, fwPorts) - appReadySpinner.End(err == nil) - if err != nil { - log.Warningf("Port forwarding might not work correctly: %v", err) - log.Warning("Running `odo logs --follow --platform podman` might help in identifying the problem.") - fmt.Fprintln(options.Out) } // By default, Podman will not forward to container applications listening on the loopback interface. From 51644be1e0e4c7f87b0094f29730c22dbf5ef10c Mon Sep 17 00:00:00 2001 From: Armel Soro Date: Mon, 12 Jun 2023 15:34:17 +0200 Subject: [PATCH 05/10] Implement '--no-command' on cluster --- pkg/dev/kubedev/innerloop.go | 280 +++++++++++++++++++---------------- 1 file changed, 153 insertions(+), 127 deletions(-) diff --git a/pkg/dev/kubedev/innerloop.go b/pkg/dev/kubedev/innerloop.go index 1d0c4194159..568552d59e8 100644 --- a/pkg/dev/kubedev/innerloop.go +++ b/pkg/dev/kubedev/innerloop.go @@ -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" @@ -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 @@ -109,80 +67,94 @@ 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, - }, - ) + 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, err = libdevfile.ValidateAndGetCommand(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( - 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) + var commandType devfilev1.CommandType + commandType, err = parsercommon.GetCommandType(cmd) + if err != nil { return err } + var running bool + var isComposite bool - err = libdevfile.ExecuteCommandByNameAndKind(ctx, parameters.Devfile, cmdName, cmdKind, cmdHandler, false) - if err != nil { - return err + 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, + }, + ) + + 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 + + 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 + } + + err = libdevfile.ExecuteCommandByNameAndKind(ctx, parameters.Devfile, cmdName, cmdKind, cmdHandler, false) + if err != nil { + return err + } } } @@ -190,14 +162,16 @@ func (o *DevClient) innerloop(ctx context.Context, parameters common.PushParamet 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 { + // 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) @@ -210,21 +184,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 } - pushDevfileCommands[devfilev1.DebugCommandGroupKind] = pushDevfileDebugCommands + 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) + } + } + + // 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, } - return pushDevfileCommands, nil + 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 { From e862b278b381cb870acc6cb7f5903e5dca038738 Mon Sep 17 00:00:00 2001 From: Armel Soro Date: Mon, 12 Jun 2023 15:35:16 +0200 Subject: [PATCH 06/10] 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. --- pkg/odo/cli/dev/dev.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/pkg/odo/cli/dev/dev.go b/pkg/odo/cli/dev/dev.go index cd479506f6f..dccf36a26ee 100644 --- a/pkg/odo/cli/dev/dev.go +++ b/pkg/odo/cli/dev/dev.go @@ -23,7 +23,6 @@ import ( "github.com/redhat-developer/odo/pkg/kclient" "github.com/redhat-developer/odo/pkg/libdevfile" "github.com/redhat-developer/odo/pkg/log" - clierrors "github.com/redhat-developer/odo/pkg/odo/cli/errors" "github.com/redhat-developer/odo/pkg/odo/cli/messages" "github.com/redhat-developer/odo/pkg/odo/cmdline" "github.com/redhat-developer/odo/pkg/odo/commonflags" @@ -114,11 +113,10 @@ func (o *DevOptions) Complete(ctx context.Context, cmdline cmdline.Cmdline, args func (o *DevOptions) Validate(ctx context.Context) error { devfileObj := *odocontext.GetEffectiveDevfileObj(ctx) - if !o.debugFlag && !libdevfile.HasRunCommand(devfileObj.Data) { - return clierrors.NewNoCommandInDevfileError("run") - } - if o.debugFlag && !libdevfile.HasDebugCommand(devfileObj.Data) { - return clierrors.NewNoCommandInDevfileError("debug") + if o.noCommandsFlag { + if o.buildCommandFlag != "" || o.runCommandFlag != "" { + return errors.New("--no-commands cannot be used with --build-command or --run-command") + } } platform := fcontext.GetPlatform(ctx, commonflags.PlatformCluster) From 280ce6d405ff512bca003ae221a775c405d55158 Mon Sep 17 00:00:00 2001 From: Armel Soro Date: Mon, 12 Jun 2023 15:36:43 +0200 Subject: [PATCH 07/10] 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. --- tests/integration/cmd_dev_debug_test.go | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/tests/integration/cmd_dev_debug_test.go b/tests/integration/cmd_dev_debug_test.go index fc7209f88a2..d76358a7ce0 100644 --- a/tests/integration/cmd_dev_debug_test.go +++ b/tests/integration/cmd_dev_debug_test.go @@ -449,13 +449,17 @@ var _ = Describe("odo dev debug command tests", func() { Expect(helper.VerifyFileExists(".odo/env/env.yaml")).To(BeFalse()) }) - It("should fail running odo dev --debug", func() { - args := []string{"dev", "--debug"} - if podman { - args = append(args, "--platform", "podman") - } - output := helper.Cmd("odo", args...).ShouldFail().Err() - Expect(output).To(ContainSubstring("no command of kind \"debug\" found in the devfile")) + It("should log error about missing debug command when running odo dev --debug", func() { + devSession, out, _, _, err := helper.StartDevMode(helper.DevSessionOpts{ + RunOnPodman: podman, + CmdlineArgs: []string{"--debug"}, + }) + Expect(err).ShouldNot(HaveOccurred()) + defer func() { + devSession.Stop() + devSession.WaitEnd() + }() + Expect(string(out)).To(ContainSubstring("no debug command found in devfile")) }) })) } From a0e0020dc19cd0c59598d38bdbf5189ed833ee47 Mon Sep 17 00:00:00 2001 From: Armel Soro Date: Tue, 13 Jun 2023 10:13:16 +0200 Subject: [PATCH 08/10] 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 --- pkg/dev/kubedev/innerloop.go | 89 ++++++++++++++++++------------- pkg/dev/podmandev/reconcile.go | 51 +++++++++++------- tests/integration/cmd_dev_test.go | 52 +++++++++--------- 3 files changed, 108 insertions(+), 84 deletions(-) diff --git a/pkg/dev/kubedev/innerloop.go b/pkg/dev/kubedev/innerloop.go index 568552d59e8..19f11c7de7d 100644 --- a/pkg/dev/kubedev/innerloop.go +++ b/pkg/dev/kubedev/innerloop.go @@ -78,50 +78,57 @@ func (o *DevClient) innerloop(ctx context.Context, parameters common.PushParamet cmdName = parameters.StartOptions.DebugCommand } - var cmd devfilev1.Command - cmd, err = libdevfile.ValidateAndGetCommand(parameters.Devfile, cmdName, cmdKind) + var ( + cmd devfilev1.Command + hasRunOrDebugCmd bool + ) + cmd, hasRunOrDebugCmd, err = libdevfile.GetCommand(parameters.Devfile, cmdName, cmdKind) if err != nil { return err } - var commandType devfilev1.CommandType - 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, - }, - ) - - if commandType == devfilev1.ExecCommandType { - running, err = cmdHandler.IsRemoteProcessForCommandRunning(ctx, cmd, pod.Name) + var runHandler libdevfile.Handler + if hasRunOrDebugCmd { + var commandType devfilev1.CommandType + commandType, err = parsercommon.GetCommandType(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) - } - cmdHandler.ComponentExists = running || isComposite + 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, + }, + ) + + 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 + } klog.V(4).Infof("running=%v, execRequired=%v", running, execRequired) @@ -151,9 +158,17 @@ func (o *DevClient) innerloop(ctx context.Context, parameters common.PushParamet return err } - err = libdevfile.ExecuteCommandByNameAndKind(ctx, parameters.Devfile, cmdName, cmdKind, cmdHandler, false) - if err != nil { - return err + if hasRunOrDebugCmd { + err = libdevfile.ExecuteCommandByNameAndKind(ctx, parameters.Devfile, cmdName, cmdKind, runHandler, false) + if err != nil { + return err + } + } 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) } } } diff --git a/pkg/dev/podmandev/reconcile.go b/pkg/dev/podmandev/reconcile.go index ccf2da2a5af..ff1c5010d5f 100644 --- a/pkg/dev/podmandev/reconcile.go +++ b/pkg/dev/podmandev/reconcile.go @@ -116,28 +116,41 @@ func (o *DevClient) reconcile( cmdKind = devfilev1.DebugCommandGroupKind cmdName = options.DebugCommand } - - cmdHandler := component.NewRunHandler( - ctx, - o.podmanClient, - o.execClient, - nil, // TODO(feloy) set this value when we want to support exec on new container on podman - - o.fs, - image.SelectBackend(ctx), - - // TODO(feloy) set to deploy Kubernetes/Openshift components - component.HandlerOptions{ - PodName: pod.Name, - ComponentExists: componentStatus.RunExecuted, - ContainersRunning: component.GetContainersNames(pod), - }, - ) - err = libdevfile.ExecuteCommandByNameAndKind(ctx, devfileObj, cmdName, cmdKind, cmdHandler, false) + var hasRunOrDebugCmd bool + _, hasRunOrDebugCmd, err = libdevfile.GetCommand(parameters.Devfile, cmdName, cmdKind) if err != nil { return err } - componentStatus.RunExecuted = true + + if hasRunOrDebugCmd { + cmdHandler := component.NewRunHandler( + ctx, + o.podmanClient, + o.execClient, + nil, // TODO(feloy) set this value when we want to support exec on new container on podman + + o.fs, + image.SelectBackend(ctx), + + // TODO(feloy) set to deploy Kubernetes/Openshift components + component.HandlerOptions{ + PodName: pod.Name, + ComponentExists: componentStatus.RunExecuted, + ContainersRunning: component.GetContainersNames(pod), + }, + ) + err = libdevfile.ExecuteCommandByNameAndKind(ctx, devfileObj, cmdName, cmdKind, cmdHandler, 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) + } } } diff --git a/tests/integration/cmd_dev_test.go b/tests/integration/cmd_dev_test.go index 5f46a9343b7..cf6d9c09f60 100644 --- a/tests/integration/cmd_dev_test.go +++ b/tests/integration/cmd_dev_test.go @@ -4913,41 +4913,19 @@ CMD ["npm", "start"] cmpName) }) - for _, noCommandsFlag := range []string{"", "--no-commands", "--no-commands=false"} { - noCommandsFlag := noCommandsFlag - suffix := "without --no-commands" - if noCommandsFlag != "" { - suffix = "with " + noCommandsFlag - } - - It("should start the Dev Session "+suffix, func() { - var args []string - if noCommandsFlag != "" { - args = append(args, noCommandsFlag) - } - opts := helper.DevSessionOpts{ + for _, noCommands := range []bool{false, true} { + noCommands := noCommands + It(fmt.Sprintf("should start the Dev Session with --no-commands=%v", noCommands), func() { + devSession, stdout, stderr, ports, err := helper.StartDevMode(helper.DevSessionOpts{ RunOnPodman: podman, - CmdlineArgs: args, - } - devSession, stdout, stderr, ports, err := helper.StartDevMode(opts) + NoCommands: noCommands, + }) Expect(err).ShouldNot(HaveOccurred()) defer func() { devSession.Stop() devSession.WaitEnd() }() - By("not executing any build command", func() { - for _, out := range [][]byte{stdout, stderr} { - Expect(string(out)).ShouldNot(ContainSubstring("Building your application in container on cluster")) - } - }) - - By("not executing any run command", func() { - for _, out := range [][]byte{stdout, stderr} { - Expect(out).ShouldNot(ContainSubstring("Executing the application")) - } - }) - By("syncing the files", func() { Expect(string(stdout)).Should(ContainSubstring("Syncing files into the container")) @@ -4961,6 +4939,24 @@ CMD ["npm", "start"] Expect(execResult).Should(ContainSubstring("App started")) }) + By("not executing any build command", func() { + for _, out := range [][]byte{stdout, stderr} { + Expect(string(out)).ShouldNot(ContainSubstring("Building your application in container on cluster")) + } + }) + + By("not executing any run command", func() { + for _, out := range [][]byte{stdout, stderr} { + Expect(string(out)).ShouldNot(ContainSubstring("Executing the application")) + } + }) + + if !noCommands { + By("warning about missing default run command", func() { + Expect(string(stderr)).Should(ContainSubstring("Missing default run command")) + }) + } + By("setting up port forwarding", func() { Expect(ports).ShouldNot(BeEmpty()) _, ok := ports["3000"] From e4b7df16fd888ce6dcd63f12f0ef5168ffae3e57 Mon Sep 17 00:00:00 2001 From: Armel Soro Date: Tue, 13 Jun 2023 10:14:10 +0200 Subject: [PATCH 09/10] 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. --- pkg/dev/kubedev/innerloop.go | 9 ++++----- pkg/dev/podmandev/reconcile.go | 4 ++-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/pkg/dev/kubedev/innerloop.go b/pkg/dev/kubedev/innerloop.go index 19f11c7de7d..b4d723cf0dc 100644 --- a/pkg/dev/kubedev/innerloop.go +++ b/pkg/dev/kubedev/innerloop.go @@ -67,6 +67,7 @@ func (o *DevClient) innerloop(ctx context.Context, parameters common.PushParamet } componentStatus.PostStartEventsDone = true + var hasRunOrDebugCmd bool innerLoopWithCommands := !parameters.StartOptions.SkipCommands if innerLoopWithCommands { var ( @@ -78,10 +79,7 @@ func (o *DevClient) innerloop(ctx context.Context, parameters common.PushParamet cmdName = parameters.StartOptions.DebugCommand } - var ( - cmd devfilev1.Command - hasRunOrDebugCmd bool - ) + var cmd devfilev1.Command cmd, hasRunOrDebugCmd, err = libdevfile.GetCommand(parameters.Devfile, cmdName, cmdKind) if err != nil { return err @@ -163,6 +161,7 @@ func (o *DevClient) innerloop(ctx context.Context, parameters common.PushParamet if err != nil { return err } + componentStatus.RunExecuted = true } else { msg := fmt.Sprintf("Missing default %v command", cmdKind) if cmdName != "" { @@ -177,7 +176,7 @@ func (o *DevClient) innerloop(ctx context.Context, parameters common.PushParamet o.portForwardClient.StopPortForwarding(ctx, componentName) } - if innerLoopWithCommands { + 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) diff --git a/pkg/dev/podmandev/reconcile.go b/pkg/dev/podmandev/reconcile.go index ff1c5010d5f..e53f4df9317 100644 --- a/pkg/dev/podmandev/reconcile.go +++ b/pkg/dev/podmandev/reconcile.go @@ -85,6 +85,7 @@ func (o *DevClient) reconcile( componentStatus.PostStartEventsDone = true innerLoopWithCommands := !parameters.StartOptions.SkipCommands + var hasRunOrDebugCmd bool if innerLoopWithCommands { if execRequired { doExecuteBuildCommand := func() error { @@ -116,7 +117,6 @@ func (o *DevClient) reconcile( cmdKind = devfilev1.DebugCommandGroupKind cmdName = options.DebugCommand } - var hasRunOrDebugCmd bool _, hasRunOrDebugCmd, err = libdevfile.GetCommand(parameters.Devfile, cmdName, cmdKind) if err != nil { return err @@ -154,7 +154,7 @@ func (o *DevClient) reconcile( } } - if innerLoopWithCommands { + if innerLoopWithCommands && hasRunOrDebugCmd && len(fwPorts) != 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, fwPorts) From 8d160d6ba2389b951f7b9001cad4000ee7e227b8 Mon Sep 17 00:00:00 2001 From: Armel Soro Date: Tue, 13 Jun 2023 11:49:59 +0200 Subject: [PATCH 10/10] Test "odo run" command against a Dev Session started with and without "--no-commands" --- tests/integration/cmd_run_test.go | 115 ++++++++++++++++-------------- 1 file changed, 60 insertions(+), 55 deletions(-) diff --git a/tests/integration/cmd_run_test.go b/tests/integration/cmd_run_test.go index 24bf914ac70..7580b5319e4 100644 --- a/tests/integration/cmd_run_test.go +++ b/tests/integration/cmd_run_test.go @@ -1,6 +1,7 @@ package integration import ( + "fmt" "path/filepath" "github.com/redhat-developer/odo/tests/helper" @@ -63,7 +64,7 @@ var _ = Describe("odo run command tests", func() { }) By("failing when trying to run on podman", func() { - output := helper.Cmd("odo", "run", "build", "--platform", "podman").ShouldFail().Err() + output := helper.Cmd("odo", "run", "build", "--platform", "podman").AddEnv("PODMAN_CMD=false").ShouldFail().Err() Expect(output).To(ContainSubstring(`unable to access podman`)) }) }) @@ -76,71 +77,75 @@ var _ = Describe("odo run command tests", func() { for _, podman := range []bool{false, true} { podman := podman - When("odo dev is executed and ready", helper.LabelPodmanIf(podman, func() { + for _, noCommands := range []bool{false, true} { + noCommands := noCommands + When(fmt.Sprintf("odo dev is executed with --no-commands=%v and ready", noCommands), helper.LabelPodmanIf(podman, func() { - var devSession helper.DevSession + var devSession helper.DevSession - BeforeEach(func() { - var err error - devSession, _, _, _, err = helper.StartDevMode(helper.DevSessionOpts{ - RunOnPodman: podman, - }) - Expect(err).ToNot(HaveOccurred()) - }) - - AfterEach(func() { - devSession.Stop() - devSession.WaitEnd() - }) - - It("should execute commands", func() { - platform := "cluster" - if podman { - platform = "podman" - } - - By("executing an exec command and displaying output", func() { - output := helper.Cmd("odo", "run", "list-files", "--platform", platform).ShouldPass().Out() - Expect(output).To(ContainSubstring("etc")) + BeforeEach(func() { + var err error + devSession, _, _, _, err = helper.StartDevMode(helper.DevSessionOpts{ + RunOnPodman: podman, + NoCommands: noCommands, + }) + Expect(err).ToNot(HaveOccurred()) }) - By("executing an exec command in another container and displaying output", func() { - output := helper.Cmd("odo", "run", "list-files-in-other-container", "--platform", platform).ShouldPass().Out() - Expect(output).To(ContainSubstring("etc")) + AfterEach(func() { + devSession.Stop() + devSession.WaitEnd() }) - if !podman { - By("executing apply command on Kubernetes component", func() { - output := helper.Cmd("odo", "run", "deploy-config", "--platform", platform).ShouldPass().Out() - Expect(output).To(ContainSubstring("Creating resource ConfigMap/my-config")) - out := commonVar.CliRunner.Run("get", "configmap", "my-config", "-n", - commonVar.Project).Wait().Out.Contents() - Expect(out).To(ContainSubstring("my-config")) - }) - } - - if podman { - By("executing apply command on Image component", func() { - // Will fail because Dockerfile is not present, but we just want to check the build is started - // We cannot use PODMAN_CMD=echo with --platform=podman - output := helper.Cmd("odo", "run", "build-image", "--platform", platform).ShouldFail().Out() - Expect(output).To(ContainSubstring("Building image locally")) + It("should execute commands", func() { + platform := "cluster" + if podman { + platform = "podman" + } + + By("executing an exec command and displaying output", func() { + output := helper.Cmd("odo", "run", "list-files", "--platform", platform).ShouldPass().Out() + Expect(output).To(ContainSubstring("etc")) }) - } else { - By("executing apply command on Image component", func() { - output := helper.Cmd("odo", "run", "build-image", "--platform", platform).AddEnv("PODMAN_CMD=echo").ShouldPass().Out() - Expect(output).To(ContainSubstring("Building image locally")) - Expect(output).To(ContainSubstring("Pushing image to container registry")) + By("executing an exec command in another container and displaying output", func() { + output := helper.Cmd("odo", "run", "list-files-in-other-container", "--platform", platform).ShouldPass().Out() + Expect(output).To(ContainSubstring("etc")) }) - } - By("exiting with a status 1 when the exec command fails and displaying error output", func() { - out := helper.Cmd("odo", "run", "error-cmd", "--platform", platform).ShouldFail().Err() - Expect(out).To(ContainSubstring("No such file or directory")) + if !podman { + By("executing apply command on Kubernetes component", func() { + output := helper.Cmd("odo", "run", "deploy-config", "--platform", platform).ShouldPass().Out() + Expect(output).To(ContainSubstring("Creating resource ConfigMap/my-config")) + out := commonVar.CliRunner.Run("get", "configmap", "my-config", "-n", + commonVar.Project).Wait().Out.Contents() + Expect(out).To(ContainSubstring("my-config")) + }) + } + + if podman { + By("executing apply command on Image component", func() { + // Will fail because Dockerfile is not present, but we just want to check the build is started + // We cannot use PODMAN_CMD=echo with --platform=podman + output := helper.Cmd("odo", "run", "build-image", "--platform", platform).ShouldFail().Out() + Expect(output).To(ContainSubstring("Building image locally")) + }) + } else { + By("executing apply command on Image component", func() { + output := helper.Cmd("odo", "run", "build-image", "--platform", platform).AddEnv("PODMAN_CMD=echo").ShouldPass().Out() + Expect(output).To(ContainSubstring("Building image locally")) + Expect(output).To(ContainSubstring("Pushing image to container registry")) + + }) + } + + By("exiting with a status 1 when the exec command fails and displaying error output", func() { + out := helper.Cmd("odo", "run", "error-cmd", "--platform", platform).ShouldFail().Err() + Expect(out).To(ContainSubstring("No such file or directory")) + }) }) - }) - })) + })) + } } }) })