Skip to content
This repository has been archived by the owner on Aug 25, 2021. It is now read-only.

envoy versioning is now set at the global level #585

Merged
merged 12 commits into from Aug 31, 2020

Conversation

kschoche
Copy link
Contributor

@kschoche kschoche commented Aug 26, 2020

Remove meshGateway.imageEnvoy and connectInject.imageEnvoy in favour of global.imageEnvoy which is what ingress and terminating gateways use, this will standardize imageEnvoy across all the gateways and connectInject.

Note : we are removing the option to leave connectInject.imageEnvoy: "null"

@kschoche kschoche added the enhancement New feature or request label Aug 26, 2020
@kschoche kschoche requested review from a team, lkysow and ishustava and removed request for a team August 26, 2020 21:00
@kschoche kschoche added area/connect Related to Connect, e.g. injection area/gateways Related to gateway components labels Aug 26, 2020
@kschoche kschoche self-assigned this Aug 26, 2020
Copy link
Member

@ishustava ishustava left a comment

Choose a reason for hiding this comment

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

That's awesome! Thanks for making this change 🙏

I left a couple of edits. I'm also now thinking about whether we should keep the behavior when you set imageEnvoy to null for connect? Since we're consolidating it into one property, I think it'd make sense for it to behave the same way across connect and gateways. What do you think?

I'm also curious if you have thought about what to do with the default in consul-k8s. Since we're making this change and it'll be a breaking change, would it makes sense to also make a breaking change in consul-k8s and remove the default?

templates/connect-inject-deployment.yaml Outdated Show resolved Hide resolved
values.yaml Outdated Show resolved Hide resolved
values.yaml Outdated Show resolved Hide resolved
templates/connect-inject-deployment.yaml Outdated Show resolved Hide resolved
templates/mesh-gateway-deployment.yaml Outdated Show resolved Hide resolved
kschoche and others added 3 commits August 28, 2020 10:08
Co-authored-by: Iryna Shustava <ishustava@users.noreply.github.com>
Co-authored-by: Iryna Shustava <ishustava@users.noreply.github.com>
Copy link
Member

@lkysow lkysow left a comment

Choose a reason for hiding this comment

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

Looks great! Needs changelog entry.

templates/connect-inject-deployment.yaml Outdated Show resolved Hide resolved
templates/connect-inject-deployment.yaml Outdated Show resolved Hide resolved
templates/mesh-gateway-deployment.yaml Outdated Show resolved Hide resolved
values.yaml Outdated Show resolved Hide resolved
values.yaml Outdated Show resolved Hide resolved
kschoche and others added 4 commits August 28, 2020 11:25
Co-authored-by: Luke Kysow <1034429+lkysow@users.noreply.github.com>
Co-authored-by: Luke Kysow <1034429+lkysow@users.noreply.github.com>
Co-authored-by: Luke Kysow <1034429+lkysow@users.noreply.github.com>
values.yaml Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Member

@lkysow lkysow left a comment

Choose a reason for hiding this comment

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

Nice! Love the fail messages as well, nice touch for users that miss the changelog.

Co-authored-by: Luke Kysow <1034429+lkysow@users.noreply.github.com>
Copy link
Member

@ishustava ishustava left a comment

Choose a reason for hiding this comment

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

Looks good!

The only minor change is that we need to remove {{ if .Values.global.imageEnvoy -}} from the connect inject deployment since we're now requiring this value to be set. Once that's done, it's good to go.

templates/connect-inject-deployment.yaml Outdated Show resolved Hide resolved
@kschoche kschoche merged commit 2732dd1 into master Aug 31, 2020
@kschoche kschoche deleted the envoy_versioning_meshgw_connect branch August 31, 2020 15:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/connect Related to Connect, e.g. injection area/gateways Related to gateway components enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants