Add Dockerfile using RBE's Jessie base image by matt-kwong · Pull Request #15499 · grpc/grpc

matt-kwong · 2018-05-21T22:15:55Z

Add a Dockerfile that uses a RBE base image with Clang 7 to run sanitizers.

grpc-testing · 2018-05-21T22:19:22Z

****************************************************************

libgrpc.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

grpc-testing · 2018-05-21T22:27:50Z

[trickle] No significant performance differences

jtattermusch · 2018-05-21T22:31:44Z

templates/tools/dockerfile/test/cxx_jessie_x64/Dockerfile.template

nit: add an explanatory comment what's the image about - the image name is incomprehensible for someone who doesn't have lot of knowledge about foundry/RBE.

Done in new Dockerfile

jtattermusch · 2018-05-21T22:34:35Z

tools/dockerfile/test/cxx_jessie_x64/Dockerfile

as discussed, let's introduce a separate rbe clang image (cxx_sanitizers.. ) and use cxx_jessie_x64 (after removing clang) for running basic C/C++ suites.

https://github.com/grpc/grpc/blob/master/tools/run_tests/run_tests.py#L526

We actually use the current Jessie Dockerfile for clang3.5. clang3.5 is installed in https://github.com/grpc/grpc/blob/master/tools/dockerfile/test/cxx_jessie_x64/Dockerfile#L73, so we at least have to keep that version of clang, but I'm fine with removing the version installed from source.

Before updating my PR, since I forgot to remove clang, are you ok with the name cxx_rbe_jessie_x64 or do you think cxx_sanitizers_x64 is better?

I think cxx_sanitizers_... is better because it's more readable. no one knows what rbe is.

grpc-testing · 2018-05-21T23:11:18Z

[microbenchmarks] No significant performance differences

matt-kwong · 2018-05-22T00:28:58Z

All sanitizers passed except for
Asan C++ - https://source.cloud.google.com/results/invocations/3e582efc-7c5c-4553-8f7f-86240286cdf2/targets/github%2Fgrpc%2Fcpp_linux_asan_native/tests

Tsan C++ - https://source.cloud.google.com/results/invocations/3aff0c79-8da8-4313-82bd-5418a44b06ef/targets/github%2Fgrpc%2Fcpp_linux_tsan_native/tests

This was on Clang 6, using an older RBE base, but I will be updating to Clang 7

grpc-testing · 2018-05-22T01:21:03Z

****************************************************************

libgrpc.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

grpc-testing · 2018-05-22T01:28:11Z

[trickle] No significant performance differences

jtattermusch · 2018-05-22T01:38:20Z

templates/tools/dockerfile/test/cxx_jessie_x64/Dockerfile.template

can we remove <%include file="../../clang_update.include"/> from this file?

jtattermusch · 2018-05-22T01:39:16Z

templates/tools/dockerfile/test/cxx_rbe_jessie_x64/Dockerfile.template

I think not that many folks know what RBE means -> either add a comment or rename.

Changed the name of the file

grpc-testing · 2018-05-22T02:12:20Z

[microbenchmarks] No significant performance differences

grpc-testing · 2018-05-22T02:34:20Z

****************************************************************

libgrpc.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

grpc-testing · 2018-05-22T02:43:51Z

[trickle] No significant performance differences

grpc-testing · 2018-05-22T03:25:06Z

[microbenchmarks] No significant performance differences

grpc-testing · 2018-05-22T07:17:27Z

****************************************************************

libgrpc.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

grpc-testing · 2018-05-22T07:27:51Z

[trickle] No significant performance differences

grpc-testing · 2018-05-22T08:08:38Z

[microbenchmarks] No significant performance differences

grpc-testing · 2018-05-23T01:35:50Z

****************************************************************

libgrpc.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

grpc-testing · 2018-05-23T01:43:34Z

[trickle] No significant performance differences

grpc-testing · 2018-05-23T02:27:28Z

[microbenchmarks] No significant performance differences

grpc-testing · 2018-05-23T18:05:26Z

****************************************************************

libgrpc.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

matt-kwong · 2018-05-23T18:09:04Z

@jtattermusch I think this is ready for review. This PR introduces two ASAN C++ failures, both bm_clousure. It introduces at least one TSAN C++ failure, but there might be more failures since we sample tests for most sanitizers.

grpc-testing · 2018-05-23T18:12:51Z

[trickle] No significant performance differences

jtattermusch · 2018-05-23T18:24:26Z

tools/run_tests/run_tests.py

            return 'Makefile'

    def _clang_make_options(self, version_suffix=''):
+        if self.args.config == 'ubsan':


why do we need to special-case ubsan?

CC @nicolasnoble

I'm actually not sure why UBSAN is a special case. If anything, I expect all of the sanitizers to break. Before this change, UBSAN wouldn't build from a linking error because it would use clang instead of clang++ as the LD. Our Makefile specifies that the LD for all sanitizers should be clang++, but I'm guessing run_tests.py overrides that behavior.

jtattermusch

LGTM (but please see the comment about special-casing ubsan).

grpc-testing · 2018-05-23T18:56:51Z

[microbenchmarks] No significant performance differences

matt-kwong · 2018-05-23T20:52:46Z

MacOS failure - #15027

After a master sanitizer build runs all tests, I'll open issues for the new test failures.

matt-kwong requested review from jtattermusch and nicolasnoble as code owners May 21, 2018 22:15

matt-kwong added the kokoro:force-run label May 21, 2018

kokoro-team removed the kokoro:force-run label May 21, 2018

jtattermusch reviewed May 21, 2018

View reviewed changes

matt-kwong force-pushed the rbe-docker branch from 6d21e83 to 1d0a0cb Compare May 22, 2018 01:17

matt-kwong changed the title ~~Use RBE's Jessie base image~~ Add Dockerfile using RBE's Jessie base image May 22, 2018

jtattermusch reviewed May 22, 2018

View reviewed changes

matt-kwong force-pushed the rbe-docker branch from 1d0a0cb to 70c2f52 Compare May 22, 2018 02:30

matt-kwong force-pushed the rbe-docker branch from 70c2f52 to a20c5de Compare May 22, 2018 07:13

matt-kwong added 2 commits May 23, 2018 10:38

Support building sanitizers with Clang5+

67d7cdc

Add Jessie Dockerfile using RBE's base

528775d

matt-kwong added 3 commits May 23, 2018 10:40

Remove installed from source Clang in standard Jessie Dockerfile

048bb07

Readd cmake backport to Jessie

cacc2cc

Force UBSan to use clang++ as ld

6578c34

matt-kwong force-pushed the rbe-docker branch from f9c07c9 to 6578c34 Compare May 23, 2018 18:02

jtattermusch reviewed May 23, 2018

View reviewed changes

jtattermusch approved these changes May 23, 2018

View reviewed changes

matt-kwong merged commit f30db9d into grpc:master May 23, 2018

matt-kwong mentioned this pull request May 23, 2018

Upgrade clang to version 5 in Dockerfile #15301

Closed

jtattermusch mentioned this pull request May 23, 2018

Bazel test failure: //test/cpp/qps:qps_openloop_test fails with tsan and dbg #14257

Closed

lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019

Conversation

matt-kwong commented May 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

grpc-testing commented May 21, 2018

Uh oh!

grpc-testing commented May 21, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

grpc-testing commented May 21, 2018

Uh oh!

matt-kwong commented May 22, 2018

Uh oh!

grpc-testing commented May 22, 2018

Uh oh!

grpc-testing commented May 22, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

grpc-testing commented May 22, 2018

Uh oh!

grpc-testing commented May 22, 2018

Uh oh!

grpc-testing commented May 22, 2018

Uh oh!

grpc-testing commented May 22, 2018

Uh oh!

grpc-testing commented May 22, 2018

Uh oh!

grpc-testing commented May 22, 2018

Uh oh!

grpc-testing commented May 22, 2018

Uh oh!

grpc-testing commented May 23, 2018

Uh oh!

grpc-testing commented May 23, 2018

Uh oh!

grpc-testing commented May 23, 2018

Uh oh!

grpc-testing commented May 23, 2018

Uh oh!

matt-kwong commented May 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

grpc-testing commented May 23, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jtattermusch left a comment

Choose a reason for hiding this comment

Uh oh!

grpc-testing commented May 23, 2018

Uh oh!

matt-kwong commented May 23, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

matt-kwong commented May 21, 2018 •

edited

Loading

matt-kwong commented May 23, 2018 •

edited

Loading