From 2321bddf2eb53e1343c01a7598bcc90e5dc19082 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Fri, 6 Nov 2015 11:23:27 -0800 Subject: [PATCH] Add Valid command to spawner and make executors check when opening --- client/driver/executor/exec_basic.go | 2 +- client/driver/executor/exec_linux.go | 3 +- client/driver/executor/test_harness.go | 30 ++++++++++++ client/driver/spawn/spawn.go | 15 ++++++ client/driver/spawn/spawn_posix.go | 10 +++- client/driver/spawn/spawn_test.go | 68 ++++++++++++++++++++++++++ 6 files changed, 123 insertions(+), 5 deletions(-) diff --git a/client/driver/executor/exec_basic.go b/client/driver/executor/exec_basic.go index 8f2d6fb2a..4b5f6a9f1 100644 --- a/client/driver/executor/exec_basic.go +++ b/client/driver/executor/exec_basic.go @@ -89,7 +89,7 @@ func (e *BasicExecutor) Open(id string) error { // Setup the executor. e.spawn = &spawn - return nil + return e.spawn.Valid() } func (e *BasicExecutor) Wait() error { diff --git a/client/driver/executor/exec_linux.go b/client/driver/executor/exec_linux.go index a7bbdd03c..5a361248f 100644 --- a/client/driver/executor/exec_linux.go +++ b/client/driver/executor/exec_linux.go @@ -95,8 +95,7 @@ func (e *LinuxExecutor) Open(id string) error { e.groups = execID.Groups e.spawn = execID.Spawn e.taskDir = execID.TaskDir - - return nil + return e.spawn.Valid() } func (e *LinuxExecutor) ID() (string, error) { diff --git a/client/driver/executor/test_harness.go b/client/driver/executor/test_harness.go index 6eabb556d..1e37f8eff 100644 --- a/client/driver/executor/test_harness.go +++ b/client/driver/executor/test_harness.go @@ -229,3 +229,33 @@ func Executor_Open(t *testing.T, command buildExecCommand, newExecutor func() Ex log.Panicf("Command output incorrectly: want %v; got %v", expected, act) } } + +func Executor_Open_Invalid(t *testing.T, command buildExecCommand, newExecutor func() Executor) { + task, alloc := mockAllocDir(t) + e := command("echo", "foo") + + if err := e.Limit(constraint); err != nil { + log.Panicf("Limit() failed: %v", err) + } + + if err := e.ConfigureTaskDir(task, alloc); err != nil { + log.Panicf("ConfigureTaskDir(%v, %v) failed: %v", task, alloc, err) + } + + if err := e.Start(); err != nil { + log.Panicf("Start() failed: %v", err) + } + + id, err := e.ID() + if err != nil { + log.Panicf("ID() failed: %v", err) + } + + // Destroy the allocdir which removes the exit code. + alloc.Destroy() + + e2 := newExecutor() + if err := e2.Open(id); err == nil { + log.Panicf("Open(%v) should have failed", id) + } +} diff --git a/client/driver/spawn/spawn.go b/client/driver/spawn/spawn.go index b962a1ab4..ab1f22f3d 100644 --- a/client/driver/spawn/spawn.go +++ b/client/driver/spawn/spawn.go @@ -285,3 +285,18 @@ func (s *Spawner) readExitCode() (int, error) { return exitStatus.ExitCode, nil } + +// Valid checks that the state of the Spawner is valid and that a subsequent +// Wait could be called. This is useful to call when reopening a Spawner +// throught client restarts. If Valid a nil error is returned. +func (s *Spawner) Valid() error { + if s.Alive() { + return nil + } + + if _, err := s.readExitCode(); err == nil { + return nil + } + + return fmt.Errorf("Spawner not alive and exit code not written") +} diff --git a/client/driver/spawn/spawn_posix.go b/client/driver/spawn/spawn_posix.go index 7df381064..9b10caddc 100644 --- a/client/driver/spawn/spawn_posix.go +++ b/client/driver/spawn/spawn_posix.go @@ -2,11 +2,17 @@ package spawn -import "syscall" +import ( + "os" + "syscall" +) func (s *Spawner) Alive() bool { if s.spawn == nil { - return false + var err error + if s.spawn, err = os.FindProcess(s.SpawnPid); err != nil { + return false + } } err := s.spawn.Signal(syscall.Signal(0)) diff --git a/client/driver/spawn/spawn_test.go b/client/driver/spawn/spawn_test.go index bbb8c8dca..de5d8e97c 100644 --- a/client/driver/spawn/spawn_test.go +++ b/client/driver/spawn/spawn_test.go @@ -298,3 +298,71 @@ func TestSpawn_DeadSpawnDaemon_NonParent(t *testing.T) { t.Fatalf("Wait() should have failed: %v", err) } } + +func TestSpawn_Valid_TaskRunning(t *testing.T) { + f, err := ioutil.TempFile("", "") + if err != nil { + t.Fatalf("TempFile() failed") + } + defer os.Remove(f.Name()) + + spawn := NewSpawner(f.Name()) + spawn.SetCommand(exec.Command("sleep", "2")) + if err := spawn.Spawn(nil); err != nil { + t.Fatalf("Spawn() failed %v", err) + } + + if err := spawn.Valid(); err != nil { + t.Fatalf("Valid() failed: %v", err) + } + + if _, err := spawn.Wait(); err != nil { + t.Fatalf("Wait() failed %v", err) + } +} + +func TestSpawn_Valid_TaskExit_ExitCode(t *testing.T) { + f, err := ioutil.TempFile("", "") + if err != nil { + t.Fatalf("TempFile() failed") + } + defer os.Remove(f.Name()) + + spawn := NewSpawner(f.Name()) + spawn.SetCommand(exec.Command("echo", "foo")) + if err := spawn.Spawn(nil); err != nil { + t.Fatalf("Spawn() failed %v", err) + } + + if _, err := spawn.Wait(); err != nil { + t.Fatalf("Wait() failed %v", err) + } + + if err := spawn.Valid(); err != nil { + t.Fatalf("Valid() failed: %v", err) + } +} + +func TestSpawn_Valid_TaskExit_NoExitCode(t *testing.T) { + f, err := ioutil.TempFile("", "") + if err != nil { + t.Fatalf("TempFile() failed") + } + + spawn := NewSpawner(f.Name()) + spawn.SetCommand(exec.Command("echo", "foo")) + if err := spawn.Spawn(nil); err != nil { + t.Fatalf("Spawn() failed %v", err) + } + + if _, err := spawn.Wait(); err != nil { + t.Fatalf("Wait() failed %v", err) + } + + // Delete the file so that it can't find the exit code. + os.Remove(f.Name()) + + if err := spawn.Valid(); err == nil { + t.Fatalf("Valid() should have failed") + } +}