From d1bda4a954aa154c9114173de6be9c0fa22bb140 Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Tue, 19 Apr 2022 09:13:38 -0500 Subject: [PATCH] ci: fixup task runner chroot test This PR is 2 fixes for the flaky TestTaskRunner_TaskEnv_Chroot test. And also the TestTaskRunner_Download_ChrootExec test. - Use TinyChroot to stop copying gigabytes of junk, which causes GHA to fail to create the environment in time. - Pre-create cgroups on V2 systems. Normally the cgroup directory is managed by the cpuset manager, but that is not active in taskrunner tests, so create it by hand in the test framework. --- ci/slow.go | 38 ++++++++++++ .../taskrunner/task_runner_test.go | 58 ++++++++++++------- client/config/testing.go | 7 +++ plugins/drivers/testutils/testing.go | 38 +----------- 4 files changed, 85 insertions(+), 56 deletions(-) diff --git a/ci/slow.go b/ci/slow.go index f6b8d066a..e2779e51e 100644 --- a/ci/slow.go +++ b/ci/slow.go @@ -26,3 +26,41 @@ func Parallel(t *testing.T) { t.Parallel() } } + +// TinyChroot is useful for testing, where we do not use anything other than +// trivial /bin commands like sleep and sh. Copying a minimal chroot helps in +// environments like GHA with very poor [network] disk performance. +// +// Note that you cannot chroot a symlink. +// +// Do not modify this value. +var TinyChroot = map[string]string{ + // destination: /bin + "/usr/bin/sleep": "/bin/sleep", + "/usr/bin/dash": "/bin/sh", + "/usr/bin/bash": "/bin/bash", + "/usr/bin/cat": "/bin/cat", + + // destination: /usr/bin + "/usr/bin/stty": "/usr/bin/stty", + "/usr/bin/head": "/usr/bin/head", + "/usr/bin/mktemp": "/usr/bin/mktemp", + "/usr/bin/echo": "/usr/bin/echo", + "/usr/bin/touch": "/usr/bin/touch", + "/usr/bin/stat": "/usr/bin/stat", + + // destination: /etc/ + "/etc/ld.so.cache": "/etc/ld.so.cache", + "/etc/ld.so.conf": "/etc/ld.so.conf", + "/etc/ld.so.conf.d": "/etc/ld.so.conf.d", + "/etc/passwd": "/etc/passwd", + "/etc/resolv.conf": "/etc/resolv.conf", + + // others + "/lib": "/lib", + "/lib32": "/lib32", + "/lib64": "/lib64", + "/usr/lib/jvm": "/usr/lib/jvm", + "/run/resolvconf": "/run/resolvconf", + "/run/systemd/resolve": "/run/systemd/resolve", +} diff --git a/client/allocrunner/taskrunner/task_runner_test.go b/client/allocrunner/taskrunner/task_runner_test.go index 4c0cc84bf..b6c0fdc53 100644 --- a/client/allocrunner/taskrunner/task_runner_test.go +++ b/client/allocrunner/taskrunner/task_runner_test.go @@ -15,15 +15,12 @@ import ( "github.com/golang/snappy" "github.com/hashicorp/nomad/ci" - "github.com/kr/pretty" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/client/allocrunner/interfaces" "github.com/hashicorp/nomad/client/config" consulapi "github.com/hashicorp/nomad/client/consul" "github.com/hashicorp/nomad/client/devicemanager" + "github.com/hashicorp/nomad/client/lib/cgutil" "github.com/hashicorp/nomad/client/pluginmanager/drivermanager" regMock "github.com/hashicorp/nomad/client/serviceregistration/mock" "github.com/hashicorp/nomad/client/serviceregistration/wrapper" @@ -41,6 +38,9 @@ import ( "github.com/hashicorp/nomad/plugins/device" "github.com/hashicorp/nomad/plugins/drivers" "github.com/hashicorp/nomad/testutil" + "github.com/kr/pretty" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) type MockTaskStateUpdater struct { @@ -92,10 +92,26 @@ func testTaskRunnerConfig(t *testing.T, alloc *structs.Allocation, taskName stri } taskDir := allocDir.NewTaskDir(taskName) + // Compute the name of the v2 cgroup in case we need it in creation, configuration, and cleanup + cgroup := filepath.Join(cgutil.CgroupRoot, "testing.slice", cgutil.CgroupScope(alloc.ID, taskName)) + + // Create the cgroup if we are in v2 mode + if cgutil.UseV2 { + if err := os.MkdirAll(cgroup, 0755); err != nil { + t.Fatalf("failed to setup v2 cgroup for test: %v:", err) + } + } + trCleanup := func() { if err := allocDir.Destroy(); err != nil { t.Logf("error destroying alloc dir: %v", err) } + + // Cleanup the cgroup if we are in v2 mode + if cgutil.UseV2 { + _ = os.RemoveAll(cgroup) + } + cleanup() } @@ -130,6 +146,14 @@ func testTaskRunnerConfig(t *testing.T, alloc *structs.Allocation, taskName stri ShutdownDelayCancelFn: shutdownDelayCancelFn, ServiceRegWrapper: wrapperMock, } + + // Set the cgroup path getter if we are in v2 mode + if cgutil.UseV2 { + conf.CpusetCgroupPathGetter = func(context.Context) (string, error) { + return filepath.Join(cgutil.CgroupRoot, "testing.slice", alloc.ID, thisTask.Name), nil + } + } + return conf, trCleanup } @@ -587,7 +611,6 @@ func TestTaskRunner_TaskEnv_Interpolated(t *testing.T) { func TestTaskRunner_TaskEnv_Chroot(t *testing.T) { ctestutil.ExecCompatible(t) ci.Parallel(t) - require := require.New(t) alloc := mock.BatchAlloc() task := alloc.Job.TaskGroups[0].Tasks[0] @@ -611,33 +634,27 @@ func TestTaskRunner_TaskEnv_Chroot(t *testing.T) { conf, cleanup := testTaskRunnerConfig(t, alloc, task.Name) defer cleanup() - // Remove /sbin and /usr from chroot - conf.ClientConfig.ChrootEnv = map[string]string{ - "/bin": "/bin", - "/etc": "/etc", - "/lib": "/lib", - "/lib32": "/lib32", - "/lib64": "/lib64", - "/run/resolvconf": "/run/resolvconf", - } - tr, err := NewTaskRunner(conf) - require.NoError(err) + require.NoError(t, err) go tr.Run() defer tr.Kill(context.Background(), structs.NewTaskEvent("cleanup")) // Wait for task to exit + timeout := 15 * time.Second + if testutil.IsCI() { + timeout = 120 * time.Second + } select { case <-tr.WaitCh(): - case <-time.After(15 * time.Second): - require.Fail("timeout waiting for task to exit") + case <-time.After(timeout): + require.Fail(t, "timeout waiting for task to exit") } // Read stdout p := filepath.Join(conf.TaskDir.LogDir, task.Name+".stdout.0") stdout, err := ioutil.ReadFile(p) - require.NoError(err) - require.Equalf(exp, string(stdout), "expected: %s\n\nactual: %s\n", exp, stdout) + require.NoError(t, err) + require.Equalf(t, exp, string(stdout), "expected: %s\n\nactual: %s\n", exp, stdout) } // TestTaskRunner_TaskEnv_Image asserts image drivers use chroot paths and @@ -1722,6 +1739,7 @@ func TestTaskRunner_Download_ChrootExec(t *testing.T) { task.Config = map[string]interface{}{ "command": "noop.sh", } + task.Artifacts = []*structs.TaskArtifact{ { GetterSource: fmt.Sprintf("%s/testdata/noop.sh", ts.URL), diff --git a/client/config/testing.go b/client/config/testing.go index 642cafdf7..a326cfbd6 100644 --- a/client/config/testing.go +++ b/client/config/testing.go @@ -5,6 +5,7 @@ import ( "os" "path/filepath" + "github.com/hashicorp/nomad/ci" "github.com/hashicorp/nomad/helper" "github.com/hashicorp/nomad/helper/testlog" "github.com/hashicorp/nomad/nomad/mock" @@ -51,6 +52,12 @@ func TestClientConfig(t testing.T) (*Config, func()) { } conf.StateDir = stateDir + // Use a minimal chroot environment + conf.ChrootEnv = ci.TinyChroot + + // Helps make sure we are respecting configured parent + conf.CgroupParent = "testing.slice" + conf.VaultConfig.Enabled = helper.BoolToPtr(false) conf.DevMode = true diff --git a/plugins/drivers/testutils/testing.go b/plugins/drivers/testutils/testing.go index c08220e3c..d568db148 100644 --- a/plugins/drivers/testutils/testing.go +++ b/plugins/drivers/testutils/testing.go @@ -12,6 +12,7 @@ import ( hclog "github.com/hashicorp/go-hclog" plugin "github.com/hashicorp/go-plugin" + "github.com/hashicorp/nomad/ci" "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/lib/cgutil" @@ -109,41 +110,6 @@ func (h *DriverHarness) cleanupCgroup() { } } -// tinyChroot is useful for testing, where we do not use anything other than -// trivial /bin commands like sleep and sh. -// -// Note that you cannot chroot a symlink. -var tinyChroot = map[string]string{ - // destination: /bin - "/usr/bin/sleep": "/bin/sleep", - "/usr/bin/dash": "/bin/sh", - "/usr/bin/bash": "/bin/bash", - "/usr/bin/cat": "/bin/cat", - - // destination: /usr/bin - "/usr/bin/stty": "/usr/bin/stty", - "/usr/bin/head": "/usr/bin/head", - "/usr/bin/mktemp": "/usr/bin/mktemp", - "/usr/bin/echo": "/usr/bin/echo", - "/usr/bin/touch": "/usr/bin/touch", - "/usr/bin/stat": "/usr/bin/stat", - - // destination: /etc/ - "/etc/ld.so.cache": "/etc/ld.so.cache", - "/etc/ld.so.conf": "/etc/ld.so.conf", - "/etc/ld.so.conf.d": "/etc/ld.so.conf.d", - "/etc/passwd": "/etc/passwd", - "/etc/resolv.conf": "/etc/resolv.conf", - - // others - "/lib": "/lib", - "/lib32": "/lib32", - "/lib64": "/lib64", - "/usr/lib/jvm": "/usr/lib/jvm", - "/run/resolvconf": "/run/resolvconf", - "/run/systemd/resolve": "/run/systemd/resolve", -} - // MkAllocDir creates a temporary directory and allocdir structure. // If enableLogs is set to true a logmon instance will be started to write logs // to the LogDir of the task @@ -165,7 +131,7 @@ func (h *DriverHarness) MkAllocDir(t *drivers.TaskConfig, enableLogs bool) func( fsi := caps.FSIsolation h.logger.Trace("FS isolation", "fsi", fsi) - require.NoError(h.t, taskDir.Build(fsi == drivers.FSIsolationChroot, tinyChroot)) + require.NoError(h.t, taskDir.Build(fsi == drivers.FSIsolationChroot, ci.TinyChroot)) task := &structs.Task{ Name: t.Name,