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

config: bump to envoy-api 422332b. #1202

Merged
merged 7 commits into from
Jul 7, 2017
Merged

Conversation

htuch
Copy link
Member

@htuch htuch commented Jul 3, 2017

Updated build targets and switched canary to a metadata field.

Updated build targets and switched canary to a metadata field.
@htuch
Copy link
Member Author

htuch commented Jul 4, 2017

Rekicking Travis. There was a TSAN failure that seems surprising, can't repeat locally with --runs_per_test=1000, need to see if Travis hits this again.

@htuch htuch closed this Jul 4, 2017
@htuch htuch reopened this Jul 4, 2017
Copy link
Member

@mattklein123 mattklein123 left a 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.

@@ -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();
Copy link
Member

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.

@htuch
Copy link
Member Author

htuch commented Jul 5, 2017

The TSAN flake bites again. I'll dig into this.

I think TSAN was triggering some non-deterministic behavior on conflicting EXPECT_CALLs.
@htuch
Copy link
Member Author

htuch commented Jul 6, 2017

Root cause of sds_test flake appears to be protocolbuffers/protobuf#3322.

htuch added a commit to htuch/envoy that referenced this pull request Jul 7, 2017
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.
htuch added a commit that referenced this pull request Jul 7, 2017
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.
@htuch
Copy link
Member Author

htuch commented Jul 7, 2017

@mattklein123 This should now pass CI, can I get an approval? Thanks.

@htuch htuch merged commit 1d330b5 into envoyproxy:master Jul 7, 2017
@htuch htuch deleted the envoy-api-bump branch July 7, 2017 23:02
rshriram pushed a commit to rshriram/envoy that referenced this pull request Oct 30, 2018
* Fix env vars in Makefile

* run tsan in circle ci
jpsim pushed a commit that referenced this pull request Nov 28, 2022
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>
jpsim pushed a commit that referenced this pull request Nov 29, 2022
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants