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

backport pr 15979 into 1.14.x #17287

Merged
merged 2 commits into from
May 10, 2023

Conversation

malizz
Copy link
Contributor

@malizz malizz commented May 9, 2023

manually backport pr #15979 into 1.14.x (backport assist failed)
This PR update protobuf package to google.golang.org/protobuf since github.com/golang/protobuf is deprecated.

PR Checklist

  • updated test coverage
  • [NA] external facing docs updated
  • appropriate backport labels added
  • not a security concern

@github-actions github-actions bot added theme/api Relating to the HTTP API interface theme/envoy/xds Related to Envoy support labels May 9, 2023
@malizz malizz marked this pull request as ready for review May 9, 2023 23:31
Copy link
Contributor

@DanStough DanStough left a comment

Choose a reason for hiding this comment

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

A couple non-blocking comments. Thank you for this cleanup!

Copy link
Contributor

Choose a reason for hiding this comment

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

Not blocking, but you may want to change this to #17287 if you are changing other things.

@@ -192,8 +193,15 @@ func ToOutlierDetection(p *structs.PassiveHealthCheck) *envoy_cluster_v3.Outlier
}

if p.EnforcingConsecutive5xx != nil {
// NOTE: EnforcingConsecutive5xx must be greater than 0 for ingress-gateway
od.EnforcingConsecutive_5Xx = &wrappers.UInt32Value{Value: *p.EnforcingConsecutive5xx}
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like wrappers comes from the deprecated GH package. I don't think it's critical to this PR, but if we went through the process of upgrading the proto generation to the google.golang version, we might want to clean it out completely. Not sure how much of a lift this is, though. Not blocking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I can change it in another PR. I want to make sure it won't break the backport assist PRs with out of band changes.

@malizz malizz merged commit 842139f into release/1.14.x May 10, 2023
@malizz malizz deleted the malizz/NET-1646/backport-pr-15979-into-1.14.x branch May 10, 2023 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/no-backport theme/api Relating to the HTTP API interface theme/envoy/xds Related to Envoy support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants