Clean-up go_grpc_client/server build rules

Summary: Use the new pl_aux_go_binary() rule to simplify the build of the go binaries in go_grpc_client/server. Test Plan: Existing tests. Reviewers: #stirling, yzhao Reviewed By: #stirling, yzhao Signed-off-by: Omid Azizi <[email protected]> Differential Revision: https://phab.corp.pixielabs.ai/D10618 GitOrigin-RevId: ca5e459
pixie-io · Jan 28, 2022 · 629f368 · 629f368
1 parent 9c09831
commit 629f368
Show file tree

Hide file tree

Showing 12 changed files with 64 additions and 135 deletions.
diff --git a/src/stirling/e2e_tests/BUILD.bazel b/src/stirling/e2e_tests/BUILD.bazel
@@ -83,13 +83,13 @@ sh_test(
     srcs = ["stirling_wrapper_test.sh"],
     args = [
         "$(location //src/stirling/binaries:stirling_wrapper)",
-        "$(location //src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:grpc_server)",
-        "$(location //src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:grpc_client)",
+        "$(location //src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_16_grpc_server_with_certs)",
+        "$(location //src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_16_grpc_client)",
     ],
     data = [
         "//src/stirling/binaries:stirling_wrapper",
-        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:grpc_client",
-        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:grpc_server",
+        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_16_grpc_client",
+        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_16_grpc_server_with_certs",
     ],
     # TODO(PL-1462): Uprobe attaching sometimes fails.
     flaky = True,
@@ -130,13 +130,13 @@ sh_test(
     srcs = ["stirling_wrapper_container_test.sh"],
     args = [
         "$(location //src/stirling/binaries:stirling_wrapper_image.tar)",
-        "$(location //src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:grpc_server)",
-        "$(location //src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:grpc_client)",
+        "$(location //src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_16_grpc_server_with_certs)",
+        "$(location //src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_16_grpc_client)",
     ],
     data = [
         "//src/stirling/binaries:stirling_wrapper_image.tar",
-        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:grpc_client",
-        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:grpc_server",
+        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_16_grpc_client",
+        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_16_grpc_server_with_certs",
     ],
     flaky = True,
     tags = [

diff --git a/src/stirling/e2e_tests/stirling_wrapper_container_test.sh b/src/stirling/e2e_tests/stirling_wrapper_container_test.sh
@@ -31,8 +31,8 @@ source "$pixie_root"/src/stirling/scripts/test_utils.sh
 if [ -z "$BUILD_WORKSPACE_DIRECTORY" ] && [ -z "$TEST_TMPDIR" ]; then
     # If the script was run in a stand-alone way, then build and set paths.
     stirling_image=$pixie_root/$(bazel_build //src/stirling/binaries:stirling_wrapper_image.tar)
-    go_grpc_server=$pixie_root/$(bazel_build //src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:grpc_server)
-    go_grpc_client=$pixie_root/$(bazel_build //src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:grpc_client)
+    go_grpc_server=$pixie_root/$(bazel_build //src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:go_1_16_grpc_server)
+    go_grpc_client=$pixie_root/$(bazel_build //src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:go_1_16_grpc_client)
 else
     # If the script was run through bazel, the locations are passed as arguments.
     stirling_image=$1

diff --git a/src/stirling/e2e_tests/stirling_wrapper_test.sh b/src/stirling/e2e_tests/stirling_wrapper_test.sh
@@ -33,8 +33,8 @@ if [ -z "$BUILD_WORKSPACE_DIRECTORY" ] && [ -z "$TEST_TMPDIR" ]; then
     echo "Running test in stand-alone mode"
     echo "Building stirling_wrapper and test binaries. This may take some time."
     stirling_wrapper=$pixie_root/$(bazel_build //src/stirling/binaries:stirling_wrapper)
-    go_grpc_server=$pixie_root/$(bazel_build //src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:grpc_server)
-    go_grpc_client=$pixie_root/$(bazel_build //src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:grpc_client)
+    go_grpc_server=$pixie_root/$(bazel_build //src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:go_1_16_grpc_server)
+    go_grpc_client=$pixie_root/$(bazel_build //src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:go_1_16_grpc_client)
 else
     # If the script was run through bazel, the locations are passed as arguments.
     stirling_wrapper=$1

diff --git a/src/stirling/source_connectors/dynamic_tracer/BUILD.bazel b/src/stirling/source_connectors/dynamic_tracer/BUILD.bazel
@@ -48,8 +48,8 @@ pl_cc_test(
     timeout = "moderate",
     srcs = ["dynamic_trace_bpf_test.cc"],
     data = [
-        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:grpc_client",
-        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:grpc_server",
+        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_16_grpc_client",
+        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_16_grpc_server_with_certs",
     ],
     tags = ["requires_bpf"],
     deps = [

diff --git a/src/stirling/source_connectors/dynamic_tracer/dynamic_trace_bpf_test.cc b/src/stirling/source_connectors/dynamic_tracer/dynamic_trace_bpf_test.cc
@@ -32,10 +32,10 @@
 
 constexpr std::string_view kClientPath =
     "src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client/"
-    "golang_1_16_grpc_client_binary/go/src/grpc_client/grpc_client";
+    "golang_1_16_grpc_client";
 constexpr std::string_view kServerPath =
     "src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server/"
-    "golang_1_16_grpc_server_binary/go/src/grpc_server/grpc_server";
+    "golang_1_16_grpc_server";
 
 namespace px {
 namespace stirling {

diff --git a/src/stirling/source_connectors/dynamic_tracer/dynamic_tracing/BUILD.bazel b/src/stirling/source_connectors/dynamic_tracer/dynamic_tracing/BUILD.bazel
@@ -92,7 +92,7 @@ pl_cc_test(
     srcs = ["dynamic_tracer_test.cc"],
     data = [
         "//src/stirling/obj_tools/testdata/go:test_go_binary",
-        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:grpc_server",
+        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_16_grpc_server_with_certs",
     ],
     tags = [
         "exclusive",

diff --git a/src/stirling/source_connectors/dynamic_tracer/dynamic_tracing/dynamic_tracer_test.cc b/src/stirling/source_connectors/dynamic_tracer/dynamic_tracing/dynamic_tracer_test.cc
@@ -45,7 +45,7 @@ using ::testing::SizeIs;
 
 constexpr char kServerPath[] =
     "src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server/"
-    "golang_1_16_grpc_server_binary/go/src/grpc_server/grpc_server";
+    "golang_1_16_grpc_server";
 
 constexpr char kPod0UpdateTxt[] = R"(
   uid: "pod0"

diff --git a/src/stirling/source_connectors/socket_tracer/BUILD.bazel b/src/stirling/source_connectors/socket_tracer/BUILD.bazel
@@ -256,10 +256,10 @@ pl_cc_test(
     timeout = "moderate",
     srcs = ["grpc_trace_bpf_test.cc"],
     data = [
-        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:go_1_17_grpc_client",
-        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:grpc_client",
-        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:go_1_17_grpc_server",
-        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:grpc_server",
+        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_16_grpc_client",
+        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client:golang_1_17_grpc_client",
+        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_16_grpc_server_with_certs",
+        "//src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server:golang_1_17_grpc_server_with_certs",
     ],
     flaky = True,
     shard_count = 3,

diff --git a/src/stirling/source_connectors/socket_tracer/grpc_trace_bpf_test.cc b/src/stirling/source_connectors/socket_tracer/grpc_trace_bpf_test.cc
@@ -72,7 +72,7 @@ class GRPCServer {
  public:
   static constexpr std::string_view kServerPath =
       "src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server/"
-      "golang_$0_grpc_server_binary/go/src/grpc_server/grpc_server";
+      "golang_$0_grpc_server";
 
   GRPCServer() = default;
 
@@ -105,7 +105,7 @@ class GRPCClient {
  public:
   static constexpr std::string_view kClientPath =
       "src/stirling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client/"
-      "golang_1_16_grpc_client_binary/go/src/grpc_client/grpc_client";
+      "golang_$0_grpc_client";
 
   void LaunchClient(std::string_view go_version, bool use_https, int port) {
     std::string client_path = absl::Substitute(kClientPath, go_version);

diff --git a/...irling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client/BUILD.bazel b/...irling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_client/BUILD.bazel
@@ -17,76 +17,31 @@
 # Build of test data binaries is controlled by dockerized builds, not bazel/gazelle.
 #gazelle:ignore
 
-load("@io_bazel_rules_docker//container:container.bzl", "container_image", "container_push")
-load("@io_bazel_rules_docker//docker/util:run.bzl", "container_run_and_commit", "container_run_and_extract")
+load("@io_bazel_rules_docker//container:container.bzl", "container_push")
+load("//src/stirling/testing/bazel:containerized_build.bzl", "pl_aux_go_binary")
 
-container_image(
-    name = "golang_1_16_grpc_client_source",
-    base = "@golang_1_16_image//image",
-    directory = "/go/src/grpc_client",
-    files = ["main.go"],
-)
+package(default_visibility = ["//src/stirling:__subpackages__"])
 
-container_run_and_commit(
-    name = "golang_1_16_grpc_client_build",
-    commands = [
-        "go mod init",
-        "go get",
-        "go build -v",
-    ],
-    docker_run_flags = ["-w /go/src/grpc_client"],
-    image = ":golang_1_16_grpc_client_source.tar",
-)
+#TODO(oazizi): The px.dev/pixie/src/stirling/source_connectors/socket_tracer/protocols/http2/testing/proto
+#              should be made local to the build containers, so that things don't break on proto update.
 
-container_run_and_extract(
-    name = "golang_1_16_grpc_client_binary",
-    commands = ["echo"],
-    extract_file = "/go/src/grpc_client/grpc_client",
-    image = ":golang_1_16_grpc_client_build_commit.tar",
+pl_aux_go_binary(
+    name = "golang_1_16_grpc_client",
+    base = "@golang_1_16_image//image",
+    files = ["main.go"],
 )
 
-filegroup(
-    name = "grpc_client",
-    srcs = [":golang_1_16_grpc_client_binary/go/src/grpc_client/grpc_client"],
-    visibility = ["//src/stirling:__subpackages__"],
+pl_aux_go_binary(
+    name = "golang_1_17_grpc_client",
+    base = "@golang_1_17_image//image",
+    files = ["main.go"],
 )
 
 container_push(
     name = "push_image",
     format = "Docker",
-    image = ":golang_1_16_grpc_client_build_commit.tar",
+    image = ":golang_1_16_grpc_client_image_with_binary_commit.tar",
     registry = "gcr.io",
     repository = "pl-dev-infra/demos/go_grpc_client",
     tag = "{BUILD_USER}",
 )
-
-container_image(
-    name = "golang_1_17_grpc_client_source",
-    base = "@golang_1_17_image//image",
-    directory = "/go/src/grpc_client",
-    files = ["main.go"],
-)
-
-container_run_and_commit(
-    name = "golang_1_17_grpc_client_build",
-    commands = [
-        "go mod init",
-        "go get",
-        "go build -v",
-    ],
-    docker_run_flags = ["-w /go/src/grpc_client"],
-    image = ":golang_1_17_grpc_client_source.tar",
-)
-
-container_run_and_extract(
-    name = "golang_1_17_grpc_client_binary",
-    commands = ["echo"],
-    extract_file = "/go/src/grpc_client/grpc_client",
-    image = ":golang_1_17_grpc_client_build_commit.tar",
-)
-
-filegroup(
-    name = "go_1_17_grpc_client",
-    srcs = [":golang_1_17_grpc_client_binary/go/src/grpc_client/grpc_client"],
-    visibility = ["//src/stirling:__subpackages__"],
-)
diff --git a/...irling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server/BUILD.bazel b/...irling/source_connectors/socket_tracer/protocols/http2/testing/go_grpc_server/BUILD.bazel
@@ -17,37 +17,39 @@
 # Build of test data binaries is controlled by dockerized builds, not bazel/gazelle.
 #gazelle:ignore
 
-load("@io_bazel_rules_docker//container:container.bzl", "container_image", "container_push")
-load("@io_bazel_rules_docker//docker/util:run.bzl", "container_run_and_commit", "container_run_and_extract")
+load("@io_bazel_rules_docker//container:container.bzl", "container_push")
+load("//src/stirling/testing/bazel:containerized_build.bzl", "pl_aux_go_binary")
 
-container_image(
-    name = "golang_1_16_grpc_server_source",
+package(default_visibility = ["//src/stirling:__subpackages__"])
+
+#TODO(oazizi): The px.dev/pixie/src/stirling/source_connectors/socket_tracer/protocols/http2/testing/proto
+#              should be made local to the build containers, so that things don't break on proto update.
+
+pl_aux_go_binary(
+    name = "golang_1_16_grpc_server",
     base = "@golang_1_16_image//image",
-    directory = "/go/src/grpc_server",
     files = ["main.go"],
 )
 
-container_run_and_commit(
-    name = "golang_1_16_grpc_server_build",
-    commands = [
-        "go mod init",
-        "go get",
-        "go build -v",
+filegroup(
+    name = "golang_1_16_grpc_server_with_certs",
+    srcs = [":golang_1_16_grpc_server"],
+    data = [
+        "https-server.crt",
+        "https-server.key",
     ],
-    docker_run_flags = ["-w /go/src/grpc_server"],
-    image = ":golang_1_16_grpc_server_source.tar",
+    visibility = ["//src/stirling:__subpackages__"],
 )
 
-container_run_and_extract(
-    name = "golang_1_16_grpc_server_binary",
-    commands = ["echo"],
-    extract_file = "/go/src/grpc_server/grpc_server",
-    image = ":golang_1_16_grpc_server_build_commit.tar",
+pl_aux_go_binary(
+    name = "golang_1_17_grpc_server",
+    base = "@golang_1_17_image//image",
+    files = ["main.go"],
 )
 
 filegroup(
-    name = "grpc_server",
-    srcs = [":golang_1_16_grpc_server_binary/go/src/grpc_server/grpc_server"],
+    name = "golang_1_17_grpc_server_with_certs",
+    srcs = [":golang_1_17_grpc_server"],
     data = [
         "https-server.crt",
         "https-server.key",
@@ -58,39 +60,8 @@ filegroup(
 container_push(
     name = "push_image",
     format = "Docker",
-    image = ":golang_1_16_grpc_server_build_commit.tar",
+    image = ":golang_1_16_grpc_server_image_with_binary_commit.tar",
     registry = "gcr.io",
     repository = "pl-dev-infra/demos/go_grpc_server",
     tag = "{BUILD_USER}",
 )
-
-container_image(
-    name = "golang_1_17_grpc_server_source",
-    base = "@golang_1_17_image//image",
-    directory = "/go/src/grpc_server",
-    files = ["main.go"],
-)
-
-container_run_and_commit(
-    name = "golang_1_17_grpc_server_build",
-    commands = [
-        "go mod init",
-        "go get",
-        "go build -v",
-    ],
-    docker_run_flags = ["-w /go/src/grpc_server"],
-    image = ":golang_1_17_grpc_server_source.tar",
-)
-
-container_run_and_extract(
-    name = "golang_1_17_grpc_server_binary",
-    commands = ["echo"],
-    extract_file = "/go/src/grpc_server/grpc_server",
-    image = ":golang_1_17_grpc_server_build_commit.tar",
-)
-
-filegroup(
-    name = "go_1_17_grpc_server",
-    srcs = [":golang_1_17_grpc_server_binary/go/src/grpc_server/grpc_server"],
-    visibility = ["//src/stirling:__subpackages__"],
-)
diff --git a/src/stirling/testing/bazel/containerized_build.bzl b/src/stirling/testing/bazel/containerized_build.bzl
@@ -22,6 +22,9 @@ load("@io_bazel_rules_docker//docker/util:run.bzl", "container_run_and_commit",
 # This lets us control the go version with which to build the auxiliary test binaries,
 # so we can ensure Stirling works on different versions of Go.
 # It also provides more determinism in our tests (e.g. less churn on go toolchain upgrades).
+# Main outputs:
+#  <name>: The stand-alone binary
+#  <name>_image_with_binary_commit.tar: A container with the built binary in the CWD.
 def pl_aux_go_binary(name, files, base, build_flags = ""):
     # Build path within the binary where the sources will be placed and built.
     container_build_dir = "/go/src/" + name