From 18cf91d5851bf89876fa2fd108f304b2d53e7480 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Wed, 31 May 2023 15:13:05 +0200 Subject: [PATCH 01/14] Add a run command --- pkg/odo/cli/cli.go | 2 ++ pkg/odo/cli/run/run.go | 74 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) create mode 100644 pkg/odo/cli/run/run.go diff --git a/pkg/odo/cli/cli.go b/pkg/odo/cli/cli.go index c4e7aea18f2..859c19037a7 100644 --- a/pkg/odo/cli/cli.go +++ b/pkg/odo/cli/cli.go @@ -10,6 +10,7 @@ import ( "unicode" "github.com/redhat-developer/odo/pkg/odo/cli/logs" + "github.com/redhat-developer/odo/pkg/odo/cli/run" "github.com/redhat-developer/odo/pkg/odo/commonflags" "github.com/redhat-developer/odo/pkg/log" @@ -195,6 +196,7 @@ func odoRootCmd(ctx context.Context, name, fullName string) *cobra.Command { set.NewCmdSet(set.RecommendedCommandName, util.GetFullName(fullName, set.RecommendedCommandName)), logs.NewCmdLogs(logs.RecommendedCommandName, util.GetFullName(fullName, logs.RecommendedCommandName)), completion.NewCmdCompletion(completion.RecommendedCommandName, util.GetFullName(fullName, completion.RecommendedCommandName)), + run.NewCmdRun(run.RecommendedCommandName, util.GetFullName(fullName, run.RecommendedCommandName)), ) // Add all subcommands to base commands diff --git a/pkg/odo/cli/run/run.go b/pkg/odo/cli/run/run.go new file mode 100644 index 00000000000..956614c333c --- /dev/null +++ b/pkg/odo/cli/run/run.go @@ -0,0 +1,74 @@ +package run + +import ( + "context" + "fmt" + + "github.com/spf13/cobra" + + "github.com/redhat-developer/odo/pkg/odo/cmdline" + "github.com/redhat-developer/odo/pkg/odo/commonflags" + "github.com/redhat-developer/odo/pkg/odo/genericclioptions" + "github.com/redhat-developer/odo/pkg/odo/genericclioptions/clientset" + odoutil "github.com/redhat-developer/odo/pkg/odo/util" + + ktemplates "k8s.io/kubectl/pkg/util/templates" +) + +const ( + RecommendedCommandName = "run" +) + +type RunOptions struct { + // Clients + clientset *clientset.Clientset +} + +var _ genericclioptions.Runnable = (*RunOptions)(nil) + +func NewRunOptions() *RunOptions { + return &RunOptions{} +} + +var runExample = ktemplates.Examples(` + # Run the command "my-command" in the Dev mode + %[1]s my-command + +`) + +func (o *RunOptions) SetClientset(clientset *clientset.Clientset) { + o.clientset = clientset +} + +func (o *RunOptions) Complete(ctx context.Context, cmdline cmdline.Cmdline, args []string) error { + return nil +} + +func (o *RunOptions) Validate(ctx context.Context) error { + return nil +} + +func (o *RunOptions) Run(ctx context.Context) (err error) { + return nil +} + +func NewCmdRun(name, fullName string) *cobra.Command { + o := NewRunOptions() + runCmd := &cobra.Command{ + Use: name, + Short: "Run a specific command in the Dev mode", + Long: `odo run executes a specific command of the Devfile during the Dev mode ("odo dev" needs to be running)`, + Example: fmt.Sprintf(runExample, fullName), + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + return genericclioptions.GenericRun(o, cmd, args) + }, + } + // clientset.Add(devCmd, + // ) + + odoutil.SetCommandGroup(runCmd, odoutil.MainGroup) + runCmd.SetUsageTemplate(odoutil.CmdUsageTemplate) + commonflags.UsePlatformFlag(runCmd) + return runCmd +} From 8d0968899f389fdc66676ef655d722ca6e25d863 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Wed, 31 May 2023 15:42:45 +0200 Subject: [PATCH 02/14] Check command name passed as arg --- pkg/odo/cli/errors/errors.go | 14 ++++++++ pkg/odo/cli/run/run.go | 26 +++++++++++++-- tests/integration/cmd_run_test.go | 55 +++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 2 deletions(-) create mode 100644 tests/integration/cmd_run_test.go diff --git a/pkg/odo/cli/errors/errors.go b/pkg/odo/cli/errors/errors.go index 48d105b0794..3a553536e97 100644 --- a/pkg/odo/cli/errors/errors.go +++ b/pkg/odo/cli/errors/errors.go @@ -19,6 +19,20 @@ func (o NoCommandInDevfileError) Error() string { return fmt.Sprintf("no command of kind %q found in the devfile", o.command) } +type NoCommandNameInDevfileError struct { + name string +} + +func NewNoCommandNameInDevfileError(name string) NoCommandNameInDevfileError { + return NoCommandNameInDevfileError{ + name: name, + } +} + +func (o NoCommandNameInDevfileError) Error() string { + return fmt.Sprintf("no command named %q found in the devfile", o.name) +} + type Warning struct { msg string err error diff --git a/pkg/odo/cli/run/run.go b/pkg/odo/cli/run/run.go index 956614c333c..665f84cb9c3 100644 --- a/pkg/odo/cli/run/run.go +++ b/pkg/odo/cli/run/run.go @@ -4,10 +4,13 @@ import ( "context" "fmt" + "github.com/devfile/library/v2/pkg/devfile/parser/data/v2/common" "github.com/spf13/cobra" + "github.com/redhat-developer/odo/pkg/odo/cli/errors" "github.com/redhat-developer/odo/pkg/odo/cmdline" "github.com/redhat-developer/odo/pkg/odo/commonflags" + odocontext "github.com/redhat-developer/odo/pkg/odo/context" "github.com/redhat-developer/odo/pkg/odo/genericclioptions" "github.com/redhat-developer/odo/pkg/odo/genericclioptions/clientset" odoutil "github.com/redhat-developer/odo/pkg/odo/util" @@ -22,6 +25,9 @@ const ( type RunOptions struct { // Clients clientset *clientset.Clientset + + // Args + commandName string } var _ genericclioptions.Runnable = (*RunOptions)(nil) @@ -41,10 +47,25 @@ func (o *RunOptions) SetClientset(clientset *clientset.Clientset) { } func (o *RunOptions) Complete(ctx context.Context, cmdline cmdline.Cmdline, args []string) error { + o.commandName = args[0] // Value at 0 is expected to exist, thanks to ExactArgs(1) return nil } func (o *RunOptions) Validate(ctx context.Context) error { + var ( + devfileObj = odocontext.GetEffectiveDevfileObj(ctx) + ) + + if devfileObj == nil { + return genericclioptions.NewNoDevfileError(odocontext.GetWorkingDirectory(ctx)) + } + + commands, err := devfileObj.Data.GetCommands(common.DevfileOptions{ + FilterByName: o.commandName, + }) + if err != nil || len(commands) != 1 { + return errors.NewNoCommandNameInDevfileError(o.commandName) + } return nil } @@ -64,8 +85,9 @@ func NewCmdRun(name, fullName string) *cobra.Command { return genericclioptions.GenericRun(o, cmd, args) }, } - // clientset.Add(devCmd, - // ) + clientset.Add(runCmd, + clientset.FILESYSTEM, + ) odoutil.SetCommandGroup(runCmd, odoutil.MainGroup) runCmd.SetUsageTemplate(odoutil.CmdUsageTemplate) diff --git a/tests/integration/cmd_run_test.go b/tests/integration/cmd_run_test.go new file mode 100644 index 00000000000..8d07361561a --- /dev/null +++ b/tests/integration/cmd_run_test.go @@ -0,0 +1,55 @@ +package integration + +import ( + "path/filepath" + + "github.com/redhat-developer/odo/tests/helper" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("odo run command tests", func() { + var cmpName string + var commonVar helper.CommonVar + + // This is run before every Spec (It) + var _ = BeforeEach(func() { + commonVar = helper.CommonBeforeEach() + cmpName = helper.RandString(6) + _ = cmpName // TODO remove when used + helper.Chdir(commonVar.Context) + Expect(helper.VerifyFileExists(".odo/env/env.yaml")).To(BeFalse()) + }) + + // This is run after every Spec (It) + var _ = AfterEach(func() { + helper.CommonAfterEach(commonVar) + }) + + When("directory is empty", Label(helper.LabelNoCluster), func() { + BeforeEach(func() { + Expect(helper.ListFilesInDir(commonVar.Context)).To(HaveLen(0)) + }) + + It("should error", func() { + output := helper.Cmd("odo", "run", "my-command").ShouldFail().Err() + Expect(output).To(ContainSubstring("The current directory does not represent an odo component")) + }) + }) + + When("a component is bootstrapped", func() { + BeforeEach(func() { + helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project"), commonVar.Context) + helper.Cmd("odo", "init", "--name", cmpName, "--devfile-path", helper.GetExamplePath("source", "devfiles", "nodejs", "devfile.yaml")).ShouldPass() + Expect(helper.VerifyFileExists(".odo/env/env.yaml")).To(BeFalse()) + }) + + It("should fail if command is not found in devfile", Label(helper.LabelNoCluster), func() { + output := helper.Cmd("odo", "run", "unknown-command").ShouldFail().Err() + Expect(output).To(ContainSubstring(`no command named "unknown-command" found in the devfile`)) + + }) + }) + +}) From 30fb3265976690775a7616ea2c556d1151dbe0db Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Wed, 31 May 2023 15:59:24 +0200 Subject: [PATCH 03/14] Check platform is available --- pkg/odo/cli/run/run.go | 22 ++++++++++++++++++++++ tests/integration/cmd_run_test.go | 17 +++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/pkg/odo/cli/run/run.go b/pkg/odo/cli/run/run.go index 665f84cb9c3..cb6a47f6731 100644 --- a/pkg/odo/cli/run/run.go +++ b/pkg/odo/cli/run/run.go @@ -7,13 +7,17 @@ import ( "github.com/devfile/library/v2/pkg/devfile/parser/data/v2/common" "github.com/spf13/cobra" + "github.com/redhat-developer/odo/pkg/kclient" "github.com/redhat-developer/odo/pkg/odo/cli/errors" "github.com/redhat-developer/odo/pkg/odo/cmdline" "github.com/redhat-developer/odo/pkg/odo/commonflags" + fcontext "github.com/redhat-developer/odo/pkg/odo/commonflags/context" odocontext "github.com/redhat-developer/odo/pkg/odo/context" "github.com/redhat-developer/odo/pkg/odo/genericclioptions" "github.com/redhat-developer/odo/pkg/odo/genericclioptions/clientset" odoutil "github.com/redhat-developer/odo/pkg/odo/util" + "github.com/redhat-developer/odo/pkg/podman" + scontext "github.com/redhat-developer/odo/pkg/segment/context" ktemplates "k8s.io/kubectl/pkg/util/templates" ) @@ -54,6 +58,7 @@ func (o *RunOptions) Complete(ctx context.Context, cmdline cmdline.Cmdline, args func (o *RunOptions) Validate(ctx context.Context) error { var ( devfileObj = odocontext.GetEffectiveDevfileObj(ctx) + platform = fcontext.GetPlatform(ctx, commonflags.PlatformCluster) ) if devfileObj == nil { @@ -66,6 +71,21 @@ func (o *RunOptions) Validate(ctx context.Context) error { if err != nil || len(commands) != 1 { return errors.NewNoCommandNameInDevfileError(o.commandName) } + + switch platform { + + case commonflags.PlatformCluster: + if o.clientset.KubernetesClient == nil { + return kclient.NewNoConnectionError() + } + scontext.SetPlatform(ctx, o.clientset.KubernetesClient) + + case commonflags.PlatformPodman: + if o.clientset.PodmanClient == nil { + return podman.NewPodmanNotFoundError(nil) + } + scontext.SetPlatform(ctx, o.clientset.PodmanClient) + } return nil } @@ -87,6 +107,8 @@ func NewCmdRun(name, fullName string) *cobra.Command { } clientset.Add(runCmd, clientset.FILESYSTEM, + clientset.KUBERNETES_NULLABLE, + clientset.PODMAN_NULLABLE, ) odoutil.SetCommandGroup(runCmd, odoutil.MainGroup) diff --git a/tests/integration/cmd_run_test.go b/tests/integration/cmd_run_test.go index 8d07361561a..160d153ec8f 100644 --- a/tests/integration/cmd_run_test.go +++ b/tests/integration/cmd_run_test.go @@ -50,6 +50,23 @@ var _ = Describe("odo run command tests", func() { Expect(output).To(ContainSubstring(`no command named "unknown-command" found in the devfile`)) }) + + It("should fail if platform is not available", Label(helper.LabelNoCluster), func() { + By("failing when trying to run on default platform", func() { + output := helper.Cmd("odo", "run", "build").ShouldFail().Err() + Expect(output).To(ContainSubstring(`unable to access the cluster`)) + + }) + By("failing when trying to run on cluster", func() { + output := helper.Cmd("odo", "run", "build", "--platform", "cluster").ShouldFail().Err() + Expect(output).To(ContainSubstring(`unable to access the cluster`)) + + }) + By("failing when trying to run on podman", func() { + output := helper.Cmd("odo", "run", "build", "--platform", "podman").ShouldFail().Err() + Expect(output).To(ContainSubstring(`unable to access podman`)) + }) + }) }) }) From cf9d50651b1e03780023e85395a67aa11b48507d Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Wed, 31 May 2023 16:05:18 +0200 Subject: [PATCH 04/14] Add a Run method to the DevClient --- pkg/dev/interface.go | 5 +++++ pkg/dev/kubedev/run.go | 15 +++++++++++++++ pkg/dev/podmandev/run.go | 15 +++++++++++++++ pkg/odo/cli/run/run.go | 3 ++- 4 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 pkg/dev/kubedev/run.go create mode 100644 pkg/dev/podmandev/run.go diff --git a/pkg/dev/interface.go b/pkg/dev/interface.go index 177831b2b36..02cde68a596 100644 --- a/pkg/dev/interface.go +++ b/pkg/dev/interface.go @@ -48,6 +48,11 @@ type Client interface { options StartOptions, ) error + Run( + ctx context.Context, + commandName string, + ) error + // CleanupResources deletes the component created using the context's devfile and writes any outputs to out CleanupResources(ctx context.Context, out io.Writer) error } diff --git a/pkg/dev/kubedev/run.go b/pkg/dev/kubedev/run.go new file mode 100644 index 00000000000..29132862a81 --- /dev/null +++ b/pkg/dev/kubedev/run.go @@ -0,0 +1,15 @@ +package kubedev + +import ( + "context" + + "k8s.io/klog" +) + +func (o *DevClient) Run( + ctx context.Context, + commandName string, +) error { + klog.V(4).Infof("running command %q on cluster", commandName) + return nil +} diff --git a/pkg/dev/podmandev/run.go b/pkg/dev/podmandev/run.go new file mode 100644 index 00000000000..cba0d0540ae --- /dev/null +++ b/pkg/dev/podmandev/run.go @@ -0,0 +1,15 @@ +package podmandev + +import ( + "context" + + "k8s.io/klog" +) + +func (o *DevClient) Run( + ctx context.Context, + commandName string, +) error { + klog.V(4).Infof("running command %q on podman", commandName) + return nil +} diff --git a/pkg/odo/cli/run/run.go b/pkg/odo/cli/run/run.go index cb6a47f6731..c5e98217457 100644 --- a/pkg/odo/cli/run/run.go +++ b/pkg/odo/cli/run/run.go @@ -90,7 +90,7 @@ func (o *RunOptions) Validate(ctx context.Context) error { } func (o *RunOptions) Run(ctx context.Context) (err error) { - return nil + return o.clientset.DevClient.Run(ctx, o.commandName) } func NewCmdRun(name, fullName string) *cobra.Command { @@ -109,6 +109,7 @@ func NewCmdRun(name, fullName string) *cobra.Command { clientset.FILESYSTEM, clientset.KUBERNETES_NULLABLE, clientset.PODMAN_NULLABLE, + clientset.DEV, ) odoutil.SetCommandGroup(runCmd, odoutil.MainGroup) From 8b8a95a80c3e2f7399af7cc00d6f9aacd9e74cf0 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Wed, 31 May 2023 17:39:15 +0200 Subject: [PATCH 05/14] Run command on cluster --- pkg/dev/kubedev/run.go | 35 ++++++++++++++++++++++++++++++++++- pkg/libdevfile/errors.go | 3 +++ pkg/libdevfile/libdevfile.go | 20 ++++++++++++++++++++ 3 files changed, 57 insertions(+), 1 deletion(-) diff --git a/pkg/dev/kubedev/run.go b/pkg/dev/kubedev/run.go index 29132862a81..503cec3efc3 100644 --- a/pkg/dev/kubedev/run.go +++ b/pkg/dev/kubedev/run.go @@ -2,7 +2,12 @@ package kubedev import ( "context" + "fmt" + "github.com/redhat-developer/odo/pkg/component" + "github.com/redhat-developer/odo/pkg/devfile/image" + "github.com/redhat-developer/odo/pkg/libdevfile" + odocontext "github.com/redhat-developer/odo/pkg/odo/context" "k8s.io/klog" ) @@ -10,6 +15,34 @@ func (o *DevClient) Run( ctx context.Context, commandName string, ) error { + var ( + componentName = odocontext.GetComponentName(ctx) + devfileObj = odocontext.GetEffectiveDevfileObj(ctx) + devfilePath = odocontext.GetDevfilePath(ctx) + ) + klog.V(4).Infof("running command %q on cluster", commandName) - return nil + + pod, err := o.kubernetesClient.GetPodUsingComponentName(componentName) + if err != nil { + return fmt.Errorf("unable to get pod for component %s: %w", componentName, err) + } + + handler := component.NewRunHandler( + ctx, + o.kubernetesClient, + o.execClient, + o.configAutomountClient, + pod.Name, + false, + component.GetContainersNames(pod), + "Executing command in container", + + o.filesystem, + image.SelectBackend(ctx), + *devfileObj, + devfilePath, + ) + + return libdevfile.ExecuteCommandByName(ctx, *devfileObj, commandName, handler, false) } diff --git a/pkg/libdevfile/errors.go b/pkg/libdevfile/errors.go index 491af5d56f8..dd6806cd230 100644 --- a/pkg/libdevfile/errors.go +++ b/pkg/libdevfile/errors.go @@ -22,6 +22,9 @@ func (e NoCommandFoundError) Error() string { if e.name == "" { return fmt.Sprintf("no %s command found in devfile", e.kind) } + if e.kind == "" { + return fmt.Sprintf("no command named %q found in devfile", e.name) + } return fmt.Sprintf("no %s command with name %q found in Devfile", e.kind, e.name) } diff --git a/pkg/libdevfile/libdevfile.go b/pkg/libdevfile/libdevfile.go index da4409bb6b3..d740c57d4d3 100644 --- a/pkg/libdevfile/libdevfile.go +++ b/pkg/libdevfile/libdevfile.go @@ -66,6 +66,26 @@ func ExecuteCommandByNameAndKind(ctx context.Context, devfileObj parser.DevfileO return executeCommand(ctx, devfileObj, cmd, handler) } +// ExecuteCommandByNameAndKind executes the specified command cmdName of the given kind in the Devfile. +// If cmdName is empty, it executes the default command for the given kind or returns an error if there is no default command. +// If ignoreCommandNotFound is true, nothing is executed if the command is not found and no error is returned. +func ExecuteCommandByName(ctx context.Context, devfileObj parser.DevfileObj, cmdName string, handler Handler, ignoreCommandNotFound bool) error { + commands, err := devfileObj.Data.GetCommands( + common.DevfileOptions{ + FilterByName: cmdName, + }, + ) + if err != nil { + return err + } + if len(commands) != 1 { + return NewNoCommandFoundError("", cmdName) + } + + cmd := commands[0] + return executeCommand(ctx, devfileObj, cmd, handler) +} + // executeCommand executes a specific command of a devfile using handler as backend func executeCommand(ctx context.Context, devfileObj parser.DevfileObj, command v1alpha2.Command, handler Handler) error { cmd, err := newCommand(devfileObj, command) From a6eca7c6ce68420d72fc37b60f9afb018415e0ce Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Thu, 1 Jun 2023 09:57:11 +0200 Subject: [PATCH 06/14] Add test with run command on cluster --- .../devfiles/nodejs/devfile-for-run.yaml | 55 +++++++++++++++++++ tests/integration/cmd_run_test.go | 34 +++++++++++- 2 files changed, 87 insertions(+), 2 deletions(-) create mode 100644 tests/examples/source/devfiles/nodejs/devfile-for-run.yaml diff --git a/tests/examples/source/devfiles/nodejs/devfile-for-run.yaml b/tests/examples/source/devfiles/nodejs/devfile-for-run.yaml new file mode 100644 index 00000000000..275a0b01221 --- /dev/null +++ b/tests/examples/source/devfiles/nodejs/devfile-for-run.yaml @@ -0,0 +1,55 @@ +schemaVersion: 2.0.0 +metadata: + name: nodejs + projectType: nodejs + language: nodejs +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-12:1-36 + memoryLimit: 1024Mi + endpoints: + - name: "3000-tcp" + targetPort: 3000 + mountSources: true +commands: + - id: devbuild + exec: + component: runtime + commandLine: npm install + workingDir: ${PROJECTS_ROOT} + group: + kind: build + isDefault: true + - id: build + exec: + component: runtime + commandLine: npm install + workingDir: ${PROJECTS_ROOT} + group: + kind: build + - id: devrun + exec: + component: runtime + commandLine: npm start + workingDir: ${PROJECTS_ROOT} + group: + kind: run + isDefault: true + - id: run + exec: + component: runtime + commandLine: npm start + workingDir: ${PROJECTS_ROOT} + group: + kind: run + - id: create-file + exec: + component: runtime + commandLine: touch /tmp/new-file + workingDir: ${PROJECTS_ROOT} diff --git a/tests/integration/cmd_run_test.go b/tests/integration/cmd_run_test.go index 160d153ec8f..1f6a7fafe49 100644 --- a/tests/integration/cmd_run_test.go +++ b/tests/integration/cmd_run_test.go @@ -41,7 +41,7 @@ var _ = Describe("odo run command tests", func() { When("a component is bootstrapped", func() { BeforeEach(func() { helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project"), commonVar.Context) - helper.Cmd("odo", "init", "--name", cmpName, "--devfile-path", helper.GetExamplePath("source", "devfiles", "nodejs", "devfile.yaml")).ShouldPass() + helper.Cmd("odo", "init", "--name", cmpName, "--devfile-path", helper.GetExamplePath("source", "devfiles", "nodejs", "devfile-for-run.yaml")).ShouldPass() Expect(helper.VerifyFileExists(".odo/env/env.yaml")).To(BeFalse()) }) @@ -67,6 +67,36 @@ var _ = Describe("odo run command tests", func() { Expect(output).To(ContainSubstring(`unable to access podman`)) }) }) - }) + for _, podman := range []bool{false} { // TODO add true + podman := podman + When("odo dev is executed and ready", helper.LabelPodmanIf(podman, func() { + + 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 a command", func() { + platform := "cluster" + if podman { + platform = "podman" + } + output := helper.Cmd("odo", "run", "create-file", "--platform", platform).ShouldPass().Out() + Expect(output).To(ContainSubstring("Executing command in container (command: create-file)")) + }) + + })) + } + }) }) From 5dc290d184f3cb7e72b826afa5b1d1dba72fc54d Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Thu, 1 Jun 2023 10:58:12 +0200 Subject: [PATCH 07/14] Implement and test run on podman --- pkg/dev/mock.go | 14 ++++++++++++ pkg/dev/podmandev/run.go | 37 +++++++++++++++++++++++++++++-- pkg/podman/component.go | 15 +++++++++++-- pkg/podman/interface.go | 2 ++ pkg/podman/mock.go | 15 +++++++++++++ pkg/podman/pods.go | 10 +++++++++ tests/integration/cmd_run_test.go | 2 +- 7 files changed, 90 insertions(+), 5 deletions(-) diff --git a/pkg/dev/mock.go b/pkg/dev/mock.go index c615af8e388..b9ad7e49778 100644 --- a/pkg/dev/mock.go +++ b/pkg/dev/mock.go @@ -49,6 +49,20 @@ func (mr *MockClientMockRecorder) CleanupResources(ctx, out interface{}) *gomock return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CleanupResources", reflect.TypeOf((*MockClient)(nil).CleanupResources), ctx, out) } +// Run mocks base method. +func (m *MockClient) Run(ctx context.Context, commandName string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Run", ctx, commandName) + ret0, _ := ret[0].(error) + return ret0 +} + +// Run indicates an expected call of Run. +func (mr *MockClientMockRecorder) Run(ctx, commandName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Run", reflect.TypeOf((*MockClient)(nil).Run), ctx, commandName) +} + // Start mocks base method. func (m *MockClient) Start(ctx context.Context, options StartOptions) error { m.ctrl.T.Helper() diff --git a/pkg/dev/podmandev/run.go b/pkg/dev/podmandev/run.go index cba0d0540ae..baa63603531 100644 --- a/pkg/dev/podmandev/run.go +++ b/pkg/dev/podmandev/run.go @@ -2,7 +2,12 @@ package podmandev import ( "context" + "fmt" + "github.com/redhat-developer/odo/pkg/component" + "github.com/redhat-developer/odo/pkg/devfile/image" + "github.com/redhat-developer/odo/pkg/libdevfile" + odocontext "github.com/redhat-developer/odo/pkg/odo/context" "k8s.io/klog" ) @@ -10,6 +15,34 @@ func (o *DevClient) Run( ctx context.Context, commandName string, ) error { - klog.V(4).Infof("running command %q on podman", commandName) - return nil + var ( + componentName = odocontext.GetComponentName(ctx) + devfileObj = odocontext.GetEffectiveDevfileObj(ctx) + devfilePath = odocontext.GetDevfilePath(ctx) + ) + + klog.V(4).Infof("running command %q on cluster", commandName) + + pod, err := o.podmanClient.GetPodUsingComponentName(componentName) + if err != nil { + return fmt.Errorf("unable to get pod for component %s: %w", componentName, err) + } + + handler := component.NewRunHandler( + ctx, + o.podmanClient, + o.execClient, + nil, // TODO(feloy) set when running on new container is supported on podman + pod.Name, + false, + component.GetContainersNames(pod), + "Executing command in container", + + o.fs, + image.SelectBackend(ctx), + *devfileObj, + devfilePath, + ) + + return libdevfile.ExecuteCommandByName(ctx, *devfileObj, commandName, handler, false) } diff --git a/pkg/podman/component.go b/pkg/podman/component.go index e0633ad040e..2ab99d7637a 100644 --- a/pkg/podman/component.go +++ b/pkg/podman/component.go @@ -6,6 +6,7 @@ import ( "os/exec" "strings" + corev1 "k8s.io/api/core/v1" "k8s.io/klog" "github.com/redhat-developer/odo/pkg/api" @@ -15,8 +16,13 @@ import ( // ListPodsReport contains the result of the `podman pod ps --format json` command type ListPodsReport struct { - Name string - Labels map[string]string + Name string + Labels map[string]string + Containers []ListPodsContainer `json:"Containers,omitempty"` +} + +type ListPodsContainer struct { + Names string `json:"Names,omitempty"` } func (o *PodmanCli) ListAllComponents() ([]api.ComponentAbstract, error) { @@ -86,3 +92,8 @@ func (o *PodmanCli) ListAllComponents() ([]api.ComponentAbstract, error) { return components, nil } + +func (o *PodmanCli) GetPodUsingComponentName(componentName string) (*corev1.Pod, error) { + podSelector := fmt.Sprintf("component=%s", componentName) + return o.GetRunningPodFromSelector(podSelector) +} diff --git a/pkg/podman/interface.go b/pkg/podman/interface.go index 22b720032ec..f4449e24a29 100644 --- a/pkg/podman/interface.go +++ b/pkg/podman/interface.go @@ -38,5 +38,7 @@ type Client interface { ListAllComponents() ([]api.ComponentAbstract, error) + GetPodUsingComponentName(componentName string) (*corev1.Pod, error) + Version(ctx context.Context) (SystemVersionReport, error) } diff --git a/pkg/podman/mock.go b/pkg/podman/mock.go index aecc762329c..0eba45104db 100644 --- a/pkg/podman/mock.go +++ b/pkg/podman/mock.go @@ -111,6 +111,21 @@ func (mr *MockClientMockRecorder) GetPodLogs(podName, containerName, followLog i return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPodLogs", reflect.TypeOf((*MockClient)(nil).GetPodLogs), podName, containerName, followLog) } +// GetPodUsingComponentName mocks base method. +func (m *MockClient) GetPodUsingComponentName(componentName string) (*v1.Pod, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetPodUsingComponentName", componentName) + ret0, _ := ret[0].(*v1.Pod) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetPodUsingComponentName indicates an expected call of GetPodUsingComponentName. +func (mr *MockClientMockRecorder) GetPodUsingComponentName(componentName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPodUsingComponentName", reflect.TypeOf((*MockClient)(nil).GetPodUsingComponentName), componentName) +} + // GetPodsMatchingSelector mocks base method. func (m *MockClient) GetPodsMatchingSelector(selector string) (*v1.PodList, error) { m.ctrl.T.Helper() diff --git a/pkg/podman/pods.go b/pkg/podman/pods.go index 44c4eaf331e..e22b847ef86 100644 --- a/pkg/podman/pods.go +++ b/pkg/podman/pods.go @@ -95,6 +95,16 @@ func (o *PodmanCli) GetRunningPodFromSelector(selector string) (*corev1.Pod, err if inspect.State == "Running" { pod.Status.Phase = corev1.PodRunning } + + for _, container := range podReport.Containers { + // Names of users containers are prefixed with pod name by podman + if !strings.HasPrefix(container.Names, podReport.Name+"-") { + continue + } + pod.Spec.Containers = append(pod.Spec.Containers, corev1.Container{ + Name: strings.TrimPrefix(container.Names, podReport.Name+"-"), + }) + } return &pod, nil } diff --git a/tests/integration/cmd_run_test.go b/tests/integration/cmd_run_test.go index 1f6a7fafe49..80d50f7a5d0 100644 --- a/tests/integration/cmd_run_test.go +++ b/tests/integration/cmd_run_test.go @@ -68,7 +68,7 @@ var _ = Describe("odo run command tests", func() { }) }) - for _, podman := range []bool{false} { // TODO add true + for _, podman := range []bool{false, true} { podman := podman When("odo dev is executed and ready", helper.LabelPodmanIf(podman, func() { From abf3033e7b58762aec09eb6625e7b06bfacceb62 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Thu, 1 Jun 2023 11:05:32 +0200 Subject: [PATCH 08/14] Enhance test to check that command has been executed in container --- tests/integration/cmd_run_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/integration/cmd_run_test.go b/tests/integration/cmd_run_test.go index 80d50f7a5d0..77119ab4ca5 100644 --- a/tests/integration/cmd_run_test.go +++ b/tests/integration/cmd_run_test.go @@ -3,7 +3,9 @@ package integration import ( "path/filepath" + "github.com/redhat-developer/odo/pkg/labels" "github.com/redhat-developer/odo/tests/helper" + "k8s.io/utils/pointer" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -94,8 +96,9 @@ var _ = Describe("odo run command tests", func() { } output := helper.Cmd("odo", "run", "create-file", "--platform", platform).ShouldPass().Out() Expect(output).To(ContainSubstring("Executing command in container (command: create-file)")) + component := helper.NewComponent(cmpName, "app", labels.ComponentDevMode, commonVar.Project, commonVar.CliRunner) + component.Exec("runtime", []string{"ls", "/tmp/new-file"}, pointer.Bool(true)) }) - })) } }) From cf33c0974c29cef47558163a2ac73f9f8f07599b Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Thu, 1 Jun 2023 13:34:47 +0200 Subject: [PATCH 09/14] Fix `odo help` test --- cmd/odo/help_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/odo/help_test.go b/cmd/odo/help_test.go index 03c927997b4..91efbd67bed 100644 --- a/cmd/odo/help_test.go +++ b/cmd/odo/help_test.go @@ -34,6 +34,7 @@ Examples: init Init bootstraps a new project logs Show logs of all containers of the component registry List all components from the Devfile registry + run Run a specific command in the Dev mode ` From a06926174469e6e56611201daf5e5f724ec8f0ff Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Thu, 1 Jun 2023 15:50:33 +0200 Subject: [PATCH 10/14] Refactor common code for podman/cluster --- pkg/dev/common/run.go | 53 +++++++++++++++++++++++++++++++++++++++ pkg/dev/kubedev/run.go | 31 +++-------------------- pkg/dev/podmandev/run.go | 31 +++-------------------- pkg/exec/exec_test.go | 4 +++ pkg/platform/interface.go | 2 ++ pkg/platform/mock.go | 15 +++++++++++ pkg/podman/interface.go | 2 -- 7 files changed, 80 insertions(+), 58 deletions(-) create mode 100644 pkg/dev/common/run.go diff --git a/pkg/dev/common/run.go b/pkg/dev/common/run.go new file mode 100644 index 00000000000..22b7b6bd178 --- /dev/null +++ b/pkg/dev/common/run.go @@ -0,0 +1,53 @@ +package common + +import ( + "context" + "fmt" + + "github.com/redhat-developer/odo/pkg/component" + "github.com/redhat-developer/odo/pkg/configAutomount" + "github.com/redhat-developer/odo/pkg/devfile/image" + "github.com/redhat-developer/odo/pkg/exec" + "github.com/redhat-developer/odo/pkg/libdevfile" + odocontext "github.com/redhat-developer/odo/pkg/odo/context" + "github.com/redhat-developer/odo/pkg/platform" + "github.com/redhat-developer/odo/pkg/testingutil/filesystem" +) + +func Run( + ctx context.Context, + commandName string, + platformClient platform.Client, + execClient exec.Client, + configAutomountClient configAutomount.Client, + filesystem filesystem.Filesystem, +) error { + var ( + componentName = odocontext.GetComponentName(ctx) + devfileObj = odocontext.GetEffectiveDevfileObj(ctx) + devfilePath = odocontext.GetDevfilePath(ctx) + ) + + pod, err := platformClient.GetPodUsingComponentName(componentName) + if err != nil { + return fmt.Errorf("unable to get pod for component %s: %w", componentName, err) + } + + handler := component.NewRunHandler( + ctx, + platformClient, + execClient, + configAutomountClient, + pod.Name, + false, + component.GetContainersNames(pod), + "Executing command in container", + + filesystem, + image.SelectBackend(ctx), + *devfileObj, + devfilePath, + ) + + return libdevfile.ExecuteCommandByName(ctx, *devfileObj, commandName, handler, false) +} diff --git a/pkg/dev/kubedev/run.go b/pkg/dev/kubedev/run.go index 503cec3efc3..6dee6972108 100644 --- a/pkg/dev/kubedev/run.go +++ b/pkg/dev/kubedev/run.go @@ -2,12 +2,8 @@ package kubedev import ( "context" - "fmt" - "github.com/redhat-developer/odo/pkg/component" - "github.com/redhat-developer/odo/pkg/devfile/image" - "github.com/redhat-developer/odo/pkg/libdevfile" - odocontext "github.com/redhat-developer/odo/pkg/odo/context" + "github.com/redhat-developer/odo/pkg/dev/common" "k8s.io/klog" ) @@ -15,34 +11,13 @@ func (o *DevClient) Run( ctx context.Context, commandName string, ) error { - var ( - componentName = odocontext.GetComponentName(ctx) - devfileObj = odocontext.GetEffectiveDevfileObj(ctx) - devfilePath = odocontext.GetDevfilePath(ctx) - ) - klog.V(4).Infof("running command %q on cluster", commandName) - - pod, err := o.kubernetesClient.GetPodUsingComponentName(componentName) - if err != nil { - return fmt.Errorf("unable to get pod for component %s: %w", componentName, err) - } - - handler := component.NewRunHandler( + return common.Run( ctx, + commandName, o.kubernetesClient, o.execClient, o.configAutomountClient, - pod.Name, - false, - component.GetContainersNames(pod), - "Executing command in container", - o.filesystem, - image.SelectBackend(ctx), - *devfileObj, - devfilePath, ) - - return libdevfile.ExecuteCommandByName(ctx, *devfileObj, commandName, handler, false) } diff --git a/pkg/dev/podmandev/run.go b/pkg/dev/podmandev/run.go index baa63603531..a978c11dc95 100644 --- a/pkg/dev/podmandev/run.go +++ b/pkg/dev/podmandev/run.go @@ -2,12 +2,8 @@ package podmandev import ( "context" - "fmt" - "github.com/redhat-developer/odo/pkg/component" - "github.com/redhat-developer/odo/pkg/devfile/image" - "github.com/redhat-developer/odo/pkg/libdevfile" - odocontext "github.com/redhat-developer/odo/pkg/odo/context" + "github.com/redhat-developer/odo/pkg/dev/common" "k8s.io/klog" ) @@ -15,34 +11,13 @@ func (o *DevClient) Run( ctx context.Context, commandName string, ) error { - var ( - componentName = odocontext.GetComponentName(ctx) - devfileObj = odocontext.GetEffectiveDevfileObj(ctx) - devfilePath = odocontext.GetDevfilePath(ctx) - ) - klog.V(4).Infof("running command %q on cluster", commandName) - - pod, err := o.podmanClient.GetPodUsingComponentName(componentName) - if err != nil { - return fmt.Errorf("unable to get pod for component %s: %w", componentName, err) - } - - handler := component.NewRunHandler( + return common.Run( ctx, + commandName, o.podmanClient, o.execClient, nil, // TODO(feloy) set when running on new container is supported on podman - pod.Name, - false, - component.GetContainersNames(pod), - "Executing command in container", - o.fs, - image.SelectBackend(ctx), - *devfileObj, - devfilePath, ) - - return libdevfile.ExecuteCommandByName(ctx, *devfileObj, commandName, handler, false) } diff --git a/pkg/exec/exec_test.go b/pkg/exec/exec_test.go index 866902706f1..4b5119f6deb 100644 --- a/pkg/exec/exec_test.go +++ b/pkg/exec/exec_test.go @@ -44,6 +44,10 @@ func (o fakePlatform) GetRunningPodFromSelector(selector string) (*corev1.Pod, e panic("not implemented yet") } +func (o fakePlatform) GetPodUsingComponentName(componentName string) (*corev1.Pod, error) { + panic("not implemented yet") +} + func TestExecuteCommand(t *testing.T) { for _, tt := range []struct { name string diff --git a/pkg/platform/interface.go b/pkg/platform/interface.go index 20b0d03c5f3..baf5f6489eb 100644 --- a/pkg/platform/interface.go +++ b/pkg/platform/interface.go @@ -31,4 +31,6 @@ type Client interface { // GetRunningPodFromSelector returns any pod matching the given label selector. // If multiple pods are found, implementations might have different behavior, by either returning an error or returning any element. GetRunningPodFromSelector(selector string) (*corev1.Pod, error) + + GetPodUsingComponentName(componentName string) (*corev1.Pod, error) } diff --git a/pkg/platform/mock.go b/pkg/platform/mock.go index 317e645b2d0..82cc718b363 100644 --- a/pkg/platform/mock.go +++ b/pkg/platform/mock.go @@ -96,6 +96,21 @@ func (mr *MockClientMockRecorder) GetPodLogs(podName, containerName, followLog i return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPodLogs", reflect.TypeOf((*MockClient)(nil).GetPodLogs), podName, containerName, followLog) } +// GetPodUsingComponentName mocks base method. +func (m *MockClient) GetPodUsingComponentName(componentName string) (*v1.Pod, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetPodUsingComponentName", componentName) + ret0, _ := ret[0].(*v1.Pod) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetPodUsingComponentName indicates an expected call of GetPodUsingComponentName. +func (mr *MockClientMockRecorder) GetPodUsingComponentName(componentName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPodUsingComponentName", reflect.TypeOf((*MockClient)(nil).GetPodUsingComponentName), componentName) +} + // GetPodsMatchingSelector mocks base method. func (m *MockClient) GetPodsMatchingSelector(selector string) (*v1.PodList, error) { m.ctrl.T.Helper() diff --git a/pkg/podman/interface.go b/pkg/podman/interface.go index f4449e24a29..22b720032ec 100644 --- a/pkg/podman/interface.go +++ b/pkg/podman/interface.go @@ -38,7 +38,5 @@ type Client interface { ListAllComponents() ([]api.ComponentAbstract, error) - GetPodUsingComponentName(componentName string) (*corev1.Pod, error) - Version(ctx context.Context) (SystemVersionReport, error) } From 1fdda50c9ed14136f9034ee4d0e5ef894c5887b6 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Thu, 1 Jun 2023 16:33:50 +0200 Subject: [PATCH 11/14] Test Apply commands on Kubernetes/Images --- .../devfiles/nodejs/devfile-for-run.yaml | 22 +++++++++++++- tests/integration/cmd_run_test.go | 30 +++++++++++++++---- 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/tests/examples/source/devfiles/nodejs/devfile-for-run.yaml b/tests/examples/source/devfiles/nodejs/devfile-for-run.yaml index 275a0b01221..3b4d87d1d84 100644 --- a/tests/examples/source/devfiles/nodejs/devfile-for-run.yaml +++ b/tests/examples/source/devfiles/nodejs/devfile-for-run.yaml @@ -1,4 +1,4 @@ -schemaVersion: 2.0.0 +schemaVersion: 2.2.0 metadata: name: nodejs projectType: nodejs @@ -17,6 +17,20 @@ components: - name: "3000-tcp" targetPort: 3000 mountSources: true + - name: config + kubernetes: + inlined: | + apiVersion: v1 + kind: ConfigMap + metadata: + name: my-config + - name: image + image: + imageName: my-image + dockerfile: + uri: ./Dockerfile + buildContext: ${PROJECTS_ROOT} + rootRequired: false commands: - id: devbuild exec: @@ -53,3 +67,9 @@ commands: component: runtime commandLine: touch /tmp/new-file workingDir: ${PROJECTS_ROOT} + - id: deploy-config + apply: + component: config + - id: build-image + apply: + component: image diff --git a/tests/integration/cmd_run_test.go b/tests/integration/cmd_run_test.go index 77119ab4ca5..031bd5143b7 100644 --- a/tests/integration/cmd_run_test.go +++ b/tests/integration/cmd_run_test.go @@ -89,15 +89,35 @@ var _ = Describe("odo run command tests", func() { devSession.WaitEnd() }) - It("should execute a command", func() { + It("should execute commands", func() { platform := "cluster" if podman { platform = "podman" } - output := helper.Cmd("odo", "run", "create-file", "--platform", platform).ShouldPass().Out() - Expect(output).To(ContainSubstring("Executing command in container (command: create-file)")) - component := helper.NewComponent(cmpName, "app", labels.ComponentDevMode, commonVar.Project, commonVar.CliRunner) - component.Exec("runtime", []string{"ls", "/tmp/new-file"}, pointer.Bool(true)) + + By("executing an exec command", func() { + output := helper.Cmd("odo", "run", "create-file", "--platform", platform).ShouldPass().Out() + Expect(output).To(ContainSubstring("Executing command in container (command: create-file)")) + component := helper.NewComponent(cmpName, "app", labels.ComponentDevMode, commonVar.Project, commonVar.CliRunner) + component.Exec("runtime", []string{"ls", "/tmp/new-file"}, pointer.Bool(true)) + }) + + 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")) + }) + } + + 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")) + }) }) })) } From 8c6f3350c40dc88df604c85308e7952a76b7da87 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Thu, 1 Jun 2023 16:44:32 +0200 Subject: [PATCH 12/14] Test a msg is displayed when executing odo run without odo dev --- pkg/dev/common/run.go | 2 +- tests/integration/cmd_run_test.go | 27 +++++++++++++++++++++------ 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/pkg/dev/common/run.go b/pkg/dev/common/run.go index 22b7b6bd178..28689b310b3 100644 --- a/pkg/dev/common/run.go +++ b/pkg/dev/common/run.go @@ -30,7 +30,7 @@ func Run( pod, err := platformClient.GetPodUsingComponentName(componentName) if err != nil { - return fmt.Errorf("unable to get pod for component %s: %w", componentName, err) + return fmt.Errorf("unable to get pod for component %s: %w. Please check the command odo dev is running", componentName, err) } handler := component.NewRunHandler( diff --git a/tests/integration/cmd_run_test.go b/tests/integration/cmd_run_test.go index 031bd5143b7..cfbecaa7979 100644 --- a/tests/integration/cmd_run_test.go +++ b/tests/integration/cmd_run_test.go @@ -70,6 +70,12 @@ var _ = Describe("odo run command tests", func() { }) }) + It("should fail if odo dev is not running", func() { + output := helper.Cmd("odo", "run", "build").ShouldFail().Err() + Expect(output).To(ContainSubstring(`unable to get pod for component`)) + Expect(output).To(ContainSubstring(`Please check the command odo dev is running`)) + }) + for _, podman := range []bool{false, true} { podman := podman When("odo dev is executed and ready", helper.LabelPodmanIf(podman, func() { @@ -112,12 +118,21 @@ var _ = Describe("odo run command tests", func() { }) } - 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")) - }) + 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")) + + }) + } }) })) } From 852077f4ddfb736a6cbb83b9108bef16c5167ba8 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Fri, 2 Jun 2023 11:18:11 +0200 Subject: [PATCH 13/14] Review --- pkg/dev/common/run.go | 2 +- pkg/dev/podmandev/run.go | 2 +- pkg/libdevfile/libdevfile.go | 3 +-- pkg/odo/cli/run/run.go | 5 ++++- .../source/devfiles/nodejs/devfile-for-run.yaml | 11 +++++++++++ tests/integration/cmd_run_test.go | 9 ++++++++- 6 files changed, 26 insertions(+), 6 deletions(-) diff --git a/pkg/dev/common/run.go b/pkg/dev/common/run.go index 28689b310b3..ddc89be77f4 100644 --- a/pkg/dev/common/run.go +++ b/pkg/dev/common/run.go @@ -30,7 +30,7 @@ func Run( pod, err := platformClient.GetPodUsingComponentName(componentName) if err != nil { - return fmt.Errorf("unable to get pod for component %s: %w. Please check the command odo dev is running", componentName, err) + return fmt.Errorf("unable to get pod for component %s: %w. Please check the command 'odo dev' is running", componentName, err) } handler := component.NewRunHandler( diff --git a/pkg/dev/podmandev/run.go b/pkg/dev/podmandev/run.go index a978c11dc95..c1b5af77cc4 100644 --- a/pkg/dev/podmandev/run.go +++ b/pkg/dev/podmandev/run.go @@ -11,7 +11,7 @@ func (o *DevClient) Run( ctx context.Context, commandName string, ) error { - klog.V(4).Infof("running command %q on cluster", commandName) + klog.V(4).Infof("running command %q on podman", commandName) return common.Run( ctx, commandName, diff --git a/pkg/libdevfile/libdevfile.go b/pkg/libdevfile/libdevfile.go index d740c57d4d3..48489498e6d 100644 --- a/pkg/libdevfile/libdevfile.go +++ b/pkg/libdevfile/libdevfile.go @@ -66,8 +66,7 @@ func ExecuteCommandByNameAndKind(ctx context.Context, devfileObj parser.DevfileO return executeCommand(ctx, devfileObj, cmd, handler) } -// ExecuteCommandByNameAndKind executes the specified command cmdName of the given kind in the Devfile. -// If cmdName is empty, it executes the default command for the given kind or returns an error if there is no default command. +// ExecuteCommandByName executes the specified command cmdName in the Devfile. // If ignoreCommandNotFound is true, nothing is executed if the command is not found and no error is returned. func ExecuteCommandByName(ctx context.Context, devfileObj parser.DevfileObj, cmdName string, handler Handler, ignoreCommandNotFound bool) error { commands, err := devfileObj.Data.GetCommands( diff --git a/pkg/odo/cli/run/run.go b/pkg/odo/cli/run/run.go index c5e98217457..c150e8b3dd7 100644 --- a/pkg/odo/cli/run/run.go +++ b/pkg/odo/cli/run/run.go @@ -68,7 +68,10 @@ func (o *RunOptions) Validate(ctx context.Context) error { commands, err := devfileObj.Data.GetCommands(common.DevfileOptions{ FilterByName: o.commandName, }) - if err != nil || len(commands) != 1 { + if err != nil { + return err + } + if len(commands) != 1 { return errors.NewNoCommandNameInDevfileError(o.commandName) } diff --git a/tests/examples/source/devfiles/nodejs/devfile-for-run.yaml b/tests/examples/source/devfiles/nodejs/devfile-for-run.yaml index 3b4d87d1d84..2dfadf93705 100644 --- a/tests/examples/source/devfiles/nodejs/devfile-for-run.yaml +++ b/tests/examples/source/devfiles/nodejs/devfile-for-run.yaml @@ -17,6 +17,12 @@ components: - name: "3000-tcp" targetPort: 3000 mountSources: true + - name: other-container + container: + image: registry.access.redhat.com/ubi8/nodejs-12:1-36 + command: ["tail"] + args: ["-f", "/dev/null"] + memoryLimit: 1024Mi - name: config kubernetes: inlined: | @@ -67,6 +73,11 @@ commands: component: runtime commandLine: touch /tmp/new-file workingDir: ${PROJECTS_ROOT} + - id: create-file-in-other-container + exec: + component: other-container + commandLine: touch /tmp/new-file-in-other-container + workingDir: ${PROJECTS_ROOT} - id: deploy-config apply: component: config diff --git a/tests/integration/cmd_run_test.go b/tests/integration/cmd_run_test.go index cfbecaa7979..2aa2e50df1c 100644 --- a/tests/integration/cmd_run_test.go +++ b/tests/integration/cmd_run_test.go @@ -73,7 +73,7 @@ var _ = Describe("odo run command tests", func() { It("should fail if odo dev is not running", func() { output := helper.Cmd("odo", "run", "build").ShouldFail().Err() Expect(output).To(ContainSubstring(`unable to get pod for component`)) - Expect(output).To(ContainSubstring(`Please check the command odo dev is running`)) + Expect(output).To(ContainSubstring(`Please check the command 'odo dev' is running`)) }) for _, podman := range []bool{false, true} { @@ -108,6 +108,13 @@ var _ = Describe("odo run command tests", func() { component.Exec("runtime", []string{"ls", "/tmp/new-file"}, pointer.Bool(true)) }) + By("executing an exec command in another container", func() { + output := helper.Cmd("odo", "run", "create-file-in-other-container", "--platform", platform).ShouldPass().Out() + Expect(output).To(ContainSubstring("Executing command in container (command: create-file-in-other-container)")) + component := helper.NewComponent(cmpName, "app", labels.ComponentDevMode, commonVar.Project, commonVar.CliRunner) + component.Exec("other-container", []string{"ls", "/tmp/new-file-in-other-container"}, pointer.Bool(true)) + }) + if !podman { By("executing apply command on Kubernetes component", func() { output := helper.Cmd("odo", "run", "deploy-config", "--platform", platform).ShouldPass().Out() From 2ada482812e043a78f7ebac2fd89adf51c38bbd6 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Fri, 2 Jun 2023 11:50:58 +0200 Subject: [PATCH 14/14] makes GetRunningPodFromSelector return only Running pods on Podman --- pkg/component/component.go | 6 +----- pkg/component/delete/delete.go | 6 ------ pkg/component/delete/delete_test.go | 19 ------------------- pkg/podman/pods.go | 4 ++-- 4 files changed, 3 insertions(+), 32 deletions(-) diff --git a/pkg/component/component.go b/pkg/component/component.go index 9581687cb85..96a7d02bd12 100644 --- a/pkg/component/component.go +++ b/pkg/component/component.go @@ -94,11 +94,7 @@ func Log(platformClient platform.Client, componentName string, appName string, f pod, err := platformClient.GetRunningPodFromSelector(odolabels.GetSelector(componentName, appName, odolabels.ComponentDevMode, false)) if err != nil { - return nil, fmt.Errorf("the component %s doesn't exist on the cluster", componentName) - } - - if pod.Status.Phase != corev1.PodRunning { - return nil, fmt.Errorf("unable to show logs, component is not in running state. current status=%v", pod.Status.Phase) + return nil, fmt.Errorf("a running component %s doesn't exist on the cluster: %w", componentName, err) } containerName := command.Exec.Component diff --git a/pkg/component/delete/delete.go b/pkg/component/delete/delete.go index ffac61e4467..9756cd82dc1 100644 --- a/pkg/component/delete/delete.go +++ b/pkg/component/delete/delete.go @@ -212,12 +212,6 @@ func (do *DeleteComponentClient) ExecutePreStopEvents(ctx context.Context, devfi return fmt.Errorf("unable to determine if component %s exists; cause: %v", componentName, err.Error()) } - // do not fail Delete operation if if the pod is not running or if the event execution fails - if pod.Status.Phase != corev1.PodRunning { - klog.V(4).Infof("unable to execute preStop events, pod for component %q is not running", componentName) - return nil - } - 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 handler := component.NewRunHandler( diff --git a/pkg/component/delete/delete_test.go b/pkg/component/delete/delete_test.go index cdf4641ee21..c153ea0fbee 100644 --- a/pkg/component/delete/delete_test.go +++ b/pkg/component/delete/delete_test.go @@ -700,25 +700,6 @@ func TestDeleteComponentClient_ExecutePreStopEvents(t *testing.T) { }, wantErr: false, }, - { - name: "did not execute PreStopEvents because the pod is not in the running state", - fields: fields{ - kubeClient: func(ctrl *gomock.Controller) kclient.ClientInterface { - client := kclient.NewMockClientInterface(ctrl) - - selector := odolabels.GetSelector(componentName, "app", odolabels.ComponentDevMode, false) - pod := odoTestingUtil.CreateFakePod(componentName, "mypod", "runtime") - pod.Status.Phase = corev1.PodFailed - client.EXPECT().GetRunningPodFromSelector(selector).Return(pod, nil) - return client - }, - }, - args: args{ - devfileObj: devfileObjWithPreStopEvents, - appName: appName, - }, - wantErr: false, - }, { name: "failed to execute PreStopEvents because it failed to execute the command inside the container, but no error returned", fields: fields{ diff --git a/pkg/podman/pods.go b/pkg/podman/pods.go index e22b847ef86..e4bae249f4e 100644 --- a/pkg/podman/pods.go +++ b/pkg/podman/pods.go @@ -92,8 +92,8 @@ func (o *PodmanCli) GetRunningPodFromSelector(selector string) (*corev1.Pod, err if err != nil { return nil, err } - if inspect.State == "Running" { - pod.Status.Phase = corev1.PodRunning + if inspect.State != "Running" { + return nil, fmt.Errorf("a pod exists but is not in Running state. Current status=%v", inspect.State) } for _, container := range podReport.Containers {