From 64de87610200a32650f13438b17a8d5e819bb2f9 Mon Sep 17 00:00:00 2001 From: Etienne Perot Date: Wed, 11 Sep 2024 14:38:34 -0700 Subject: [PATCH] Do not embed the `run_sniffer` binary in the `dockerutil` library. This is causing nogo test failures. Use a `data` dependency instead. Updates #10885 PiperOrigin-RevId: 673541771 --- Makefile | 32 ++++++++++++++++---------------- pkg/test/dockerutil/BUILD | 15 ++------------- pkg/test/dockerutil/gpu.go | 25 ++++++++++++++----------- 3 files changed, 32 insertions(+), 40 deletions(-) diff --git a/Makefile b/Makefile index 34781b7cd..c0ed0e0cb 100644 --- a/Makefile +++ b/Makefile @@ -284,15 +284,15 @@ gpu-smoke-images: load-basic_cuda-vector-add load-gpu_cuda-tests .PHONY: gpu-smoke-images gpu-smoke-tests: gpu-smoke-images $(RUNTIME_BIN) - @$(call sudo,test/gpu:smoke_test,--runtime=runc -test.v $(ARGS)) + @$(call test,test/gpu:smoke_test,--runtime=runc -test.v $(ARGS)) @$(call install_runtime,$(RUNTIME),--nvproxy=true --nvproxy-docker=true) - @$(call sudo,test/gpu:smoke_test,--runtime=$(RUNTIME) -test.v $(ARGS)) + @$(call test,test/gpu:smoke_test,--runtime=$(RUNTIME) -test.v $(ARGS)) .PHONY: gpu-smoke-tests cos-gpu-smoke-tests: gpu-smoke-images $(RUNTIME_BIN) - @$(call sudo,test/gpu:smoke_test,--runtime=runc -test.v --cos-gpu $(ARGS)) + @$(call test,test/gpu:smoke_test,--runtime=runc -test.v --cos-gpu $(ARGS)) @$(call install_runtime,$(RUNTIME),--nvproxy=true) - @$(call sudo,test/gpu:smoke_test,--runtime=$(RUNTIME) -test.v --cos-gpu $(ARGS)) + @$(call test,test/gpu:smoke_test,--runtime=$(RUNTIME) -test.v --cos-gpu $(ARGS)) .PHONY: cos-gpu-smoke-tests # Images needed for GPU tests. @@ -304,22 +304,22 @@ gpu-images: gpu-smoke-images load-gpu_pytorch load-gpu_ollama load-gpu_ollama_cl gpu-all-tests: gpu-images gpu-smoke-tests $(RUNTIME_BIN) @$(call install_runtime,$(RUNTIME),--nvproxy=true --nvproxy-docker=true) - @$(call sudo,test/gpu:pytorch_test,--runtime=$(RUNTIME) -test.v $(ARGS)) - @$(call sudo,test/gpu:textgen_test,--runtime=$(RUNTIME) -test.v $(ARGS)) - @$(call sudo,test/gpu:imagegen_test,--runtime=$(RUNTIME) -test.v $(ARGS)) - @$(call sudo,test/gpu:sr_test,--runtime=$(RUNTIME) -test.v $(ARGS)) - @$(call sudo,test/gpu:nccl_test,--runtime=$(RUNTIME) -test.v $(ARGS)) - @$(call sudo,test/gpu:sniffer_test,--runtime=$(RUNTIME) -test.v $(ARGS)) + @$(call test,test/gpu:pytorch_test,--runtime=$(RUNTIME) -test.v $(ARGS)) + @$(call test,test/gpu:textgen_test,--runtime=$(RUNTIME) -test.v $(ARGS)) + @$(call test,test/gpu:imagegen_test,--runtime=$(RUNTIME) -test.v $(ARGS)) + @$(call test,test/gpu:sr_test,--runtime=$(RUNTIME) -test.v $(ARGS)) + @$(call test,test/gpu:nccl_test,--runtime=$(RUNTIME) -test.v $(ARGS)) + @$(call test,test/gpu:sniffer_test,--runtime=$(RUNTIME) -test.v $(ARGS)) .PHONY: gpu-all-tests cos-gpu-all-tests: gpu-images cos-gpu-smoke-tests $(RUNTIME_BIN) @$(call install_runtime,$(RUNTIME),--nvproxy=true) - @$(call sudo,test/gpu:pytorch_test,--runtime=$(RUNTIME) -test.v --cos-gpu $(ARGS)) - @$(call sudo,test/gpu:textgen_test,--runtime=$(RUNTIME) -test.v --cos-gpu $(ARGS)) - @$(call sudo,test/gpu:imagegen_test,--runtime=$(RUNTIME) -test.v --cos-gpu $(ARGS)) - @$(call sudo,test/gpu:sr_test,--runtime=$(RUNTIME) -test.v --cos-gpu $(ARGS)) - @$(call sudo,test/gpu:nccl_test,--runtime=$(RUNTIME) -test.v --cos-gpu $(ARGS)) - @$(call sudo,test/gpu:sniffer_test,--runtime=$(RUNTIME) -test.v --cos-gpu $(ARGS)) + @$(call test,test/gpu:pytorch_test,--runtime=$(RUNTIME) -test.v --cos-gpu $(ARGS)) + @$(call test,test/gpu:textgen_test,--runtime=$(RUNTIME) -test.v --cos-gpu $(ARGS)) + @$(call test,test/gpu:imagegen_test,--runtime=$(RUNTIME) -test.v --cos-gpu $(ARGS)) + @$(call test,test/gpu:sr_test,--runtime=$(RUNTIME) -test.v --cos-gpu $(ARGS)) + @$(call test,test/gpu:nccl_test,--runtime=$(RUNTIME) -test.v --cos-gpu $(ARGS)) + @$(call test,test/gpu:sniffer_test,--runtime=$(RUNTIME) -test.v --cos-gpu $(ARGS)) .PHONY: cos-gpu-all-tests portforward-tests: load-basic_redis load-basic_nginx $(RUNTIME_BIN) diff --git a/pkg/test/dockerutil/BUILD b/pkg/test/dockerutil/BUILD index 2b120c614..c41c2100f 100644 --- a/pkg/test/dockerutil/BUILD +++ b/pkg/test/dockerutil/BUILD @@ -5,17 +5,6 @@ package( licenses = ["notice"], ) -# We copy the `run_sniffer` binary here because `go:embed` can only embed -# from the current directory or subdirectories, not parents of it. -genrule( - name = "run_sniffer_bin", - srcs = [ - "//tools/ioctl_sniffer:run_sniffer", - ], - outs = ["run_sniffer_copy"], - cmd = "cat < $(SRCS) > $@", -) - go_library( name = "dockerutil", testonly = 1, @@ -27,8 +16,8 @@ go_library( "network.go", "profile.go", ], - embedsrcs = [ - ":run_sniffer_bin", # keep + data = [ + "//tools/ioctl_sniffer:run_sniffer", ], visibility = ["//:sandbox"], deps = [ diff --git a/pkg/test/dockerutil/gpu.go b/pkg/test/dockerutil/gpu.go index 6419cb08c..999925723 100644 --- a/pkg/test/dockerutil/gpu.go +++ b/pkg/test/dockerutil/gpu.go @@ -18,13 +18,12 @@ package dockerutil import ( "flag" "fmt" + "io" "os" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/mount" - - // Needed for go:embed - _ "embed" + "gvisor.dev/gvisor/pkg/test/testutil" ) // Flags. @@ -32,9 +31,6 @@ var ( setCOSGPU = flag.Bool("cos-gpu", false, "set to configure GPU settings for COS, as opposed to Docker") ) -//go:embed run_sniffer_copy -var runSnifferBinary []byte - const ( // ioctlSnifferMountPath is the in-container path at which the ioctl sniffer is mounted. ioctlSnifferMountPath = "/ioctl_sniffer" @@ -54,12 +50,22 @@ const ( func GPURunOpts(sniffGPUOpts SniffGPUOpts) (RunOpts, error) { var mounts []mount.Mount if sniffGPUOpts.DisableSnifferReason == "" { - // Extract the sniffer binary to a temporary location. + snifferRunfilesPath, err := testutil.FindFile("tools/ioctl_sniffer/run_sniffer") + if err != nil { + return RunOpts{}, fmt.Errorf("failed to find run_sniffer binary: %w", err) + } + snifferRunfilesFile, err := os.Open(snifferRunfilesPath) + if err != nil { + return RunOpts{}, fmt.Errorf("failed to open run_sniffer binary: %w", err) + } + defer snifferRunfilesFile.Close() + // Copy the sniffer binary to a temporary location. runSniffer, err := os.CreateTemp("", "run_sniffer.*") if err != nil { return RunOpts{}, fmt.Errorf("failed to create temporary file: %w", err) } - if _, err := runSniffer.Write(runSnifferBinary); err != nil { + defer runSniffer.Close() + if _, err := io.Copy(runSniffer, snifferRunfilesFile); err != nil { return RunOpts{}, fmt.Errorf("failed to write to temporary file: %w", err) } if err := runSniffer.Sync(); err != nil { @@ -68,9 +74,6 @@ func GPURunOpts(sniffGPUOpts SniffGPUOpts) (RunOpts, error) { if err := runSniffer.Chmod(0o555); err != nil { return RunOpts{}, fmt.Errorf("failed to chmod temporary file: %w", err) } - if err := runSniffer.Close(); err != nil { - return RunOpts{}, fmt.Errorf("failed to close temporary file: %w", err) - } sniffGPUOpts.runSniffer = runSniffer mounts = append(mounts, mount.Mount{ Source: runSniffer.Name(),