Skip to content

Commit

Permalink
Revert "Revert "Revert "Revert "[core] Support encrypted redis connec…
Browse files Browse the repository at this point in the history
…tion." (ray-project#28991)""" (ray-project#29109)

Reverts ray-project#29078

There are two issues: tsan fail and mac build fail.

For TSAN, if we build redis + openssl with tsan the redis will segfault. So we exclude the redis target from the tsan build.

For mac failure, the root cause is that in the CI somehow the toolchain picked in rules_foreign_cc is wrong. Failed to find any useful solution within a day. So just use the old build for mac and pick in the runtime.

Since redis is only used for testing, win/mac/linux both support rediss.
  • Loading branch information
fishbone committed Oct 7, 2022
1 parent 366f2a9 commit 2b5f041
Show file tree
Hide file tree
Showing 15 changed files with 318 additions and 54 deletions.
4 changes: 2 additions & 2 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,9 @@ build:llvm --action_env=BAZEL_LINKOPTS=-lm:-pthread
build:llvm --define force_libcpp=enabled

# Thread sanitizer configuration:
build:tsan --per_file_copt="-bazel-ray/external/com_github_antirez_redis/.*$@-fsanitize=thread"
build:tsan --per_file_copt="-bazel-ray/external/com_github_antirez_redis/.*$@-DTHREAD_SANITIZER"
build:tsan --strip=never
build:tsan --copt -fsanitize=thread
build:tsan --copt -DTHREAD_SANITIZER
build:tsan --copt -O2
build:tsan --copt -g
build:tsan --copt -fno-omit-frame-pointer
Expand Down
2 changes: 2 additions & 0 deletions BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -2708,6 +2708,7 @@ alias(
name = "redis-server",
actual = select({
"@bazel_tools//src/conditions:windows": "@com_github_tporadowski_redis_bin//:redis-server.exe",
"@bazel_tools//src/conditions:darwin": "@com_github_antirez_redis//:redis-server-mac",
"//conditions:default": "@com_github_antirez_redis//:redis-server",
}),
visibility = ["//visibility:public"],
Expand All @@ -2717,6 +2718,7 @@ alias(
name = "redis-cli",
actual = select({
"@bazel_tools//src/conditions:windows": "@com_github_tporadowski_redis_bin//:redis-cli.exe",
"@bazel_tools//src/conditions:darwin": "@com_github_antirez_redis//:redis-cli-mac",
"//conditions:default": "@com_github_antirez_redis//:redis-cli",
}),
visibility = ["//visibility:public"],
Expand Down
7 changes: 5 additions & 2 deletions bazel/BUILD.hiredis
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
COPTS = [] + select({
COPTS = ["-DUSE_SSL=1"] + select({
"@bazel_tools//src/conditions:windows": [
"-D_CRT_DECLARE_NONSTDC_NAMES=0", # don't define off_t, to avoid conflicts
"-DWIN32",
"-DWIN32_LEAN_AND_MEAN"
],
"//conditions:default": [
],
Expand Down Expand Up @@ -32,7 +34,6 @@ cc_library(
],
exclude =
[
"ssl.c",
"test.c",
],
),
Expand All @@ -44,6 +45,8 @@ cc_library(
include_prefix = "hiredis",
deps = [
":_hiredis",
"@boringssl//:ssl",
"@boringssl//:crypto"
],
visibility = ["//visibility:public"],
)
100 changes: 61 additions & 39 deletions bazel/BUILD.redis
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
load("@rules_foreign_cc//foreign_cc:defs.bzl", "make")

exports_files(
[
"redis-server.exe",
Expand All @@ -6,13 +8,67 @@ exports_files(
visibility = ["//visibility:public"],
)

filegroup(
name = "all_srcs",
srcs = glob(
include = ["**"],
exclude = ["*.bazel"],
),
)


make(
name = "redis",
args = [
"BUILD_TLS=yes",
"-s",
],
copts = [
"-DLUA_USE_MKSTEMP",
"-Wno-pragmas",
"-Wno-empty-body",
"-fPIC",
],
visibility = ["//visibility:public"],
lib_source = ":all_srcs",
deps = [
"@openssl//:openssl",
],
out_binaries = [
"redis-server",
"redis-cli"
]
)

genrule(
name = "bin",
srcs = glob(["**"]),
srcs = [":redis"],
outs = [
"redis-server",
"redis-cli",
],
cmd = """
mkdir tmp-redis-bin
cp -rf $(locations :redis) ./tmp-redis-bin/
cp tmp-redis-bin/redis-server $(location redis-server)
chmod +x $(location redis-server)
cp tmp-redis-bin/redis-cli $(location redis-cli)
chmod +x $(location redis-cli)
rm -rf tmp-redis-bin
""",
visibility = ["//visibility:public"],
tags = ["local"],
)

# This is a build for mac without openssl.
# rules_foreign_cc somehow pick the wrong toolchain in CI.
genrule(
name = "bin-mac",
srcs = glob(["**"]),
outs = [
"redis-server-mac",
"redis-cli-mac",
],
cmd = """
unset CC LDFLAGS CXX CXXFLAGS
tmpdir="redis.tmp"
Expand All @@ -21,47 +77,13 @@ genrule(
chmod +x "$${tmpdir}"/deps/jemalloc/configure
parallel="$$(getconf _NPROCESSORS_ONLN || echo 1)"
make -s -C "$${tmpdir}" -j"$${parallel}" V=0 CFLAGS="$${CFLAGS-} -DLUA_USE_MKSTEMP -Wno-pragmas -Wno-empty-body"
mv "$${tmpdir}"/src/redis-server $(location redis-server)
chmod +x $(location redis-server)
mv "$${tmpdir}"/src/redis-cli $(location redis-cli)
chmod +x $(location redis-cli)
mv "$${tmpdir}"/src/redis-server $(location redis-server-mac)
chmod +x $(location redis-server-mac)
mv "$${tmpdir}"/src/redis-cli $(location redis-cli-mac)
chmod +x $(location redis-cli-mac)
rm -r -f -- "$${tmpdir}"
""",
visibility = ["//visibility:public"],
tags = ["local"],
)

# This library is for internal hiredis use, because hiredis assumes a
# different include prefix for itself than external libraries do.
cc_library(
name = "_hiredis",
hdrs = [
"deps/hiredis/dict.c",
"deps/hiredis/dict.h",
"deps/hiredis/fmacros.h",
],
strip_include_prefix = "deps/hiredis",
)

cc_library(
name = "hiredis",
srcs = glob(
[
"deps/hiredis/*.c",
"deps/hiredis/*.h",
],
exclude =
[
"deps/hiredis/test.c",
],
),
hdrs = glob([
"deps/hiredis/*.h",
"deps/hiredis/adapters/*.h",
]),
strip_include_prefix = "deps",
deps = [
":_hiredis",
],
visibility = ["//visibility:public"],
)
5 changes: 5 additions & 0 deletions bazel/ray_deps_build_all.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ load("@com_github_grpc_grpc//third_party/py:python_configure.bzl", "python_confi
load("@com_github_grpc_grpc//bazel:grpc_deps.bzl", "grpc_deps")
load("@rules_proto_grpc//:repositories.bzl", "rules_proto_grpc_toolchains")
load("@com_github_johnynek_bazel_jar_jar//:jar_jar.bzl", "jar_jar_repositories")
load("@rules_foreign_cc//foreign_cc:repositories.bzl", "rules_foreign_cc_dependencies")
load("@rules_foreign_cc_thirdparty//openssl:openssl_setup.bzl", "openssl_setup")



def ray_deps_build_all():
Expand All @@ -17,3 +20,5 @@ def ray_deps_build_all():
grpc_deps()
rules_proto_grpc_toolchains()
jar_jar_repositories()
rules_foreign_cc_dependencies()
openssl_setup()
32 changes: 32 additions & 0 deletions bazel/ray_deps_setup.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,13 @@ def ray_deps_setup():
auto_http_archive(
name = "com_github_antirez_redis",
build_file = "@com_github_ray_project_ray//bazel:BUILD.redis",
patch_args = ["-p1"],
url = "https://github.com/redis/redis/archive/refs/tags/7.0.5.tar.gz",
sha256 = "40827fcaf188456ad9b3be8e27a4f403c43672b6bb6201192dc15756af6f1eae",
patches = [
"@com_github_ray_project_ray//thirdparty/patches:redis-quiet.patch",
],
workspace_file_content = 'workspace(name = "com_github_antirez_redis")'
)

auto_http_archive(
Expand Down Expand Up @@ -243,6 +245,36 @@ def ray_deps_setup():
"@com_github_ray_project_ray//thirdparty/patches:grpc-python.patch",
],
)

http_archive(
name = "openssl",
strip_prefix = "openssl-1.1.1f",
sha256 = "186c6bfe6ecfba7a5b48c47f8a1673d0f3b0e5ba2e25602dd23b629975da3f35",
urls = [
"https://www.openssl.org/source/openssl-1.1.1f.tar.gz",
],
build_file = "@rules_foreign_cc_thirdparty//openssl:BUILD.openssl.bazel",
)

http_archive(
name = "rules_foreign_cc",
sha256 = "2a4d07cd64b0719b39a7c12218a3e507672b82a97b98c6a89d38565894cf7c51",
strip_prefix = "rules_foreign_cc-0.9.0",
url = "https://github.com/bazelbuild/rules_foreign_cc/archive/refs/tags/0.9.0.tar.gz",
)

git_repository(
name = "rules_perl",
remote = "https://github.com/bazelbuild/rules_perl.git",
commit = "022b8daf2bb4836ac7a50e4a1d8ea056a3e1e403",
)

http_archive(
name = "rules_foreign_cc_thirdparty",
sha256 = "2a4d07cd64b0719b39a7c12218a3e507672b82a97b98c6a89d38565894cf7c51",
strip_prefix = "rules_foreign_cc-0.9.0/examples/third_party",
url = "https://github.com/bazelbuild/rules_foreign_cc/archive/refs/tags/0.9.0.tar.gz",
)

http_archive(
# This rule is used by @com_github_grpc_grpc, and using a GitHub mirror
Expand Down
29 changes: 27 additions & 2 deletions python/ray/_private/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import ray
import ray._private.ray_constants as ray_constants
from ray._private.gcs_utils import GcsClient
from ray._raylet import GcsClientOptions
from ray._raylet import GcsClientOptions, Config
from ray.core.generated.common_pb2 import Language

resource = None
Expand Down Expand Up @@ -1107,11 +1107,36 @@ def _start_redis_instance(
if " " in password:
raise ValueError("Spaces not permitted in redis password.")
command += ["--requirepass", password]
command += ["--port", str(port), "--loglevel", "warning"]

if not Config.REDIS_ENABLE_SSL():
command += ["--port", str(port), "--loglevel", "warning"]
else:
import socket

with socket.socket() as s:
s.bind(("", 0))
free_port = s.getsockname()[1]
command += [
"--tls-port",
str(port),
"--loglevel",
"warning",
"--port",
str(free_port),
]

if listen_to_localhost_only:
command += ["--bind", "127.0.0.1"]
pidfile = os.path.join(session_dir_path, "redis-" + uuid.uuid4().hex + ".pid")
command += ["--pidfile", pidfile]
if Config.REDIS_ENABLE_SSL():
if Config.REDIS_CA_CERT():
command += ["--tls-ca-cert-file", Config.REDIS_CA_CERT()]
if Config.REDIS_CLIENT_CERT():
command += ["--tls-cert-file", Config.REDIS_CLIENT_CERT()]
if Config.REDIS_CLIENT_KEY():
command += ["--tls-key-file", Config.REDIS_CLIENT_KEY()]
command += ["--tls-replication", "yes"]
if sys.platform != "win32":
command += ["--save", "", "--appendonly", "no"]
process_info = start_ray_process(
Expand Down
12 changes: 12 additions & 0 deletions python/ray/includes/ray_config.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,15 @@ cdef extern from "ray/common/ray_config.h" nogil:
c_bool start_python_importer_thread() const

c_bool use_ray_syncer() const

c_bool REDIS_ENABLE_SSL() const

c_string REDIS_CA_CERT() const

c_string REDIS_CA_PATH() const

c_string REDIS_CLIENT_CERT() const

c_string REDIS_CLIENT_KEY() const

c_string REDIS_SERVER_NAME() const
24 changes: 24 additions & 0 deletions python/ray/includes/ray_config.pxi
Original file line number Diff line number Diff line change
Expand Up @@ -116,3 +116,27 @@ cdef class Config:
@staticmethod
def use_ray_syncer():
return RayConfig.instance().use_ray_syncer()

@staticmethod
def REDIS_ENABLE_SSL():
return RayConfig.instance().REDIS_ENABLE_SSL()

@staticmethod
def REDIS_CA_CERT():
return RayConfig.instance().REDIS_CA_CERT()

@staticmethod
def REDIS_CA_PATH():
return RayConfig.instance().REDIS_CA_PATH()

@staticmethod
def REDIS_CLIENT_CERT():
return RayConfig.instance().REDIS_CLIENT_CERT()

@staticmethod
def REDIS_CLIENT_KEY():
return RayConfig.instance().REDIS_CLIENT_KEY()

@staticmethod
def REDIS_SERVER_NAME():
return RayConfig.instance().REDIS_SERVER_NAME()
1 change: 1 addition & 0 deletions python/ray/tests/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ py_test_module_list(
"test_raylet_output.py",
"test_scheduling_performance.py",
"test_get_or_create_actor.py",
"test_redis_tls.py",
],
size = "small",
tags = ["exclusive", "small_size_python_tests", "team:core"],
Expand Down
Loading

0 comments on commit 2b5f041

Please sign in to comment.