-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
To start VS Code, use post-start
rather than overriding the entrypoint
#21879
Comments
post-start
rather than overriding the entrypoint
@RomanNikitenko I have been testing: 1. With this editor definition that uses 2. With this editor definition that uses a poststart event (link to start it on dogfooding instance) but it's failing with message
and I noticed that the DWT doesn't have the |
About the second option. Yes, before our meeting I tried to set the command for the Then I went forward and tried to set the command for the I continue to investigate the problem... |
Update for #21879 (comment) It looks like the entrypoint is not executed when entrypoint_from_terminal.mp4 |
depends on #21971 |
Today I've tested on the
The result is: the error So, I guess, some changes are required on the |
DWO issue devfile/devworkspace-operator#1029 has an impact on this. I've opened a PR to fix this in upstream DWO: devfile/devworkspace-operator#1033 |
Historically, it was (maybe still is) necessary to create entries in If I remember right when I configured a web-terminal-tooling image to use |
This would be great. The last time I looked I found that cri-o is patching automatically |
Update:
At the same time there is still a problem when I create a workspace from the dashboard. So - I guess:
I'm going to test similar changes for the JetBrains editor. |
About changes on the dashboard side (please see my previous comment) - I think resolving this issue should fix the problem. |
Looking into using alternative shells, che-code, (current
To test setting
To summarize, as far as I can tell: If In addition, |
Thank you @amisevsk this is great news. We should get rid of the patches to the UDI /etc/passwd now! I will create a separate issue. |
When I tested it against OpenShift 4.11/4.12, I didn't get the cri-o injected entry in However, if UDI images installed alternate shells (e.g. zsh), we could enable switching shells by just setting an environment variable in the devfile, which would be a nice bonus. I'll create an issue to follow up here, as we're getting off-topic for this issue. Edit: Created issues to move discussion out of this thread: |
I noticed one small side-effect of adding the command of
The best option for hiding it - filter in Che-Code by the well-known ID @RomanNikitenko WDYT about ^^? I used the test project provided in the PR eclipse-che/che-plugin-registry#1566. |
@azatsarynnyy About filtering by ID - I don't like hardcoding in general:
From architecture point of view, I would say, we should have some mechanism/approach how to filter out In this case the command was defined in the |
@RomanNikitenko We can filter them out by the |
The editor-contributed Devfile Commands are filtered out now - che-incubator/che-code#192 |
I'm concerned the approach to filtering editor-contributed commands is potentially fragile, as the |
Thanks @amisevsk! I implemented your suggestion in che-incubator/che-code#194. Now, it doesn't depend on the |
Is your enhancement related to a problem? Please describe
VS Code
che-code-runtime-description
component contributes the containercommand
to the tooling container:As a result, the script
/checode/entrypoint-volume.sh
is executed at workspace startup, but the tooling container entrypoint (for example UDI's) is not.This is problematic as the tooling container entrypoint may be used to initialise development tools and runtimes.
Describe the solution you'd like
We should use a
post-start
event rather than thecommand
to start VS Code.That should be tested though and may require some tweaks to the VS Code startup script (or not).
This is currently VS Code specific but will JetBrains and Theia may be affected too if this feature is implemented in the meantime.
The text was updated successfully, but these errors were encountered: