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

Fix QuotaSpec crash #24276

Closed
wants to merge 1 commit into from

Conversation

howardjohn
Copy link
Member

Fixes #24264

I have no strong confidence this works, as we clearly have no end to end tests or this would have been caught. Apparently no one has used QuotaSpec with istiod since this should be broken since 1.5.0

Basically the issue is we are using the golang proto struct instead of gogo proto struct.

@howardjohn howardjohn requested a review from a team as a code owner June 1, 2020 17:27
@istio-policy-bot
Copy link

🤔 🐛 You appear to be fixing a bug in Go code, yet your PR doesn't include updates to any test files. Did you forget to add a test?

Courtesy of your friendly test nag.

@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Jun 1, 2020
@istio-testing istio-testing added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jun 1, 2020
@istio-testing
Copy link
Collaborator

@howardjohn: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
lint_istio 5fdf235 link /test lint_istio
gencheck_istio 5fdf235 link /test gencheck_istio
unit-tests_istio 5fdf235 link /test unit-tests_istio

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@gargnupur
Copy link
Contributor

@howardjohn : I thought we hade ratelimiting tests running in post submit?
We would have to change mixer.go too.. pilot/pkg/networking/plugin/mixer/mixer.go.. that's using mpb "istio.io/istio/pilot/pkg/networking/plugin/mixer/client"....

@howardjohn
Copy link
Member Author

@gargnupur the problem is the CRD reading library only supports gogo proto and XDS only supports golang protobuf. We run pure envoy tests for rate limitting but none that go through the full pilot pipeline

@howardjohn howardjohn closed this Jun 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
6 participants