-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
connect: Remove support for Envoy 1.16 #11354
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
agent/xds/server.go
Outdated
|
||
return err | ||
} | ||
|
||
const ( | ||
stateInit int = iota |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these can be deleted too. I forked them for delta.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM again, with one comment about something else to possibly delete.
🍒 If backport labels were added before merging, cherry-picking will start automatically. To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/487837. |
Similar for what we did with Envoy 1.19/1.15 in #11115 and #11118 respectively, this stacks on top of #11277 to remove support for Envoy 1.16.5. The stacking helps keep these changes isolated and easier to review while also allowing me to merge them in close succession.
As for the actual removal of 1.16.5, this is a relatively significant change since, for versions of Envoy 1.17+, xDS v2 is officially deprecated. This means we no longer need the v2 <-> v3 conversion code so it has been removed.
Question to reviewers: is there any reason why we need v2 code? @rboyer and I landed on no but it would be good to get more eyes on it.
TODO: