-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
config: bump to envoy-api 422332b. #1202
Conversation
Updated build targets and switched canary to a metadata field.
Rekicking Travis. There was a TSAN failure that seems surprising, can't repeat locally with |
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.
looks good, small comment.
source/common/upstream/eds.cc
Outdated
@@ -47,11 +47,13 @@ void EdsClusterImpl::onConfigUpdate(const ResourceVector& resources) { | |||
const std::string& zone = locality_lb_endpoint.locality().zone(); | |||
|
|||
for (const auto& lb_endpoint : locality_lb_endpoint.lb_endpoints()) { | |||
const bool canary = | |||
Config::Utility::metadataValue(lb_endpoint.metadata(), "envoy.lb", "canary").bool_value(); |
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.
Can we add "envoy.lb" and "canary" to some type of MetadataConstants class or something? We will be referencing well known metadata in various places in the code and it would be good to put the well known things in a central place.
The TSAN flake bites again. I'll dig into this. |
I think TSAN was triggering some non-deterministic behavior on conflicting EXPECT_CALLs.
Root cause of |
Needed as a workaround for protocolbuffers/protobuf#3322. I'm going to work on an upstream fix once I unblock current dependencies on the merge of envoyproxy#1202. This increases image size from 2.3GB to 2.9GB. While it might be possible to only rebuild protobuf/lightstep, it's significantly more complicated throwaway build hacking to get there.
Needed as a workaround for protocolbuffers/protobuf#3322. I'm going to work on an upstream fix once I unblock current dependencies on the merge of #1202. This increases image size from 2.3GB to 2.9GB. While it might be possible to only rebuild protobuf/lightstep, it's significantly more complicated throwaway build hacking to get there.
@mattklein123 This should now pass CI, can I get an approval? Thanks. |
* Fix env vars in Makefile * run tsan in circle ci
Description: iOS parallel to #1188 Risk Level: low Testing: example app. Pending test with assertion filter. Docs Changes: pending full API docs Signed-off-by: Jose Nino <jnino@lyft.com> Signed-off-by: JP Simard <jp@jpsim.com>
Description: iOS parallel to #1188 Risk Level: low Testing: example app. Pending test with assertion filter. Docs Changes: pending full API docs Signed-off-by: Jose Nino <jnino@lyft.com> Signed-off-by: JP Simard <jp@jpsim.com>
Updated build targets and switched canary to a metadata field.