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

Bazel hack #1

Open
wants to merge 120 commits into
base: master
Choose a base branch
from
Open

Bazel hack #1

wants to merge 120 commits into from

Conversation

wattli
Copy link
Owner

@wattli wattli commented Mar 21, 2017

No description provided.

lizan and others added 30 commits February 2, 2017 08:22
…envoyproxy#432)

do_ci.sh runs tests with heapcheck enabled. This can discover heap leaks
but requires pprof (in google-perftools) to perform the symbolization
and printing of allocation site stack traces.

With this PR, a failed run will produce output such as:

Have memory regions w/o callers: might report false leaks
Leak check _main_ detected leaks of 16 bytes in 1 objects
The 1 largest leaks:
Using local file /source/build/test/envoy-test.
Leak of 16 bytes in 1 objects allocated from:
        @ 15d6b6f Network::DnsImplTest_Cancel_Test::TestBody
        @ 1e91e66 testing::internal::HandleSehExceptionsInMethodIfSupported
        @ 1e8cba8 testing::internal::HandleExceptionsInMethodIfSupported
        @ 1e73af5 testing::Test::Run
        @ 1e74381 testing::TestInfo::Run
        @ 1e74a12 testing::TestCase::Run
        @ 1e7b1bd testing::internal::UnitTestImpl::RunAllTests
        @ 1e92f66 testing::internal::HandleSehExceptionsInMethodIfSupported
        @ 1e8d86a testing::internal::HandleExceptionsInMethodIfSupported
        @ 1e79e47 testing::UnitTest::Run
        @ 181925a RUN_ALL_TESTS
        @ 181844d main
The MOCK_METHOD* macros from gtest are not including the override
attribute and it's triggering warnings. This change includes
`-Wno-inconsistent-missing-override` to that target.
Some of these types were assuming that `size_t == uint64_t` or that
`uint64_t == unsigned long` but that's not universally true, some BSDs
such as Darwin define uint64_t as unsigned long long which warns on
literals that are defined as `UL`.

Additionally there were also some assumptions made on the pch as to
forward declarations. So I explicitly added some needed ones.
This change allows copy elision on unreferenced temporary objects
This change adds three main things:
1.  Pushing a lyft/envoy:latest image to dockerhub every time master passes tests.
2.  Check that the docker examples work on every PR.
3.  Add SHA versioned lyft/envoy-build images.
* Some extra developer conveniences for Docker image.

* Refactor do_ci.sh to split the cmake and environment setup from the
  build and test. This makes it convenient for devs to invoke arbitrary
  make targets in the Docker image, e.g.

  docker run -t -i -v $PWD:/source lyft/envoy-build:latest /bin/bash -c \
    "cd /source && . ./ci/build_setup.sh && make fix_format"

* Add EXTRA_TEST_ARGS env var to pass addition flags to envoy-test. This
  is useful, for example, when applying a --gtest_filter to restrict the
  test run to only failing tests.

* Add a do_ci.sh fix_format target.

* Add gdb, strace to the image and a RUN_TEST_UNDER environment variable
  that can be used to have envoy-test run under these programs.

* UNIT_TEST_ONLY env var that will skip the non-envoy-test aspects of
  the test process.

Example command for debugging only tests matching *Dns*:

docker run -t -i -v $PWD:/source lyft/envoy-build:latest /bin/bash -c \
  "cd /source && UNIT_TEST_ONLY=1 RUN_TEST_UNDER='gdb --args' ./ci/do_ci.sh debug \
  --gtest_filter='*Dns*'
This is a large refactor that moves away from representing addresses
internally as strings of "URL" form into proper objects. With this
change in place, it should be very straightforward to add IPv6
support, as most places in the code no longer need to understand
anything about the underlying address. Additionally, adding NT support
in the future should also be easier.

There are still some unfortunate legacy aspects, such as the fact that
when loading from configuration we still parse tcp:// as well as unix://
style addresses. In the future this should likely become ip:// as well
as pipe:// but we will probably need to continue to support both.

