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

It is not possible to run more than one Che 7 workspace at a time #12238

Closed
l0rd opened this issue Dec 19, 2018 · 16 comments
Closed

It is not possible to run more than one Che 7 workspace at a time #12238

l0rd opened this issue Dec 19, 2018 · 16 comments
Labels
kind/enhancement A feature request - must adhere to the feature request template.

Comments

@l0rd
Copy link
Contributor

l0rd commented Dec 19, 2018

Description

If a Che 7 workspace is running and I try to start a new one I get the following error:

Message: services "theia" already exists

image

One possible solution to address this problem would be to suffix the name of services with the workspace id:

  • suffixing services with workspaceid would not be a problem for theia remote plugins as of today because they don't use the hostname but localhost (since everything run in the same pod)
  • That may be a problem for some plugins anyway that want to contact a given service (e.g. the che hello world plugin)
  • We fix this in 2 steps:
    1. prefixing the name of the services with the workspace id when starting a che 7 workspace: that make it easy to start workspaces
    2. extending the API of the che plugin that @evidolob and IDE 2 are working on to retrieve the real endpoint of a service: that make it easy for plugins developers to consume other plugins APIs
@l0rd l0rd added kind/enhancement A feature request - must adhere to the feature request template. target/che7 labels Dec 19, 2018
@sleshchenko
Copy link
Member

It is caused by the issue: service discovery of APIs running in tooling containers of Workspace.Next #9913

@l0rd
Copy link
Contributor Author

l0rd commented Dec 20, 2018

@sleshchenko I have added some notes about a possible solution. That's something we discussed with @benoitf last week. cc @evidolob

@skabashnyuk
Copy link
Contributor

Linking original PR made by @garagatyi #10528

@ibuziuk
Copy link
Member

ibuziuk commented Dec 20, 2018

@l0rd is this issue a priority for the next sprint ?

@slemeur slemeur mentioned this issue Dec 20, 2018
69 tasks
@l0rd
Copy link
Contributor Author

l0rd commented Dec 20, 2018

@ibuziuk no

@garagatyi
Copy link

@l0rd

suffixing services with workspaceid would not be a problem for theia remote plugins as of today because they don't use the hostname but localhost (since everything run in the same pod)

I wonder why you think that they use localhost?

prefixing the name of the services with the workspace id when starting a che 7 workspace: that make it easy to start workspaces

Do you mean prefixing only services created by plugins or also those that are in the user recipe of a workspace?

extending the API of the che plugin that @evidolob and IDE 2 are working on to retrieve the real endpoint of a service: that make it easy for plugins developers to consume other plugins APIs

For the start we might be OK with just having an env var with workspace ID and knowing that we need to prefix service name with it prefix.

@l0rd
Copy link
Contributor Author

l0rd commented Dec 20, 2018

I wonder why you think that they use localhost?

Because if you started an HTTP service on port 80 of container A, you can reach it as http://localhost:80 from container B (assumed that A and B are in the same pod). That's how Theia plugin adapter talk with the original Theia container right now (@benoitf can correct me if I am wrong)

Do you mean prefixing only services created by plugins or also those that are in the user recipe of a workspace?

That's a good question. But no we should not prefix services defined in the user runtime. It will be the user responsibility not make them collide with other services and that it's perfectly fine because he should complete control of the services running in the namespace.

For the start we might be OK with just having an env var with workspace ID and knowing that we need to prefix service name with it prefix.

That's a hack that plugin developer should apply in their plugins and we don't want because their plugins may be broken as soon as we fix it in the right way. We want to go with the clean solution right away.

@garagatyi
Copy link

@l0rd actually our plugins don't use localhost. They use k8s Service. And this is a PITA. I have a POC for this but it leads to:

  • Che master knows that when Che plugin endpoint gets converted to k8s Service it needs to prefix the name
  • VS Code and Theia brokers create Endpoint with name port4321 and insert environment variable to theia container with a value "ws://workspace123456789port4321:4321"

This solution makes us split the knowledge about prefixing k8s services with workspace ID among different components with different lifecycles. You mentioned that we want to go with a clean solution and I believe that this solution is not clean solution at all.

extending the API of the che plugin that @evidolob and IDE 2 are working on to retrieve the real endpoint of a service: that make it easy for plugins developers to consume other plugins APIs

We do not show internal plugin endpoints in Che workspace API at the moment.

Hello world plugin doesn't work as it was expected.

@garagatyi
Copy link

Here is another way to implement this:
Brokers would add workspace ID to endpoint names, while Che master would behave as it is now. This would isolate changes in brokers only.
Theia and VS Code brokers actually don't care what is the name of the endpoint since they just put it into the env var needed by Theia.
Common che plugin would have to use API to get server it needs. As far as I understand exec which is the only plugin of that type already does it.
So such changes would not break existing plugins because of above-described reasons.

I'm going to provide PR with this solution. If somebody has any thought why should we go to another direction, please, comment.

@garagatyi
Copy link

@l0rd @sleshchenko FYI

@garagatyi
Copy link

Just discussed this idea with @sleshchenko and he pointed out that Theia editor server name would be changed. Which is not OK. So, can't proceed with this solution

@garagatyi
Copy link

One problem with the solution suggested in the issue description is that since service discovery would be dependent on the workspace ID we can't just cache brokering results without modifying on each request with a certain workspace ID.

@garagatyi
Copy link

garagatyi commented Jan 25, 2019

