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

Envoy is failing on Bazel CI #16542

Open
Wyverald opened this issue May 18, 2021 · 18 comments
Open

Envoy is failing on Bazel CI #16542

Wyverald opened this issue May 18, 2021 · 18 comments
Assignees
Labels
area/build bug no stalebot Disables stalebot from closing an issue

Comments

@Wyverald
Copy link
Contributor

https://buildkite.com/bazel/envoy/builds/1550#aee9a8a9-d127-466b-9dff-3e6b21bda840

The logs are extremely long but the error seems to originate from this:

test/common/common/safe_memcpy_test.cc:40:15: error: no viable constructor or deduction guide for deduction of template arguments of 'vector'
--
| EXPECT_THAT(std::vector(dst, dst + sizeof(src)), ElementsAre(1, 2, 3, 4));

More info can be found by following the link above and Ctrl+F'ing the excerpt above.

@Wyverald Wyverald added bug triage Issue requires triage labels May 18, 2021
@antoniovicente
Copy link
Contributor

What compiler and tools/library versions are used by the bazel CI?

It seems like the intent was to use the 2 iterator vector constructor, but it seems to me that this code should at the very least look something like:

EXPECT_THAT(std::vector<uint8_t>(dst, dst + sizeof(src)),

@antoniovicente
Copy link
Contributor

cc @rialg @jmarantz

@rialg
Copy link
Contributor

rialg commented May 18, 2021

With the following environment :

  • ENVOY_BUILD_IMAGE=envoyproxy/envoy-build-ubuntu:e33c93e6d79804bf95ff80426d10bdcc9096c785
  • bazel 3.7.2

Tests succeeded

$ ci/run_envoy_docker.sh 'bazel run //test/common/common:safe_memcpy_test'
Starting local Bazel server and connecting to it...
INFO: Analyzed target //test/common/common:safe_memcpy_test (313 packages loaded, 12721 targets configured).
INFO: Found 1 target...
Target //test/common/common:safe_memcpy_test up-to-date:
  bazel-bin/test/common/common/safe_memcpy_test
INFO: Elapsed time: 32.568s, Critical Path: 21.95s
INFO: 5 processes: 3 internal, 2 processwrapper-sandbox.
INFO: Build completed successfully, 5 total actions
INFO: Running command line: external/bazel_tools/tools/test/test-setup.sh test/common/common/safe_memcpy_test '--gmock_default_moINFO: Build completed successfully, 5 total actions
exec ${PAGER:-/usr/bin/less} "$0" || exit 1
Executing tests from //test/common/common:safe_memcpy_test
-----------------------------------------------------------------------------
[==========] Running 3 tests from 3 test suites.
[----------] Global test environment set-up.
[----------] 1 test from SafeMemcpyTest
[ RUN      ] SafeMemcpyTest.CopyUint8
[       OK ] SafeMemcpyTest.CopyUint8 (0 ms)
[----------] 1 test from SafeMemcpyTest (0 ms total)

[----------] 1 test from SafeMemcpyUnsafeSrcTest
[ RUN      ] SafeMemcpyUnsafeSrcTest.CopyUint8Pointer
[       OK ] SafeMemcpyUnsafeSrcTest.CopyUint8Pointer (0 ms)
[----------] 1 test from SafeMemcpyUnsafeSrcTest (0 ms total)

[----------] 1 test from SafeMemcpyUnsafeDstTest
[ RUN      ] SafeMemcpyUnsafeDstTest.PrependGrpcFrameHeader
[       OK ] SafeMemcpyUnsafeDstTest.PrependGrpcFrameHeader (0 ms)
[----------] 1 test from SafeMemcpyUnsafeDstTest (0 ms total)

[----------] Global test environment tear-down
[==========] 3 tests from 3 test suites ran. (0 ms total)
[  PASSED  ] 3 tests.

Nonetheless, if required, I can submit a PR with the following patch:

diff --git a/test/common/common/safe_memcpy_test.cc b/test/common/common/safe_memcpy_test.cc
index bbe8010..1f5f39b 100644
--- a/test/common/common/safe_memcpy_test.cc
+++ b/test/common/common/safe_memcpy_test.cc
@@ -37,7 +37,7 @@ TEST(SafeMemcpyUnsafeDstTest, PrependGrpcFrameHeader) {
   uint8_t* dst = new uint8_t[4];
   memset(dst, 0, 4);
   safeMemcpyUnsafeDst(dst, &src);
-  EXPECT_THAT(std::vector(dst, dst + sizeof(src)), ElementsAre(1, 2, 3, 4));
+  EXPECT_THAT(std::vector<uint8_t>(dst, dst + sizeof(src)), ElementsAre(1, 2, 3, 4));
   delete[] dst;
 }

@jmarantz
Copy link
Contributor

@rialg sounds good to me. I assume @Wyverald is using some combination of processor and compiler revision where it can't figure out the right vector type, though it looks derivable to me :)

@antoniovicente
Copy link
Contributor

Great to hear. Feel free to send me a PR with the diff above.

@Wyverald
Copy link
Contributor Author

The environment for the coverage build on Bazel CI is specified here: https://github.com/envoyproxy/envoy/blob/main/.bazelci/presubmit.yml#L19

It specifies that settings from https://github.com/envoyproxy/envoy/blob/main/bazel/setup_clang.sh should be used for the coverage run. I'm not completely sure what in there is tripping up the build, but someone with more knowledge of the setup could maybe tell?

@antoniovicente antoniovicente removed the triage Issue requires triage label May 19, 2021
@antoniovicente
Copy link
Contributor

@Wyverald: fix is merged, let us know if it fixed the bazel build.

@Wyverald
Copy link
Contributor Author

Thanks! That fixed the coverage test, but now fuzz_coverage is failing: https://buildkite.com/bazel/envoy/builds/1553#72726748-2aab-47ec-94ea-d90a382768dc

In file included from source/common/common/base_logger.cc:1:
--
 | bazel-out/k8-fastbuild/bin/source/common/common/_virtual_includes/base_logger_lib/common/common/base_logger.h:3:10: fatal error: 'memory' file not found
 | #include <memory>
 |          ^~~~~~~~
 | 1 error generated.

Now this looks like a real toolchain problem. I think we could, in the meantime, just disable the fuzz_coverage build and get a green commit (the last one is still in February).

@meteorcloudy
Copy link
Contributor

https://buildkite.com/bazel/envoy/builds/1572#193bb419-dcd3-4556-be76-7c76af5d69fb

@antoniovicente @rialg Can you help fix the fuzz_coverage failure on Bazel CI?

@meteorcloudy
Copy link
Contributor

Ping!

@antoniovicente
Copy link
Contributor

cc @lizan @asraa @phlax

meteorcloudy added a commit to meteorcloudy/envoy that referenced this issue Jun 14, 2021
envoyproxy#16542 has been keep Envoy red on Bazel CI for a long time, it doesn't make sense to keep a failing test if it's not going to be fixed.
Signed-off-by: Yun Peng <pcloudy@google.com>
@antoniovicente antoniovicente assigned phlax and lizan and unassigned antoniovicente and rialg Jun 14, 2021
antoniovicente pushed a commit that referenced this issue Jun 14, 2021
#16542 has been keep Envoy red on Bazel CI for a long time, it doesn't make sense to keep a failing test if it's not going to be fixed.

Signed-off-by: Yun Peng <pcloudy@google.com>
@meteorcloudy
Copy link
Contributor

This is now fixed, thanks!

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Jul 29, 2021
@meteorcloudy
Copy link
Contributor

Envoy is green again after disabling the tests: https://buildkite.com/bazel/envoy/builds?branch=main

@github-actions github-actions bot removed the stale stalebot believes this issue/PR has not been touched recently label Jul 29, 2021
@github-actions
Copy link

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Aug 28, 2021
@github-actions
Copy link

github-actions bot commented Sep 4, 2021

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.

@github-actions github-actions bot closed this as completed Sep 4, 2021
leyao-daily pushed a commit to leyao-daily/envoy that referenced this issue Sep 30, 2021
envoyproxy#16542 has been keep Envoy red on Bazel CI for a long time, it doesn't make sense to keep a failing test if it's not going to be fixed.

Signed-off-by: Yun Peng <pcloudy@google.com>
@keith
Copy link
Member

keith commented Nov 30, 2022

this test is still disabled, we should probably fix at some point

@keith keith reopened this Nov 30, 2022
@github-actions github-actions bot removed the stale stalebot believes this issue/PR has not been touched recently label Nov 30, 2022
@github-actions
Copy link

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Dec 31, 2022
@keith keith added no stalebot Disables stalebot from closing an issue and removed stale stalebot believes this issue/PR has not been touched recently labels Jan 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build bug no stalebot Disables stalebot from closing an issue
Projects
None yet
Development

No branches or pull requests

8 participants