From c97ce9057b861048cec920e47d70e69e8ad3fcec Mon Sep 17 00:00:00 2001 From: Eytan Kidron Date: Tue, 29 Oct 2019 15:40:28 -0400 Subject: [PATCH] Simplify the platform and constraints settings for remote execution * Remove the toolchains //third_party/toolchains:local and //third_party/toolchains:local_large. * Remove the platforms :rbe_ubuntu1604, :rbe_ubuntu1604_large, :local and :local_large. * No longer inherit from @rbe_default//config:platform but instead use it directly. It is now the only non-windows platform. * When creating @rbe_default//config:platform directly with rbe_autoconfig, set dockerAddCapabilities and dockerPrivileged directly in the exec_properties field. No need to set dockerNetwork to "off" and dockerSiblingContainers to false since these are the defaults. * Also set gceMachineType = "n1-highmem-2" on the default platform. This value can be overridden by specific targets that want to use LARGE_MACHINE. * Use create_exec_properties_dict where appropriate. * Use custom_exec_properties to define LARGE_MACHINE. I wasn't able to test thoroughly that this PR does not break any existing targets. I was not able to run anything on windows/mac and I also don't have access to gRPC's RBE setup. --- WORKSPACE | 19 +++++++- bazel/grpc_deps.bzl | 8 +-- test/core/gprpp/BUILD | 3 +- test/core/iomgr/BUILD | 3 +- test/cpp/qps/BUILD | 3 +- third_party/toolchains/BUILD | 59 +++-------------------- third_party/toolchains/machine_size/BUILD | 31 ------------ tools/remote_build/rbe_common.bazelrc | 8 +-- 8 files changed, 38 insertions(+), 96 deletions(-) delete mode 100644 third_party/toolchains/machine_size/BUILD diff --git a/WORKSPACE b/WORKSPACE index 1b46a6c1348..c1862c15435 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -13,8 +13,6 @@ load("//bazel:grpc_extra_deps.bzl", "grpc_extra_deps") grpc_extra_deps() register_execution_platforms( - "//third_party/toolchains:local", - "//third_party/toolchains:local_large", "//third_party/toolchains:rbe_windows", ) @@ -22,6 +20,15 @@ register_toolchains( "//third_party/toolchains/bazel_0.26.0_rbe_windows:cc-toolchain-x64_windows", ) +load("@bazel_toolchains//rules/exec_properties:exec_properties.bzl", "create_exec_properties_dict", "custom_exec_properties") + +custom_exec_properties( + name = "grpc_custom_exec_properties", + constants = { + "LARGE_MACHINE": create_exec_properties_dict(gce_machine_type = "n1-standard-8"), + }, +) + load("@bazel_toolchains//rules:rbe_repo.bzl", "rbe_autoconfig") # Create toolchain configuration for remote execution. @@ -29,6 +36,14 @@ rbe_autoconfig( name = "rbe_default", # use exec_properties instead of deprecated remote_execution_properties use_legacy_platform_definition = False, + exec_properties = create_exec_properties_dict( + # n1-highmem-2 is the default (small machine) machine type. Targets + # that want to use other machines (such as LARGE_MACHINE) will override + # this value. + gce_machine_type = "n1-highmem-2", + docker_add_capabilities = "SYS_PTRACE", + docker_privileged = True, + ), ) load("@bazel_toolchains//rules:environments.bzl", "clang_env") diff --git a/bazel/grpc_deps.bzl b/bazel/grpc_deps.bzl index a6b62554f38..ec571423df1 100644 --- a/bazel/grpc_deps.bzl +++ b/bazel/grpc_deps.bzl @@ -174,11 +174,11 @@ def grpc_deps(): # list of releases is at https://releases.bazel.build/bazel-toolchains.html http_archive( name = "bazel_toolchains", - sha256 = "e9bab54199722935f239cb1cd56a80be2ac3c3843e1a6d3492e2bc11f9c21daf", - strip_prefix = "bazel-toolchains-1.0.0", + sha256 = "0b36eef8a66f39c8dbae88e522d5bbbef49d5e66e834a982402c79962281be10", + strip_prefix = "bazel-toolchains-1.0.1", urls = [ - "https://github.com/bazelbuild/bazel-toolchains/releases/download/1.0.0/bazel-toolchains-1.0.0.tar.gz", - "https://mirror.bazel.build/github.com/bazelbuild/bazel-toolchains/archive/1.0.0.tar.gz", + "https://github.com/bazelbuild/bazel-toolchains/releases/download/1.0.1/bazel-toolchains-1.0.1.tar.gz", + "https://mirror.bazel.build/github.com/bazelbuild/bazel-toolchains/archive/1.0.1.tar.gz", ], ) diff --git a/test/core/gprpp/BUILD b/test/core/gprpp/BUILD index 3e23d78b124..c0d3d0d275e 100644 --- a/test/core/gprpp/BUILD +++ b/test/core/gprpp/BUILD @@ -12,7 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. -load("//bazel:grpc_build_system.bzl", "grpc_cc_library", "grpc_cc_test", "grpc_cc_binary", "grpc_package", "LARGE_MACHINE") +load("//bazel:grpc_build_system.bzl", "grpc_cc_library", "grpc_cc_test", "grpc_cc_binary", "grpc_package") +load("@grpc_custom_exec_properties//:constants.bzl", "LARGE_MACHINE") licenses(["notice"]) # Apache v2 diff --git a/test/core/iomgr/BUILD b/test/core/iomgr/BUILD index 13327db112f..6ebca950988 100644 --- a/test/core/iomgr/BUILD +++ b/test/core/iomgr/BUILD @@ -12,7 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. -load("//bazel:grpc_build_system.bzl", "grpc_cc_library", "grpc_cc_test", "grpc_cc_binary", "grpc_package", "LARGE_MACHINE") +load("//bazel:grpc_build_system.bzl", "grpc_cc_library", "grpc_cc_test", "grpc_cc_binary", "grpc_package") +load("@grpc_custom_exec_properties//:constants.bzl", "LARGE_MACHINE") licenses(["notice"]) # Apache v2 diff --git a/test/cpp/qps/BUILD b/test/cpp/qps/BUILD index 60596c8e31d..1b8fe306a1b 100644 --- a/test/cpp/qps/BUILD +++ b/test/cpp/qps/BUILD @@ -14,8 +14,9 @@ licenses(["notice"]) # Apache v2 -load("//bazel:grpc_build_system.bzl", "grpc_cc_test", "grpc_cc_library", "grpc_cc_binary", "grpc_package", "LARGE_MACHINE") +load("//bazel:grpc_build_system.bzl", "grpc_cc_test", "grpc_cc_library", "grpc_cc_binary", "grpc_package") load("//test/cpp/qps:qps_benchmark_script.bzl", "qps_json_driver_batch", "json_run_localhost_batch") +load("@grpc_custom_exec_properties//:constants.bzl", "LARGE_MACHINE") grpc_package(name = "test/cpp/qps") diff --git a/third_party/toolchains/BUILD b/third_party/toolchains/BUILD index 93b8bead777..a581d1fe1e3 100644 --- a/third_party/toolchains/BUILD +++ b/third_party/toolchains/BUILD @@ -16,6 +16,8 @@ licenses(["notice"]) # Apache v2 package(default_visibility = ["//visibility:public"]) +load("@bazel_toolchains//rules/exec_properties:exec_properties.bzl", "create_exec_properties_dict") + alias( name = "rbe_windows", actual = ":rbe_windows_1803", @@ -29,56 +31,9 @@ platform( "@bazel_tools//platforms:windows", "@bazel_tools//tools/cpp:msvc", ], - exec_properties = { - "container-image" : "docker://gcr.io/grpc-testing/rbe_windows_toolchain@sha256:75728e7d6d804090f71095e5fec627b18cfa1f47adc52096c209f2a687e06b2e", - "gceMachineType" : "n1-highmem-2", - "OSFamily" : "Windows", - }, -) - -# RBE Ubuntu16_04 r346485 -platform( - name = "rbe_ubuntu1604", - parents = ["@rbe_default//config:platform"], - constraint_values = [ - "//third_party/toolchains/machine_size:standard", - ], - exec_properties = { - "gceMachineType" : "n1-highmem-2", # Small machines for majority of tests - "dockerSiblingContainers" : "false", - "dockerNetwork" : "off", - "dockerAddCapabilities" : "SYS_PTRACE", - "dockerPrivileged" : "true" - }, -) - -platform( - name = "rbe_ubuntu1604_large", - parents = ["@rbe_default//config:platform"], - constraint_values = [ - "//third_party/toolchains/machine_size:large", - ], - exec_properties = { - "gceMachineType" : "n1-standard-8", # Large machines for some resource demanding tests (TSAN). - "dockerSiblingContainers" : "false", - "dockerNetwork" : "off", - "dockerAddCapabilities" : "SYS_PTRACE", - "dockerPrivileged" : "true" - }, -) - -platform( - name = "local", - parents = ["@bazel_tools//platforms:target_platform"], - constraint_values = [ - "//third_party/toolchains/machine_size:standard", - ], -) - -platform( - name = "local_large", - parents = ["@bazel_tools//platforms:target_platform"], - constraint_values = [ - "//third_party/toolchains/machine_size:large", - ], + exec_properties = create_exec_properties_dict( + container_image = "docker://gcr.io/grpc-testing/rbe_windows_toolchain@sha256:75728e7d6d804090f71095e5fec627b18cfa1f47adc52096c209f2a687e06b2e", + gce_machine_type = "n1-highmem-2", + os_family = "Windows", + ), ) diff --git a/third_party/toolchains/machine_size/BUILD b/third_party/toolchains/machine_size/BUILD deleted file mode 100644 index cc962946c31..00000000000 --- a/third_party/toolchains/machine_size/BUILD +++ /dev/null @@ -1,31 +0,0 @@ -# Copyright 2018 gRPC authors. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -licenses(["notice"]) # Apache v2 - -package(default_visibility = ["//visibility:public"]) - -constraint_setting(name = "machine_size") - -constraint_value( - name = "large", - constraint_setting = ":machine_size", -) - -constraint_value( - name = "standard", - constraint_setting = ":machine_size", -) - -# Add other constraint values as needed (tiny, huge, etc.) in the future. diff --git a/tools/remote_build/rbe_common.bazelrc b/tools/remote_build/rbe_common.bazelrc index d4242baaae9..78f29206ced 100644 --- a/tools/remote_build/rbe_common.bazelrc +++ b/tools/remote_build/rbe_common.bazelrc @@ -22,9 +22,9 @@ startup --host_jvm_args=-Dbazel.DigestFunction=SHA256 build --crosstool_top=@rbe_default//cc:toolchain build --extra_toolchains=@rbe_default//config:cc-toolchain # Use custom execution platforms defined in third_party/toolchains -build --extra_execution_platforms=//third_party/toolchains:rbe_ubuntu1604,//third_party/toolchains:rbe_ubuntu1604_large -build --host_platform=//third_party/toolchains:rbe_ubuntu1604 -build --platforms=//third_party/toolchains:rbe_ubuntu1604 +build --extra_execution_platforms=@rbe_default//config:platform +build --host_platform=@rbe_default//config:platform +build --platforms=@rbe_default//config:platform build --spawn_strategy=remote build --strategy=Javac=remote @@ -81,7 +81,7 @@ build:tsan --copt=-gmlt # TODO(jtattermusch): use more reasonable test timeout build:tsan --test_timeout=3600 build:tsan --test_tag_filters=-no_linux,-qps_json_driver -build:tsan --extra_execution_platforms=//third_party/toolchains:rbe_ubuntu1604,//third_party/toolchains:rbe_ubuntu1604_large +build:tsan --extra_execution_platforms=@rbe_default//config:platform # undefined behavior sanitizer: most settings are already in %workspace%/.bazelrc # we only need a few additional ones that are Foundry specific