Plugin Broker refactoring

[amisevsk]

This issue is to collect the various open design-level issues for the plugin broker and discuss the path forward.

Current Issues

• [1] Merge plugin brokers into a single image: Merge plugin brokers (init, unified) into a single container image build #14296
• [2] Plugin broker should only make necessary changes to plugins: Plugin broker should only make necessary changes to plugins #13452
• [3] Plugin broker as separate service shared among workspaces: [Spike] Investigate a possibility to move plugin broker to a separate service shared among all the workspaces redhat-developer/rh-che#1592
• [4] REST backend/merge plugin and devfile registries: Merge devfile and plugin registries #14268
• [5] Broker's role as a library in Che Operator

Current broker functionality

The current version of the plugin broker (v0.20), broadly, does two things:

• Compute workspace metadata changes necessary to run the requested plugins (e.g. env vars, sidecars, etc.)
• Download extension artifacts so that Theia/VSCode plugins can be run.

This is done in one image (ignoring the init broker for now); the broker takes plugin FQNs, downloads meta.yamls, converts those meta.yamls to Che Plugins, and downloads their artifacts.

Proposed changes

The plugin broker should be refactored into two images with separate concerns;

• One image (a metadata broker) is only responsible for computing Che Plugins from meta.yamls and returning that information to the Che server so that the workspace can be provisioned
• One image (an "artifact" broker) is only responsible for ensuring any files needed for plugins are present in their appropriate folders (/plugins or /plugins/sidecars)

A proof-of-concept implementation is available in my fork: https://github.com/eclipse/che-plugin-broker/compare/master...amisevsk:refactor-brokers?ts=4

These changes would address the issues listed above

• Plugin broker as separate service shared among workspaces: Separating out the metadata actions is a necessary step in having a shared "plugin broker" service; the artifact broker will always have to exist, as something has to mount /plugins to download artifacts. The metadata broker could easily be adapted into a library or service that runs separate from workspace starts/stops
• REST backend/merge plugin and devfile registries: The metadata broker could be integrated into the plugin-registry REST backend, allowing the registry to precompute/cache Che Plugins without running a separate image.
• Broker's role as a library in Che Operator: The adapted brokers are API compatible with the unified broker, so the metadata broker can be used as a library. It also avoids the need to specify "only metadata actions" when running the broker, since the metadata broker is always "only metadata actions"
• Merge plugin brokers into a single image: The goal of a single image has issues with the other goals listed above; the metadata broker, if integrated into e.g. the registry, could be removed, leaving us with one image as hoped
• Plugin broker should only make necessary changes to plugins: Since the refactor removes the init broker, it would be fairly simple to add this functionality to the artifact broker.

Additionally, one improvement with this approach is that it removes the clumsy way we run the broker in ephemeral workspaces (run the entire broker and download everything twice). The metadata broker can run separately to get the workspace configuration information, and then the artifact broker can run as an init container on the workspace pod.

Alternatives

One goal would be to get rid of the plugin broker altogether. This would likely involve reworking Theia to do this fetching at startup. Even in this case, the refactor above would be useful, as it would easily allow separate deprecation of broker functions (i.e. we could deprecate/move the metadata broker to the registry and deprecate the artifact broker when Theia has this functionality).

Summary

Workspace startup with and without changes (ephemeral workspace to show worst-case process):

[amisevsk]

cc: @ibuziuk @davidfestal @l0rd @nickboldt your thoughts?

[davidfestal]

That seems a very good and clear explanation and I fully agree with proposed changes.

[gorkem]

Is metadata broker an interim solution until we have a plugin broker REST service that we can add this functionality into?
If that is the case does the metadata functionality have to be on the plugin broker REST service? Can it be on the Che server (as a library) and calculated during the server creation/startup?

[davidfestal]

Can it be on the Che server (as a library) and calculated during the server creation/startup?

It is planned to be a library, also available for the Che Worskpace CRD operator (which already uses the broker as a library). However it would be a Go library, and the Che server is a Java application...

[amisevsk]

Is metadata broker an interim solution until we have a plugin broker REST service that we can add this functionality into?

The metadata broker is basically just a copy-paste of the current broker code, with some light re-writing. I think it's a necessary step regardless of what we do -- as for where it ends up, that's up to PMs. I think keeping it as a Go library is important for the Che operator, so I don't know how we would integrate it into the Che server, but it could be reworked into a standalone service, run as a sidecar on the registry, or be a part of the registry itself.

[amisevsk]

The incremental change in merging the refactor would require updating Che and the operator to

1. Run the metadata broker standalone, as we currently run the broker.
2. Add the artifact broker as an init container on the workspace pod.

Once the two steps are separated, it would be possible to move each broker separately with less effort.

[l0rd]

@amisevsk nice work!

I thought that metadata broker should become part of the che-server too (as a library reused by the quarkus app in the workspace CRD) but if it need be written in Go it makes sense to integrate it in the plugin registry.

And for the artifact broker I was thinking about it as part of the plugin registry (that's where the caching of the vsix should naturally happen). The init container would only run a 3 lines bash script that downloads the list of vsix. But honestly these are implementation details. The important thing is to satisfy the requirements you mention in the issue description. And your solution seems to do it.

[amisevsk]

@l0rd I agree that the init container situation could be simplified (basically we only need a list of extensions); my reason for maintaining the current broker approach for now is

1. The plugins dir needs to be managed on every workspace start (if e.g. I remove a plugin from my workspace, all downloaded files for that plugin also need to be cleaned up)
2. Changing how the actual downloading of .vsix and .theia works is a more significant change on the Che server side. This is a smaller change and thus requires less work on Che for now.
3. Support for brokering plugins that aren't in the default registry requires some care, and there's a conceptual difference between for plugins (we're not referring to metas stored in the registry).

In my ideal case, the plugin registry (once it is reworked into an actual REST API) would

• offer an endpoint, something like /provision/pluginFQN, that serves the json Che Plugin metadata that needs to be added to the workspace. The Che server makes requests on this endpoint to figure out how to provision a workspace.
  • With the metadata broker, this could be done by calling its API currently, if it's integrated.
• offer an endpoint, e.g. /artifacts/pluginFQN, that provides the .theia and .vsix files (+metadata) that need to be downloaded. The workspace init container would make this request and download the respective files to /plugins. I'm not sure how possible this is, however, since my quick attempts to fix Plugin broker should only make necessary changes to plugins #13452 require some processing.

[amisevsk]

The data flow with the refactor would be something like

We could fairly easily close the green loop by incorporating the metadata step into the registry (since it's pretty much static computation) -- this is the /provision/pluginFQN endpoint. The blue loop could also potentially be closed via /artifacts/PluginFQN but I don't know if that's as useful.

[amisevsk]

The relevant PRs are finally open:

• Plugin brokers: Refactor plugin brokers che-plugin-broker#80
• Che server: Refactor plugin brokering process #15386

Changes can be tested by starting a Che server using the CI image eclipseche/che-server:15386 and adding env vars:

CHE_WORKSPACE_PLUGIN__BROKER_METADATA_IMAGE=amisevsk/che-metadata-plugin-broker:dev
CHE_WORKSPACE_PLUGIN__BROKER_ARTIFACTS_IMAGE=amisevsk/che-artifacts-plugin-broker:dev

[amisevsk]

The refactor changes have been merged into Che master.

[nickboldt]

Moved to milestone 7.8 as that's where it was delivered. "Backlog" makes it seem like it wasn't done. :)