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

Support deployment of Turing routers with RPC Experiment Engine plugins #158

Merged
merged 31 commits into from
Jan 20, 2022

Conversation

romanwozniak
Copy link
Contributor

@romanwozniak romanwozniak commented Jan 19, 2022

Context:

#153 made it possible to deploy Turing API with experiment engines implemented as RPC plugins, however, these plugins were not propagated into turing routers, deployed by the API. This PR addresses this part.

Details

Deployment of Turing API with RPC plugins is supported through the k8s initContainers: during the deployment, plugin binaries from init containers are copied into a shared emptyDir volume, which later is mounted into the main turing-api container. However, neither initContainers, nor shared volumes are currently fully supported by the Knative (which is used for deploying routers), hence it requires a workaround to overcome these limitations.

There are two parts to this workaround:

  • Support of PluginURL configuration was added into the ExperimentEngine factory, to make it possible for the factory to fetch plugin's binary from the provided URL
  • Together with the router, Turing API deploys plugins-server k8s service (nginx server), that serves plugin executables as static files via HTTP

So if turing-router is configured with RPC-based experiment engine, then during the initialization, router fetches plugin's binary from the plugins-server, stores it inside the container, and then starts the regular communication between host (turing-router) and the plugin (experiment-runner).

@romanwozniak romanwozniak requested a review from a team January 19, 2022 10:09
@romanwozniak romanwozniak self-assigned this Jan 19, 2022
description: The GitHub repository
type: string
required: true
default: "charts"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make it possible to manually trigger publish-chart pipeline from the dev branch

Copy link
Collaborator

@krithika369 krithika369 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some small comments / questions. LGTM!

.github/workflows/turing.yaml Show resolved Hide resolved
api/turing/config/config.go Outdated Show resolved Hide resolved
api/turing/config/config_test.go Show resolved Hide resolved
api/turing/models/events.go Outdated Show resolved Hide resolved
infra/charts/turing/Chart.yaml Outdated Show resolved Hide resolved
@romanwozniak romanwozniak merged commit 6c16861 into caraml-dev:main Jan 20, 2022
@romanwozniak romanwozniak deleted the deploy_router_with_plugins branch January 20, 2022 13:22
deadlycoconuts pushed a commit to deadlycoconuts/turing that referenced this pull request Jan 21, 2022
…ns (caraml-dev#158)

* - Remove unused `RouterDefaults.Experiment` and update tests

* - Update service builder to prepare persistence volume for router and copy experiment engine plugins into it

* - update fiber config if external plugin is used

* - publish test chart

* - publish test chart

* - fix lint

* - make it possible to configure manual charts publishing

* - unit tests

* - fix chart

* - fix permissions inside the dockerfile

* - use bash alpine image

* - use bash image

* - fix nil pointer

* - use nginx to server plugin binary

* - disable adding PluginBinary data to fiber config

* - disable adding PluginBinary data to fiber config

* - Support downloading plugins from URL

* fix lint

* - bug fixes

* - set executable permission for the plugin binary

* - re-use PluginConfig model

* - Delete plugins server before knative services are deleted

* - fix test

* - address naming suggestiong from the PR review, bumping up charts version

* - add test case for plugin config

* - add test case to cover k8s services for router with plugins

* - fix chart

* - move mlp mock service into mocks directory, clean up tests

* - fix lint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants