search

redhat-developer / odo

odo - Developer-focused CLI for fast & iterative container-based application development on Podman and Kubernetes. Implementation of the open Devfile standard.
https://odo.dev
Apache License 2.0
777 stars 242 forks source link

Make sure to run parallel commands part of a composite command in parallel #7075

Closed rm3l closed 10 months ago

rm3l commented 10 months ago

What type of PR is this:

/kind bug /area devfile-spec

What does this PR do / why we need it:

Which issue(s) this PR fixes: Fixes #6681

PR acceptance criteria:

How to test changes / Special notes to the reviewer: See the repro steps in #6681. With the changes in this PR, the output of odo dev should reflect that the commands are running in parallel:

$ odo init \
  --name my-component-with-parallel-commands \
  --devfile-path https://gist.githubusercontent.com/rm3l/5691e926a4cfe37e369d93695187bdd6/raw/95abc768e83bd83e7acda035a6053f57b20f4939/devfile-with-parallel-commands.yaml

$ odo dev

[...]
 •  Waiting for Kubernetes resources  ...
 ⚠  Pod is Pending
 ✓  Pod is Running
 ✓  Syncing files into the container [57ms]
 ✓  Building your application in container (command: exec-3) [40ms]
 ✓  Building your application in container (command: exec-2) [41ms]
 ✓  Building your application in container (command: exec-1) [5s]
 •  Executing the application (command: run-1)  ...
 •  Executing the application (command: run-3)  ...
 •  Executing the application (command: run-2)  ...
 ✓  Finished executing the application (command: run-3) [145ms]
 ✓  Finished executing the application (command: run-2) [145ms]
 ✓  Finished executing the application (command: run-1) [5s]

p

Pushing files...

 •  Waiting for Kubernetes resources  ...
 ✓  Syncing files into the container 
 ✓  Building your application in container (command: exec-2) [30ms]
 ✓  Building your application in container (command: exec-3) [30ms]
 ✓  Building your application in container (command: exec-1) [5s]
 •  Executing the application (command: run-2)  ...
 •  Executing the application (command: run-3)  ...
 •  Executing the application (command: run-1)  ...
 ✓  Finished executing the application (command: run-3) [2s]
 ✓  Finished executing the application (command: run-2) [2s]
 ✓  Finished executing the application (command: run-1) [7s]
[...]
netlify[bot] commented 10 months ago

Deploy Preview for odo-docusaurus-preview canceled.

Name Link
Latest commit 39a6b5468a33344147b47c018010eac140775afe
Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/64f8a601cfe6a50008298116
odo-robot[bot] commented 10 months ago

Windows Tests (OCP) on commit 336abf62a2f117953af7fdb92500a52d94c7218c finished successfully. View logs: TXT HTML

odo-robot[bot] commented 10 months ago

OpenShift Unauthenticated Tests on commit 336abf62a2f117953af7fdb92500a52d94c7218c finished successfully. View logs: TXT HTML

odo-robot[bot] commented 10 months ago

NoCluster Tests on commit 336abf62a2f117953af7fdb92500a52d94c7218c finished successfully. View logs: TXT HTML

odo-robot[bot] commented 10 months ago

Unit Tests on commit 336abf62a2f117953af7fdb92500a52d94c7218c finished successfully. View logs: TXT HTML

odo-robot[bot] commented 10 months ago

Validate Tests on commit 336abf62a2f117953af7fdb92500a52d94c7218c finished successfully. View logs: TXT HTML

odo-robot[bot] commented 10 months ago

Kubernetes Tests on commit 336abf62a2f117953af7fdb92500a52d94c7218c finished successfully. View logs: TXT HTML

odo-robot[bot] commented 10 months ago

OpenShift Tests on commit 336abf62a2f117953af7fdb92500a52d94c7218c finished successfully. View logs: TXT HTML

feloy commented 10 months ago

I can see this error happening in the different platforms, buu I cannot see the relation with the changes made in this PR:

