Skip to content
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

Run vsx in same container if the container specs match #15373

Closed
benoitf opened this issue Dec 2, 2019 · 7 comments · Fixed by eclipse-che/che-plugin-broker#111
Closed

Run vsx in same container if the container specs match #15373

benoitf opened this issue Dec 2, 2019 · 7 comments · Fixed by eclipse-che/che-plugin-broker#111
Assignees
Labels
area/plugin-broker lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. severity/P1 Has a major impact to usage or development of the system.
Milestone

Comments

@benoitf
Copy link
Contributor

benoitf commented Dec 2, 2019

Is your enhancement related to a problem? Please describe.

As part of #15272 to add support of VS Code Extension Pack, there is a need to have several VS Code extensions running in the same container.

Describe the solution you'd like

Plug-in broker should use only one container for all VS Code extensions specified in a meta.yaml if the plug-in sidecar image is the same.

Example of current meta.yaml for redhat/java11:

...
spec:
  containers:
    - image: "docker.io/eclipse/che-remote-plugin-runner-java11:next"
      name: vscode-java
      memoryLimit: "1500Mi"
      volumes:
      - mountPath: "/home/theia/.m2"
        name: m2
  extensions:
    - https://github.com/microsoft/vscode-java-debug/releases/download/0.20.0/vscode-java-debug-0.20.0.vsix
    - https://download.jboss.org/jbosstools/static/jdt.ls/stable/java-0.50.0-1825.vsix

so if there is VS Code yaml with

repository: https://github.com/redhat-developer/vscode-xml
category: Language
firstPublicationDate: "2019-10-28"
spec:
  containers:
    - image: "docker.io/eclipse/che-remote-plugin-runner-java11:next"
      name: vscode-xml
      memoryLimit: "768Mi"
  extensions:
    - https://github.com/redhat-developer/vscode-xml/releases/download/0.9.1/redhat.vscode-xml-0.9.1.vsix

at the end, we should have only one container using the image docker.io/eclipse/che-remote-plugin-runner-java11:next and 3 vsix files will be provided in this container.

Note:
It works only if there is only one container defined or if we use spec/container instead of spec/containers

About the memory limit, for now it could be average or sum of memoryLimit

Describe alternatives you've considered

None

Additional context

JDT.ls need to have access to other jars files that are provided through VS code extensions.
Also some VS Code extensions may depend on other Extensions at runtime

@benoitf benoitf added the kind/enhancement A feature request - must adhere to the feature request template. label Dec 2, 2019
@che-bot che-bot added the status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. label Dec 2, 2019
@benoitf benoitf changed the title https://github.com/eclipse/che/issues/15272 Merge VS Code Extensions in a single container if they're using the same container definition Dec 2, 2019
@ibuziuk ibuziuk added severity/P2 Has a minor but important impact to the usage or development of the system. team/osio and removed status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. labels Dec 2, 2019
@ibuziuk
Copy link
Member

ibuziuk commented Dec 2, 2019

@benoitf I put P2, but feel free to update priority

@benoitf
Copy link
Contributor Author

benoitf commented Dec 2, 2019

I think this one is the most important one of the three related issues so maybe P1
cc @l0rd @davidfestal

@tolusha
Copy link
Contributor

tolusha commented Dec 3, 2019

It should be P1

@l0rd l0rd added severity/P1 Has a major impact to usage or development of the system. and removed severity/P2 Has a minor but important impact to the usage or development of the system. labels Dec 4, 2019
@l0rd
Copy link
Contributor

l0rd commented Dec 4, 2019

I have set it as P1. @benoitf @tolusha for the memory limit I would say that it should be the sum of the 2 memory limits.

@slemeur slemeur added kind/epic A long-lived, PM-driven feature request. Must include a checklist of items that must be completed. and removed kind/enhancement A feature request - must adhere to the feature request template. labels Dec 5, 2019
@amisevsk amisevsk mentioned this issue Dec 12, 2019
3 tasks
@sleshchenko sleshchenko removed the kind/epic A long-lived, PM-driven feature request. Must include a checklist of items that must be completed. label Dec 13, 2019
@sleshchenko sleshchenko added this to the Backlog - Controller milestone Dec 13, 2019
@amisevsk
Copy link
Contributor

Can anyone confirm that the remote plugin runtime injected by Theia will handle multiple extensions in this way?

@benoitf
Copy link
Contributor Author

benoitf commented Jan 13, 2020

there are already multiple VS code extensions for some plugins

@amisevsk amisevsk self-assigned this Jan 15, 2020
@che-bot
Copy link
Contributor

che-bot commented Jul 27, 2020

Issues go stale after 180 days of inactivity. lifecycle/stale issues rot after an additional 7 days of inactivity and eventually close.

Mark the issue as fresh with /remove-lifecycle stale in a new comment.

If this issue is safe to close now please do so.

Moderators: Add lifecycle/frozen label to avoid stale mode.

@che-bot che-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 27, 2020
@benoitf benoitf added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jul 27, 2020
@l0rd l0rd changed the title Merge VS Code Extensions in a single container if they're using the same container definition Run vsx in same container if the container specs match Jul 30, 2020
@sleshchenko sleshchenko modified the milestones: 7.18, 7.19 Aug 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/plugin-broker lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. severity/P1 Has a major impact to usage or development of the system.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants