Skip to content

ci: update do_ci.sh targets and docs to the bazel test matrix.#837

Merged
mattklein123 merged 21 commits intoenvoyproxy:masterfrom
htuch:travis-bazel
Apr 27, 2017
Merged

ci: update do_ci.sh targets and docs to the bazel test matrix.#837
mattklein123 merged 21 commits intoenvoyproxy:masterfrom
htuch:travis-bazel

Conversation

@htuch
Copy link
Member

@htuch htuch commented Apr 25, 2017

We've lost Mac OS X build information for do_ci.sh, since it's stale and I don't have a way to
verify it. Feel free to add in updated info later, but I think it might be better to use Bazel
directly here instead of via the container build.

htuch added 2 commits April 25, 2017 11:13
We've lost Mac OS X build information for do_ci.sh, since it's stale and I don't have a way to
verify it. Feel free to add in updated info later, but I think it might be better to use Bazel
directly here instead of via the container build.

Also, the Alpine build is going to atrophy. I'll take an action item to chat with folks about
updating this.
@@ -1,7 +0,0 @@
DOCS_OUTPUT_DIR ?= generated/docs
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if anyone was relying on this to test docs locally. If we don't have it already, can we add a short section in the dev docs on how to build/view docs locally?

@mattklein123
Copy link
Member

Per offline convo, ASAN build is much faster in bazel vs. cmake. Maybe it's due to parallelism? But also surprised we aren't destroying crappy travis VMs without setting --jobs.

@htuch
Copy link
Member Author

htuch commented Apr 25, 2017

It wasn't doing ASAN on bazel.asan, I think because of the fact I was building from a consumer project and missed the bazel.rc.

@mattklein123
Copy link
Member

Ah OK, I had this same problem in our private build. I worked around by symlinking tools/bazel.rc -> envoy/tools/bazel.rc. +1 to building this into the bzl file.

@mattklein123
Copy link
Member

This looks good, but I realized that we have various processing things that are done on the "normal" build like pushing images, etc. I think we need to fix all of that.

@mattklein123
Copy link
Member

Also, do we care to understand why optimized build/test is now so slow? Seems like almost 2x regression from cmake.

@htuch
Copy link
Member Author

htuch commented Apr 25, 2017

I can confirm that locally (not on CI) that a bazel test --jobs=2 //test/... takes approximately 3x longer for -c opt than fastbuild (14m 09 s vs. 5m 21s). I'll look at a --profile` next locally to see what's going on.

@htuch
Copy link
Member Author

htuch commented Apr 25, 2017

Looking at individual C++ build commands, I can see the -O2 build take 267% the time of the -O0 build in one example (test/mocks/router/mocks). So, the build time that CI is reporting seems legit. What I wonder then is why the release build was so fast with cmake. Will look into what this was doing next.

@htuch
Copy link
Member Author

htuch commented Apr 26, 2017

-c opt builds both PIC and non-PIC for a reason I still can't figure out after reading https://groups.google.com/forum/#!topic/bazel-discuss/agycffdH0R0, hence doubling the test build time. Anyway, fixed by forcing the toolchain to be non-PIC in cc_configure.bzl.

@htuch htuch closed this Apr 26, 2017
@htuch htuch reopened this Apr 26, 2017
@htuch
Copy link
Member Author

htuch commented Apr 26, 2017

Close/reopen due to a flake that might be in async_client_impl_test

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 docs question, needs master merge when the flake fix merges.

ci/README.md Outdated
The output artifacts can be found in `/build/envoy/bazel-bin/source/exe/envoy-static` in the
container.

The `do_ci.sh` targets are:
Copy link
Member

Choose a reason for hiding this comment

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

Should references to do_ci.sh in the docs be updated to point to run_envoy_docker.sh ?

@mattklein123
Copy link
Member

@htuch you should be good to merge master.

@htuch
Copy link
Member Author

htuch commented Apr 27, 2017

@mattklein123 ready for final approval and merge.

@mattklein123 mattklein123 merged commit def05ef into envoyproxy:master Apr 27, 2017
htuch added a commit to htuch/envoy that referenced this pull request Apr 27, 2017
Alpine build was failing, fixed and changed to build even when not pushing to master to catch this
next time.
mattklein123 pushed a commit that referenced this pull request Apr 27, 2017
Alpine build was failing, fixed and changed to build even when not pushing to master to catch this
next time.
mathetake added a commit that referenced this pull request Mar 3, 2026
**Description**

Before #793, the case where no matching route found was handled in the
extproc and the 404 immediate response was returned from there, but
after that, it naturally results in the "unreachable" default route and
swallowed the indication of no matching and it made it impossible to
reason about the 500 error on that case. In other words, this fixes the
regression in #793 to return the proper 404 response.

---------

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.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