Skip to content

Commit

Permalink
Do not embed the run_sniffer binary in the dockerutil library.
Browse files Browse the repository at this point in the history 
This is causing nogo test failures. Use a `data` dependency instead.

Updates #10885

PiperOrigin-RevId: 673541771
  • Loading branch information
@EtiennePerot @gvisor-bot
EtiennePerot authored and gvisor-bot committed Sep 11, 2024
1 parent 1f4299e commit 64de876
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 40 deletions.
32 changes: 16 additions & 16 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
Expand Down
15 changes: 2 additions & 13 deletions pkg/test/dockerutil/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -27,8 +16,8 @@ go_library(
"network.go",
"profile.go",
],
embedsrcs = [
":run_sniffer_bin", # keep
data = [
"//tools/ioctl_sniffer:run_sniffer",
],
visibility = ["//:sandbox"],
deps = [
Expand Down
25 changes: 14 additions & 11 deletions pkg/test/dockerutil/gpu.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,19 @@ 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.
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"
Expand All @@ -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 {
Expand All @@ -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(),
Expand Down

0 comments on commit 64de876

Please sign in to comment.