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

Refactor envoy configuration to using upstream yaml.file #100

Closed
orfeas-k opened this issue May 27, 2024 · 1 comment · Fixed by #101
Closed

Refactor envoy configuration to using upstream yaml.file #100

orfeas-k opened this issue May 27, 2024 · 1 comment · Fixed by #101
Labels
enhancement New feature or request

Comments

@orfeas-k
Copy link
Contributor

orfeas-k commented May 27, 2024

Context

We should refactor the charm to be configured using a simple yaml file instead of generating it using the envoy_data_plane package for the following reasons.

  1. During the update of envoy for Kubeflow 1.9, there were updates to the envoy.yaml file that are visible in PR fix(metadata envoy): upgrade envoy and config from 1.12 to 1.27 kubeflow/pipelines#10589. Regarding those:
  1. In general, it is a more cumbersome developer experience to map changes in the envoy.yaml file to the correspond. In other words, it's easier to just copy changes from upstream if we also use a yaml file.
  2. This has caused us trouble in the past e.g. in this PR fix: Update envoy configuration #64 (comment) we had to revert changes because the python package didn't work the same way with the yaml file (see bullet about revert max_grpc_timeout). This also resulted in us having a slightly different configuration than upstream.

Thus, in order to make our lifes easier, I think we should refactor the

What needs to get done

Replace the config generator component with a simple yaml template being rendered and pushed in the container.

Definition of Done

Envoy is configurable through a yaml file.

@orfeas-k orfeas-k added the enhancement New feature or request label May 27, 2024
Copy link

Thank you for reporting us your feedback!

The internal ticket has been created: https://warthogs.atlassian.net/browse/KF-5742.

This message was autogenerated

orfeas-k added a commit that referenced this issue May 29, 2024
* Remove the config generation component that used a python package in 
  order to generate the envoy configuration file. Instead, render a simple `yaml` 
  file with the proper context. 
* Omit the usage of `upstream_service` value (which comes from `mlmd` charm and 
  by default has the value `metadata-grpc-service`) in naming the `cluster`. This does 
  not serve any purpose and also makes us diverge from upstream.

Closes #100
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant