Add init container to inject remote binary.

[AndrienkoAleksandr]

Referenced issue

eclipse-che/che#13387

What does this PR do?

Add init container to inject remote binary.

Earlier I had pr #210 where is I changed model to implement remote runtime injection, but some developers were against changing model. Current pr don't change model.

In this pr I propose to apply init container for che-theia editor plugin. Plugin broker should find this init container by name remote-runtime-injector and only for che-theia editor plugin. Then plugin-broker should analize env variables and volume from init container.

REMOTE_ENDPOINT_VOLUME_NAME env we are using to mark which one volume should be shared with other sidecar containers. In this volume init container on start should copy remote binary.

PLUGIN_REMOTE_ENDPOINT_EXECUTABLE env will be shared with all sidecar containers.

Depends on:
eclipse-che/che-theia#410
eclipse-che/che-plugin-broker#76

Signed-off-by: Oleksandr Andriienko oandriie@redhat.com

[benoitf]

hello, I think it's missing a lot of content in the description of the PR as well in referenced PR

[nickboldt]

What is the impact on docs for this change? #definitionOfDone

[amisevsk]

What is the timeline on moving the remote runtime injector to an eclipse-owned image repo?

[AndrienkoAleksandr]

hello, I think it's missing a lot of content in the description of the PR as well in referenced PR

I'm sorry, I hurry up yesterday and wrote bad description. I updated description.

[amisevsk]

How will this affect compatibility between versions of Che pre-remote injection and post-remote injection?

Is it possible to break existing workspaces by updating to a new release of the plugin registry?

[amisevsk]

I don't really like the idea of updating all our old versions of plugins, especially since it requires understanding how remote plugin runner starts up; I think the entrypoint update should either be done in the plugin runner image or by the plugin broker when it provisions a remote runtime injector.

Also, does overriding this remove the UID fixes in the generic remote runtime entrypoint?

[amisevsk]

Preferably the plugin runner images could be updated to automatically look for and execute the injected runtime executable.

[davidfestal]

I think the entrypoint update should either be done in the plugin runner image or by the plugin broker when it provisions a remote runtime injector

+1

[AndrienkoAleksandr]

Preferably the plugin runner images could be updated to automatically look for and execute the injected runtime executable.

@amisevsk @davidfestal one of our goal for remote binary is ability to reuse existed rhel, ubi images without patching and rebuilding this images for CodeReady workspaces. And there a lot of VsCode extension which could work without UID fixes. But to improve this flow we really can use alternative solution. In the image with remote binary we can provide one more file - some script entrypoint.sh. Instead of running remote-binary in a straight way we will start it using this script. And this script could do some additional work: on start take a look if in the root '/' located entrypoint.sh - than start it and detach, if we don't have this file script could try to do uid fix and the end execute remote binary.
Proposed draft content:

entrypoint.sh for remote binary

#!/bin/sh


fixUID() {
        # Ensure $HOME exists when starting
        if [ ! -d "${HOME}" ]; then
          mkdir -p "${HOME}"
        fi
        
        # Setup $PS1 for a consistent and reasonable prompt
        if [ -w "${HOME}" ] && [ ! -f "${HOME}"/.bashrc ]; then
          echo "PS1='\s-\v \w \$ '" > "${HOME}"/.bashrc
        fi
        
        # Add current (arbitrary) user to /etc/passwd
        if ! whoami 2>&1 /dev/null; then
          if [ -w /etc/passwd ]; then
            echo "${USER_NAME:-user}:x:$(id -u):0:${USER_NAME:-user} user:${HOME}:/bin/bash" >> /etc/passwd
          fi
        fi
}


if [ -f "/entrypoint.sh" ]; then
        /entrypoint.sh &
else 
        fixUID
fi

${PLUGIN_REMOTE_ENDPOINT_EXECUTABLE}

@amisevsk @davidfestal This solution is OK for you?

[amisevsk]

@AndrienkoAleksandr +1 to that suggestion; I'd prefer to not have to override entrypoint on plugin runner -- ideally we could add this functionality without changing meta.yamls in the plugin registry.

[nickboldt]

Should you do /entrypoint.sh & ? Why not just /entrypoint.sh; ? With an & the entrypoint script may run in parallel with ${PLUGIN_REMOTE_ENDPOINT_EXECUTABLE} ... is that what you want?

[nickboldt]

Also you're creating ${HOME} and then checking if it's writeable, but why not chmod & chown ${USER_NAME:-user} ${HOME} it so it IS writeable by the correct user, rather than checking it?

[amisevsk]

Also you're creating ${HOME} and then checking if it's writeable, but why not chmod & chown ${USER_NAME:-user} ${HOME} it so it IS writeable by the correct user, rather than checking it?

The way to do this would be to chmod -R g+rwX ${HOME}, I don't think chown would work in an entrypoint.

[AndrienkoAleksandr]

@amisevsk What is the timeline on moving the remote runtime injector to an eclipse-owned image repo?

It was merged.

@amisevsk How will this affect compatibility between versions of Che pre-remote injection and post-remote injection?
@amisevsk Is it possible to break existing workspaces by updating to a new release of the plugin registry?

For older theia runtime injection doesn't work, because in the old che-theia meta.yaml we don't have information about init container with runime injection, so they will work in the same way like earlier.

[AndrienkoAleksandr]

Should you do /entrypoint.sh & ? Why not just /entrypoint.sh; ? With an & the entrypoint script may run in parallel with ${PLUGIN_REMOTE_ENDPOINT_EXECUTABLE} ... is that what you want?

Hmm... I really meant process detach with &. Because I think about case when user hardcoded in the entrypoint.sh launching some endless process.
But OK, seems detach is really bad idea for scripts with UID fix. Because processes will be really parallel with remote runtime and we could not predict where is located entrypoint.sh. For example in the some code ready workspace images I saw entrypoint.sh is located not in the / folder.

[AndrienkoAleksandr]

@amisevsk @benoitf @nickboldt I created pr to apply entrypoint.sh for remote binary, but seems we don't have common opinion how it should be. Could we consider eclipse-che/che-theia#499 what should we do? Because it would be nice to merge remote binary feature earlier to save time for testing and we need it for code-ready.

[amisevsk]

Hey @AndrienkoAleksandr, sorry I forgot to update on this issue.

I think the way to finish this task would be to

• Remove entrypoint overrides from plugin meta.yamls
• Update plugin runner images to be structured

  ENTRYPOINT <whatever entrypoint script we need, e.g. UID fix>
  CMD <path to remote binary executable>
  

Basically, going forward (release 7.4.0 or whenever we enable runtime injection), plugin runner images should expect to have a ${PLUGIN_REMOTE_EXECUTABLE} and be written so that their entrypoint and command do not need to be updated at runtime.

What do you think about this solution? Is there a reason it wouldn't work?

[AndrienkoAleksandr]

Hello, I think it should work. But than we have two options: 1) remove override Dockerfile#Entrypoint => eclipse-che/che-plugin-broker#79 at all 2) Change code and instead of override Dockerfile#Entrypoint we can override Dockerfile#CMD . What is the best way 1) or 2) ? P.S. yes, for both cases we also should change images and remove entrypoints from plugin-registry.

[amisevsk]

@AndrienkoAleksandr I would favor option 1, but both make sense to me so whichever is easier to test/implement works. The downside to option 2 (plugin broker overrides dockerfile CMD (k8s args) is that it requires the plugin runner to have an entrypoint that includes exec $@, but that's fairly easy to manage.

[AndrienkoAleksandr]

@amisevsk @benoitf @sleshchenko pr updated.