-
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
Remove support for Envoy 1.15 #11118
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, one minor suggestion
default: | ||
return nil, fmt.Errorf("Invalid service kind: %v", cfgSnap.Kind) | ||
} | ||
} | ||
|
||
func (s *ResourceGenerator) maybeInjectStubClusterForGateways(resources []proto.Message) ([]proto.Message, error) { |
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.
yay!
@@ -310,12 +310,6 @@ func (s *Server) processDelta(stream ADSDeltaStream, reqCh <-chan *envoy_discove | |||
} | |||
if sent { | |||
sentType[op.TypeUrl] = struct{}{} | |||
if generator.ProxyFeatures.IncrementalXDSUpdatesMustBeSerial { |
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.
yay!
GatewaysNeedStubClusterWhenEmptyWithIncrementalXDS bool | ||
|
||
// IncrementalXDSUpdatesMustBeSerial is needed to avoid an envoy crash. | ||
// For example, we previously had flags for Envoy < 1.16 called: |
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 like this. Prior to these 2 flags existing the struct was empty for a bit. Prior to that it had completely different features available. When I went to add these two flags I had to look at my own file history to see how to re-add a feature flag.
…eculver/remove-envoy-1.15
🍒 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/463501. |
Refs #11100, stacked on #11115.
This is a follow on to #11115 to remove support for Envoy 1.15. The only notable changes are around the feature flags that we had in place for Envoy < 1.16, which means only 1.15. As such, I am removing these flags and code that references them.
Note: As this is pretty unfamiliar territory for me, I'd like to specifically call out the bits in
./agent/xds/clusters.go
that are getting removed (maybeInjectStubClusterForGateways
and calling code). This appears correct to me, but need someone to confirm.I'll rebase on
main
once #11115 lands but for now this will be based oneculver/envoy-1.19
.