From 1a13400f2d75e380f11c029db0c720da85a2b912 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Mon, 8 Feb 2016 16:08:29 -0800 Subject: [PATCH] Moved the destroycgroup method into executor --- client/driver/exec.go | 8 ++-- client/driver/executor/executor.go | 8 +++- client/driver/executor/executor_basic.go | 6 ++- client/driver/executor/executor_linux.go | 50 ++++++++++-------------- client/driver/java.go | 8 ++-- client/driver/utils_linux.go | 24 ------------ client/driver/utils_posix.go | 6 --- client/driver/utils_windows.go | 6 --- 8 files changed, 39 insertions(+), 77 deletions(-) diff --git a/client/driver/exec.go b/client/driver/exec.go index 430b387ca..fb09e1394 100644 --- a/client/driver/exec.go +++ b/client/driver/exec.go @@ -162,15 +162,15 @@ func (d *ExecDriver) Open(ctx *ExecContext, handleID string) (DriverHandle, erro pluginConfig := &plugin.ClientConfig{ Reattach: id.PluginConfig.PluginConfig(), } - executor, client, err := createExecutor(pluginConfig, d.config.LogOutput, d.config) + exec, client, err := createExecutor(pluginConfig, d.config.LogOutput, d.config) if err != nil { d.logger.Println("[ERROR] driver.exec: error connecting to plugin so destroying plugin pid and user pid") if e := destroyPlugin(id.PluginConfig.Pid, id.UserPid); e != nil { d.logger.Printf("[ERROR] driver.exec: error destroying plugin and userpid: %v", e) } if id.IsolationConfig != nil { - if e := destroyCgroup(id.IsolationConfig.Cgroup); e != nil { - d.logger.Printf("[ERROR] driver.exec: %v", e) + if e := executor.DestroyCgroup(id.IsolationConfig.Cgroup); e != nil { + d.logger.Printf("[ERROR] driver.exec: destroying cgroup failed: %v", e) } } if e := ctx.AllocDir.UnmountSpecialDirs(id.TaskDir); e != nil { @@ -182,7 +182,7 @@ func (d *ExecDriver) Open(ctx *ExecContext, handleID string) (DriverHandle, erro // Return a driver handle h := &execHandle{ pluginClient: client, - executor: executor, + executor: exec, userPid: id.UserPid, taskDir: id.TaskDir, isolationConfig: id.IsolationConfig, diff --git a/client/driver/executor/executor.go b/client/driver/executor/executor.go index 572042d48..4384e1ce4 100644 --- a/client/driver/executor/executor.go +++ b/client/driver/executor/executor.go @@ -186,7 +186,9 @@ func (e *UniversalExecutor) wait() { e.removeChrootMounts() } if e.ctx.ResourceLimits { - e.destroyCgroup() + e.lock.Lock() + DestroyCgroup(e.groups) + e.lock.Unlock() } e.exitState = &ProcessState{Pid: 0, ExitCode: exitCode, Time: time.Now()} } @@ -212,9 +214,11 @@ func (e *UniversalExecutor) Exit() error { } } if e.ctx.ResourceLimits { - if err := e.destroyCgroup(); err != nil { + e.lock.Lock() + if err := DestroyCgroup(e.groups); err != nil { merr.Errors = append(merr.Errors, err) } + e.lock.Unlock() } return merr.ErrorOrNil() } diff --git a/client/driver/executor/executor_basic.go b/client/driver/executor/executor_basic.go index 84d080233..9e531a547 100644 --- a/client/driver/executor/executor_basic.go +++ b/client/driver/executor/executor_basic.go @@ -2,11 +2,15 @@ package executor +import ( + cgroupConfig "github.com/opencontainers/runc/libcontainer/configs" +) + func (e *UniversalExecutor) configureChroot() error { return nil } -func (e *UniversalExecutor) destroyCgroup() error { +func DestroyCgroup(groups *cgroupConfig.Cgroup) error { return nil } diff --git a/client/driver/executor/executor_linux.go b/client/driver/executor/executor_linux.go index 630ea7a18..99780e1a0 100644 --- a/client/driver/executor/executor_linux.go +++ b/client/driver/executor/executor_linux.go @@ -46,7 +46,7 @@ func (e *UniversalExecutor) configureIsolation() error { return fmt.Errorf("error creating cgroups: %v", err) } if err := e.applyLimits(os.Getpid()); err != nil { - if er := e.destroyCgroup(); er != nil { + if er := DestroyCgroup(e.groups); er != nil { e.logger.Printf("[ERROR] executor: error destroying cgroup: %v", er) } if er := e.removeChrootMounts(); er != nil { @@ -65,7 +65,7 @@ func (e *UniversalExecutor) applyLimits(pid int) error { } // Entering the process in the cgroup - manager := e.getCgroupManager(e.groups) + manager := getCgroupManager(e.groups) if err := manager.Apply(pid); err != nil { e.logger.Printf("[ERROR] executor: unable to join cgroup: %v", err) if err := e.Exit(); err != nil { @@ -183,49 +183,39 @@ func (e *UniversalExecutor) removeChrootMounts() error { // destroyCgroup kills all processes in the cgroup and removes the cgroup // configuration from the host. -func (e *UniversalExecutor) destroyCgroup() error { - if e.groups == nil { +func DestroyCgroup(groups *cgroupConfig.Cgroup) error { + merrs := new(multierror.Error) + if groups == nil { return fmt.Errorf("Can't destroy: cgroup configuration empty") } - // Prevent a race between Wait/ForceStop - e.lock.Lock() - defer e.lock.Unlock() - - manager := e.getCgroupManager(e.groups) - pids, err := manager.GetPids() - if err != nil { - return fmt.Errorf("Failed to get pids in the cgroup %v: %v", e.groups.Name, err) - } - - errs := new(multierror.Error) - for _, pid := range pids { - process, err := os.FindProcess(pid) - if err != nil { - multierror.Append(errs, fmt.Errorf("Failed to find Pid %v: %v", pid, err)) - continue - } - - if err := process.Kill(); err != nil && err.Error() != "os: process already finished" { - multierror.Append(errs, fmt.Errorf("Failed to kill Pid %v: %v", pid, err)) - continue + manager := getCgroupManager(groups) + if pids, perr := manager.GetPids(); perr == nil { + for _, pid := range pids { + proc, err := os.FindProcess(pid) + if err != nil { + merrs.Errors = append(merrs.Errors, fmt.Errorf("error finding process %v: %v", pid, err)) + } else { + if e := proc.Kill(); e != nil { + merrs.Errors = append(merrs.Errors, fmt.Errorf("error killing process %v: %v", pid, e)) + } + } } } // Remove the cgroup. if err := manager.Destroy(); err != nil { - multierror.Append(errs, fmt.Errorf("Failed to delete the cgroup directories: %v", err)) + multierror.Append(merrs, fmt.Errorf("Failed to delete the cgroup directories: %v", err)) } - if len(errs.Errors) != 0 { - return fmt.Errorf("Failed to destroy cgroup: %v", errs) + if len(merrs.Errors) != 0 { + return fmt.Errorf("errors while destroying cgroup: %v", merrs) } - return nil } // getCgroupManager returns the correct libcontainer cgroup manager. -func (e *UniversalExecutor) getCgroupManager(groups *cgroupConfig.Cgroup) cgroups.Manager { +func getCgroupManager(groups *cgroupConfig.Cgroup) cgroups.Manager { var manager cgroups.Manager manager = &cgroupFs.Manager{Cgroups: groups} if systemd.UseSystemd() { diff --git a/client/driver/java.go b/client/driver/java.go index 3109b42ff..ee33d8b88 100644 --- a/client/driver/java.go +++ b/client/driver/java.go @@ -209,15 +209,15 @@ func (d *JavaDriver) Open(ctx *ExecContext, handleID string) (DriverHandle, erro pluginConfig := &plugin.ClientConfig{ Reattach: id.PluginConfig.PluginConfig(), } - executor, pluginClient, err := createExecutor(pluginConfig, d.config.LogOutput, d.config) + exec, pluginClient, err := createExecutor(pluginConfig, d.config.LogOutput, d.config) if err != nil { d.logger.Println("[ERROR] driver.java: error connecting to plugin so destroying plugin pid and user pid") if e := destroyPlugin(id.PluginConfig.Pid, id.UserPid); e != nil { d.logger.Printf("[ERROR] driver.java: error destroying plugin and userpid: %v", e) } if id.IsolationConfig != nil { - if e := destroyCgroup(id.IsolationConfig.Cgroup); e != nil { - d.logger.Printf("[ERROR] driver.exec: %v", e) + if e := executor.DestroyCgroup(id.IsolationConfig.Cgroup); e != nil { + d.logger.Printf("[ERROR] driver.exec: destroying cgroup failed: %v", e) } } if e := ctx.AllocDir.UnmountSpecialDirs(id.TaskDir); e != nil { @@ -230,7 +230,7 @@ func (d *JavaDriver) Open(ctx *ExecContext, handleID string) (DriverHandle, erro // Return a driver handle h := &javaHandle{ pluginClient: pluginClient, - executor: executor, + executor: exec, userPid: id.UserPid, isolationConfig: id.IsolationConfig, taskDir: id.TaskDir, diff --git a/client/driver/utils_linux.go b/client/driver/utils_linux.go index eb7022cd0..693bba6f6 100644 --- a/client/driver/utils_linux.go +++ b/client/driver/utils_linux.go @@ -3,12 +3,6 @@ package driver import ( "os/exec" "syscall" - - "fmt" - "github.com/opencontainers/runc/libcontainer/cgroups" - cgroupFs "github.com/opencontainers/runc/libcontainer/cgroups/fs" - "github.com/opencontainers/runc/libcontainer/cgroups/systemd" - cgroupConfig "github.com/opencontainers/runc/libcontainer/configs" ) // isolateCommand sets the setsid flag in exec.Cmd to true so that the process @@ -20,21 +14,3 @@ func isolateCommand(cmd *exec.Cmd) { } cmd.SysProcAttr.Setsid = true } - -// destroyCgroup destroys a cgroup and thereby killing all the processes in that -// group -func destroyCgroup(group *cgroupConfig.Cgroup) error { - if group == nil { - return nil - } - var manager cgroups.Manager - manager = &cgroupFs.Manager{Cgroups: group} - if systemd.UseSystemd() { - manager = &systemd.Manager{Cgroups: group} - } - - if err := manager.Destroy(); err != nil { - return fmt.Errorf("failed to destroy cgroup: %v", err) - } - return nil -} diff --git a/client/driver/utils_posix.go b/client/driver/utils_posix.go index cf90d109d..fef4a002f 100644 --- a/client/driver/utils_posix.go +++ b/client/driver/utils_posix.go @@ -5,8 +5,6 @@ package driver import ( "os/exec" "syscall" - - cgroupConfig "github.com/opencontainers/runc/libcontainer/configs" ) // isolateCommand sets the setsid flag in exec.Cmd to true so that the process @@ -18,7 +16,3 @@ func isolateCommand(cmd *exec.Cmd) { } cmd.SysProcAttr.Setsid = true } - -func destroyCgroup(group *cgroupConfig.Cgroup) error { - return nil -} diff --git a/client/driver/utils_windows.go b/client/driver/utils_windows.go index 84aff1e5f..5b2b7d842 100644 --- a/client/driver/utils_windows.go +++ b/client/driver/utils_windows.go @@ -2,14 +2,8 @@ package driver import ( "os/exec" - - cgroupConfig "github.com/opencontainers/runc/libcontainer/configs" ) // TODO Figure out if this is needed in Wondows func isolateCommand(cmd *exec.Cmd) { } - -func destroyCgroup(group *cgroupConfig.Cgroup) error { - return nil -}