[FAILED] Expected
      <string>: Searching resources to delete, please wait...
      This will delete "nodejs" from the namespace "cmd-delete-test517hno".
       •  The following resources will get deleted from cluster:
       •      - Deployment: nodejs-app

      This will also delete the following files and directories:
        - /tmp/1997028204/.odo
        - /tmp/1997028204/devfile.yaml
       •  Deleting resources from cluster  ...
      ↵
 ✓  Deleting resources from cluster [108ms]
      The component "nodejs" is successfully deleted from namespace "cmd-delete-test517hno"

  to contain substring
      <string>: Executing pre-stop command in container (command: myprestop)
  In [It] at: /go/odo_1/tests/integration/cmd_delete_test.go:520 @ 09/06/23 09:22:03.405
------------------------------
rm3l commented 10 months ago

I can see this error happening in the different platforms, buu I cannot see the relation with the changes made in this PR:

[FAILED] Expected
      <string>: Searching resources to delete, please wait...
      This will delete "nodejs" from the namespace "cmd-delete-test517hno".
       •  The following resources will get deleted from cluster:
       •    - Deployment: nodejs-app

      This will also delete the following files and directories:
          - /tmp/1997028204/.odo
          - /tmp/1997028204/devfile.yaml
       •  Deleting resources from cluster  ...
      ↵
 ✓  Deleting resources from cluster [108ms]
      The component "nodejs" is successfully deleted from namespace "cmd-delete-test517hno"

  to contain substring
      <string>: Executing pre-stop command in container (command: myprestop)
  In [It] at: /go/odo_1/tests/integration/cmd_delete_test.go:520 @ 09/06/23 09:22:03.405
------------------------------

I was also able to reproduce the same failure locally; not sure either how it is related to the changes here. I'm taking a look..

rm3l commented 10 months ago

I can see this error happening in the different platforms, buu I cannot see the relation with the changes made in this PR:

[FAILED] Expected
      <string>: Searching resources to delete, please wait...
      This will delete "nodejs" from the namespace "cmd-delete-test517hno".
       •  The following resources will get deleted from cluster:
       •      - Deployment: nodejs-app

      This will also delete the following files and directories:
        - /tmp/1997028204/.odo
        - /tmp/1997028204/devfile.yaml
       •  Deleting resources from cluster  ...
      ↵
 ✓  Deleting resources from cluster [108ms]
      The component "nodejs" is successfully deleted from namespace "cmd-delete-test517hno"

  to contain substring
      <string>: Executing pre-stop command in container (command: myprestop)
  In [It] at: /go/odo_1/tests/integration/cmd_delete_test.go:520 @ 09/06/23 09:22:03.405
------------------------------

I was also able to reproduce the same failure locally; not sure either how it is related to the changes here. I'm taking a look..

Okay, I think this is a bit related, because the failing test is using a devfile with a pre-stop event trying to execute a composite command with parallel sub-commands. And with the changes here, we should now be using the actual composite parallel implementation.

From the job logs, I noticed the following message:

Failed to execute "preStop" event commands for component "nodejs", cause: unable to execute devfile command "mycompcmd": unknown command type

After digging, the sub-commands in the Devfile have an upper case, which seems to be a valid Devfile (even if all command IDs are expected to be lower-case).

https://github.com/redhat-developer/odo/blob/adc96994d9a393c19999fc1247529a9fa088c816/tests/examples/source/devfiles/nodejs/devfile-with-valid-events.yaml#L70-L76

The composite implementation lowers the case of the sub-commands, while the composite parallel implementation does not. We need to use the same logic in both implementations. I'll update the implementation in command_composite_parallel.go accordingly.

odo-robot[bot] commented 10 months ago

Kubernetes Docs Tests on commit 725a64014ada827d852615670b9fc09b8193ccd9 finished successfully. View logs: TXT HTML

sonarcloud[bot] commented 10 months ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication