diff --git a/Makefile b/Makefile index f45345471..1983e32d3 100644 --- a/Makefile +++ b/Makefile @@ -99,6 +99,7 @@ endif ## RUNTIME_BIN - The runtime binary (default: $RUNTIME_DIR/runsc). ## RUNTIME_LOG_DIR - The logs directory (default: $RUNTIME_DIR/logs). ## RUNTIME_LOGS - The log pattern (default: $RUNTIME_LOG_DIR/runsc.log.%TEST%.%TIMESTAMP%.%COMMAND%). +## RUNTIME_ARGS - Arguments passed to the runtime when installed. ## STAGED_BINARIES - A tarball of staged binaries. If this is set, then binaries ## will be installed from this staged bundle instead of built. ## @@ -112,6 +113,7 @@ endif RUNTIME_BIN := $(RUNTIME_DIR)/runsc RUNTIME_LOG_DIR := $(RUNTIME_DIR)/logs RUNTIME_LOGS := $(RUNTIME_LOG_DIR)/runsc.log.%TEST%.%TIMESTAMP%.%COMMAND% +RUNTIME_ARGS ?= ifeq ($(shell stat -f -c "%T" /sys/fs/cgroup 2>/dev/null),cgroup2fs) CGROUPV2 := true @@ -133,7 +135,7 @@ endif # Configure helpers for below. configure_noreload = \ $(call header,CONFIGURE $(1) → $(RUNTIME_BIN) $(2)); \ - sudo $(RUNTIME_BIN) install --experimental=true --runtime="$(1)" -- --debug-log "$(RUNTIME_LOGS)" $(2) && \ + sudo $(RUNTIME_BIN) install --experimental=true --runtime="$(1)" -- $(RUNTIME_ARGS) --debug-log "$(RUNTIME_LOGS)" $(2) && \ sudo rm -rf "$(RUNTIME_LOG_DIR)" && mkdir -p "$(RUNTIME_LOG_DIR)" reload_docker = \ sudo systemctl reload docker && \ @@ -147,7 +149,7 @@ configure = $(call configure_noreload,$(1),$(2)) && $(reload_docker) install_runtime = $(call configure,$(1),$(2) --TESTONLY-test-name-env=RUNSC_TEST_NAME) # Don't use cached results, otherwise multiple runs using different runtimes # may be skipped, if all other inputs are the same. -test_runtime = $(call test,--test_arg=--runtime=$(1) --nocache_test_results $(PARTITIONS) $(2)) +test_runtime = $(call test,--test_env=RUNTIME=$(1) --nocache_test_results $(PARTITIONS) $(2)) refresh: $(RUNTIME_BIN) ## Updates the runtime binary. .PHONY: refresh @@ -175,7 +177,7 @@ dev: $(RUNTIME_BIN) ## Installs a set of local runtimes. Requires sudo. ## PARTITION ?= 1 TOTAL_PARTITIONS ?= 1 -PARTITIONS := --test_arg=--partition=$(PARTITION) --test_arg=--total_partitions=$(TOTAL_PARTITIONS) +PARTITIONS := --test_env=PARTITION=$(PARTITION) --test_env=TOTAL_PARTITIONS=$(TOTAL_PARTITIONS) runsc: ## Builds the runsc binary. @$(call build,-c opt //runsc) @@ -205,7 +207,7 @@ unit-tests: ## Local package unit tests in pkg/..., tools/.., etc. # See unit-tests: this includes runsc/container. container-tests: $(RUNTIME_BIN) ## Run all tests in runsc/container/... - @$(call test,--test_arg=--runsc=$(RUNTIME_BIN) runsc/container/...) + @$(call test,--test_env=RUNTIME=$(RUNTIME_BIN) runsc/container/...) .PHONY: container-tests tests: ## Runs all unit tests and syscall tests. @@ -221,18 +223,8 @@ network-tests: ## Run all networking integration tests. network-tests: iptables-tests packetdrill-tests packetimpact-tests .PHONY: network-tests -# The set of system call targets. -SYSCALL_TARGETS := test/syscalls/... test/fuse/... - -syscall-%-tests: - @$(call test,--test_tag_filters=runsc_$* $(PARTITIONS) test/syscalls/...) - -syscall-native-tests: - @$(call test,--test_tag_filters=native $(PARTITIONS) test/syscalls/...) -.PHONY: syscall-native-tests - -syscall-tests: ## Run all system call tests. - @$(call test,$(PARTITIONS) test/syscalls/...) +syscall-tests: $(RUNTIME_BIN) ## Run all system call tests. + @$(call test,--test_env=RUNTIME=$(RUNTIME_BIN) $(PARTITIONS) test/syscalls/... test/fuse/...) .PHONY: syscall-tests packetimpact-tests: @@ -286,7 +278,7 @@ swgso-tests: load-basic $(RUNTIME_BIN) hostnet-tests: load-basic $(RUNTIME_BIN) @$(call install_runtime,$(RUNTIME),--network=host) - @$(call test_runtime,$(RUNTIME),--test_arg=-checkpoint=false --test_arg=-hostnet=true $(INTEGRATION_TARGETS)) + @$(call test_runtime,$(RUNTIME),--test_env=CHECKPOINT=false --test_env=HOSTNET=true $(INTEGRATION_TARGETS)) .PHONY: hostnet-tests kvm-tests: load-basic $(RUNTIME_BIN) @@ -300,7 +292,10 @@ kvm-tests: load-basic $(RUNTIME_BIN) iptables-tests: load-iptables $(RUNTIME_BIN) @sudo modprobe iptable_filter @sudo modprobe ip6table_filter - @$(call test,--test_arg=-runtime=runc $(PARTITIONS) //test/iptables:iptables_test) + @sudo modprobe iptable_nat + @sudo modprobe ip6table_nat + @# FIXME(b/218923513): Need to fix permissions issues. + @#$(call test,--test_env=RUNTIME=runc //test/iptables:iptables_test) @$(call install_runtime,$(RUNTIME),--net-raw) @$(call test_runtime,$(RUNTIME),//test/iptables:iptables_test) .PHONY: iptables-tests @@ -358,7 +353,6 @@ containerd-tests: containerd-test-1.5.4 ## BENCHMARKS_FILTER - filter to be applied to the test suite. ## BENCHMARKS_OPTIONS - options to be passed to the test. ## BENCHMARKS_PROFILE - profile options to be passed to the test. -## BENCH_RUNTIME_ARGS - args to configure the runtime which runs the benchmarks. ## BENCHMARKS_PROJECT ?= gvisor-benchmarks BENCHMARKS_DATASET ?= kokoro @@ -371,8 +365,6 @@ BENCHMARKS_FILTER := . BENCHMARKS_OPTIONS := -test.benchtime=30s BENCHMARKS_ARGS := -test.v -test.bench=$(BENCHMARKS_FILTER) $(BENCHMARKS_OPTIONS) BENCHMARKS_PROFILE := -pprof-dir=/tmp/profile -pprof-cpu -pprof-heap -pprof-block -pprof-mutex -BENCH_VFS := --vfs2 -BENCH_RUNTIME_ARGS ?= init-benchmark-table: ## Initializes a BigQuery table with the benchmark schema. @$(call run,//tools/parsers:parser,init --project=$(BENCHMARKS_PROJECT) --dataset=$(BENCHMARKS_DATASET) --table=$(BENCHMARKS_TABLE)) @@ -394,14 +386,13 @@ run_benchmark = \ benchmark-platforms: load-benchmarks $(RUNTIME_BIN) ## Runs benchmarks for runc and all platforms. @set -xe; for PLATFORM in $$($(RUNTIME_BIN) help platforms); do \ export PLATFORM; \ - $(call run_benchmark,$${PLATFORM},--platform=$${PLATFORM} $(BENCH_RUNTIME_ARGS) --vfs2); \ + $(call run_benchmark,$${PLATFORM},--platform=$${PLATFORM}); \ done @$(call run_benchmark,runc) .PHONY: benchmark-platforms run-benchmark: load-benchmarks $(RUNTIME_BIN) ## Runs single benchmark and optionally sends data to BigQuery. - @if test "$(RUNTIME)" = "runc"; then $(call run_benchmark,$(RUNTIME)); fi; - @if test "$(RUNTIME)" != "runc"; then $(call run_benchmark,$(RUNTIME)$(BENCH_VFS),$(BENCH_RUNTIME_ARGS) $(BENCH_VFS)); fi; + @$(call run_benchmark,$(RUNTIME)) .PHONY: run-benchmark ## diff --git a/pkg/test/testutil/BUILD b/pkg/test/testutil/BUILD index 7ff13cf12..8b4797e12 100644 --- a/pkg/test/testutil/BUILD +++ b/pkg/test/testutil/BUILD @@ -15,6 +15,7 @@ go_library( "//pkg/sentry/watchdog", "//pkg/sync", "//runsc/config", + "//runsc/flag", "//runsc/specutils", "@com_github_cenkalti_backoff//:go_default_library", "@com_github_kr_pty//:go_default_library", diff --git a/pkg/test/testutil/testutil.go b/pkg/test/testutil/testutil.go index 1cbe190de..15d3e6ef2 100644 --- a/pkg/test/testutil/testutil.go +++ b/pkg/test/testutil/testutil.go @@ -21,7 +21,6 @@ import ( "debug/elf" "encoding/base32" "encoding/json" - "flag" "fmt" "io" "io/ioutil" @@ -45,17 +44,43 @@ import ( "gvisor.dev/gvisor/pkg/sentry/watchdog" "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/runsc/config" + "gvisor.dev/gvisor/runsc/flag" "gvisor.dev/gvisor/runsc/specutils" ) var ( - checkpoint = flag.Bool("checkpoint", true, "control checkpoint/restore support") - partition = flag.Int("partition", 1, "partition number, this is 1-indexed") - totalPartitions = flag.Int("total_partitions", 1, "total number of partitions") - isRunningWithHostNet = flag.Bool("hostnet", false, "whether test is running with hostnet") - runscPath = flag.String("runsc", "", "path to runsc binary") + checkpoint = flag.Bool("checkpoint", boolFromEnv("CHECKPOINT", true), "control checkpoint/restore support") + partition = flag.Int("partition", intFromEnv("PARTITION", 1), "partition number, this is 1-indexed") + totalPartitions = flag.Int("total_partitions", intFromEnv("TOTAL_PARTITIONS", 1), "total number of partitions") + isRunningWithHostNet = flag.Bool("hostnet", boolFromEnv("HOSTNET", false), "whether test is running with hostnet") + runscPath = flag.String("runsc", os.Getenv("RUNTIME"), "path to runsc binary") ) +func intFromEnv(name string, def int) int { + str := os.Getenv(name) + if str == "" { + return def + } + v, err := strconv.ParseInt(str, 10, 64) + if err != nil { + // N.B. This library is testonly, so a panic here is reasonable. + panic(fmt.Errorf("invalid environment variable %q; got %q expected integer: %w", name, str, err)) + } + return int(v) +} + +func boolFromEnv(name string, def bool) bool { + str := strings.ToLower(os.Getenv(name)) + if str == "" { + return def + } + v, err := strconv.ParseBool(str) + if err != nil { + panic(fmt.Errorf("invalid environment variable %q; got %q expected bool: %w", name, str, err)) + } + return v +} + // IsCheckpointSupported returns the relevant command line flag. func IsCheckpointSupported() bool { return *checkpoint @@ -174,13 +199,11 @@ func TestConfig(t *testing.T) *config.Config { logDir = dir + "/" } - // Only register flags if config is being used. Otherwise anyone that uses - // testutil will get flags registered and they may conflict. - config.RegisterFlags() - - conf, err := config.NewFromFlags() + testFlags := flag.NewFlagSet("test", flag.ContinueOnError) + config.RegisterFlags(testFlags) + conf, err := config.NewFromFlags(testFlags) if err != nil { - panic(err) + t.Fatalf("error loading configuration from flags: %v", err) } // Change test defaults. conf.Debug = true diff --git a/runsc/boot/BUILD b/runsc/boot/BUILD index aa4566bb4..e7ace61be 100644 --- a/runsc/boot/BUILD +++ b/runsc/boot/BUILD @@ -148,6 +148,7 @@ go_test( "//pkg/sync", "//pkg/unet", "//runsc/config", + "//runsc/flag", "//runsc/fsgofer", "@com_github_opencontainers_runtime_spec//specs-go:go_default_library", "@org_golang_x_sys//unix:go_default_library", diff --git a/runsc/boot/loader_test.go b/runsc/boot/loader_test.go index eb56bc563..86334dbba 100644 --- a/runsc/boot/loader_test.go +++ b/runsc/boot/loader_test.go @@ -35,6 +35,7 @@ import ( "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/pkg/unet" "gvisor.dev/gvisor/runsc/config" + "gvisor.dev/gvisor/runsc/flag" "gvisor.dev/gvisor/runsc/fsgofer" ) @@ -44,11 +45,12 @@ func init() { if err := fsgofer.OpenProcSelfFD(); err != nil { panic(err) } - config.RegisterFlags() } func testConfig() *config.Config { - conf, err := config.NewFromFlags() + testFlags := flag.NewFlagSet("test", flag.ContinueOnError) + config.RegisterFlags(testFlags) + conf, err := config.NewFromFlags(testFlags) if err != nil { panic(err) } diff --git a/runsc/cli/main.go b/runsc/cli/main.go index da02a699b..9070b8a5c 100644 --- a/runsc/cli/main.go +++ b/runsc/cli/main.go @@ -100,7 +100,8 @@ func Main(version string) { subcommands.Register(new(cmd.Boot), internalGroup) subcommands.Register(new(cmd.Gofer), internalGroup) - config.RegisterFlags() + // Register with the main command line. + config.RegisterFlags(flag.CommandLine) // All subcommands must be registered before flag parsing. flag.Parse() @@ -114,7 +115,7 @@ func Main(version string) { } // Create a new Config from the flags. - conf, err := config.NewFromFlags() + conf, err := config.NewFromFlags(flag.CommandLine) if err != nil { cmd.Fatalf(err.Error()) } diff --git a/runsc/cmd/install.go b/runsc/cmd/install.go index fbef4f020..69cb75d62 100644 --- a/runsc/cmd/install.go +++ b/runsc/cmd/install.go @@ -24,6 +24,8 @@ import ( "path" "github.com/google/subcommands" + "gvisor.dev/gvisor/pkg/sentry/platform" + "gvisor.dev/gvisor/runsc/config" "gvisor.dev/gvisor/runsc/flag" ) @@ -63,6 +65,26 @@ func (i *Install) SetFlags(fs *flag.FlagSet) { func (i *Install) Execute(_ context.Context, f *flag.FlagSet, _ ...interface{}) subcommands.ExitStatus { // Grab the name and arguments. runtimeArgs := f.Args() + testFlags := flag.NewFlagSet("test", flag.ContinueOnError) + config.RegisterFlags(testFlags) + testFlags.Parse(runtimeArgs) + conf, err := config.NewFromFlags(testFlags) + if err != nil { + log.Fatalf("invalid runtime arguments: %v", err) + } + + // Check the platform. + p, err := platform.Lookup(conf.Platform) + if err != nil { + log.Fatalf("invalid platform: %v", err) + } + deviceFile, err := p.OpenDevice() + if err != nil { + log.Printf("WARNING: unable to open platform, runsc may fail to start: %v", err) + } + if deviceFile != nil { + deviceFile.Close() + } // Extract the executable. path, err := os.Executable() diff --git a/runsc/config/BUILD b/runsc/config/BUILD index 64295d283..004a98332 100644 --- a/runsc/config/BUILD +++ b/runsc/config/BUILD @@ -13,7 +13,6 @@ go_library( "//pkg/refs", "//pkg/sentry/control:control_go_proto", "//pkg/sentry/watchdog", - "//pkg/sync", "//runsc/flag", ], ) diff --git a/runsc/config/config_test.go b/runsc/config/config_test.go index 420df1868..2e34bb11b 100644 --- a/runsc/config/config_test.go +++ b/runsc/config/config_test.go @@ -22,12 +22,10 @@ import ( "gvisor.dev/gvisor/runsc/flag" ) -func init() { - RegisterFlags() -} - func TestDefault(t *testing.T) { - c, err := NewFromFlags() + testFlags := flag.NewFlagSet("test", flag.ContinueOnError) + RegisterFlags(testFlags) + c, err := NewFromFlags(testFlags) if err != nil { t.Fatal(err) } @@ -42,46 +40,26 @@ func TestDefault(t *testing.T) { } } -func setDefault(name string) error { - fl := flag.CommandLine.Lookup(name) - return fl.Value.Set(fl.DefValue) -} - func TestFromFlags(t *testing.T) { - if err := flag.CommandLine.Lookup("root").Value.Set("some-path"); err != nil { + testFlags := flag.NewFlagSet("test", flag.ContinueOnError) + RegisterFlags(testFlags) + if err := testFlags.Lookup("root").Value.Set("some-path"); err != nil { t.Errorf("Flag set: %v", err) } - if err := flag.CommandLine.Lookup("debug").Value.Set("true"); err != nil { + if err := testFlags.Lookup("debug").Value.Set("true"); err != nil { t.Errorf("Flag set: %v", err) } - if err := flag.CommandLine.Lookup("num-network-channels").Value.Set("123"); err != nil { + if err := testFlags.Lookup("num-network-channels").Value.Set("123"); err != nil { t.Errorf("Flag set: %v", err) } - if err := flag.CommandLine.Lookup("network").Value.Set("none"); err != nil { + if err := testFlags.Lookup("network").Value.Set("none"); err != nil { t.Errorf("Flag set: %v", err) } - if err := flag.CommandLine.Lookup("controls").Value.Set("EVENTS,FS"); err != nil { + if err := testFlags.Lookup("controls").Value.Set("EVENTS,FS"); err != nil { t.Errorf("Flag set: %v", err) } - defer func() { - if err := setDefault("root"); err != nil { - t.Errorf("Flag set: %v", err) - } - if err := setDefault("debug"); err != nil { - t.Errorf("Flag set: %v", err) - } - if err := setDefault("num-network-channels"); err != nil { - t.Errorf("Flag set: %v", err) - } - if err := setDefault("network"); err != nil { - t.Errorf("Flag set: %v", err) - } - if err := setDefault("controls"); err != nil { - t.Errorf("Flag set: %v", err) - } - }() - c, err := NewFromFlags() + c, err := NewFromFlags(testFlags) if err != nil { t.Fatal(err) } @@ -106,7 +84,9 @@ func TestFromFlags(t *testing.T) { } func TestToFlags(t *testing.T) { - c, err := NewFromFlags() + testFlags := flag.NewFlagSet("test", flag.ContinueOnError) + RegisterFlags(testFlags) + c, err := NewFromFlags(testFlags) if err != nil { t.Fatal(err) } @@ -175,8 +155,9 @@ func TestInvalidFlags(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - defer setDefault(tc.name) - if err := flag.CommandLine.Lookup(tc.name).Value.Set("invalid"); err == nil || !strings.Contains(err.Error(), tc.error) { + testFlags := flag.NewFlagSet("test", flag.ContinueOnError) + RegisterFlags(testFlags) + if err := testFlags.Lookup(tc.name).Value.Set("invalid"); err == nil || !strings.Contains(err.Error(), tc.error) { t.Errorf("flag.Value.Set(invalid) wrong error reported: %v", err) } }) @@ -206,13 +187,14 @@ func TestValidationFail(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { + testFlags := flag.NewFlagSet("test", flag.ContinueOnError) + RegisterFlags(testFlags) for name, val := range tc.flags { - defer setDefault(name) - if err := flag.CommandLine.Lookup(name).Value.Set(val); err != nil { + if err := testFlags.Lookup(name).Value.Set(val); err != nil { t.Errorf("%s=%q: %v", name, val, err) } } - if _, err := NewFromFlags(); err == nil || !strings.Contains(err.Error(), tc.error) { + if _, err := NewFromFlags(testFlags); err == nil || !strings.Contains(err.Error(), tc.error) { t.Errorf("NewFromFlags() wrong error reported: %v", err) } }) @@ -220,7 +202,9 @@ func TestValidationFail(t *testing.T) { } func TestOverride(t *testing.T) { - c, err := NewFromFlags() + testFlags := flag.NewFlagSet("test", flag.ContinueOnError) + RegisterFlags(testFlags) + c, err := NewFromFlags(testFlags) if err != nil { t.Fatal(err) } @@ -228,10 +212,9 @@ func TestOverride(t *testing.T) { t.Run("string", func(t *testing.T) { c.RootDir = "foobar" - if err := c.Override("root", "bar"); err != nil { + if err := c.Override(testFlags, "root", "bar"); err != nil { t.Fatalf("Override(root, bar) failed: %v", err) } - defer setDefault("root") if c.RootDir != "bar" { t.Errorf("Override(root, bar) didn't work: %+v", c) } @@ -239,10 +222,9 @@ func TestOverride(t *testing.T) { t.Run("bool", func(t *testing.T) { c.Debug = true - if err := c.Override("debug", "false"); err != nil { + if err := c.Override(testFlags, "debug", "false"); err != nil { t.Fatalf("Override(debug, false) failed: %v", err) } - defer setDefault("debug") if c.Debug { t.Errorf("Override(debug, false) didn't work: %+v", c) } @@ -250,10 +232,9 @@ func TestOverride(t *testing.T) { t.Run("enum", func(t *testing.T) { c.FileAccess = FileAccessShared - if err := c.Override("file-access", "exclusive"); err != nil { + if err := c.Override(testFlags, "file-access", "exclusive"); err != nil { t.Fatalf("Override(file-access, exclusive) failed: %v", err) } - defer setDefault("file-access") if c.FileAccess != FileAccessExclusive { t.Errorf("Override(file-access, exclusive) didn't work: %+v", c) } @@ -261,18 +242,22 @@ func TestOverride(t *testing.T) { } func TestOverrideDisabled(t *testing.T) { - c, err := NewFromFlags() + testFlags := flag.NewFlagSet("test", flag.ContinueOnError) + RegisterFlags(testFlags) + c, err := NewFromFlags(testFlags) if err != nil { t.Fatal(err) } const errMsg = "flag override disabled" - if err := c.Override("root", "path"); err == nil || !strings.Contains(err.Error(), errMsg) { + if err := c.Override(testFlags, "root", "path"); err == nil || !strings.Contains(err.Error(), errMsg) { t.Errorf("Override() wrong error: %v", err) } } func TestOverrideError(t *testing.T) { - c, err := NewFromFlags() + testFlags := flag.NewFlagSet("test", flag.ContinueOnError) + RegisterFlags(testFlags) + c, err := NewFromFlags(testFlags) if err != nil { t.Fatal(err) } @@ -299,7 +284,7 @@ func TestOverrideError(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - if err := c.Override(tc.name, tc.value); err == nil || !strings.Contains(err.Error(), tc.error) { + if err := c.Override(testFlags, tc.name, tc.value); err == nil || !strings.Contains(err.Error(), tc.error) { t.Errorf("Override(%q, %q) wrong error: %v", tc.name, tc.value, err) } }) @@ -307,7 +292,9 @@ func TestOverrideError(t *testing.T) { } func TestOverrideAllowlist(t *testing.T) { - c, err := NewFromFlags() + testFlags := flag.NewFlagSet("test", flag.ContinueOnError) + RegisterFlags(testFlags) + c, err := NewFromFlags(testFlags) if err != nil { t.Fatal(err) } @@ -351,7 +338,7 @@ func TestOverrideAllowlist(t *testing.T) { }, } { t.Run(tc.flag, func(t *testing.T) { - err := c.Override(tc.flag, tc.value) + err := c.Override(testFlags, tc.flag, tc.value) if len(tc.error) == 0 { if err != nil { t.Errorf("Unexpected error: %v", err) diff --git a/runsc/config/flags.go b/runsc/config/flags.go index f5ca6b868..be0b1ac97 100644 --- a/runsc/config/flags.go +++ b/runsc/config/flags.go @@ -23,84 +23,79 @@ import ( "gvisor.dev/gvisor/pkg/refs" "gvisor.dev/gvisor/pkg/sentry/watchdog" - "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/runsc/flag" ) -var registration sync.Once - // RegisterFlags registers flags used to populate Config. -func RegisterFlags() { - registration.Do(func() { - // Although these flags are not part of the OCI spec, they are used by - // Docker, and thus should not be changed. - flag.String("root", "", "root directory for storage of container state.") - flag.String("log", "", "file path where internal debug information is written, default is stdout.") - flag.String("log-format", "text", "log format: text (default), json, or json-k8s.") - flag.Bool("debug", false, "enable debug logging.") +func RegisterFlags(flagSet *flag.FlagSet) { + // Although these flags are not part of the OCI spec, they are used by + // Docker, and thus should not be changed. + flagSet.String("root", "", "root directory for storage of container state.") + flagSet.String("log", "", "file path where internal debug information is written, default is stdout.") + flagSet.String("log-format", "text", "log format: text (default), json, or json-k8s.") + flagSet.Bool("debug", false, "enable debug logging.") - // These flags are unique to runsc, and are used to configure parts of the - // system that are not covered by the runtime spec. + // These flags are unique to runsc, and are used to configure parts of the + // system that are not covered by the runtime spec. - // Debugging flags. - flag.String("debug-log", "", "additional location for logs. If it ends with '/', log files are created inside the directory with default names. The following variables are available: %TIMESTAMP%, %COMMAND%.") - flag.String("panic-log", "", "file path where panic reports and other Go's runtime messages are written.") - flag.String("coverage-report", "", "file path where Go coverage reports are written. Reports will only be generated if runsc is built with --collect_code_coverage and --instrumentation_filter Bazel flags.") - flag.Bool("log-packets", false, "enable network packet logging.") - flag.String("debug-log-format", "text", "log format: text (default), json, or json-k8s.") - flag.Bool("alsologtostderr", false, "send log messages to stderr.") - flag.Bool("allow-flag-override", false, "allow OCI annotations (dev.gvisor.flag.) to override flags for debugging.") - flag.String("traceback", "system", "golang runtime's traceback level") + // Debugging flags. + flagSet.String("debug-log", "", "additional location for logs. If it ends with '/', log files are created inside the directory with default names. The following variables are available: %TIMESTAMP%, %COMMAND%.") + flagSet.String("panic-log", "", "file path where panic reports and other Go's runtime messages are written.") + flagSet.String("coverage-report", "", "file path where Go coverage reports are written. Reports will only be generated if runsc is built with --collect_code_coverage and --instrumentation_filter Bazel flags.") + flagSet.Bool("log-packets", false, "enable network packet logging.") + flagSet.String("debug-log-format", "text", "log format: text (default), json, or json-k8s.") + flagSet.Bool("alsologtostderr", false, "send log messages to stderr.") + flagSet.Bool("allow-flag-override", false, "allow OCI annotations (dev.gvisor.flag.) to override flags for debugging.") + flagSet.String("traceback", "system", "golang runtime's traceback level") - // Debugging flags: strace related - flag.Bool("strace", false, "enable strace.") - flag.String("strace-syscalls", "", "comma-separated list of syscalls to trace. If --strace is true and this list is empty, then all syscalls will be traced.") - flag.Uint("strace-log-size", 1024, "default size (in bytes) to log data argument blobs.") - flag.Bool("strace-event", false, "send strace to event.") + // Debugging flags: strace related + flagSet.Bool("strace", false, "enable strace.") + flagSet.String("strace-syscalls", "", "comma-separated list of syscalls to trace. If --strace is true and this list is empty, then all syscalls will be traced.") + flagSet.Uint("strace-log-size", 1024, "default size (in bytes) to log data argument blobs.") + flagSet.Bool("strace-event", false, "send strace to event.") - // Flags that control sandbox runtime behavior. - flag.String("platform", "ptrace", "specifies which platform to use: ptrace (default), kvm.") - flag.Var(watchdogActionPtr(watchdog.LogWarning), "watchdog-action", "sets what action the watchdog takes when triggered: log (default), panic.") - flag.Int("panic-signal", -1, "register signal handling that panics. Usually set to SIGUSR2(12) to troubleshoot hangs. -1 disables it.") - flag.Bool("profile", false, "prepares the sandbox to use Golang profiler. Note that enabling profiler loosens the seccomp protection added to the sandbox (DO NOT USE IN PRODUCTION).") - flag.String("profile-block", "", "collects a block profile to this file path for the duration of the container execution. Requires -profile=true.") - flag.String("profile-cpu", "", "collects a CPU profile to this file path for the duration of the container execution. Requires -profile=true.") - flag.String("profile-heap", "", "collects a heap profile to this file path for the duration of the container execution. Requires -profile=true.") - flag.String("profile-mutex", "", "collects a mutex profile to this file path for the duration of the container execution. Requires -profile=true.") - flag.String("trace", "", "collects a Go runtime execution trace to this file path for the duration of the container execution.") - flag.Bool("rootless", false, "it allows the sandbox to be started with a user that is not root. Sandbox and Gofer processes may run with same privileges as current user.") - flag.Var(leakModePtr(refs.NoLeakChecking), "ref-leak-mode", "sets reference leak check mode: disabled (default), log-names, log-traces.") - flag.Bool("cpu-num-from-quota", false, "set cpu number to cpu quota (least integer greater or equal to quota value, but not less than 2)") - flag.Bool("oci-seccomp", false, "Enables loading OCI seccomp filters inside the sandbox.") - flag.Var(defaultControlConfig(), "controls", "Sentry control endpoints.") + // Flags that control sandbox runtime behavior. + flagSet.String("platform", "ptrace", "specifies which platform to use: ptrace (default), kvm.") + flagSet.Var(watchdogActionPtr(watchdog.LogWarning), "watchdog-action", "sets what action the watchdog takes when triggered: log (default), panic.") + flagSet.Int("panic-signal", -1, "register signal handling that panics. Usually set to SIGUSR2(12) to troubleshoot hangs. -1 disables it.") + flagSet.Bool("profile", false, "prepares the sandbox to use Golang profiler. Note that enabling profiler loosens the seccomp protection added to the sandbox (DO NOT USE IN PRODUCTION).") + flagSet.String("profile-block", "", "collects a block profile to this file path for the duration of the container execution. Requires -profile=true.") + flagSet.String("profile-cpu", "", "collects a CPU profile to this file path for the duration of the container execution. Requires -profile=true.") + flagSet.String("profile-heap", "", "collects a heap profile to this file path for the duration of the container execution. Requires -profile=true.") + flagSet.String("profile-mutex", "", "collects a mutex profile to this file path for the duration of the container execution. Requires -profile=true.") + flagSet.String("trace", "", "collects a Go runtime execution trace to this file path for the duration of the container execution.") + flagSet.Bool("rootless", false, "it allows the sandbox to be started with a user that is not root. Sandbox and Gofer processes may run with same privileges as current user.") + flagSet.Var(leakModePtr(refs.NoLeakChecking), "ref-leak-mode", "sets reference leak check mode: disabled (default), log-names, log-traces.") + flagSet.Bool("cpu-num-from-quota", false, "set cpu number to cpu quota (least integer greater or equal to quota value, but not less than 2)") + flagSet.Bool("oci-seccomp", false, "Enables loading OCI seccomp filters inside the sandbox.") + flagSet.Var(defaultControlConfig(), "controls", "Sentry control endpoints.") - // Flags that control sandbox runtime behavior: FS related. - flag.Var(fileAccessTypePtr(FileAccessExclusive), "file-access", "specifies which filesystem validation to use for the root mount: exclusive (default), shared.") - flag.Var(fileAccessTypePtr(FileAccessShared), "file-access-mounts", "specifies which filesystem validation to use for volumes other than the root mount: shared (default), exclusive.") - flag.Bool("overlay", false, "wrap filesystem mounts with writable overlay. All modifications are stored in memory inside the sandbox.") - flag.Bool("verity", false, "specifies whether a verity file system will be mounted.") - flag.Bool("fsgofer-host-uds", false, "allow the gofer to mount Unix Domain Sockets.") - flag.Bool("vfs2", true, "enables VFSv2. This uses the new VFS layer that is faster than the previous one.") - flag.Bool("fuse", false, "TEST ONLY; This allows the use of the new experimental FUSE filesystem. Only works with VFS2.") - flag.Bool("lisafs", false, "Enables lisafs protocol instead of 9P. Only works with VFS2.") - flag.Bool("cgroupfs", false, "Automatically mount cgroupfs.") - flag.Bool("ignore-cgroups", false, "don't configure cgroups.") + // Flags that control sandbox runtime behavior: FS related. + flagSet.Var(fileAccessTypePtr(FileAccessExclusive), "file-access", "specifies which filesystem validation to use for the root mount: exclusive (default), shared.") + flagSet.Var(fileAccessTypePtr(FileAccessShared), "file-access-mounts", "specifies which filesystem validation to use for volumes other than the root mount: shared (default), exclusive.") + flagSet.Bool("overlay", false, "wrap filesystem mounts with writable overlay. All modifications are stored in memory inside the sandbox.") + flagSet.Bool("verity", false, "specifies whether a verity file system will be mounted.") + flagSet.Bool("fsgofer-host-uds", false, "allow the gofer to mount Unix Domain Sockets.") + flagSet.Bool("vfs2", true, "enables VFSv2. This uses the new VFS layer that is faster than the previous one.") + flagSet.Bool("fuse", false, "TEST ONLY; use while FUSE in VFSv2 is landing. This allows the use of the new experimental FUSE filesystem.") + flagSet.Bool("lisafs", false, "Enables lisafs protocol instead of 9P. This is only effective with VFS2.") + flagSet.Bool("cgroupfs", false, "Automatically mount cgroupfs.") + flagSet.Bool("ignore-cgroups", false, "don't configure cgroups.") - // Flags that control sandbox runtime behavior: network related. - flag.Var(networkTypePtr(NetworkSandbox), "network", "specifies which network to use: sandbox (default), host, none. Using network inside the sandbox is more secure because it's isolated from the host network.") - flag.Bool("net-raw", false, "enable raw sockets. When false, raw sockets are disabled by removing CAP_NET_RAW from containers (`runsc exec` will still be able to utilize raw sockets). Raw sockets allow malicious containers to craft packets and potentially attack the network.") - flag.Bool("gso", true, "enable hardware segmentation offload if it is supported by a network device.") - flag.Bool("software-gso", true, "enable software segmentation offload when hardware offload can't be enabled.") - flag.Bool("tx-checksum-offload", false, "enable TX checksum offload.") - flag.Bool("rx-checksum-offload", true, "enable RX checksum offload.") - flag.Var(queueingDisciplinePtr(QDiscFIFO), "qdisc", "specifies which queueing discipline to apply by default to the non loopback nics used by the sandbox.") - flag.Int("num-network-channels", 1, "number of underlying channels(FDs) to use for network link endpoints.") + // Flags that control sandbox runtime behavior: network related. + flagSet.Var(networkTypePtr(NetworkSandbox), "network", "specifies which network to use: sandbox (default), host, none. Using network inside the sandbox is more secure because it's isolated from the host network.") + flagSet.Bool("net-raw", false, "enable raw sockets. When false, raw sockets are disabled by removing CAP_NET_RAW from containers (`runsc exec` will still be able to utilize raw sockets). Raw sockets allow malicious containers to craft packets and potentially attack the network.") + flagSet.Bool("gso", true, "enable hardware segmentation offload if it is supported by a network device.") + flagSet.Bool("software-gso", true, "enable software segmentation offload when hardware offload can't be enabled.") + flagSet.Bool("tx-checksum-offload", false, "enable TX checksum offload.") + flagSet.Bool("rx-checksum-offload", true, "enable RX checksum offload.") + flagSet.Var(queueingDisciplinePtr(QDiscFIFO), "qdisc", "specifies which queueing discipline to apply by default to the non loopback nics used by the sandbox.") + flagSet.Int("num-network-channels", 1, "number of underlying channels(FDs) to use for network link endpoints.") - // Test flags, not to be used outside tests, ever. - flag.Bool("TESTONLY-unsafe-nonroot", false, "TEST ONLY; do not ever use! This skips many security measures that isolate the host from the sandbox.") - flag.String("TESTONLY-test-name-env", "", "TEST ONLY; do not ever use! Used for automated tests to improve logging.") - flag.Bool("TESTONLY-allow-packet-endpoint-write", false, "TEST ONLY; do not ever use! Used for tests to allow writes on packet sockets.") - }) + // Test flags, not to be used outside tests, ever. + flagSet.Bool("TESTONLY-unsafe-nonroot", false, "TEST ONLY; do not ever use! This skips many security measures that isolate the host from the sandbox.") + flagSet.String("TESTONLY-test-name-env", "", "TEST ONLY; do not ever use! Used for automated tests to improve logging.") + flagSet.Bool("TESTONLY-allow-packet-endpoint-write", false, "TEST ONLY; do not ever use! Used for tests to allow writes on packet sockets.") } // overrideAllowlist lists all flags that can be changed using OCI @@ -131,7 +126,7 @@ func checkOciSeccomp(name string, value string) error { } // NewFromFlags creates a new Config with values coming from command line flags. -func NewFromFlags() (*Config, error) { +func NewFromFlags(flagSet *flag.FlagSet) (*Config, error) { conf := &Config{} obj := reflect.ValueOf(conf).Elem() @@ -143,7 +138,7 @@ func NewFromFlags() (*Config, error) { // No flag set for this field. continue } - fl := flag.CommandLine.Lookup(name) + fl := flagSet.Lookup(name) if fl == nil { panic(fmt.Sprintf("Flag %q not found", name)) } @@ -170,6 +165,10 @@ func NewFromFlags() (*Config, error) { func (c *Config) ToFlags() []string { var rv []string + // Construct a temporary set for default plumbing. + flagSet := flag.NewFlagSet("tmp", flag.ContinueOnError) + RegisterFlags(flagSet) + obj := reflect.ValueOf(c).Elem() st := obj.Type() for i := 0; i < st.NumField(); i++ { @@ -181,7 +180,7 @@ func (c *Config) ToFlags() []string { } val := getVal(obj.Field(i)) - flag := flag.CommandLine.Lookup(name) + flag := flagSet.Lookup(name) if flag == nil { panic(fmt.Sprintf("Flag %q not found", name)) } @@ -194,7 +193,7 @@ func (c *Config) ToFlags() []string { } // Override writes a new value to a flag. -func (c *Config) Override(name string, value string) error { +func (c *Config) Override(flagSet *flag.FlagSet, name string, value string) error { obj := reflect.ValueOf(c).Elem() st := obj.Type() for i := 0; i < st.NumField(); i++ { @@ -204,7 +203,7 @@ func (c *Config) Override(name string, value string) error { // Not a flag field, or flag name doesn't match. continue } - fl := flag.CommandLine.Lookup(name) + fl := flagSet.Lookup(name) if fl == nil { // Flag must exist if there is a field match above. panic(fmt.Sprintf("Flag %q not found", name)) diff --git a/runsc/container/BUILD b/runsc/container/BUILD index e0e4f4149..fe2ef3c69 100644 --- a/runsc/container/BUILD +++ b/runsc/container/BUILD @@ -46,10 +46,12 @@ go_test( # Only run the default platform for the tsan test, which should # be compatible. For non-tsan builds, run all platforms. args = select({ - "//conditions:default": ["-test_platforms=%s" % ",".join( - [p for (p, tags) in platforms.items() if "manual" not in tags], - )], - "//tools:gotsan": ["-test_platforms=%s" % default_platform], + "//conditions:default": [ + "--test_platforms=%s" % ",".join([p for (p, tags) in platforms.items() if "manual" not in tags]) or default_platform, + ], + "//tools:gotsan": [ + "--test_platforms=%s" % default_platform, + ], }), data = [ "//runsc", diff --git a/runsc/container/container_test.go b/runsc/container/container_test.go index c61f3c4b1..28c310417 100644 --- a/runsc/container/container_test.go +++ b/runsc/container/container_test.go @@ -48,6 +48,7 @@ import ( ) func TestMain(m *testing.M) { + config.RegisterFlags(flag.CommandLine) log.SetLevel(log.Debug) if err := testutil.ConfigureExePath(); err != nil { panic(err.Error()) @@ -390,16 +391,21 @@ func run(spec *specs.Spec, conf *config.Config) error { } // platforms must be provided by the BUILD rule, or all platforms are included. -var platforms = flag.String("test_platforms", strings.Join(platform.List(), ","), "Platforms to test with.") +var platforms = flag.String("test_platforms", os.Getenv("TEST_PLATFORMS"), "Platforms to test with.") // configs generates different configurations to run tests. // // TODO(gvisor.dev/issue/1624): Remove VFS1 dimension. func configs(t *testing.T, noOverlay bool) map[string]*config.Config { - cs := make(map[string]*config.Config) - ps := strings.Split(*platforms, ",") + var ps []string + if *platforms == "" { + ps = platform.List() + } else { + ps = strings.Split(*platforms, ",") + } // Non-overlay versions. + cs := make(map[string]*config.Config) for _, p := range ps { c := testutil.TestConfig(t) c.Platform = p diff --git a/runsc/specutils/BUILD b/runsc/specutils/BUILD index 679d8bc8e..945e0131f 100644 --- a/runsc/specutils/BUILD +++ b/runsc/specutils/BUILD @@ -17,6 +17,7 @@ go_library( "//pkg/log", "//pkg/sentry/kernel/auth", "//runsc/config", + "//runsc/flag", "@com_github_cenkalti_backoff//:go_default_library", "@com_github_mohae_deepcopy//:go_default_library", "@com_github_opencontainers_runtime_spec//specs-go:go_default_library", diff --git a/runsc/specutils/specutils.go b/runsc/specutils/specutils.go index 9d3b97277..363c57747 100644 --- a/runsc/specutils/specutils.go +++ b/runsc/specutils/specutils.go @@ -37,6 +37,7 @@ import ( "gvisor.dev/gvisor/pkg/log" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" "gvisor.dev/gvisor/runsc/config" + "gvisor.dev/gvisor/runsc/flag" ) // ExePath must point to runsc binary, which is normally the same binary. It's @@ -200,7 +201,7 @@ func ReadSpecFromFile(bundleDir string, specFile *os.File, conf *config.Config) if strings.HasPrefix(annotation, flagPrefix) { name := annotation[len(flagPrefix):] log.Infof("Overriding flag: %s=%q", name, val) - if err := conf.Override(name, val); err != nil { + if err := conf.Override(flag.CommandLine, name, val); err != nil { return nil, err } } diff --git a/test/iptables/README.md b/test/iptables/README.md index 1327adce1..bf703d3e3 100644 --- a/test/iptables/README.md +++ b/test/iptables/README.md @@ -80,5 +80,5 @@ $ bazel test //test/iptables:iptables_test --test_filter= To run an individual test with `runc`: ```bash -$ bazel test //test/iptables:iptables_test --test_filter= --test_arg=--runtime=runc +$ bazel test //test/iptables:iptables_test --test_filter= --test_env=RUNTIME=runc ``` diff --git a/test/root/BUILD b/test/root/BUILD index 8d9fff578..a2deafea7 100644 --- a/test/root/BUILD +++ b/test/root/BUILD @@ -33,7 +33,9 @@ go_test( "//pkg/test/dockerutil", "//pkg/test/testutil", "//runsc/cgroup", + "//runsc/config", "//runsc/container", + "//runsc/flag", "//runsc/specutils", "@com_github_cenkalti_backoff//:go_default_library", "@com_github_opencontainers_runtime_spec//specs-go:go_default_library", diff --git a/test/root/main_test.go b/test/root/main_test.go index 9fb17e0dd..cae850bc4 100644 --- a/test/root/main_test.go +++ b/test/root/main_test.go @@ -15,13 +15,14 @@ package root import ( - "flag" "fmt" "os" "testing" "github.com/syndtr/gocapability/capability" "gvisor.dev/gvisor/pkg/test/dockerutil" + "gvisor.dev/gvisor/runsc/config" + "gvisor.dev/gvisor/runsc/flag" "gvisor.dev/gvisor/runsc/specutils" ) @@ -29,6 +30,7 @@ import ( // supported docker version, required capabilities, and configures the executable // path for runsc. func TestMain(m *testing.M) { + config.RegisterFlags(flag.CommandLine) flag.Parse() if !specutils.HasCapabilities(capability.CAP_SYS_ADMIN, capability.CAP_DAC_OVERRIDE) {