From 617a0eb9b82e9437343e0b8573178159f3fac0ca Mon Sep 17 00:00:00 2001 From: hc-github-team-nomad-core <82989552+hc-github-team-nomad-core@users.noreply.github.com> Date: Fri, 12 May 2023 12:56:20 -0500 Subject: [PATCH] allocrunner: provide factory function so we can build mock ARs (#17161) (#17173) Tools like `nomad-nodesim` are unable to implement a minimal implementation of an allocrunner so that we can test the client communication without having to lug around the entire allocrunner/taskrunner code base. The allocrunner was implemented with an interface specifically for this purpose, but there were circular imports that made it challenging to use in practice. Move the AllocRunner interface into an inner package and provide a factory function type. Provide a minimal test that exercises the new function so that consumers have some idea of what the minimum implementation required is. Co-authored-by: Tim Gross --- client/allocrunner/alloc_runner.go | 14 +- client/allocrunner/alloc_runner_test.go | 140 ++++++++------- client/allocrunner/alloc_runner_unix_test.go | 27 +-- client/allocrunner/config.go | 95 ---------- client/allocrunner/csi_hook.go | 5 +- client/allocrunner/interfaces/runner.go | 41 ++++- client/allocrunner/migrate_hook.go | 6 +- client/allocrunner/testing.go | 10 +- client/allocrunner/upstream_allocs_hook.go | 6 +- client/allocwatcher/alloc_watcher.go | 33 +--- client/allocwatcher/group_alloc_watcher.go | 8 +- .../allocwatcher/group_alloc_watcher_test.go | 5 +- client/client.go | 63 ++----- client/client_interface_test.go | 165 ++++++++++++++++++ client/client_test.go | 2 +- client/config/arconfig.go | 136 +++++++++++++++ client/config/config.go | 2 + client/gc.go | 10 +- client/gc_test.go | 3 +- client/heartbeatstop.go | 6 +- client/state/upgrade_int_test.go | 3 +- drivers/mock/driver_test.go | 3 +- plugins/drivers/testutils/testing.go | 13 +- 23 files changed, 497 insertions(+), 299 deletions(-) create mode 100644 client/client_interface_test.go create mode 100644 client/config/arconfig.go diff --git a/client/allocrunner/alloc_runner.go b/client/allocrunner/alloc_runner.go index 9de657b788e..517c970e7af 100644 --- a/client/allocrunner/alloc_runner.go +++ b/client/allocrunner/alloc_runner.go @@ -14,7 +14,6 @@ import ( "github.com/hashicorp/nomad/client/allocrunner/state" "github.com/hashicorp/nomad/client/allocrunner/tasklifecycle" "github.com/hashicorp/nomad/client/allocrunner/taskrunner" - "github.com/hashicorp/nomad/client/allocwatcher" "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/consul" "github.com/hashicorp/nomad/client/devicemanager" @@ -144,10 +143,10 @@ type allocRunner struct { // prevAllocWatcher allows waiting for any previous or preempted allocations // to exit - prevAllocWatcher allocwatcher.PrevAllocWatcher + prevAllocWatcher config.PrevAllocWatcher // prevAllocMigrator allows the migration of a previous allocations alloc dir. - prevAllocMigrator allocwatcher.PrevAllocMigrator + prevAllocMigrator config.PrevAllocMigrator // dynamicRegistry contains all locally registered dynamic plugins (e.g csi // plugins). @@ -182,7 +181,7 @@ type allocRunner struct { // rpcClient is the RPC Client that should be used by the allocrunner and its // hooks to communicate with Nomad Servers. - rpcClient RPCer + rpcClient config.RPCer // serviceRegWrapper is the handler wrapper that is used by service hooks // to perform service and check registration and deregistration. @@ -195,13 +194,8 @@ type allocRunner struct { getter cinterfaces.ArtifactGetter } -// RPCer is the interface needed by hooks to make RPC calls. -type RPCer interface { - RPC(method string, args interface{}, reply interface{}) error -} - // NewAllocRunner returns a new allocation runner. -func NewAllocRunner(config *Config) (*allocRunner, error) { +func NewAllocRunner(config *config.AllocRunnerConfig) (interfaces.AllocRunner, error) { alloc := config.Alloc tg := alloc.Job.LookupTaskGroup(alloc.TaskGroup) if tg == nil { diff --git a/client/allocrunner/alloc_runner_test.go b/client/allocrunner/alloc_runner_test.go index f9292afbd78..9e857f0237a 100644 --- a/client/allocrunner/alloc_runner_test.go +++ b/client/allocrunner/alloc_runner_test.go @@ -13,6 +13,7 @@ import ( multierror "github.com/hashicorp/go-multierror" "github.com/hashicorp/nomad/ci" "github.com/hashicorp/nomad/client/allochealth" + "github.com/hashicorp/nomad/client/allocrunner/interfaces" "github.com/hashicorp/nomad/client/allocrunner/tasklifecycle" "github.com/hashicorp/nomad/client/allocrunner/taskrunner" "github.com/hashicorp/nomad/client/allocwatcher" @@ -29,7 +30,7 @@ import ( ) // destroy does a blocking destroy on an alloc runner -func destroy(ar *allocRunner) { +func destroy(ar interfaces.AllocRunner) { ar.Destroy() <-ar.DestroyCh() } @@ -45,7 +46,7 @@ func TestAllocRunner_AllocState_Initialized(t *testing.T) { defer cleanup() ar, err := NewAllocRunner(conf) - require.NoError(t, err) + must.NoError(t, err) allocState := ar.AllocState() @@ -86,7 +87,7 @@ func TestAllocRunner_TaskLeader_KillTG(t *testing.T) { conf, cleanup := testAllocRunnerConfig(t, alloc) defer cleanup() ar, err := NewAllocRunner(conf) - require.NoError(t, err) + must.NoError(t, err) defer destroy(ar) go ar.Run() @@ -169,7 +170,7 @@ func TestAllocRunner_Lifecycle_Poststart(t *testing.T) { conf, cleanup := testAllocRunnerConfig(t, alloc) defer cleanup() ar, err := NewAllocRunner(conf) - require.NoError(t, err) + must.NoError(t, err) defer destroy(ar) go ar.Run() @@ -306,7 +307,7 @@ func TestAllocRunner_TaskMain_KillTG(t *testing.T) { conf, cleanup := testAllocRunnerConfig(t, alloc) defer cleanup() ar, err := NewAllocRunner(conf) - require.NoError(t, err) + must.NoError(t, err) defer destroy(ar) go ar.Run() @@ -428,7 +429,7 @@ func TestAllocRunner_Lifecycle_Poststop(t *testing.T) { conf, cleanup := testAllocRunnerConfig(t, alloc) defer cleanup() ar, err := NewAllocRunner(conf) - require.NoError(t, err) + must.NoError(t, err) defer destroy(ar) go ar.Run() @@ -513,13 +514,13 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) { taskDefs []mock.LifecycleTaskDef isBatch bool hasLeader bool - action func(*allocRunner, *structs.Allocation) error + action func(interfaces.AllocRunner, *structs.Allocation) error expectedErr string expectedAfter map[string]structs.TaskState }{ { name: "restart entire allocation", - action: func(ar *allocRunner, alloc *structs.Allocation) error { + action: func(ar interfaces.AllocRunner, alloc *structs.Allocation) error { return ar.RestartAll(ev) }, expectedAfter: map[string]structs.TaskState{ @@ -533,7 +534,7 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) { }, { name: "restart only running tasks", - action: func(ar *allocRunner, alloc *structs.Allocation) error { + action: func(ar interfaces.AllocRunner, alloc *structs.Allocation) error { return ar.RestartRunning(ev) }, expectedAfter: map[string]structs.TaskState{ @@ -556,7 +557,7 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) { {Name: "poststop", RunFor: "1s", ExitCode: 0, Hook: "poststop", IsSidecar: false}, }, isBatch: true, - action: func(ar *allocRunner, alloc *structs.Allocation) error { + action: func(ar interfaces.AllocRunner, alloc *structs.Allocation) error { return ar.RestartAll(ev) }, expectedAfter: map[string]structs.TaskState{ @@ -579,7 +580,7 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) { {Name: "poststop", RunFor: "1s", ExitCode: 0, Hook: "poststop", IsSidecar: false}, }, isBatch: true, - action: func(ar *allocRunner, alloc *structs.Allocation) error { + action: func(ar interfaces.AllocRunner, alloc *structs.Allocation) error { return ar.RestartRunning(ev) }, expectedAfter: map[string]structs.TaskState{ @@ -594,7 +595,7 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) { { name: "restart entire allocation with leader", hasLeader: true, - action: func(ar *allocRunner, alloc *structs.Allocation) error { + action: func(ar interfaces.AllocRunner, alloc *structs.Allocation) error { return ar.RestartAll(ev) }, expectedAfter: map[string]structs.TaskState{ @@ -608,7 +609,7 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) { }, { name: "stop from server", - action: func(ar *allocRunner, alloc *structs.Allocation) error { + action: func(ar interfaces.AllocRunner, alloc *structs.Allocation) error { stopAlloc := alloc.Copy() stopAlloc.DesiredStatus = structs.AllocDesiredStatusStop ar.Update(stopAlloc) @@ -625,7 +626,7 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) { }, { name: "restart main task", - action: func(ar *allocRunner, alloc *structs.Allocation) error { + action: func(ar interfaces.AllocRunner, alloc *structs.Allocation) error { return ar.RestartTask("main", ev) }, expectedAfter: map[string]structs.TaskState{ @@ -640,7 +641,7 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) { { name: "restart leader main task", hasLeader: true, - action: func(ar *allocRunner, alloc *structs.Allocation) error { + action: func(ar interfaces.AllocRunner, alloc *structs.Allocation) error { return ar.RestartTask("main", ev) }, expectedAfter: map[string]structs.TaskState{ @@ -662,7 +663,7 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) { {Name: "poststart-sidecar", RunFor: "100s", ExitCode: 0, Hook: "poststart", IsSidecar: true}, {Name: "poststop", RunFor: "1s", ExitCode: 0, Hook: "poststop", IsSidecar: false}, }, - action: func(ar *allocRunner, alloc *structs.Allocation) error { + action: func(ar interfaces.AllocRunner, alloc *structs.Allocation) error { time.Sleep(3 * time.Second) // make sure main task has exited return nil }, @@ -686,7 +687,7 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) { {Name: "poststop", RunFor: "1s", ExitCode: 0, Hook: "poststop", IsSidecar: false}, }, hasLeader: true, - action: func(ar *allocRunner, alloc *structs.Allocation) error { + action: func(ar interfaces.AllocRunner, alloc *structs.Allocation) error { time.Sleep(3 * time.Second) // make sure main task has exited return nil }, @@ -709,7 +710,7 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) { {Name: "poststart-sidecar", RunFor: "100s", ExitCode: 0, Hook: "poststart", IsSidecar: true}, {Name: "poststop", RunFor: "1s", ExitCode: 0, Hook: "poststop", IsSidecar: false}, }, - action: func(ar *allocRunner, alloc *structs.Allocation) error { + action: func(ar interfaces.AllocRunner, alloc *structs.Allocation) error { time.Sleep(3 * time.Second) // make sure main task has exited return nil }, @@ -732,7 +733,7 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) { {Name: "poststart-sidecar", RunFor: "100s", ExitCode: 0, Hook: "poststart", IsSidecar: true}, {Name: "poststop", RunFor: "1s", ExitCode: 0, Hook: "poststop", IsSidecar: false}, }, - action: func(ar *allocRunner, alloc *structs.Allocation) error { + action: func(ar interfaces.AllocRunner, alloc *structs.Allocation) error { time.Sleep(3 * time.Second) // make sure main task has exited return nil }, @@ -755,7 +756,7 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) { {Name: "poststart-sidecar", RunFor: "100s", ExitCode: 0, Hook: "poststart", IsSidecar: true}, {Name: "poststop", RunFor: "1s", ExitCode: 0, Hook: "poststop", IsSidecar: false}, }, - action: func(ar *allocRunner, alloc *structs.Allocation) error { + action: func(ar interfaces.AllocRunner, alloc *structs.Allocation) error { // make sure main task has had a chance to restart once on its // own and fail again before we try to manually restart it time.Sleep(5 * time.Second) @@ -773,7 +774,7 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) { }, { name: "restart prestart-sidecar task", - action: func(ar *allocRunner, alloc *structs.Allocation) error { + action: func(ar interfaces.AllocRunner, alloc *structs.Allocation) error { return ar.RestartTask("prestart-sidecar", ev) }, expectedAfter: map[string]structs.TaskState{ @@ -787,7 +788,7 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) { }, { name: "restart poststart-sidecar task", - action: func(ar *allocRunner, alloc *structs.Allocation) error { + action: func(ar interfaces.AllocRunner, alloc *structs.Allocation) error { return ar.RestartTask("poststart-sidecar", ev) }, expectedAfter: map[string]structs.TaskState{ @@ -829,7 +830,7 @@ func TestAllocRunner_Lifecycle_Restart(t *testing.T) { conf, cleanup := testAllocRunnerConfig(t, alloc) defer cleanup() ar, err := NewAllocRunner(conf) - require.NoError(t, err) + must.NoError(t, err) defer destroy(ar) go ar.Run() @@ -988,7 +989,7 @@ func TestAllocRunner_TaskGroup_ShutdownDelay(t *testing.T) { conf, cleanup := testAllocRunnerConfig(t, alloc) defer cleanup() ar, err := NewAllocRunner(conf) - require.NoError(t, err) + must.NoError(t, err) defer destroy(ar) go ar.Run() @@ -1113,7 +1114,7 @@ func TestAllocRunner_TaskLeader_StopTG(t *testing.T) { conf, cleanup := testAllocRunnerConfig(t, alloc) defer cleanup() ar, err := NewAllocRunner(conf) - require.NoError(t, err) + must.NoError(t, err) defer destroy(ar) go ar.Run() @@ -1211,15 +1212,15 @@ func TestAllocRunner_TaskLeader_StopRestoredTG(t *testing.T) { conf.StateDB = state.NewMemDB(conf.Logger) ar, err := NewAllocRunner(conf) - require.NoError(t, err) + must.NoError(t, err) // Mimic Nomad exiting before the leader stopping is able to stop other tasks. - ar.tasks["leader"].UpdateState(structs.TaskStateDead, structs.NewTaskEvent(structs.TaskKilled)) - ar.tasks["follower1"].UpdateState(structs.TaskStateRunning, structs.NewTaskEvent(structs.TaskStarted)) + ar.(*allocRunner).tasks["leader"].UpdateState(structs.TaskStateDead, structs.NewTaskEvent(structs.TaskKilled)) + ar.(*allocRunner).tasks["follower1"].UpdateState(structs.TaskStateRunning, structs.NewTaskEvent(structs.TaskStarted)) // Create a new AllocRunner to test RestoreState and Run ar2, err := NewAllocRunner(conf) - require.NoError(t, err) + must.NoError(t, err) defer destroy(ar2) if err := ar2.Restore(); err != nil { @@ -1263,8 +1264,9 @@ func TestAllocRunner_Restore_LifecycleHooks(t *testing.T) { // Use a memory backed statedb conf.StateDB = state.NewMemDB(conf.Logger) - ar, err := NewAllocRunner(conf) - require.NoError(t, err) + arIface, err := NewAllocRunner(conf) + must.NoError(t, err) + ar := arIface.(*allocRunner) go ar.Run() defer destroy(ar) @@ -1287,9 +1289,10 @@ func TestAllocRunner_Restore_LifecycleHooks(t *testing.T) { ar.tasks["web"].UpdateState(structs.TaskStateRunning, structs.NewTaskEvent(structs.TaskStarted)) // Create a new AllocRunner to test Restore and Run. - ar2, err := NewAllocRunner(conf) - require.NoError(t, err) - require.NoError(t, ar2.Restore()) + arIface2, err := NewAllocRunner(conf) + must.NoError(t, err) + ar2 := arIface2.(*allocRunner) + must.NoError(t, ar2.Restore()) go ar2.Run() defer destroy(ar2) @@ -1322,8 +1325,9 @@ func TestAllocRunner_Update_Semantics(t *testing.T) { conf, cleanup := testAllocRunnerConfig(t, alloc) defer cleanup() - ar, err := NewAllocRunner(conf) - require.NoError(err) + arIface, err := NewAllocRunner(conf) + must.NoError(t, err) + ar := arIface.(*allocRunner) upd1 := updatedAlloc(alloc) ar.Update(upd1) @@ -1383,7 +1387,7 @@ func TestAllocRunner_DeploymentHealth_Healthy_Migration(t *testing.T) { defer cleanup() ar, err := NewAllocRunner(conf) - require.NoError(t, err) + must.NoError(t, err) go ar.Run() defer destroy(ar) @@ -1437,7 +1441,7 @@ func TestAllocRunner_DeploymentHealth_Healthy_NoChecks(t *testing.T) { defer cleanup() ar, err := NewAllocRunner(conf) - require.NoError(t, err) + must.NoError(t, err) start, done := time.Now(), time.Time{} go ar.Run() @@ -1531,7 +1535,7 @@ func TestAllocRunner_DeploymentHealth_Unhealthy_Checks(t *testing.T) { } ar, err := NewAllocRunner(conf) - require.NoError(t, err) + must.NoError(t, err) go ar.Run() defer destroy(ar) @@ -1580,7 +1584,7 @@ func TestAllocRunner_Destroy(t *testing.T) { conf.StateDB = state.NewMemDB(conf.Logger) ar, err := NewAllocRunner(conf) - require.NoError(t, err) + must.NoError(t, err) go ar.Run() // Wait for alloc to be running @@ -1620,8 +1624,8 @@ func TestAllocRunner_Destroy(t *testing.T) { require.Nil(t, ts) // Assert the alloc directory was cleaned - if _, err := os.Stat(ar.allocDir.AllocDir); err == nil { - require.Fail(t, "alloc dir still exists: %v", ar.allocDir.AllocDir) + if _, err := os.Stat(ar.(*allocRunner).allocDir.AllocDir); err == nil { + require.Fail(t, "alloc dir still exists: %v", ar.(*allocRunner).allocDir.AllocDir) } else if !os.IsNotExist(err) { require.Failf(t, "expected NotExist error", "found %v", err) } @@ -1635,7 +1639,7 @@ func TestAllocRunner_SimpleRun(t *testing.T) { conf, cleanup := testAllocRunnerConfig(t, alloc) defer cleanup() ar, err := NewAllocRunner(conf) - require.NoError(t, err) + must.NoError(t, err) go ar.Run() defer destroy(ar) @@ -1670,7 +1674,8 @@ func TestAllocRunner_MoveAllocDir(t *testing.T) { conf, cleanup := testAllocRunnerConfig(t, alloc) defer cleanup() ar, err := NewAllocRunner(conf) - require.NoError(t, err) + must.NoError(t, err) + ar.Run() defer destroy(ar) @@ -1678,9 +1683,9 @@ func TestAllocRunner_MoveAllocDir(t *testing.T) { // Step 2. Modify its directory task := alloc.Job.TaskGroups[0].Tasks[0] - dataFile := filepath.Join(ar.allocDir.SharedDir, "data", "data_file") + dataFile := filepath.Join(ar.GetAllocDir().SharedDir, "data", "data_file") os.WriteFile(dataFile, []byte("hello world"), os.ModePerm) - taskDir := ar.allocDir.TaskDirs[task.Name] + taskDir := ar.GetAllocDir().TaskDirs[task.Name] taskLocalFile := filepath.Join(taskDir.LocalDir, "local_file") os.WriteFile(taskLocalFile, []byte("good bye world"), os.ModePerm) @@ -1697,7 +1702,7 @@ func TestAllocRunner_MoveAllocDir(t *testing.T) { }) defer cleanup() ar2, err := NewAllocRunner(conf2) - require.NoError(t, err) + must.NoError(t, err) ar2.Run() defer destroy(ar2) @@ -1705,11 +1710,11 @@ func TestAllocRunner_MoveAllocDir(t *testing.T) { WaitForClientState(t, ar, structs.AllocClientStatusComplete) // Ensure that data from ar was moved to ar2 - dataFile = filepath.Join(ar2.allocDir.SharedDir, "data", "data_file") + dataFile = filepath.Join(ar2.GetAllocDir().SharedDir, "data", "data_file") fileInfo, _ := os.Stat(dataFile) require.NotNilf(t, fileInfo, "file %q not found", dataFile) - taskDir = ar2.allocDir.TaskDirs[task.Name] + taskDir = ar2.GetAllocDir().TaskDirs[task.Name] taskLocalFile = filepath.Join(taskDir.LocalDir, "local_file") fileInfo, _ = os.Stat(taskLocalFile) require.NotNilf(t, fileInfo, "file %q not found", dataFile) @@ -1752,7 +1757,8 @@ func TestAllocRunner_HandlesArtifactFailure(t *testing.T) { conf, cleanup := testAllocRunnerConfig(t, alloc) defer cleanup() ar, err := NewAllocRunner(conf) - require.NoError(t, err) + must.NoError(t, err) + go ar.Run() defer destroy(ar) @@ -1856,7 +1862,8 @@ func TestAllocRunner_TaskFailed_KillTG(t *testing.T) { } ar, err := NewAllocRunner(conf) - require.NoError(t, err) + must.NoError(t, err) + defer destroy(ar) go ar.Run() upd := conf.StateUpdater.(*MockStateUpdater) @@ -1926,7 +1933,8 @@ func TestAllocRunner_TerminalUpdate_Destroy(t *testing.T) { conf, cleanup := testAllocRunnerConfig(t, alloc) defer cleanup() ar, err := NewAllocRunner(conf) - require.NoError(t, err) + must.NoError(t, err) + defer destroy(ar) go ar.Run() upd := conf.StateUpdater.(*MockStateUpdater) @@ -1946,7 +1954,7 @@ func TestAllocRunner_TerminalUpdate_Destroy(t *testing.T) { // Update the alloc to be terminal which should cause the alloc runner to // stop the tasks and wait for a destroy. - update := ar.alloc.Copy() + update := ar.Alloc().Copy() update.DesiredStatus = structs.AllocDesiredStatusStop ar.Update(update) @@ -1962,8 +1970,8 @@ func TestAllocRunner_TerminalUpdate_Destroy(t *testing.T) { } // Check the alloc directory still exists - if _, err := os.Stat(ar.allocDir.AllocDir); err != nil { - return false, fmt.Errorf("alloc dir destroyed: %v", ar.allocDir.AllocDir) + if _, err := os.Stat(ar.GetAllocDir().AllocDir); err != nil { + return false, fmt.Errorf("alloc dir destroyed: %v", ar.GetAllocDir().AllocDir) } return true, nil @@ -1986,8 +1994,8 @@ func TestAllocRunner_TerminalUpdate_Destroy(t *testing.T) { } // Check the alloc directory was cleaned - if _, err := os.Stat(ar.allocDir.AllocDir); err == nil { - return false, fmt.Errorf("alloc dir still exists: %v", ar.allocDir.AllocDir) + if _, err := os.Stat(ar.GetAllocDir().AllocDir); err == nil { + return false, fmt.Errorf("alloc dir still exists: %v", ar.GetAllocDir().AllocDir) } else if !os.IsNotExist(err) { return false, fmt.Errorf("stat err: %v", err) } @@ -2010,7 +2018,8 @@ func TestAllocRunner_PersistState_Destroyed(t *testing.T) { defer cleanup() ar, err := NewAllocRunner(conf) - require.NoError(t, err) + must.NoError(t, err) + defer destroy(ar) go ar.Run() @@ -2106,12 +2115,12 @@ func TestAllocRunner_Reconnect(t *testing.T) { defer cleanup() ar, err := NewAllocRunner(conf) - require.NoError(t, err) + must.NoError(t, err) defer destroy(ar) go ar.Run() - for _, taskRunner := range ar.tasks { + for _, taskRunner := range ar.(*allocRunner).tasks { taskRunner.UpdateState(tc.taskState, tc.taskEvent) } @@ -2185,7 +2194,8 @@ func TestAllocRunner_Lifecycle_Shutdown_Order(t *testing.T) { conf, cleanup := testAllocRunnerConfig(t, alloc) defer cleanup() ar, err := NewAllocRunner(conf) - require.NoError(t, err) + must.NoError(t, err) + defer destroy(ar) go ar.Run() @@ -2386,8 +2396,9 @@ func TestHasSidecarTasks(t *testing.T) { arConf, cleanup := testAllocRunnerConfig(t, alloc) defer cleanup() - ar, err := NewAllocRunner(arConf) - require.NoError(t, err) + arIface, err := NewAllocRunner(arConf) + must.NoError(t, err) + ar := arIface.(*allocRunner) require.Equal(t, tc.hasSidecars, hasSidecarTasks(ar.tasks), "sidecars") @@ -2423,8 +2434,9 @@ func TestAllocRunner_PreKill_RunOnDone(t *testing.T) { conf, cleanup := testAllocRunnerConfig(t, alloc.Copy()) t.Cleanup(cleanup) - ar, err := NewAllocRunner(conf) + arIface, err := NewAllocRunner(conf) must.NoError(t, err) + ar := arIface.(*allocRunner) // set our custom prekill hook hook := new(allocPreKillHook) diff --git a/client/allocrunner/alloc_runner_unix_test.go b/client/allocrunner/alloc_runner_unix_test.go index 0c2492da2e5..c2e5b760112 100644 --- a/client/allocrunner/alloc_runner_unix_test.go +++ b/client/allocrunner/alloc_runner_unix_test.go @@ -16,6 +16,7 @@ import ( "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/testutil" + "github.com/shoenig/test/must" "github.com/stretchr/testify/require" ) @@ -55,7 +56,7 @@ func TestAllocRunner_Restore_RunningTerminal(t *testing.T) { // Start and wait for task to be running ar, err := NewAllocRunner(conf) - require.NoError(t, err) + must.NoError(t, err) go ar.Run() defer destroy(ar) @@ -105,8 +106,9 @@ func TestAllocRunner_Restore_RunningTerminal(t *testing.T) { conf2.StateDB = conf.StateDB // Restore, start, and wait for task to be killed - ar2, err := NewAllocRunner(conf2) - require.NoError(t, err) + ar2Iface, err := NewAllocRunner(conf2) + must.NoError(t, err) + ar2 := ar2Iface.(*allocRunner) require.NoError(t, ar2.Restore()) @@ -165,8 +167,9 @@ func TestAllocRunner_Restore_CompletedBatch(t *testing.T) { conf.StateDB = state.NewMemDB(conf.Logger) // Start and wait for task to be running - ar, err := NewAllocRunner(conf) - require.NoError(t, err) + arIface, err := NewAllocRunner(conf) + must.NoError(t, err) + ar := arIface.(*allocRunner) go ar.Run() defer destroy(ar) @@ -198,10 +201,10 @@ func TestAllocRunner_Restore_CompletedBatch(t *testing.T) { conf2.StateDB = conf.StateDB // Restore, start, and wait for task to be killed - ar2, err := NewAllocRunner(conf2) - require.NoError(t, err) - - require.NoError(t, ar2.Restore()) + ar2Iface, err := NewAllocRunner(conf2) + must.NoError(t, err) + ar2 := ar2Iface.(*allocRunner) + must.NoError(t, ar2.Restore()) go ar2.Run() defer destroy(ar2) @@ -249,9 +252,9 @@ func TestAllocRunner_PreStartFailuresLeadToFailed(t *testing.T) { conf.StateDB = state.NewMemDB(conf.Logger) // Start and wait for task to be running - ar, err := NewAllocRunner(conf) - require.NoError(t, err) - + arIface, err := NewAllocRunner(conf) + must.NoError(t, err) + ar := arIface.(*allocRunner) ar.runnerHooks = append(ar.runnerHooks, &allocFailingPrestartHook{}) go ar.Run() diff --git a/client/allocrunner/config.go b/client/allocrunner/config.go index 4b9cbc2e23a..cc0b24938e4 100644 --- a/client/allocrunner/config.go +++ b/client/allocrunner/config.go @@ -1,96 +1 @@ package allocrunner - -import ( - log "github.com/hashicorp/go-hclog" - "github.com/hashicorp/nomad/client/allocwatcher" - clientconfig "github.com/hashicorp/nomad/client/config" - "github.com/hashicorp/nomad/client/consul" - "github.com/hashicorp/nomad/client/devicemanager" - "github.com/hashicorp/nomad/client/dynamicplugins" - "github.com/hashicorp/nomad/client/interfaces" - "github.com/hashicorp/nomad/client/lib/cgutil" - "github.com/hashicorp/nomad/client/pluginmanager/csimanager" - "github.com/hashicorp/nomad/client/pluginmanager/drivermanager" - "github.com/hashicorp/nomad/client/serviceregistration" - "github.com/hashicorp/nomad/client/serviceregistration/checks/checkstore" - "github.com/hashicorp/nomad/client/serviceregistration/wrapper" - cstate "github.com/hashicorp/nomad/client/state" - "github.com/hashicorp/nomad/client/vaultclient" - "github.com/hashicorp/nomad/nomad/structs" -) - -// Config holds the configuration for creating an allocation runner. -type Config struct { - // Logger is the logger for the allocation runner. - Logger log.Logger - - // ClientConfig is the clients configuration. - ClientConfig *clientconfig.Config - - // Alloc captures the allocation that should be run. - Alloc *structs.Allocation - - // StateDB is used to store and restore state. - StateDB cstate.StateDB - - // Consul is the Consul client used to register task services and checks - Consul serviceregistration.Handler - - // ConsulProxies is the Consul client used to lookup supported envoy versions - // of the Consul agent. - ConsulProxies consul.SupportedProxiesAPI - - // ConsulSI is the Consul client used to manage service identity tokens. - ConsulSI consul.ServiceIdentityAPI - - // Vault is the Vault client to use to retrieve Vault tokens - Vault vaultclient.VaultClient - - // StateUpdater is used to emit updated task state - StateUpdater interfaces.AllocStateHandler - - // DeviceStatsReporter is used to lookup resource usage for alloc devices - DeviceStatsReporter interfaces.DeviceStatsReporter - - // PrevAllocWatcher handles waiting on previous or preempted allocations - PrevAllocWatcher allocwatcher.PrevAllocWatcher - - // PrevAllocMigrator allows the migration of a previous allocations alloc dir - PrevAllocMigrator allocwatcher.PrevAllocMigrator - - // DynamicRegistry contains all locally registered dynamic plugins (e.g csi - // plugins). - DynamicRegistry dynamicplugins.Registry - - // CSIManager is used to wait for CSI Volumes to be attached, and by the task - // runner to manage their mounting - CSIManager csimanager.Manager - - // DeviceManager is used to mount devices as well as lookup device - // statistics - DeviceManager devicemanager.Manager - - // DriverManager handles dispensing of driver plugins - DriverManager drivermanager.Manager - - // CpusetManager configures the cpuset cgroup if supported by the platform - CpusetManager cgutil.CpusetManager - - // ServersContactedCh is closed when the first GetClientAllocs call to - // servers succeeds and allocs are synced. - ServersContactedCh chan struct{} - - // RPCClient is the RPC Client that should be used by the allocrunner and its - // hooks to communicate with Nomad Servers. - RPCClient RPCer - - // ServiceRegWrapper is the handler wrapper that is used by service hooks - // to perform service and check registration and deregistration. - ServiceRegWrapper *wrapper.HandlerWrapper - - // CheckStore contains check result information. - CheckStore checkstore.Shim - - // Getter is an interface for retrieving artifacts. - Getter interfaces.ArtifactGetter -} diff --git a/client/allocrunner/csi_hook.go b/client/allocrunner/csi_hook.go index 6012fe910e7..a6d20ac6efd 100644 --- a/client/allocrunner/csi_hook.go +++ b/client/allocrunner/csi_hook.go @@ -9,6 +9,7 @@ import ( hclog "github.com/hashicorp/go-hclog" multierror "github.com/hashicorp/go-multierror" + "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/dynamicplugins" "github.com/hashicorp/nomad/client/pluginmanager/csimanager" cstructs "github.com/hashicorp/nomad/client/structs" @@ -27,7 +28,7 @@ type csiHook struct { csimanager csimanager.Manager // interfaces implemented by the allocRunner - rpcClient RPCer + rpcClient config.RPCer taskCapabilityGetter taskCapabilityGetter hookResources *cstructs.AllocHookResources @@ -46,7 +47,7 @@ type taskCapabilityGetter interface { GetTaskDriverCapabilities(string) (*drivers.Capabilities, error) } -func newCSIHook(alloc *structs.Allocation, logger hclog.Logger, csi csimanager.Manager, rpcClient RPCer, taskCapabilityGetter taskCapabilityGetter, hookResources *cstructs.AllocHookResources, nodeSecret string) *csiHook { +func newCSIHook(alloc *structs.Allocation, logger hclog.Logger, csi csimanager.Manager, rpcClient config.RPCer, taskCapabilityGetter taskCapabilityGetter, hookResources *cstructs.AllocHookResources, nodeSecret string) *csiHook { shutdownCtx, shutdownCancelFn := context.WithCancel(context.Background()) diff --git a/client/allocrunner/interfaces/runner.go b/client/allocrunner/interfaces/runner.go index 9fe9ed28457..aa0e5ab726f 100644 --- a/client/allocrunner/interfaces/runner.go +++ b/client/allocrunner/interfaces/runner.go @@ -1,24 +1,49 @@ package interfaces import ( + "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/client/allocrunner/state" "github.com/hashicorp/nomad/client/pluginmanager/csimanager" + "github.com/hashicorp/nomad/client/pluginmanager/drivermanager" cstructs "github.com/hashicorp/nomad/client/structs" + "github.com/hashicorp/nomad/nomad/structs" + "github.com/hashicorp/nomad/plugins/drivers" ) -// AllocRunner is the interface for an allocation runner. +// AllocRunner is the interface to the allocRunner struct used by client.Client type AllocRunner interface { - // ID returns the ID of the allocation being run. - ID() string + Alloc() *structs.Allocation - // Run starts the runner and begins executing all the tasks as part of the - // allocation. Run() + Restore() error + Update(*structs.Allocation) + Reconnect(update *structs.Allocation) error + Shutdown() + Destroy() - // State returns a copy of the runners state object - State() *state.State + IsDestroyed() bool + IsMigrating() bool + IsWaiting() bool - TaskStateHandler + WaitCh() <-chan struct{} + DestroyCh() <-chan struct{} + ShutdownCh() <-chan struct{} + + AllocState() *state.State + PersistState() error + SetClientStatus(string) + + Signal(taskName, signal string) error + RestartTask(taskName string, taskEvent *structs.TaskEvent) error + RestartRunning(taskEvent *structs.TaskEvent) error + RestartAll(taskEvent *structs.TaskEvent) error + + GetTaskEventHandler(taskName string) drivermanager.EventHandler + GetTaskExecHandler(taskName string) drivermanager.TaskExecHandler + GetTaskDriverCapabilities(taskName string) (*drivers.Capabilities, error) + StatsReporter() AllocStatsReporter + Listener() *cstructs.AllocListener + GetAllocDir() *allocdir.AllocDir } // TaskStateHandler exposes a handler to be called when a task's state changes diff --git a/client/allocrunner/migrate_hook.go b/client/allocrunner/migrate_hook.go index b8ca162cb4d..5fb69c48998 100644 --- a/client/allocrunner/migrate_hook.go +++ b/client/allocrunner/migrate_hook.go @@ -6,18 +6,18 @@ import ( log "github.com/hashicorp/go-hclog" "github.com/hashicorp/nomad/client/allocdir" - "github.com/hashicorp/nomad/client/allocwatcher" + "github.com/hashicorp/nomad/client/config" ) // diskMigrationHook migrates ephemeral disk volumes. Depends on alloc dir // being built but must be run before anything else manipulates the alloc dir. type diskMigrationHook struct { allocDir *allocdir.AllocDir - allocWatcher allocwatcher.PrevAllocMigrator + allocWatcher config.PrevAllocMigrator logger log.Logger } -func newDiskMigrationHook(logger log.Logger, allocWatcher allocwatcher.PrevAllocMigrator, allocDir *allocdir.AllocDir) *diskMigrationHook { +func newDiskMigrationHook(logger log.Logger, allocWatcher config.PrevAllocMigrator, allocDir *allocdir.AllocDir) *diskMigrationHook { h := &diskMigrationHook{ allocDir: allocDir, allocWatcher: allocWatcher, diff --git a/client/allocrunner/testing.go b/client/allocrunner/testing.go index 44e3eb52479..6032d2d88de 100644 --- a/client/allocrunner/testing.go +++ b/client/allocrunner/testing.go @@ -8,8 +8,10 @@ import ( "sync" "testing" + "github.com/hashicorp/nomad/client/allocrunner/interfaces" "github.com/hashicorp/nomad/client/allocrunner/taskrunner/getter" "github.com/hashicorp/nomad/client/allocwatcher" + "github.com/hashicorp/nomad/client/config" clientconfig "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/consul" "github.com/hashicorp/nomad/client/devicemanager" @@ -64,7 +66,7 @@ func (m *MockStateUpdater) Reset() { m.mu.Unlock() } -func testAllocRunnerConfig(t *testing.T, alloc *structs.Allocation) (*Config, func()) { +func testAllocRunnerConfig(t *testing.T, alloc *structs.Allocation) (*config.AllocRunnerConfig, func()) { clientConf, cleanup := clientconfig.TestClientConfig(t) consulRegMock := mock.NewServiceRegistrationHandler(clientConf.Logger) @@ -72,7 +74,7 @@ func testAllocRunnerConfig(t *testing.T, alloc *structs.Allocation) (*Config, fu stateDB := new(state.NoopDB) - conf := &Config{ + conf := &config.AllocRunnerConfig{ // Copy the alloc in case the caller edits and reuses it Alloc: alloc.Copy(), Logger: clientConf.Logger, @@ -104,10 +106,10 @@ func TestAllocRunnerFromAlloc(t *testing.T, alloc *structs.Allocation) (*allocRu require.NoError(t, err, "Failed to setup AllocRunner") } - return ar, cleanup + return ar.(*allocRunner), cleanup } -func WaitForClientState(t *testing.T, ar *allocRunner, state string) { +func WaitForClientState(t *testing.T, ar interfaces.AllocRunner, state string) { testutil.WaitForResult(func() (bool, error) { got := ar.AllocState().ClientStatus return got == state, diff --git a/client/allocrunner/upstream_allocs_hook.go b/client/allocrunner/upstream_allocs_hook.go index 4da0d0840cc..72081f185ac 100644 --- a/client/allocrunner/upstream_allocs_hook.go +++ b/client/allocrunner/upstream_allocs_hook.go @@ -4,17 +4,17 @@ import ( "context" log "github.com/hashicorp/go-hclog" - "github.com/hashicorp/nomad/client/allocwatcher" + "github.com/hashicorp/nomad/client/config" ) // upstreamAllocsHook waits for a PrevAllocWatcher to exit before allowing // an allocation to be executed type upstreamAllocsHook struct { - allocWatcher allocwatcher.PrevAllocWatcher + allocWatcher config.PrevAllocWatcher logger log.Logger } -func newUpstreamAllocsHook(logger log.Logger, allocWatcher allocwatcher.PrevAllocWatcher) *upstreamAllocsHook { +func newUpstreamAllocsHook(logger log.Logger, allocWatcher config.PrevAllocWatcher) *upstreamAllocsHook { h := &upstreamAllocsHook{ allocWatcher: allocWatcher, } diff --git a/client/allocwatcher/alloc_watcher.go b/client/allocwatcher/alloc_watcher.go index 7c4c7d2fa80..8b54538411c 100644 --- a/client/allocwatcher/alloc_watcher.go +++ b/client/allocwatcher/alloc_watcher.go @@ -46,29 +46,6 @@ type AllocRunnerMeta interface { Alloc() *structs.Allocation } -// PrevAllocWatcher allows AllocRunners to wait for a previous allocation to -// terminate whether or not the previous allocation is local or remote. -// See `PrevAllocMigrator` for migrating workloads. -type PrevAllocWatcher interface { - // Wait for previous alloc to terminate - Wait(context.Context) error - - // IsWaiting returns true if a concurrent caller is blocked in Wait - IsWaiting() bool -} - -// PrevAllocMigrator allows AllocRunners to migrate a previous allocation -// whether or not the previous allocation is local or remote. -type PrevAllocMigrator interface { - PrevAllocWatcher - - // IsMigrating returns true if a concurrent caller is in Migrate - IsMigrating() bool - - // Migrate data from previous alloc - Migrate(ctx context.Context, dest *allocdir.AllocDir) error -} - type Config struct { // Alloc is the current allocation which may need to block on its // previous allocation stopping. @@ -94,7 +71,7 @@ type Config struct { Logger hclog.Logger } -func newMigratorForAlloc(c Config, tg *structs.TaskGroup, watchedAllocID string, m AllocRunnerMeta) PrevAllocMigrator { +func newMigratorForAlloc(c Config, tg *structs.TaskGroup, watchedAllocID string, m AllocRunnerMeta) config.PrevAllocMigrator { logger := c.Logger.Named("alloc_migrator").With("alloc_id", c.Alloc.ID).With("previous_alloc", watchedAllocID) tasks := tg.Tasks @@ -133,7 +110,7 @@ func newMigratorForAlloc(c Config, tg *structs.TaskGroup, watchedAllocID string, // Note that c.Alloc.PreviousAllocation must NOT be used in this func as it // used for preemption which has a distinct field. The caller is responsible // for passing the allocation to be watched as watchedAllocID. -func newWatcherForAlloc(c Config, watchedAllocID string, m AllocRunnerMeta) PrevAllocWatcher { +func newWatcherForAlloc(c Config, watchedAllocID string, m AllocRunnerMeta) config.PrevAllocWatcher { logger := c.Logger.Named("alloc_watcher").With("alloc_id", c.Alloc.ID).With("previous_alloc", watchedAllocID) if m != nil { @@ -164,13 +141,13 @@ func newWatcherForAlloc(c Config, watchedAllocID string, m AllocRunnerMeta) Prev // For allocs which are either running on another node or have already // terminated their alloc runners, use a remote backend which watches the alloc // status via rpc. -func NewAllocWatcher(c Config) (PrevAllocWatcher, PrevAllocMigrator) { +func NewAllocWatcher(c Config) (config.PrevAllocWatcher, config.PrevAllocMigrator) { if c.Alloc.PreviousAllocation == "" && c.PreemptedRunners == nil { return NoopPrevAlloc{}, NoopPrevAlloc{} } - var prevAllocWatchers []PrevAllocWatcher - var prevAllocMigrator PrevAllocMigrator = NoopPrevAlloc{} + var prevAllocWatchers []config.PrevAllocWatcher + var prevAllocMigrator config.PrevAllocMigrator = NoopPrevAlloc{} // We have a previous allocation, add its listener to the watchers, and // use a migrator. diff --git a/client/allocwatcher/group_alloc_watcher.go b/client/allocwatcher/group_alloc_watcher.go index 087e6256f1b..a81db98b3c1 100644 --- a/client/allocwatcher/group_alloc_watcher.go +++ b/client/allocwatcher/group_alloc_watcher.go @@ -5,10 +5,12 @@ import ( "sync" multierror "github.com/hashicorp/go-multierror" + + "github.com/hashicorp/nomad/client/config" ) type groupPrevAllocWatcher struct { - prevAllocs []PrevAllocWatcher + prevAllocs []config.PrevAllocWatcher wg sync.WaitGroup // waiting and migrating are true when alloc runner is waiting on the @@ -18,7 +20,7 @@ type groupPrevAllocWatcher struct { waitingLock sync.RWMutex } -func NewGroupAllocWatcher(watchers ...PrevAllocWatcher) PrevAllocWatcher { +func NewGroupAllocWatcher(watchers ...config.PrevAllocWatcher) config.PrevAllocWatcher { return &groupPrevAllocWatcher{ prevAllocs: watchers, } @@ -45,7 +47,7 @@ func (g *groupPrevAllocWatcher) Wait(ctx context.Context) error { g.wg.Add(len(g.prevAllocs)) for _, alloc := range g.prevAllocs { - go func(ctx context.Context, alloc PrevAllocWatcher) { + go func(ctx context.Context, alloc config.PrevAllocWatcher) { defer g.wg.Done() err := alloc.Wait(ctx) if err != nil { diff --git a/client/allocwatcher/group_alloc_watcher_test.go b/client/allocwatcher/group_alloc_watcher_test.go index 79eeaf07ec7..5d8d07be43c 100644 --- a/client/allocwatcher/group_alloc_watcher_test.go +++ b/client/allocwatcher/group_alloc_watcher_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/hashicorp/nomad/ci" + "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/testutil" "github.com/stretchr/testify/require" @@ -25,7 +26,7 @@ func TestPrevAlloc_GroupPrevAllocWatcher_Block(t *testing.T) { waiter, _ := NewAllocWatcher(conf) - groupWaiter := &groupPrevAllocWatcher{prevAllocs: []PrevAllocWatcher{waiter}} + groupWaiter := &groupPrevAllocWatcher{prevAllocs: []config.PrevAllocWatcher{waiter}} // Wait in a goroutine with a context to make sure it exits at the right time ctx, cancel := context.WithCancel(context.Background()) @@ -99,7 +100,7 @@ func TestPrevAlloc_GroupPrevAllocWatcher_BlockMulti(t *testing.T) { waiter2, _ := NewAllocWatcher(conf2) groupWaiter := &groupPrevAllocWatcher{ - prevAllocs: []PrevAllocWatcher{ + prevAllocs: []config.PrevAllocWatcher{ waiter1, waiter2, }, diff --git a/client/client.go b/client/client.go index e3cf72c553f..bc7f7c19322 100644 --- a/client/client.go +++ b/client/client.go @@ -40,7 +40,6 @@ import ( "github.com/hashicorp/nomad/client/serviceregistration/wrapper" "github.com/hashicorp/nomad/client/state" "github.com/hashicorp/nomad/client/stats" - cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/client/vaultclient" "github.com/hashicorp/nomad/command/agent/consul" "github.com/hashicorp/nomad/helper" @@ -54,7 +53,6 @@ import ( nconfig "github.com/hashicorp/nomad/nomad/structs/config" "github.com/hashicorp/nomad/plugins/csi" "github.com/hashicorp/nomad/plugins/device" - "github.com/hashicorp/nomad/plugins/drivers" vaultapi "github.com/hashicorp/vault/api" "github.com/shirou/gopsutil/v3/host" "golang.org/x/exp/maps" @@ -135,38 +133,6 @@ type ClientStatsReporter interface { LatestHostStats() *stats.HostStats } -// AllocRunner is the interface implemented by the core alloc runner. -// TODO Create via factory to allow testing Client with mock AllocRunners. -type AllocRunner interface { - Alloc() *structs.Allocation - AllocState() *arstate.State - Destroy() - Shutdown() - GetAllocDir() *allocdir.AllocDir - IsDestroyed() bool - IsMigrating() bool - IsWaiting() bool - Listener() *cstructs.AllocListener - Restore() error - Run() - StatsReporter() interfaces.AllocStatsReporter - Update(*structs.Allocation) - WaitCh() <-chan struct{} - DestroyCh() <-chan struct{} - ShutdownCh() <-chan struct{} - Signal(taskName, signal string) error - GetTaskEventHandler(taskName string) drivermanager.EventHandler - PersistState() error - - RestartTask(taskName string, taskEvent *structs.TaskEvent) error - RestartRunning(taskEvent *structs.TaskEvent) error - RestartAll(taskEvent *structs.TaskEvent) error - Reconnect(update *structs.Allocation) error - - GetTaskExecHandler(taskName string) drivermanager.TaskExecHandler - GetTaskDriverCapabilities(taskName string) (*drivers.Capabilities, error) -} - // Client is used to implement the client interaction with Nomad. Clients // are expected to register as a schedule-able node to the servers, and to // run allocations as determined by the servers. @@ -226,9 +192,12 @@ type Client struct { // allocs maps alloc IDs to their AllocRunner. This map includes all // AllocRunners - running and GC'd - until the server GCs them. - allocs map[string]AllocRunner + allocs map[string]interfaces.AllocRunner allocLock sync.RWMutex + // allocrunnerFactory is the function called to create new allocrunners + allocrunnerFactory config.AllocRunnerFactory + // invalidAllocs is a map that tracks allocations that failed because // the client couldn't initialize alloc or task runners for it. This can // happen due to driver errors @@ -387,7 +356,7 @@ func NewClient(cfg *config.Config, consulCatalog consul.CatalogAPI, consulProxie streamingRpcs: structs.NewStreamingRpcRegistry(), logger: logger, rpcLogger: logger.Named("rpc"), - allocs: make(map[string]AllocRunner), + allocs: make(map[string]interfaces.AllocRunner), allocUpdates: make(chan *structs.Allocation, 64), shutdownCh: make(chan struct{}), triggerDiscoveryCh: make(chan struct{}), @@ -400,6 +369,12 @@ func NewClient(cfg *config.Config, consulCatalog consul.CatalogAPI, consulProxie cpusetManager: cgutil.CreateCPUSetManager(cfg.CgroupParent, cfg.ReservableCores, logger), getter: getter.NewGetter(logger.Named("artifact_getter"), cfg.Artifact), EnterpriseClient: newEnterpriseClient(logger), + allocrunnerFactory: cfg.AllocRunnerFactory, + } + + // we can't have this set in the default Config because of import cycles + if c.allocrunnerFactory == nil { + c.allocrunnerFactory = allocrunner.NewAllocRunner } c.batchNodeUpdates = newBatchNodeUpdates( @@ -947,7 +922,7 @@ func (c *Client) Node() *structs.Node { // getAllocRunner returns an AllocRunner or an UnknownAllocation error if the // client has no runner for the given alloc ID. -func (c *Client) getAllocRunner(allocID string) (AllocRunner, error) { +func (c *Client) getAllocRunner(allocID string) (interfaces.AllocRunner, error) { c.allocLock.RLock() defer c.allocLock.RUnlock() @@ -1191,7 +1166,7 @@ func (c *Client) restoreState() error { prevAllocWatcher := allocwatcher.NoopPrevAlloc{} prevAllocMigrator := allocwatcher.NoopPrevAlloc{} - arConf := &allocrunner.Config{ + arConf := &config.AllocRunnerConfig{ Alloc: alloc, Logger: c.logger, ClientConfig: conf, @@ -1216,7 +1191,7 @@ func (c *Client) restoreState() error { Getter: c.getter, } - ar, err := allocrunner.NewAllocRunner(arConf) + ar, err := c.allocrunnerFactory(arConf) if err != nil { c.logger.Error("error running alloc", "error", err, "alloc_id", alloc.ID) c.handleInvalidAllocs(alloc, err) @@ -1317,7 +1292,7 @@ func (c *Client) saveState() error { wg.Add(len(runners)) for id, ar := range runners { - go func(id string, ar AllocRunner) { + go func(id string, ar interfaces.AllocRunner) { err := ar.PersistState() if err != nil { c.logger.Error("error saving alloc state", "error", err, "alloc_id", id) @@ -1334,10 +1309,10 @@ func (c *Client) saveState() error { } // getAllocRunners returns a snapshot of the current set of alloc runners. -func (c *Client) getAllocRunners() map[string]AllocRunner { +func (c *Client) getAllocRunners() map[string]interfaces.AllocRunner { c.allocLock.RLock() defer c.allocLock.RUnlock() - runners := make(map[string]AllocRunner, len(c.allocs)) + runners := make(map[string]interfaces.AllocRunner, len(c.allocs)) for id, ar := range c.allocs { runners[id] = ar } @@ -2549,7 +2524,7 @@ func (c *Client) addAlloc(alloc *structs.Allocation, migrateToken string) error } prevAllocWatcher, prevAllocMigrator := allocwatcher.NewAllocWatcher(watcherConfig) - arConf := &allocrunner.Config{ + arConf := &config.AllocRunnerConfig{ Alloc: alloc, Logger: c.logger, ClientConfig: c.GetConfig(), @@ -2573,7 +2548,7 @@ func (c *Client) addAlloc(alloc *structs.Allocation, migrateToken string) error Getter: c.getter, } - ar, err := allocrunner.NewAllocRunner(arConf) + ar, err := c.allocrunnerFactory(arConf) if err != nil { return err } diff --git a/client/client_interface_test.go b/client/client_interface_test.go new file mode 100644 index 00000000000..5a985b7bce4 --- /dev/null +++ b/client/client_interface_test.go @@ -0,0 +1,165 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package client + +import ( + "sync" + "testing" + + "github.com/hashicorp/nomad/ci" + "github.com/hashicorp/nomad/client/allocdir" + "github.com/hashicorp/nomad/client/allocrunner/interfaces" + "github.com/hashicorp/nomad/client/allocrunner/state" + "github.com/hashicorp/nomad/client/config" + cinterfaces "github.com/hashicorp/nomad/client/interfaces" + "github.com/hashicorp/nomad/client/pluginmanager/drivermanager" + cstructs "github.com/hashicorp/nomad/client/structs" + "github.com/hashicorp/nomad/nomad/mock" + "github.com/hashicorp/nomad/nomad/structs" + "github.com/hashicorp/nomad/plugins/device" + "github.com/hashicorp/nomad/plugins/drivers" + "github.com/hashicorp/nomad/testutil" +) + +// TestEmptyAllocRunner demonstrates the minimum interface necessary to +// implement a mock AllocRunner that can report client status back to the server +func TestEmptyAllocRunner(t *testing.T) { + ci.Parallel(t) + + s1, _, cleanupS1 := testServer(t, nil) + defer cleanupS1() + + _, cleanup := TestClient(t, func(c *config.Config) { + c.RPCHandler = s1 + c.AllocRunnerFactory = newEmptyAllocRunnerFunc + }) + defer cleanup() + + job := mock.Job() + job.Constraints = nil + job.TaskGroups[0].Constraints = nil + job.TaskGroups[0].Count = 1 + task := job.TaskGroups[0].Tasks[0] + task.Driver = "mock_driver" + task.Config = map[string]interface{}{ + "run_for": "10s", + } + task.Services = nil + + // WaitForRunning polls the server until the ClientStatus is running + testutil.WaitForRunning(t, s1.RPC, job) +} + +type emptyAllocRunner struct { + c cinterfaces.AllocStateHandler + alloc *structs.Allocation + allocState *state.State + allocLock sync.RWMutex +} + +func newEmptyAllocRunnerFunc(conf *config.AllocRunnerConfig) (interfaces.AllocRunner, error) { + return &emptyAllocRunner{ + c: conf.StateUpdater, + alloc: conf.Alloc, + allocState: &state.State{}, + }, nil +} + +func (ar *emptyAllocRunner) Alloc() *structs.Allocation { + ar.allocLock.RLock() + defer ar.allocLock.RUnlock() + return ar.alloc.Copy() +} + +func (ar *emptyAllocRunner) Run() { + ar.allocLock.RLock() + defer ar.allocLock.RUnlock() + ar.alloc.ClientStatus = "running" + ar.c.AllocStateUpdated(ar.alloc) +} + +func (ar *emptyAllocRunner) Restore() error { return nil } +func (ar *emptyAllocRunner) Update(update *structs.Allocation) { + ar.allocLock.Lock() + defer ar.allocLock.Unlock() + ar.alloc = update +} + +func (ar *emptyAllocRunner) Reconnect(update *structs.Allocation) error { + ar.allocLock.Lock() + defer ar.allocLock.Unlock() + ar.alloc = update + return nil +} + +func (ar *emptyAllocRunner) Shutdown() {} +func (ar *emptyAllocRunner) Destroy() {} + +func (ar *emptyAllocRunner) IsDestroyed() bool { return false } +func (ar *emptyAllocRunner) IsMigrating() bool { return false } +func (ar *emptyAllocRunner) IsWaiting() bool { return false } + +func (ar *emptyAllocRunner) WaitCh() <-chan struct{} { return make(chan struct{}) } + +func (ar *emptyAllocRunner) DestroyCh() <-chan struct{} { + ch := make(chan struct{}) + close(ch) + return ch +} + +func (ar *emptyAllocRunner) ShutdownCh() <-chan struct{} { + ch := make(chan struct{}) + close(ch) + return ch +} + +func (ar *emptyAllocRunner) AllocState() *state.State { + ar.allocLock.RLock() + defer ar.allocLock.RUnlock() + return ar.allocState.Copy() +} + +func (ar *emptyAllocRunner) PersistState() error { return nil } +func (ar *emptyAllocRunner) AcknowledgeState(*state.State) {} +func (ar *emptyAllocRunner) LastAcknowledgedStateIsCurrent(*structs.Allocation) bool { return false } + +func (ar *emptyAllocRunner) SetClientStatus(status string) { + ar.allocLock.Lock() + defer ar.allocLock.Unlock() + ar.alloc.ClientStatus = status +} + +func (ar *emptyAllocRunner) Signal(taskName, signal string) error { return nil } +func (ar *emptyAllocRunner) RestartTask(taskName string, taskEvent *structs.TaskEvent) error { + return nil +} +func (ar *emptyAllocRunner) RestartRunning(taskEvent *structs.TaskEvent) error { return nil } +func (ar *emptyAllocRunner) RestartAll(taskEvent *structs.TaskEvent) error { return nil } + +func (ar *emptyAllocRunner) GetTaskEventHandler(taskName string) drivermanager.EventHandler { + return nil +} +func (ar *emptyAllocRunner) GetTaskExecHandler(taskName string) drivermanager.TaskExecHandler { + return nil +} +func (ar *emptyAllocRunner) GetTaskDriverCapabilities(taskName string) (*drivers.Capabilities, error) { + return nil, nil +} + +func (ar *emptyAllocRunner) StatsReporter() interfaces.AllocStatsReporter { return ar } +func (ar *emptyAllocRunner) Listener() *cstructs.AllocListener { return nil } +func (ar *emptyAllocRunner) GetAllocDir() *allocdir.AllocDir { return nil } + +// LatestAllocStats lets this empty runner implement AllocStatsReporter +func (ar *emptyAllocRunner) LatestAllocStats(taskFilter string) (*cstructs.AllocResourceUsage, error) { + return &cstructs.AllocResourceUsage{ + ResourceUsage: &cstructs.ResourceUsage{ + MemoryStats: &cstructs.MemoryStats{}, + CpuStats: &cstructs.CpuStats{}, + DeviceStats: []*device.DeviceGroupStats{}, + }, + Tasks: map[string]*cstructs.TaskResourceUsage{}, + Timestamp: 0, + }, nil +} diff --git a/client/client_test.go b/client/client_test.go index f6df9e75b01..4372689b968 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -1777,7 +1777,7 @@ func TestClient_ReconnectAllocs(t *testing.T) { c1.runAllocs(updates) invalid := false - var runner AllocRunner + var runner interfaces.AllocRunner var finalAlloc *structs.Allocation // Ensure the allocation is not invalid on the client and has been marked // running on the server with the new modify index diff --git a/client/config/arconfig.go b/client/config/arconfig.go new file mode 100644 index 00000000000..7496bc26619 --- /dev/null +++ b/client/config/arconfig.go @@ -0,0 +1,136 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package config + +import ( + "context" + + log "github.com/hashicorp/go-hclog" + + "github.com/hashicorp/nomad/client/allocdir" + arinterfaces "github.com/hashicorp/nomad/client/allocrunner/interfaces" + "github.com/hashicorp/nomad/client/consul" + "github.com/hashicorp/nomad/client/devicemanager" + "github.com/hashicorp/nomad/client/dynamicplugins" + "github.com/hashicorp/nomad/client/interfaces" + "github.com/hashicorp/nomad/client/lib/cgutil" + "github.com/hashicorp/nomad/client/pluginmanager/csimanager" + "github.com/hashicorp/nomad/client/pluginmanager/drivermanager" + "github.com/hashicorp/nomad/client/serviceregistration" + "github.com/hashicorp/nomad/client/serviceregistration/checks/checkstore" + "github.com/hashicorp/nomad/client/serviceregistration/wrapper" + cstate "github.com/hashicorp/nomad/client/state" + "github.com/hashicorp/nomad/client/vaultclient" + "github.com/hashicorp/nomad/nomad/structs" +) + +// AllocRunnerFactory returns an AllocRunner interface built from the +// configuration. Note: the type for config is any because we can't count on +// test callers being able to make a real allocrunner.Config without an circular +// import +type AllocRunnerFactory func(*AllocRunnerConfig) (arinterfaces.AllocRunner, error) + +// RPCer is the interface needed by hooks to make RPC calls. +type RPCer interface { + RPC(method string, args interface{}, reply interface{}) error +} + +// AllocRunnerConfig holds the configuration for creating an allocation runner. +type AllocRunnerConfig struct { + // Logger is the logger for the allocation runner. + Logger log.Logger + + // ClientConfig is the clients configuration. + ClientConfig *Config + + // Alloc captures the allocation that should be run. + Alloc *structs.Allocation + + // StateDB is used to store and restore state. + StateDB cstate.StateDB + + // Consul is the Consul client used to register task services and checks + Consul serviceregistration.Handler + + // ConsulProxies is the Consul client used to lookup supported envoy versions + // of the Consul agent. + ConsulProxies consul.SupportedProxiesAPI + + // ConsulSI is the Consul client used to manage service identity tokens. + ConsulSI consul.ServiceIdentityAPI + + // Vault is the Vault client to use to retrieve Vault tokens + Vault vaultclient.VaultClient + + // StateUpdater is used to emit updated task state + StateUpdater interfaces.AllocStateHandler + + // DeviceStatsReporter is used to lookup resource usage for alloc devices + DeviceStatsReporter interfaces.DeviceStatsReporter + + // PrevAllocWatcher handles waiting on previous or preempted allocations + PrevAllocWatcher PrevAllocWatcher + + // PrevAllocMigrator allows the migration of a previous allocations alloc dir + PrevAllocMigrator PrevAllocMigrator + + // DynamicRegistry contains all locally registered dynamic plugins (e.g csi + // plugins). + DynamicRegistry dynamicplugins.Registry + + // CSIManager is used to wait for CSI Volumes to be attached, and by the task + // runner to manage their mounting + CSIManager csimanager.Manager + + // DeviceManager is used to mount devices as well as lookup device + // statistics + DeviceManager devicemanager.Manager + + // DriverManager handles dispensing of driver plugins + DriverManager drivermanager.Manager + + // CpusetManager configures the cpuset cgroup if supported by the platform + CpusetManager cgutil.CpusetManager + + // ServersContactedCh is closed when the first GetClientAllocs call to + // servers succeeds and allocs are synced. + ServersContactedCh chan struct{} + + // RPCClient is the RPC Client that should be used by the allocrunner and its + // hooks to communicate with Nomad Servers. + RPCClient RPCer + + // ServiceRegWrapper is the handler wrapper that is used by service hooks + // to perform service and check registration and deregistration. + ServiceRegWrapper *wrapper.HandlerWrapper + + // CheckStore contains check result information. + CheckStore checkstore.Shim + + // Getter is an interface for retrieving artifacts. + Getter interfaces.ArtifactGetter +} + +// PrevAllocWatcher allows AllocRunners to wait for a previous allocation to +// terminate whether or not the previous allocation is local or remote. +// See `PrevAllocMigrator` for migrating workloads. +type PrevAllocWatcher interface { + // Wait for previous alloc to terminate + Wait(context.Context) error + + // IsWaiting returns true if a concurrent caller is blocked in Wait + IsWaiting() bool +} + +// PrevAllocMigrator allows AllocRunners to migrate a previous allocation +// whether or not the previous allocation is local or remote. +type PrevAllocMigrator interface { + PrevAllocWatcher + + // IsMigrating returns true if a concurrent caller is in Migrate + IsMigrating() bool + + // Migrate data from previous alloc + Migrate(ctx context.Context, dest *allocdir.AllocDir) error +} diff --git a/client/config/config.go b/client/config/config.go index b5c38df0b6e..9cb23c7457f 100644 --- a/client/config/config.go +++ b/client/config/config.go @@ -245,6 +245,8 @@ type Config struct { // StateDBFactory is used to override stateDB implementations, StateDBFactory state.NewStateDBFunc + AllocRunnerFactory AllocRunnerFactory + // CNIPath is the path used to search for CNI plugins. Multiple paths can // be specified with colon delimited CNIPath string diff --git a/client/gc.go b/client/gc.go index 1d37dbf1f82..f81b83c7193 100644 --- a/client/gc.go +++ b/client/gc.go @@ -7,6 +7,8 @@ import ( "time" hclog "github.com/hashicorp/go-hclog" + + "github.com/hashicorp/nomad/client/allocrunner/interfaces" "github.com/hashicorp/nomad/client/stats" "github.com/hashicorp/nomad/nomad/structs" ) @@ -170,7 +172,7 @@ func (a *AllocGarbageCollector) keepUsageBelowThreshold() error { // destroyAllocRunner is used to destroy an allocation runner. It will acquire a // lock to restrict parallelism and then destroy the alloc runner, returning // once the allocation has been destroyed. -func (a *AllocGarbageCollector) destroyAllocRunner(allocID string, ar AllocRunner, reason string) { +func (a *AllocGarbageCollector) destroyAllocRunner(allocID string, ar interfaces.AllocRunner, reason string) { a.logger.Info("garbage collecting allocation", "alloc_id", allocID, "reason", reason) // Acquire the destroy lock @@ -335,7 +337,7 @@ func (a *AllocGarbageCollector) MakeRoomFor(allocations []*structs.Allocation) e } // MarkForCollection starts tracking an allocation for Garbage Collection -func (a *AllocGarbageCollector) MarkForCollection(allocID string, ar AllocRunner) { +func (a *AllocGarbageCollector) MarkForCollection(allocID string, ar interfaces.AllocRunner) { if a.allocRunners.Push(allocID, ar) { a.logger.Info("marking allocation for GC", "alloc_id", allocID) } @@ -346,7 +348,7 @@ func (a *AllocGarbageCollector) MarkForCollection(allocID string, ar AllocRunner type GCAlloc struct { timeStamp time.Time allocID string - allocRunner AllocRunner + allocRunner interfaces.AllocRunner index int } @@ -400,7 +402,7 @@ func NewIndexedGCAllocPQ() *IndexedGCAllocPQ { // Push an alloc runner into the GC queue. Returns true if alloc was added, // false if the alloc already existed. -func (i *IndexedGCAllocPQ) Push(allocID string, ar AllocRunner) bool { +func (i *IndexedGCAllocPQ) Push(allocID string, ar interfaces.AllocRunner) bool { i.pqLock.Lock() defer i.pqLock.Unlock() diff --git a/client/gc_test.go b/client/gc_test.go index 83a25c50ac0..5bdc394307e 100644 --- a/client/gc_test.go +++ b/client/gc_test.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/nomad/ci" "github.com/hashicorp/nomad/client/allocrunner" + "github.com/hashicorp/nomad/client/allocrunner/interfaces" "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/stats" "github.com/hashicorp/nomad/helper/testlog" @@ -29,7 +30,7 @@ func gcConfig() *GCConfig { // exitAllocRunner is a helper that updates the allocs on the given alloc // runners to be terminal -func exitAllocRunner(runners ...AllocRunner) { +func exitAllocRunner(runners ...interfaces.AllocRunner) { for _, ar := range runners { terminalAlloc := ar.Alloc().Copy() terminalAlloc.DesiredStatus = structs.AllocDesiredStatusStop diff --git a/client/heartbeatstop.go b/client/heartbeatstop.go index b75dea9c11b..15a01f3c83f 100644 --- a/client/heartbeatstop.go +++ b/client/heartbeatstop.go @@ -5,6 +5,8 @@ import ( "time" hclog "github.com/hashicorp/go-hclog" + + "github.com/hashicorp/nomad/client/allocrunner/interfaces" "github.com/hashicorp/nomad/nomad/structs" ) @@ -13,14 +15,14 @@ type heartbeatStop struct { startupGrace time.Time allocInterval map[string]time.Duration allocHookCh chan *structs.Allocation - getRunner func(string) (AllocRunner, error) + getRunner func(string) (interfaces.AllocRunner, error) logger hclog.InterceptLogger shutdownCh chan struct{} lock *sync.RWMutex } func newHeartbeatStop( - getRunner func(string) (AllocRunner, error), + getRunner func(string) (interfaces.AllocRunner, error), timeout time.Duration, logger hclog.InterceptLogger, shutdownCh chan struct{}) *heartbeatStop { diff --git a/client/state/upgrade_int_test.go b/client/state/upgrade_int_test.go index 5bcf12d5cf1..fbc94bfe6d2 100644 --- a/client/state/upgrade_int_test.go +++ b/client/state/upgrade_int_test.go @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/nomad/ci" "github.com/hashicorp/nomad/client/allocrunner" "github.com/hashicorp/nomad/client/allocwatcher" + "github.com/hashicorp/nomad/client/config" clientconfig "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/devicemanager" dmstate "github.com/hashicorp/nomad/client/devicemanager/state" @@ -197,7 +198,7 @@ func checkUpgradedAlloc(t *testing.T, path string, db StateDB, alloc *structs.Al clientConf.StateDir = path - conf := &allocrunner.Config{ + conf := &config.AllocRunnerConfig{ Alloc: alloc, Logger: clientConf.Logger, ClientConfig: clientConf, diff --git a/drivers/mock/driver_test.go b/drivers/mock/driver_test.go index f12155705c1..add9563e54f 100644 --- a/drivers/mock/driver_test.go +++ b/drivers/mock/driver_test.go @@ -16,7 +16,6 @@ import ( "github.com/hashicorp/nomad/ci" "github.com/hashicorp/nomad/client/allocdir" - "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/taskenv" "github.com/hashicorp/nomad/helper/testlog" "github.com/hashicorp/nomad/helper/testtask" @@ -158,7 +157,7 @@ func mkTestAllocDir(t *testing.T, h *dtestutil.DriverHarness, logger hclog.Logge } taskBuilder := taskenv.NewBuilder(mock.Node(), alloc, task, "global") - dtestutil.SetEnvvars(taskBuilder, drivers.FSIsolationNone, taskDir, config.DefaultConfig()) + dtestutil.SetEnvvars(taskBuilder, drivers.FSIsolationNone, taskDir) taskEnv := taskBuilder.Build() if tc.Env == nil { diff --git a/plugins/drivers/testutils/testing.go b/plugins/drivers/testutils/testing.go index 89ff1496d46..2bb87a64692 100644 --- a/plugins/drivers/testutils/testing.go +++ b/plugins/drivers/testutils/testing.go @@ -6,14 +6,12 @@ import ( "os" "path/filepath" "runtime" - "strings" "time" 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" "github.com/hashicorp/nomad/client/logmon" "github.com/hashicorp/nomad/client/taskenv" @@ -145,7 +143,7 @@ func (h *DriverHarness) MkAllocDir(t *drivers.TaskConfig, enableLogs bool) func( } taskBuilder := taskenv.NewBuilder(mock.Node(), alloc, task, "global") - SetEnvvars(taskBuilder, fsi, taskDir, config.DefaultConfig()) + SetEnvvars(taskBuilder, fsi, taskDir) taskEnv := taskBuilder.Build() if t.Env == nil { @@ -291,7 +289,7 @@ func (d *MockDriver) ExecTaskStreaming(ctx context.Context, taskID string, execO } // SetEnvvars sets path and host env vars depending on the FS isolation used. -func SetEnvvars(envBuilder *taskenv.Builder, fsi drivers.FSIsolation, taskDir *allocdir.TaskDir, conf *config.Config) { +func SetEnvvars(envBuilder *taskenv.Builder, fsi drivers.FSIsolation, taskDir *allocdir.TaskDir) { envBuilder.SetClientTaskRoot(taskDir.Dir) envBuilder.SetClientSharedAllocDir(taskDir.SharedAllocDir) @@ -314,11 +312,6 @@ func SetEnvvars(envBuilder *taskenv.Builder, fsi drivers.FSIsolation, taskDir *a // Set the host environment variables for non-image based drivers if fsi != drivers.FSIsolationImage { - // COMPAT(1.0) using inclusive language, blacklist is kept for backward compatibility. - filter := strings.Split(conf.ReadAlternativeDefault( - []string{"env.denylist", "env.blacklist"}, - config.DefaultEnvDenylist, - ), ",") - envBuilder.SetHostEnvvars(filter) + envBuilder.SetHostEnvvars([]string{"env.denylist"}) } }