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

Envoy config generated by the operator is invalid in envoy 1.22 #823

Merged
merged 1 commit into from
Jun 23, 2022

Conversation

amuraru
Copy link
Contributor

@amuraru amuraru commented Jun 17, 2022

Q A
Bug fix? yes
New feature? yes
API breaks? no
Deprecations? no
License Apache 2.0

What's in this PR?

Envoy config generated by the operator was not compatible with newer version of the envoy. More specifically in v1.22 the config is invalid:

[2022-06-16 13:27:58.425][1][info][main] [source/server/server.cc:939] exiting
Didn't find a registered implementation for 'envoy.filters.http.router' with type URL: ''

This PR addresses this by:

  • fixing the expected typedConfig in the router config
  • fixing deprecated options used in the generated envoy.yaml file
  • upgrades default envoy to v1.22
  • upgrades github.com/envoyproxy/go-control-plane to latest version

Why?

Being able to upgrade to newer version of envoy when envoy ingress is used to front Kafka clusters.

Additional context

Checklist

  • Implementation tested
  • Error handling code meets the guideline
  • Logging code meets the guideline
  • User guide and development docs updated (if needed)

@amuraru amuraru requested a review from a team as a code owner June 17, 2022 11:39
Copy link
Member

@stoader stoader left a comment

Choose a reason for hiding this comment

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

Since the user can override the image for Envoy via envoyConfig.image the config generation should take into account the version and generate config specific to the version.

The other option is we mandate the use of Envoy 1.22.2 and above from now on.

@stoader stoader requested a review from a team June 17, 2022 14:55
@amuraru
Copy link
Contributor Author

amuraru commented Jun 17, 2022

Good points @stoader - in fact the new generated version of envoy.yaml should be backward compatible with envoy < 1.22
I'll double check that and report back here

Kuvesz
Kuvesz previously approved these changes Jun 20, 2022
Copy link
Contributor

@Kuvesz Kuvesz left a comment

Choose a reason for hiding this comment

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

I think this looks good if it is indeed backward compatible.

Added explicit typeconfig for envoy.filters.http.router

```
[2022-06-16 13:27:58.425][1][info][main] [source/server/server.cc:939] exiting
Didn't find a registered implementation for 'envoy.filters.http.router' with type URL: ''
```
@amuraru
Copy link
Contributor Author

amuraru commented Jun 23, 2022

@stoader @Kuvesz I updated the PR to include only the minimal changes to make the generated config compatible with newer versions of envoy (e.g. 1.22.x)
Tested this out and it works with envoy 1.18 as well - so this is not breaking the clients still using older versions.

I'll follow-up with a separate PR to make envoy 1.22 as min required if you agree:
amuraru@1e1f9bc
This change though is not backward compatible so we may need to think through it more.

@hi-im-aren
Copy link
Member

Thanks! 👍

@stoader stoader merged commit a9f1d11 into banzaicloud:master Jun 23, 2022
@amuraru amuraru deleted the envoy122 branch July 8, 2022 08:00
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.

4 participants