A quick win could be:

  • Make Theia and exec not discoverable (the feature is already implemented), so they would not conflict when several workspaces are running. AFAIK nobody accesses them using service discovery now
  • Make Theia and VS Code brokers generate random endpoint names, so they would not conflict either

And here is a wider explanation of options we have and what are the pros and cons:

What we do now:

  • VS Code and Theia brokers:
    • Create endpoint with name port5690
    • Create Env Var to pass info about the plugin to theia container with value ws://port5690:5690
  • Che master creates k8s service with name equal to the endpoint name
  • Che plugin broker (the first one that uses tar.gz)
    • Gets endpoint from the configuration YAML
  • Che master creates k8s service with a name equals to the endpoint name for this broker

What options we have to allow running several workspaces in a single k8s namespace:

  1. Prefix with workspace ID service on wsmaster
  • VS Code and Theia brokers:
    • Create endpoint with name port5690
    • Create Env Var to pass info about the plugin to theia container with value (brokers know that endpoint will have one name but would be discoverable only after prefixing) ws://workspace12345678port5690:5690
  • Che master creates k8s service with a name that consists of workspace ID and endpoint name workspace12345678port5690​
  • Che plugin broker (the first one that uses tar.gz)
    • Gets endpoint from the configuration YAML, e.g. theia
  • Che master creates k8s service with a name that consists of workspace ID and endpoint name workspace12345678theia

Pros:

  • simple to implement
    Cons:
  • Brokers behavior is not consistent: if discoverability is needed endpoint should have a name that differs from the name used for service discovery
  • if we expose these k8s services over API as Che servers (we don't at the moment) then server name would be also inconsistent either with the endpoint or k8s service
  • if we want to move brokering process from workspace start to registry than we can't statically evaluate this since we don't have workspace ID. We will need to have a placeholder in the env var.
  1. Let brokers prefix endpoint with workspace ID themselves:
  • VS Code and Theia brokers:
    • Create endpoint with name workspace12345678port5690
    • Create Env Var to pass info about the plugin to theia container with value ws://workspace12345678port5690:5690
  • Che master creates k8s service with name equal to the endpoint name
  • Che plugin broker (the first one that uses tar.gz)
    • Gets endpoint from the configuration YAML, e.g. theia
  • Che master creates k8s service with a name that consists of workspace ID and endpoint name workspace12345678theia

Pros:

  • brokers behavior is consistent: if service discovery is needed broker uses the same name for discovery and endpoint
  • if we expose these k8s services over API as Che servers (we don't at the moment) then server name would be also consistent with endpoint and k8s service
    Cons:
  • if we want to move brokering process from workspace start to registry than we can't statically evaluate this since we don't have workspace ID. We will need to have a placeholder in the env var.
  • If some broker implementation would not prefix endpoint with workspace ID multiple workspaces with the same plugin would not start. We don't actually have third-party broker implementation and this can be avoidable by reading docs. Doesn't seem a problem for me now.
  1. Make service discovery depend on env vars instead of predictable service name:
  • Make Theia support a new way of discovering remote plugins runners:
    • Instead of just a link by which remote Theia is accessible use value env:SOME_ENV which means that actual value is written in SOME_ENV
  • VS Code and Theia brokers:
    • Create endpoint with name port5690
    • Create Env Var with value env:CHE_PLUGIN_ENDPOINT_port5690
  • Che master:
    • Creates k8s service with a name that consists of workspace ID and endpoint name workspace12345678port5690
    • Inserts following env var into all workspace containers: CHE_PLUGIN_ENDPOINT_ in our case CHE_PLUGIN_ENDPOINT_port5690 with value ://:
  • Che plugin broker (the first one that uses tar.gz)
    • Gets endpoint from the configuration YAML, e.g. theia
  • Che master creates k8s service with a name that consists of workspace ID and endpoint name workspace12345678theia and inserts env var CHE_PLUGIN_ENDPOINT_theia with value http://workspace12345678theia:3000 so if some plugin wants to connect to it it can read env var with a predictable name

Pros:

  • Consistent look of endpoint and server
  • Allows us to statically evaluate plugins config in the registry instead of on workspace start
  • Doesn't make broker knows about prefixing which is not broker responsibility
  • Simple changes in brokers
    Cons:
  • more complex change
  • require the addition of a new way of discovering remote plugin runner in Theia if env var value is env:SOME_ENV then read the value from SOME_ENV

I would go with option 3 and here is how to do it in an iterable way:

  1. Disable discoverability in endpoints that don't need it at the moment theia, exec sidecar
  2. Make VS Code and Theia brokers generate endpoints with workspace ID OR some random chars
  3. at this stage it would be possible to start several workspaces at the same time
  4. Add support of env:SOME_ENV to Theia
  5. Rework brokers to leverage this feature.

DONE!

@garagatyi
Copy link

After a discussion with @l0rd and @ibuziuk, we agreed to disable creation of services for Theia and exec sidecars since it is not used at the moment and make Theia, VS Code brokers generate random names for services to avoid collisions.

@garagatyi
Copy link

I recalled that we had Fortune and Hello world plugins that are using sidecars and rely on predictable sidecar URLs. They need to be adapted to fetch endpoint URL from Che master API.
When one of these plugins are used by the same user in more than one workspace in the same kubernetes namespace workspaces started after the first one would fail to start.

@garagatyi
Copy link

It seems that plugins like these can make UX worse, but since we do not expect heavy usage of plugins other than Theia and VS Code type this should not be a problem for the time being.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A feature request - must adhere to the feature request template.
Projects
None yet
Development

No branches or pull requests

5 participants