From 9d5e754caad0af212545b352e19c3085d8df3281 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 15 Oct 2022 15:10:25 +0200 Subject: [PATCH] move pkg/system: process to a separate package Signed-off-by: Sebastiaan van Stijn --- daemon/container_operations_unix.go | 6 ++-- libcontainerd/supervisor/remote_daemon.go | 9 +++--- .../supervisor/remote_daemon_linux.go | 8 ++--- .../supervisor/remote_daemon_windows.go | 4 +-- pkg/process/doc.go | 3 ++ pkg/{system => process}/process_unix.go | 20 ++++++++----- pkg/process/process_windows.go | 29 +++++++++++++++++++ pkg/system/process_deprecated.go | 27 +++++++++++++++++ pkg/system/process_windows.go | 18 ------------ 9 files changed, 85 insertions(+), 39 deletions(-) create mode 100644 pkg/process/doc.go rename pkg/{system => process}/process_unix.go (60%) create mode 100644 pkg/process/process_windows.go create mode 100644 pkg/system/process_deprecated.go delete mode 100644 pkg/system/process_windows.go diff --git a/daemon/container_operations_unix.go b/daemon/container_operations_unix.go index c61865ad20..290ec59a34 100644 --- a/daemon/container_operations_unix.go +++ b/daemon/container_operations_unix.go @@ -15,8 +15,8 @@ import ( "github.com/docker/docker/errdefs" "github.com/docker/docker/libnetwork" "github.com/docker/docker/pkg/idtools" + "github.com/docker/docker/pkg/process" "github.com/docker/docker/pkg/stringid" - "github.com/docker/docker/pkg/system" "github.com/docker/docker/runconfig" "github.com/moby/sys/mount" "github.com/opencontainers/selinux/go-selinux/label" @@ -352,9 +352,9 @@ func killProcessDirectly(container *container.Container) error { } // In case there were some exceptions(e.g., state of zombie and D) - if system.IsProcessAlive(pid) { + if process.Alive(pid) { // Since we can not kill a zombie pid, add zombie check here - isZombie, err := system.IsProcessZombie(pid) + isZombie, err := process.Zombie(pid) if err != nil { logrus.WithError(err).WithField("container", container.ID).Warn("Container state is invalid") return err diff --git a/libcontainerd/supervisor/remote_daemon.go b/libcontainerd/supervisor/remote_daemon.go index a81bff75b2..81c8748abb 100644 --- a/libcontainerd/supervisor/remote_daemon.go +++ b/libcontainerd/supervisor/remote_daemon.go @@ -14,6 +14,7 @@ import ( "github.com/containerd/containerd" "github.com/containerd/containerd/services/server/config" "github.com/containerd/containerd/sys" + "github.com/docker/docker/pkg/process" "github.com/docker/docker/pkg/system" "github.com/pelletier/go-toml" "github.com/pkg/errors" @@ -145,7 +146,7 @@ func (r *remote) getContainerdPid() (int, error) { if err != nil { return -1, err } - if system.IsProcessAlive(int(pid)) { + if process.Alive(int(pid)) { return int(pid), nil } } @@ -238,7 +239,7 @@ func (r *remote) startContainerd() error { err = os.WriteFile(r.pidFile, []byte(strconv.Itoa(r.daemonPid)), 0660) if err != nil { - system.KillProcess(r.daemonPid) + process.Kill(r.daemonPid) return errors.Wrap(err, "libcontainerd: failed to save daemon pid to disk") } @@ -357,7 +358,7 @@ func (r *remote) monitorDaemon(ctx context.Context) { r.logger.WithError(err).WithField("binary", binaryName).Debug("daemon is not responding") transientFailureCount++ - if transientFailureCount < maxConnectionRetryCount || system.IsProcessAlive(r.daemonPid) { + if transientFailureCount < maxConnectionRetryCount || process.Alive(r.daemonPid) { delay = time.Duration(transientFailureCount) * 200 * time.Millisecond continue } @@ -365,7 +366,7 @@ func (r *remote) monitorDaemon(ctx context.Context) { client = nil } - if system.IsProcessAlive(r.daemonPid) { + if process.Alive(r.daemonPid) { r.logger.WithField("pid", r.daemonPid).Info("killing and restarting containerd") r.killDaemon() } diff --git a/libcontainerd/supervisor/remote_daemon_linux.go b/libcontainerd/supervisor/remote_daemon_linux.go index 9523b04005..626f66d3c6 100644 --- a/libcontainerd/supervisor/remote_daemon_linux.go +++ b/libcontainerd/supervisor/remote_daemon_linux.go @@ -7,7 +7,7 @@ import ( "time" "github.com/containerd/containerd/defaults" - "github.com/docker/docker/pkg/system" + "github.com/docker/docker/pkg/process" ) const ( @@ -35,13 +35,13 @@ func (r *remote) stopDaemon() { syscall.Kill(r.daemonPid, syscall.SIGTERM) // Wait up to 15secs for it to stop for i := time.Duration(0); i < shutdownTimeout; i += time.Second { - if !system.IsProcessAlive(r.daemonPid) { + if !process.Alive(r.daemonPid) { break } time.Sleep(time.Second) } - if system.IsProcessAlive(r.daemonPid) { + if process.Alive(r.daemonPid) { r.logger.WithField("pid", r.daemonPid).Warn("daemon didn't stop within 15 secs, killing it") syscall.Kill(r.daemonPid, syscall.SIGKILL) } @@ -51,7 +51,7 @@ func (r *remote) killDaemon() { // Try to get a stack trace syscall.Kill(r.daemonPid, syscall.SIGUSR1) <-time.After(100 * time.Millisecond) - system.KillProcess(r.daemonPid) + process.Kill(r.daemonPid) } func (r *remote) platformCleanup() { diff --git a/libcontainerd/supervisor/remote_daemon_windows.go b/libcontainerd/supervisor/remote_daemon_windows.go index 9b254ef58a..9cb4610509 100644 --- a/libcontainerd/supervisor/remote_daemon_windows.go +++ b/libcontainerd/supervisor/remote_daemon_windows.go @@ -3,7 +3,7 @@ package supervisor // import "github.com/docker/docker/libcontainerd/supervisor" import ( "os" - "github.com/docker/docker/pkg/system" + "github.com/docker/docker/pkg/process" ) const ( @@ -40,7 +40,7 @@ func (r *remote) stopDaemon() { } func (r *remote) killDaemon() { - system.KillProcess(r.daemonPid) + process.Kill(r.daemonPid) } func (r *remote) platformCleanup() { diff --git a/pkg/process/doc.go b/pkg/process/doc.go new file mode 100644 index 0000000000..dae536d7db --- /dev/null +++ b/pkg/process/doc.go @@ -0,0 +1,3 @@ +// Package process provides a set of basic functions to manage individual +// processes. +package process diff --git a/pkg/system/process_unix.go b/pkg/process/process_unix.go similarity index 60% rename from pkg/system/process_unix.go rename to pkg/process/process_unix.go index 951bc179d4..109b56f56c 100644 --- a/pkg/system/process_unix.go +++ b/pkg/process/process_unix.go @@ -1,7 +1,7 @@ //go:build linux || freebsd || darwin // +build linux freebsd darwin -package system // import "github.com/docker/docker/pkg/system" +package process import ( "bytes" @@ -11,8 +11,8 @@ import ( "golang.org/x/sys/unix" ) -// IsProcessAlive returns true if process with a given pid is running. -func IsProcessAlive(pid int) bool { +// Alive returns true if process with a given pid is running. +func Alive(pid int) bool { err := unix.Kill(pid, 0) if err == nil || err == unix.EPERM { return true @@ -21,14 +21,18 @@ func IsProcessAlive(pid int) bool { return false } -// KillProcess force-stops a process. -func KillProcess(pid int) { - unix.Kill(pid, unix.SIGKILL) +// Kill force-stops a process. +func Kill(pid int) error { + err := unix.Kill(pid, unix.SIGKILL) + if err != nil && err != unix.ESRCH { + return err + } + return nil } -// IsProcessZombie return true if process has a state with "Z" +// Zombie return true if process has a state with "Z" // http://man7.org/linux/man-pages/man5/proc.5.html -func IsProcessZombie(pid int) (bool, error) { +func Zombie(pid int) (bool, error) { data, err := os.ReadFile(fmt.Sprintf("/proc/%d/stat", pid)) if err != nil { if os.IsNotExist(err) { diff --git a/pkg/process/process_windows.go b/pkg/process/process_windows.go new file mode 100644 index 0000000000..8de3362c72 --- /dev/null +++ b/pkg/process/process_windows.go @@ -0,0 +1,29 @@ +package process + +import "os" + +// Alive returns true if process with a given pid is running. +func Alive(pid int) bool { + _, err := os.FindProcess(pid) + + return err == nil +} + +// Kill force-stops a process. +func Kill(pid int) error { + p, err := os.FindProcess(pid) + if err == nil { + err = p.Kill() + if err != nil && err != os.ErrProcessDone { + return err + } + } + return nil +} + +// Zombie is not supported on Windows. +// +// TODO(thaJeztah): remove once we remove the stubs from pkg/system. +func Zombie(_ int) (bool, error) { + return false, nil +} diff --git a/pkg/system/process_deprecated.go b/pkg/system/process_deprecated.go new file mode 100644 index 0000000000..7b9f19acd5 --- /dev/null +++ b/pkg/system/process_deprecated.go @@ -0,0 +1,27 @@ +//go:build linux || freebsd || darwin || windows +// +build linux freebsd darwin windows + +package system + +import "github.com/docker/docker/pkg/process" + +var ( + // IsProcessAlive returns true if process with a given pid is running. + // + // Deprecated: use [process.Alive]. + IsProcessAlive = process.Alive + + // IsProcessZombie return true if process has a state with "Z" + // + // Deprecated: use [process.Zombie]. + // + // TODO(thaJeztah): remove the Windows implementation in process once we remove this stub. + IsProcessZombie = process.Zombie +) + +// KillProcess force-stops a process. +// +// Deprecated: use [process.Kill]. +func KillProcess(pid int) { + _ = process.Kill(pid) +} diff --git a/pkg/system/process_windows.go b/pkg/system/process_windows.go deleted file mode 100644 index 09bdfa0ca0..0000000000 --- a/pkg/system/process_windows.go +++ /dev/null @@ -1,18 +0,0 @@ -package system // import "github.com/docker/docker/pkg/system" - -import "os" - -// IsProcessAlive returns true if process with a given pid is running. -func IsProcessAlive(pid int) bool { - _, err := os.FindProcess(pid) - - return err == nil -} - -// KillProcess force-stops a process. -func KillProcess(pid int) { - p, err := os.FindProcess(pid) - if err == nil { - _ = p.Kill() - } -}