Che: Theia remote plug-in runtime in sidecar container improvement

[evidolob]

Description

Currently we have base image for remote plugin. So all remote plugin images should inherit it, which is not convenient by it self, and heavily increase remote plugin image build time. Also we have some problems when Thiea IDE and remote plugin runtime has different version(when some one doesn't update remote plugin image) which leads to very strange and difficult to detect bugs.

We can improve that with two options:

1. Add che-theia remote plugin runtime binary at the end of plugin image build
2. Mount and run che-theia remote plugin runtime binary on che workspace start

[evidolob]

@benoitf WDYT?

[benoitf]

@evidolob

yes I think steps are:

1. Move theia-endpoint outside of dockerfiles folder
2. Produce all-in-one binary of the package in another foler
3. @davidfestal is proposing as well to copy binary in a volume so basically we should try to have image without the runtime inside as well. (let's pick up one of the current images) and to provide binary inside a docker image (to allow to grab this binary from a init container)
4. move the insert of the binary to the end of images instead of inheriting parent endpoint image (so layers are re-used at their best) . (and if David is able to run binary outside we could avoid this step and just prune endpoint from all images)

[AndrienkoAleksandr]

1. Move theia-endpoint outside of dockerfiles folder

Seems, done by @tolusha https://github.com/eclipse/che-theia/pull/254

1. Produce all-in-one binary of the package in another foler

I complete investigation.
node.js doesn't has embedded mechanism to build executable image from the box(it's not golang...). So to build executable binary we need to use third party libs. But all of them has own pluses and minuses. To build remote-plugin-endpoint I've got not bad result only with nexe tool, but this tool has own limitation too: doesn't support dynamic require and native node libs(native node libs - it's code written on non node, f.e on the c or c++, but wrapped with help node). So for remote-plugin-endpoint we should not use these features to prevent issues with nexe.
So, To package all-in-one binary I experemented with 4 tools:

pkg - doesn't work for our remote-plugin code, because our code uses getMac dependency(like transitive dependency). pkg unable to resolve such kind of libs, see more https://github.com/zeit/pkg/issues/519

node-packer - remote-plugin-endpoint binary compiled with help this tool doesn't work for alpine images at all (https://github.com/pmq20/node-packer/issues/127), and node-packer executable binary doesn't work under alpine too. On the github repo we can see that source code contains node source code with version 8.3.0, but we need node 10.

appImage - main idea - package remote-plugin with help electron and generate with help electron-builder lib binary in the universal linux binary format - AppImage. But such binaries doesn't work in the docker containers, because appImage uses fuse filesystem, which works for docker containers only with privilege mode. Also fuse filesystem need fuse filesystem libs - I could not find any base linux images where are these dependencies are pre-installed.

nexe - I generated binary with platform flag alpine, but this binary works for all another containers which I tested. I tested this binary with images:

• registry.access.redhat.com/ubi8/ubi-minimal
• ubuntu:18.04
• debian:9
• fedora:30
• centos:7
• busybox
• node:10.15-alpine
• opensuse/leap
• cirros
• clefos
• mageia:7
• photon:latest
• proton
• alt
• amazonlinux
• open-liberty
• php
• oraclelinux
• golang:stretch
• alpine

[AndrienkoAleksandr]

All-in-one binary branch https://github.com/eclipse/che-theia/compare/remoteBinary

[AndrienkoAleksandr]

Binary size 136.8Mb

[AndrienkoAleksandr]

@davidfestal is proposing as well to copy binary in a volume so basically we should try to have image without the runtime inside as well. (let's pick up one of the current images) and to provide binary inside a docker image (to allow to grab this binary from a init container)

Hello, @davidfestal Did you mean, that we can use plugin-broker to serve remote-plugin-endpoint binary, and Eclipse CHE master could create volume for remote plugin containers and copy binary inside volume?

[davidfestal]

Well, initially my general idea was that we could provide such binaries (or any additional standalone files) in very lightweight containers that would contain:

• the files to inject,
• a lightweight cp utility (such as the busybox one)
• An env variable that indicates where to copy the provided files to inject.

Then it would be added as an initContainer in the workspace POD with a volume and the right env variable.

Such lightweight container images could be either created at build time (for the remote-plugin-endpoint) or even created on-the-fly by the plugin broker (for plugin archives for example).

[AndrienkoAleksandr]

@davidfestal Do you have a plan to implement your initial idea or this part of task should be done by ide2 team?

[davidfestal]

In fact it seems to me that it would be sufficient to simply:

• add a single optional boolean initContainer field (false by default) on the CheContainer model class,
• and add all the containers with a true value as init containers in the workspace POD.

All the rest of the work would be done in the plugin broker.

However, if I find some time I might be able to prepare the plugin-broker part of this work and test it with the Che Workspace CRD Operator POC.

[davidfestal]

@l0rd Should I open a separate issue for this ?

[l0rd]

@davidfestal ok to create a separate issue. But I have some comments:

About the build of the init-container

👍 to have an init container that copies an all-in-one binary in a volume but 👎 to build the init container image during the workspace bootstrap

About the initContainer field

If that means that we should add these containers in the meta.yaml of every vs-code extension or theia plugin I am 👎. If I am a user and want to add a VS Code extension in my workspace I should only provide 2 information:

• the reference or id to the VS Code Extension
• the sidecar image that will host my VS Code Extension

I should not care about how the Theia runtime is injected.

I am 👍 if instead there would be no mention of such init containers in the meta.yaml of the plugins: the plugin-broker automatically adds a theia-remote-runtime-copier initContainer for every theia-plugin component that needs to run in a sidecar.

But the specification of the theia-remote-runtime-copier image (what name and version) should be specified as an attribute in the che-theia meta.yaml to guarantee consistency between che-theia and the all-in-one runtime.

And I would call the field isInitContainer rather than initContainer.

About the plugin sidecar

Take into consideration that the sidecar container image that will run the all-in-one binary should match the image used in our default stacks. For example the maven image in the maven stack should be used for both the build/runtime container and the jdt LS sidecar containers: 2 containers based on the same image guarantee consistency and take less time to be pulled.

[davidfestal]

@davidfestal ok to create a separate issue.

Great.

But I have some comments:

About the build of the init-container

👍 to have an init container that copies an all-in-one binary in a volume but 👎 to build the init container image during the workspace bootstrap

OK. As soon as we have the initContainer mechanism in place in the Che server, we are free to decide how and when we build this containers.

>

About the initContainer field

If that means that we should add these containers in the meta.yaml of every vs-code extension or theia plugin I am 👎. If I am a user and want to add a VS Code extension in my workspace I should only provide 2 information:

• the reference or id to the VS Code Extension
• the sidecar image that will host my VS Code Extension

I should not care about how the Theia runtime is injected.

I am 👍 if instead there would be no mention of such init containers in the meta.yaml of the plugins: the plugin-broker automatically adds a theia-remote-runtime-copier initContainer for every theia-plugin component that needs to run in a sidecar.

That was my idea.

But the specification of the theia-remote-runtime-copier image (what name and version) should be specified as an attribute in the che-theia meta.yaml to guarantee consistency between che-theia and the all-in-one runtime.

So will the user have to ensure consistency manually ? or couldn't we use, by default, an image name based on the name of the docker image of the che-theia editor plugin ?

And I would call the field isInitContainer rather than initContainer.

Sure.

About the plugin sidecar

Take into consideration that the sidecar container image that will run the all-in-one binary should match the image used in our default stacks. For example the maven image in the maven stack should be used for both the build/runtime container and the jdt LS sidecar containers: 2 containers based on the same image guarantee consistency and take less time to be pulled.

I don't see how this is related to my proposal. This is already the case and totally depends on what the meta.yaml will contain. The good thing brought by this proposal is that the sidecar container that will run the all-in-one binary doesn't have to have it inside the docker image. It will have it available in a shared volume (having been copied by the new init container). And we will override its entrypoint to run the all-in-one binary at start. So, I assume that, in the Maven case mentioned above, it would even be the base maven image.
Or am I missing some point ?

[AndrienkoAleksandr]

Current implementation state
List pull requests:
https://github.com/eclipse/che-theia/pull/410
https://github.com/eclipse/che-plugin-registry/pull/210
https://github.com/eclipse/che/pull/14333
https://github.com/eclipse/che-plugin-broker/pull/71

To launch remote plugins we have node application https://github.com/eclipse/che-theia/tree/master/extensions/eclipse-che-theia-plugin-remote. For this application we had separated image eclipse/che-theia-endpoint-runtime. And all remote plugins extended this image, but it’s an issue for us.

Proposed enhancements:
In the pulll request https://github.com/eclipse/che-theia/pull/410 was introduced changes:
we compile application https://github.com/eclipse/che-theia/tree/master/extensions/eclipse-che-theia-plugin-remote to the binary with help of nexe tool. Compiled binary contains node runtime, that’s why this binary doesn’t need to have pre-installed node in the image.
Also we need pick up this binary to the remote plugin containers and start this binary, let’s call it “remote plugin runtime injection”.
To inject remote plugin binary we need to change plugin-broker model and che-server.

I proposed to define in the plugin meta.yaml PluginPatcher object:

type PluginPatcher struct {
// List init containers
    InitContainers []Container `json:"initContainers" yaml:"initContainers"`

     // List plugin types matched to PluginPatcher
    PluginTypeMatcher []string `json:"pluginTypeMatcher" yaml:"pluginTypeMatcher"`

     // Command to patch original plugin container command
    PluginContainerCommand []string `json:"pluginContainerCommand" yaml:"pluginContainerCommand"`
    // Argument to patch original plugin container arguments
    PluginContainerArgs []string `json:"pluginContainerArgs" yaml:"pluginContainerArgs"`
}

PluginPatcher has initContainers. eclipse/che-theia-endpoint-runtime image with remote binary we are using like init container. Main goal of this init container => copy remote binary to the volume plugins. I proposed to apply PluginPatcher to the ChePlugin:

type ChePlugin struct {
    … 
   PluginPatcher PluginPatcher 
}

Main idea: we are using PluginPatcher with editor ChePlugin. Che server take a look on the list workspace plugins, find editor plugin, get PluginPatcher. Than che-server get list of the init containers from plugin patcher and start them on workspace start. After init container we have copied remote binary in the volume plugins. Than che-server override root container command for vscode and theia plugin containers to start remote binary. When remote plugin containers started, each remote binary in the container with help of env variables detect were is (vscode Extension)/(che-theia plugin), start it and connect to the che-theia editor. remote binary and editor are always compatible by the same code version, because we defined in the meta.yaml of the eclipse/che-theia in the plugin registry compatible che-theia image and remote binary image:

#che-theia meta.yaml definitiion
apiVersion: v3
publisher: eclipse
name: che-theia
...
spec:
  endpoints:
  ....
  pluginPatcher:
    pluginTypeMatcher: ["vs code extension", "theia plugin"]
    pluginContainerCommand: ["sh", "-c"]
    pluginContainerArgs: ["/plugins/remote-launcher/entrypoint.sh"]
    initContainers:
      - name: theia-remote-plugin-launcher
        image: eclipse/che-theia-endpoint-runtime:next
        initContainer: true
        command: ['cp']
        args: ['-rf', '/remote-plugin-launcher', '/plugins/remote-launcher']
        volumes:
          - mountPath: "/plugins"
            name: plugins
  containers:
   - name: theia-ide
     image: "docker.io/eclipse/che-theia:next"
     env:
       ....
     volumes:
       ...
     ports:
       ...
     ...

@sleshchenko @mario @evidolob @davidfestal @benoitf @amisevsk what do you think?

[sleshchenko]

@AndrienkoAleksandr Some discussion already is happening here https://github.com/eclipse/che-plugin-broker/pull/71#discussion_r318062445

[tsmaeder]

@AndrienkoAleksandr we already have a "containers" field in the plugin meta.yml. Wouldn't adding a "initiContainers" solve the problem? Every version of the Che Theia editor plugin would come with an init container that copies the correct plugin runtime to a shared volume.
I feel this would be much simpler and more general.

[davidfestal]

@tsmaeder: that's although my thought. cf. my comment here: https://github.com/eclipse/che-plugin-broker/pull/71#discussion_r318508090

[l0rd]

@tsmaeder and @davidfestal solution looks simpler. But it doesn't solve the problem of overriding the sidecar container entrypoint. @AndrienkoAleksandr do we really need to override the sidecar entrypoint? Can't we assume that it's handled in the plugin meta.yaml?

[davidfestal]

If the initContainer we add had a volume mount to the /plugins volume, it would be able to copy the exe there itself, and wouldn't require overriding the side container entrypoints ?

[davidfestal]

And this initContainer could be added by the plugin broker itself, since it already knows the specifics of each type of plugin. It seems to me that part of the complexity of the solution finally proposed is due to the fact that we pushed the implementation of this into the Che server Kubernetes infrastructure.

[davidfestal]

Also implementing this injection logic in the plugin broker seems more consistent to me with 2 facts:

• the plugin broker already gathers files + specifics of each types of plugins.
• the plugin broker is reused by the POC of the Che Workspace CRD operator. So plugin broker implements defailed plugin management logic that can be used independently of the central Che server. That also seems an important point for the future.

[AndrienkoAleksandr]

@l0rd : Can't we assume that it's handled in the plugin meta.yaml

Potentially we can hard code in the each remote plugin meta.yaml commands to run remote binary. But if we change something in this mechanism, then we need to patch meta.yamls for all remote plugins...

[davidfestal]

@AndrienkoAleksandr

Pease could you look into this comment from @l0rd : https://github.com/eclipse/che/issues/13387#issuecomment-499115833 and the answer I made just after ?

let me quote a part:

I am 👍 if instead there would be no mention of such init containers in the meta.yaml of the plugins: the plugin-broker automatically adds a theia-remote-runtime-copier initContainer for every theia-plugin component that needs to run in a sidecar.

[tsmaeder]

The init container would only have to be mentioned in the che-theia editor plugin. Theia plugin sidecars would expect the "pluginhost.exe" to be at a fixed location in the shared /plugins folder.

[AndrienkoAleksandr]

I am +1 if instead there would be no mention of such init containers in the meta.yaml of the plugins: the plugin-broker automatically adds a theia-remote-runtime-copier initContainer for every theia-plugin component that needs to run in a sidecar.

I just would like to clarify this sentence the plugin-broker automatically adds a theia-remote-runtime-copier initContainer with the another quote from the same comment:

But the specification of the theia-remote-runtime-copier image (what name and version) should be specified as an attribute in the che-theia meta.yaml to guarantee consistency between che-theia and the all-in-one runtime.

@davidfestal : If the initContainer we add had a volume mount to the /plugins volume, it would be able to copy the exe there itself, and wouldn't require overriding the side container entrypoints ?

We override entrypoints for containers to run injected binary but not copy exec. Sorry, you've already mentioned in https://github.com/eclipse/che/issues/13387#issuecomment-499395581

And we will override its entrypoint to run the all-in-one binary at start.

Next remark

@davidfestal we pushed the implementation of this into the Che server Kubernetes infrastructure.

Actually, logic of patching che plugin configuration (like overriding entrypoints) could be moved to che-plugin-broker and then Che Server would just use ready-to-use plugin configurations.

Next remark

@tsmaeder The init container would only have to be mentioned in the che-theia editor plugin.

Yes, In the current implementation I used init containers only for che-theia editor. And for next round enhancements we can save this approach.

@tsmaeder Theia plugin sidecars would expect the "pluginhost.exe" to be at a fixed location in the shared /plugins folder.

I am not against this way, but for me it's looks strange(Maybe I am wrong). We hard code in the image or plugin meta.yaml path to the magic binary, which should appear. And we do that for all remote plugins. But if it's ok, then we need at least apply information about that magic stuff in the docs... And one more minus of this approach if we will change remote binary path or mechanism of injection, all previous version of the plugins could be broken with newer che...
I guess remote runtime injection definition should be bound to the che-theia editor version and definition, for remote plugins it would be nice to hide detail of this mechanism(like path and so on).

From another point overriding entypoint for container has also minus. I talked with @mmorhun. He has CHE plugin https://github.com/mmorhun/che-plugin-registry/blob/master/v3/plugins/mm4eche/git-ui-tools/latest/meta.yaml and this plugin has two containers:

• first container with che-theia plugin. It should be launched with remote endpoint runtime.
• second container with VNC installed inside. And second container has own entrypoint to start VNC.
  If we will override entrypoint, than VNC won't start for this container. To cover this case we need to join two entrypoints into one or mark some containers, which don't need remote plugin runtime injection.

[davidfestal]

Sorry, you've already mentioned in #13387 (comment)

And we will override its entrypoint to run the all-in-one binary at start.

Not sure I was meaning the same as you here, but never mind.

Next remark

@davidfestal we pushed the implementation of this into the Che server Kubernetes infrastructure.

Actually, logic of patching che plugin configuration (like overriding entrypoints) could be moved to che-plugin-broker and then Che Server would just use ready-to-use plugin configurations.

I think that might be great.

Next remark

@tsmaeder The init container would only have to be mentioned in the che-theia editor plugin.

Yes, In the current implementation I used init containers only for che-theia editor. And for next round enhancements we can save this approach.

@tsmaeder Theia plugin sidecars would expect the "pluginhost.exe" to be at a fixed location in the shared /plugins folder.

I am not against this way, but for me it's looks strange(Maybe I am wrong). We hard code in the image or plugin meta.yaml path to the magic binary, which should appear. And we do that for all remote plugins. But if it's ok, then we need at least apply information about that magic stuff in the docs... And one more minus of this approach if we will change remote binary path or mechanism of injection, all previous version of the plugins could be broken with newer che...

Can't you manage that with environment variables added to both the init container and the theia plugin side-car containers ? These environment variables would be set by the plugin broker (in the ChePlugin.Container.Env field I assume)? And the plugin-broker could make this value configurable.

I guess remote runtime injection definition should be bound to the che-theia editor version and definition, for remote plugins it would be nice to hide detail of this mechanism(like path and so on).

From another point overriding entypoint for container has also minus. I talked with @mmorhun. He has CHE plugin https://github.com/mmorhun/che-plugin-registry/blob/master/v3/plugins/mm4eche/git-ui-tools/latest/meta.yaml and this plugin has two containers:

* first container with che-theia plugin. It should be launched with remote endpoint runtime.

* second container with VNC installed inside. And second container has own entrypoint to start VNC.
  If we will override entrypoint, than VNC won't start for this container. To cover this case we need to join two entrypoints into one or mark some containers, which don't need remote plugin runtime injection.

[amisevsk]

If I'm understanding correctly, we're planning on copying the all-in-one binary into an attached volume on the workspace pod. I've got a few questions about this approach:

• Will this work for ephemeral workspaces?
• Won't this increase PVC usage by ~100Mi for every workspace?
• Is there any timing data on how long the copy takes on an actual cluster? Gluster can be very slow and has been the source of issues in the past.

[l0rd]

@amisevsk if we don't use a volume we need to repeat the copy operation for every sidecar. And I would not use a PersistentVolume (Gluster or Ceph) but rather an ephemeral volume (for the reasons that you have mentioned above)

[amisevsk]

And I would not use a PersistentVolume (Gluster or Ceph) but rather an ephemeral volume

I agree that that makes sense. This does mean that the current PRs need to be reworked, as they copy into /plugins

[tsmaeder]

@AndrienkoAleksandr we need some protocol to provide the plugin host binary to the sidecar container. This requires us to make assumptions of what is supposed to happen on the editor and/or plugin host side. We break this protocol, we break all containers using it. I really don't see a way around that. If our protocol is "the editor provides a suitable plugin host binary at path ", I think that is a very simple and robust protocol. It also can be implemented very easily with minimal (and generally useful) changes to the plugin broker.

@amisevsk conceptually, it makes sense to use a ephemeral volume, as the editor plugin version (and thus the plugin host binary) could change at each workspace start.

[AndrienkoAleksandr]

@sleshchenko proposed split work to incremental changes, so first iteration: Add an ability to add init containers in meta.yaml).
PR's:

• https://github.com/eclipse/che/pull/14397
• https://github.com/eclipse/che-plugin-broker/pull/72

Test devfile:
https://gist.github.com/AndrienkoAleksandr/96566651e7af12e0ec64aaabf21e5acf

Remaining:

• [ ] Add an ability to override command and args;
• [ ] Add an ability to forcibly mark a volume as ephemeral (emptyDir, persistVolume:false);
• [ ] (Under discussion) Rework che-plugin broker: (providing fixed remote binary path for remote plugin images) vs (overriding entypoint remote plugins);

[AndrienkoAleksandr]

Second iteration.
PR's

• https://github.com/eclipse/che/pull/14453
• https://github.com/eclipse/che-plugin-broker/pull/73

Test devfile:
https://gist.githubusercontent.com/AndrienkoAleksandr/63fa6c598744af06a58a45761141c104/raw/e15f772d2b67c7bd124a023ff09e1b760acb36d9/gistfile1.txt

[AndrienkoAleksandr]

Third iteration(Add an ability to forcibly mark a volume as ephemeral persistVolume:false):
PR's

• https://github.com/eclipse/che/pull/14539
• https://github.com/eclipse/che-plugin-broker/pull/74

Test devfile:
https://gist.github.com/AndrienkoAleksandr/c5bdf9e40db4d30edad4cb07e36f4ec9

[nickboldt]

clearly still ongoing so slip to 7.3.0

[AndrienkoAleksandr]

Detected regression: https://github.com/eclipse/che/issues/14833 Working on fix.