che task: Cannot run a che command after workspace restart for some containers

[sunix]

Describe the bug

Cannot rerun commands that are targeting a plugin container after restart of a workspace.

Che version

• nightly

Steps to reproduce

1. start a workspace with this devfile containing a command that would run on a plugin sidecar container. For instance:

    $ chectl workspace:start -f <(chectl devfile:generate --name=test-task-bug-restart --language=java --command="echo hello world")
      ✔ Retrieving Che Server URL...http://che-che.192.168.39.13.nip.io
      ✔ Verify if Che server is running...RUNNING (auth disabled)
      ✔ Create workspace from Devfile /dev/fd/63
    
    Workspace IDE URL:
    http://che-che.192.168.39.13.nip.io/dashboard/#/ide/che/test-task-bug-restart

   apiVersion: 1.0.0
   metadata:
     name: test-task-bug-restart
   components:
     - type: chePlugin
       alias: java-ls
       id: redhat/java/latest
   commands:
     - name: echo hello world
       actions:
         - type: exec
           command: echo hello world
           component: java-ls
           workdir: /projects/

2. open the workspace and execute the task: it works well

3. stop the workspace

4. restart it and reopen it

5. Try to run the task again, it doesn't work anymore. The terminal is appearing but nothing is executed.
   

   The error Uncaught (in promise) Error: container with name vscode-javaebs was not found. For workspace: workspaceeau9fy5dqm79n36l. But the vscode-javaebs was the container name when it started the first time.
   Fixing the /projects/.theia/tasks.json with the right machineName works.

Expected behavior

The task is supposed to be running after restart and display "hello world" (for the previous example)

Runtime

• minikube (include output of minikube version and kubectl version)

Installation method

• chectl

Environment

• my computer
  • Linux

[tsmaeder]

From the screenshot, it looks like the name of the container is wrong (javalce vs. javaebs): does it change when we restart the workspace? Does che remember the container by id when first running a command?

[rhopp]

I couldn't reproduce with your devfile, but my current che installation is from yesterday (chectl/operator on OCP 4), so if that's something new I wouldn't catch it.

[tsmaeder]

@rhopp redeploy che, set image pull policy to always. It's what Jesus would do :-)

[tsmaeder]

@sunix can you open terminals for the containers?

[sunix]

If I edit tasks.json and correct the container name, it works fine. The problem is that now we are persisting che commands in tasks.json ... only the first time. But the container name is changing after restart for plugins and editors. So the tasks are trying to run that on previous container name before the restart of the workspace.

[sunix]

I can open terminals

[slemeur]

Yeah that behavior is weird and not synchronized with the devfile. linked to the epic.

[slemeur]

@skabashnyuk : This is very weird. Is that the configuration given in the devfile not supposed to be persisted as workspace configuration?
cc @metlos

[sunix]

@slemeur , it is a problem introduced by latest changes in the task plugin, the container running some plugins is changing name after restart.
But now, task plugin is persisting the container name in tasks.json and tasks.json won't change after restart

[sunix]

the question is: why side cars container with plugins has that generated suffix ?

[RomanNikitenko]

@slemeur
Sun is right, the problem is on the task plugin side.

We don't have the synchronization for config file and devfile at the moment.
The tasks from tasks.json file have higher priority than tasks from devfile because user working in the running workspace and if he changes some args for command it's expected to execute this configuration from tasks.json file, not origin task from devfile.

But I didn't take in account that side cars container with plugins has that generated suffix.
I think I can provide fix for that if we can take the issue in the current sprint.

[l0rd]

@RomanNikitenko can you provide more details about the fix you are thinking about? I am setting it as milestone 7.0.0 but I don't want to fix it at any cost (it's not a blocker): we fix it if we have time and we are confident that no regression will be introduced.

[RomanNikitenko]

@l0rd
At refreshing the page/start workspace we check for conflicts by type and label for configurations from devfile and from tasks.json file. As described here configurations from config file win over configs from devfile.

As quick fix at conflict resolving we can replace containerName field of task from tasks.json file by the value of the configuration from devfile. So after merge the task will contain fields like command, workingDir from tasks.json file but containerName field from the devfile configuration. That state will be saved to tasks.json file.

For case if user changed in tasks.json file the label for a configuration and we can not match this task to any task from devfile:
Before running the task we can take containers by API and check:

• if containerNameexists in the given list - execute the config as is
• if no - display dropdawn with the list of the containers - the same behavior if the configuration doesn't contain containerName at all.

[sunix]

What is the value (for a developer) of editing a Che command from the tasks.json:

• he could test it right away, a kind of live reload of commands
• maybe some vscode extension rely on that (but I am not sure about that, it is to be verified)

But the most important thing is that the devfile is the place where you want to persist configuration because it is portable and you could share it afterwards.

We should have the devfile overrides tasks.json at workspace restart.

• Waiting for synchronisation mechanism, let's say to the user that the workaround is to copy the changes done in tasks.json to the devfile manually if the user would like to save these changes.
• We won't have to make any tricks to fix this current issue.
• Missing implementations for synchronization wouldn't involve undoing things that we would add to fix that current issue.
• It is the simplest solution in short and long term. Simplest is usually the best option :)
• Merging tasks.json and devfile is absolutly something to avoid as it would be error/bug prone and something we would eventually get rid of if we have a proper synchronisation.

[amisevsk]

We should have the devfile overrides tasks.json at workspace restart.

Editing tasks.json and having those changes persisted is much more valuable to me than having changes to the devfile override tasks.json.

Editing devfiles is an enormous pain (see #13982) and not easily accessible. OTOH, I've frequently modified tasks.json to update or change commands, even just to test devfile improvements. If the server is to overwrite my tasks.json with what's in the devfile, that would be bad UX.

In an even more involved use-case, having the devfile overwrite tasks.json would make working in a Che workspace for longer than "trying it out" very painful, since every task change would require

1. Copy devfile config to external editor (and understand its structure)
2. Modify commands fields (knowing that they will override tasks.json
3. Restart workspace

In environments where your workspace is stopped after 30 minutes of idling, this completely ends usability.

[RomanNikitenko]

@gorkem
Thank you very much for the idea!
We could do mapping before running a task on the task resolving step, but looks like I need help of platform team, so I created the corresponding issue #14280

[sunix]

Hello I have just been through this issue during a live demo at a developer conference. Could we consider a fix for this?
Thanks

[vparfonov]

@sunix What your blocker label means we don't release 7.3.0?

[sunix]

@vparfonov it means it makes commands that would run in plugin sidecar containers unsuable after restart. So plz fix it ASAP. I put it blocker because it is breaking demos we are doing at conferences (and customer demos).
It has been set for 7.0.0 and p1 initially. I am just hoping that we get that fix one day.