Note that although this change is huge in number of lines, most of it is
very straightforward, and certain portions of the code have become
substantially simpler such as listener handling and client connection
handling.

Fixes envoyproxy#390
Previously we would incorrectly keep adding hosts and essentially
leak memory.

fixes envoyproxy#433
envoyproxy#389 did not actually cache them
between filters which was the point of the previous change.
…iterating locally. (envoyproxy#455)

It's somewhat distracting to be forced to fix_format while debugging.
mattklein123 and others added 29 commits March 10, 2017 11:39
If we get additional CDS updates while we are still initializing, we would
not previously initialize new secondary clusters. This would cause init to
never complete in certain cases.

This commit also adds more init logging to help with debugging similar issues.
envoyproxy#558)

Baby steps towards flow control. This patch plumbs the TCP connection buffer limits 
to the listener and enforces it on the read buffer only. Later patches will also plumb to 
the upstream cluster connection and cover the write buffer (which requires the 
watermark API).
Previously we did not correctly handle "expect: 100-continue" in the http/2
codec. Because of protocol differences, we handle 100-continue directly in the
codec. This change implements support in the http/2 codec similar to http/1.1
where we immediately send a continue response.

This commit also adds partial support for the client codec for 1xx responses.
The code will raise decodeHeaders(...) twice in the 1xx response case. This
is good enough for testing, but will probably crash higher layers if a client
actually responded this way. Right now we effectively trust clients so this
can be handled in a follow up if necessary.

fixes envoyproxy#561
* Minimal Bazel infrastructure (envoyproxy#415).

This patch introduces the minimal Bazel infrastructure required to build and execute utility_test,
as a step towards conversion of the cmake build system to Bazel (envoyproxy#415). The main contributions are:

* Machinery for declaring Envoy C++ library and test targets (envoy_cc_library, envoy_cc_test).

* External dependency management that works both inside the Docker CI environment (using prebuilts)
  and with developer-local dependency fetch/build (based on envoyproxy#416).

* Handling of the implicit dependencies that are today sourced via prebuilts.

* Simple example of building and running a unit test with only its dependencies built. With the
  cmake system, we would need to build all of Envoy and all tests to run utility_test. E.g.

  blaze test //test/common/common:utility_test

This is not intended to be used by anyone other than those working with the Bazel conversion at this
point. The plan is to add "ci/do_ci.sh bazel.debug" as a Travis target to ensure we don't bit rot.
We don't want debug code in the coverage stats.
…y#584)

This prevents strange behavior from build/CMakeCache.txt.
…d and connect. (envoyproxy#581)

Add getUnusedPort in test/test_common/network_utility.*, which for now just selects a port at random in the ephemeral range.

Fix bug in Ipv6Instance ctor where it specified AF_INET (i.e. IPv4) as the address family. Doh!

Add testSocketBindAndConnect, and apply to both Ipv4Instance and Ipv6Instance.
…o be used in IpList (or its successor), as that only supports IPv4 ranges today. (envoyproxy#576)
…oxy#592)

Previously, we would get all buffer slices on the stack and stack overflow
if the chain was very large. This change limits the number of slices that we
write during each iteration. There are a number of potential improvements
possible in terms of collapsing small moves, enforcing fairness in terms of
number of iterations, etc.

This commit also removes a bunch of RawSlice to evbuffer_iovec conversions
for performance reasons.

NOTE: The added test repros the crash before the fix.

fixes envoyproxy#585
Also added a "set -e" to configgen.sh to ensure we catch this in presubmit in the future. As a
bonus, plumbed in arbitrary address binding to admin server config, since we had some configs that
had been updated to this already and it seems a generally useful thing to have.
This breaks statsd. I also took the opportunity to plumb through Stats::Scope
properly in a bunch of places which will be needed for LDS.
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.