From d6966951d6bddc02085f88f7d40eeb7143f00552 Mon Sep 17 00:00:00 2001 From: Vincent Batts Date: Tue, 23 Jan 2018 10:56:56 -0500 Subject: [PATCH] oci: abstract out cgroup calls per platform Signed-off-by: Vincent Batts oci: abstract out syscall for platforms Signed-off-by: Vincent Batts oci: abstract out the unix pipe per platform Signed-off-by: Vincent Batts oci: change the unix calls to be platform independent Signed-off-by: Vincent Batts --- oci/finished.go | 2 +- oci/finished_32.go | 2 +- oci/finished_unsupported.go | 23 +++++++++++++++ oci/oci.go | 57 +++++++++++++++---------------------- oci/oci_linux.go | 47 ++++++++++++++++++++++++++++++ oci/oci_unsupported.go | 20 +++++++++++++ 6 files changed, 115 insertions(+), 36 deletions(-) create mode 100644 oci/finished_unsupported.go create mode 100644 oci/oci_linux.go create mode 100644 oci/oci_unsupported.go diff --git a/oci/finished.go b/oci/finished.go index 9fedbfb7..5db4218f 100644 --- a/oci/finished.go +++ b/oci/finished.go @@ -1,4 +1,4 @@ -// +build !arm,!386 +// +build linux,!arm,!386 package oci diff --git a/oci/finished_32.go b/oci/finished_32.go index 3f24b1ba..53d37078 100644 --- a/oci/finished_32.go +++ b/oci/finished_32.go @@ -1,4 +1,4 @@ -// +build arm 386 +// +build linux,arm linux,386 package oci diff --git a/oci/finished_unsupported.go b/oci/finished_unsupported.go new file mode 100644 index 00000000..918abe48 --- /dev/null +++ b/oci/finished_unsupported.go @@ -0,0 +1,23 @@ +// +build !linux + +package oci + +import ( + "os" + "time" +) + +func getFinishedTime(fi os.FileInfo) time.Time { + // Windows would be like + //st := fi.Sys().(*syscall.Win32FileAttributeDatao) + //st.CreationTime.Nanoseconds() + + // Darwin and Freebsd would be like + //st := fi.Sys().(*syscall.Stat_t) + //st.Ctimespec.Nsec + + // openbsd would be like + //st := fi.Sys().(*syscall.Stat_t) + //st.Ctim.Nsec + return fi.ModTime() +} diff --git a/oci/oci.go b/oci/oci.go index 73b9f885..c8021c48 100644 --- a/oci/oci.go +++ b/oci/oci.go @@ -13,12 +13,10 @@ import ( "syscall" "time" - "github.com/containerd/cgroups" "github.com/kubernetes-incubator/cri-o/utils" rspec "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" "golang.org/x/net/context" - "golang.org/x/sys/unix" kwait "k8s.io/apimachinery/pkg/util/wait" ) @@ -200,9 +198,7 @@ func (r *Runtime) CreateContainer(c *Container, cgroupParent string) (err error) cmd := exec.Command(r.conmonPath, args...) cmd.Dir = c.bundlePath - cmd.SysProcAttr = &syscall.SysProcAttr{ - Setpgid: true, - } + cmd.SysProcAttr = sysProcAttrPlatform() cmd.Stdin = os.Stdin cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr @@ -231,19 +227,8 @@ func (r *Runtime) CreateContainer(c *Container, cgroupParent string) (err error) logrus.Warnf("Failed to add conmon to systemd sandbox cgroup: %v", err) } } else { - control, err := cgroups.New(cgroups.V1, cgroups.StaticPath(filepath.Join(cgroupParent, "/crio-conmon-"+c.id)), &rspec.LinuxResources{}) - if err != nil { - logrus.Warnf("Failed to add conmon to cgroupfs sandbox cgroup: %v", err) - } else { - // Here we should defer a crio-connmon- cgroup hierarchy deletion, but it will - // always fail as conmon's pid is still there. - // Fortunately, kubelet takes care of deleting this for us, so the leak will - // only happens in corner case where one does a manual deletion of the container - // through e.g. runc. This should be handled by implementing a conmon monitoring - // routine that does the cgroup cleanup once conmon is terminated. - if err := control.Add(cgroups.Process{Pid: cmd.Process.Pid}); err != nil { - logrus.Warnf("Failed to add conmon to cgroupfs sandbox cgroup: %v", err) - } + if err := createContainerPlatform(c, cgroupParent, cmd.Process.Pid); err != nil { + logrus.Warnf("%s", err) } } @@ -478,7 +463,7 @@ func (r *Runtime) ExecSync(c *Container, command []string, timeout int64) (resp err = cmd.Wait() if err != nil { if exitErr, ok := err.(*exec.ExitError); ok { - if status, ok := exitErr.Sys().(unix.WaitStatus); ok { + if status, ok := exitErr.Sys().(syscall.WaitStatus); ok { return nil, ExecSyncError{ Stdout: stdoutBuf, Stderr: stderrBuf, @@ -577,8 +562,13 @@ func waitContainerStop(ctx context.Context, c *Container, timeout time.Duration) return default: // Check if the process is still around - err := unix.Kill(c.state.Pid, 0) - if err == unix.ESRCH { + proc, err := os.FindProcess(c.state.Pid) + if err != nil { + close(done) + return + } + err = proc.Signal(syscall.Signal(0)) + if err == syscall.ESRCH { close(done) return } @@ -594,8 +584,12 @@ func waitContainerStop(ctx context.Context, c *Container, timeout time.Duration) return ctx.Err() case <-time.After(timeout): close(chControl) - err := unix.Kill(c.state.Pid, unix.SIGKILL) - if err != nil && err != unix.ESRCH { + proc, err := os.FindProcess(c.state.Pid) + if err != nil { + return fmt.Errorf("failed to find pid %d: %v", c.state.Pid, err) + } + err = proc.Signal(syscall.SIGKILL) + if err != nil && err != syscall.ESRCH { return fmt.Errorf("failed to kill process: %v", err) } } @@ -660,8 +654,12 @@ func (r *Runtime) StopContainer(ctx context.Context, c *Container, timeout int64 defer c.opLock.Unlock() // Check if the process is around before sending a signal - err := unix.Kill(c.state.Pid, 0) - if err == unix.ESRCH { + proc, err := os.FindProcess(c.state.Pid) + if err != nil { + return nil + } + err = proc.Signal(syscall.Signal(0)) + if err == syscall.ESRCH { c.state.Finished = time.Now() return nil } @@ -773,15 +771,6 @@ func (r *Runtime) ContainerStatus(c *Container) *ContainerState { return c.state } -// newPipe creates a unix socket pair for communication -func newPipe() (parent *os.File, child *os.File, err error) { - fds, err := unix.Socketpair(unix.AF_LOCAL, unix.SOCK_STREAM|unix.SOCK_CLOEXEC, 0) - if err != nil { - return nil, nil, err - } - return os.NewFile(uintptr(fds[1]), "parent"), os.NewFile(uintptr(fds[0]), "child"), nil -} - // RuntimeReady checks if the runtime is up and ready to accept // basic containers e.g. container only needs host network. func (r *Runtime) RuntimeReady() (bool, error) { diff --git a/oci/oci_linux.go b/oci/oci_linux.go new file mode 100644 index 00000000..e73fe45f --- /dev/null +++ b/oci/oci_linux.go @@ -0,0 +1,47 @@ +// +build linux + +package oci + +import ( + "fmt" + "os" + "path/filepath" + "syscall" + + "github.com/containerd/cgroups" + rspec "github.com/opencontainers/runtime-spec/specs-go" + "golang.org/x/sys/unix" +) + +func createContainerPlatform(c *Container, cgroupParent string, pid int) error { + control, err := cgroups.New(cgroups.V1, cgroups.StaticPath(filepath.Join(cgroupParent, "/crio-conmon-"+c.id)), &rspec.LinuxResources{}) + if err != nil { + return fmt.Errorf("Failed to add conmon to cgroupfs sandbox cgroup: %v", err) + } else { + // Here we should defer a crio-connmon- cgroup hierarchy deletion, but it will + // always fail as conmon's pid is still there. + // Fortunately, kubelet takes care of deleting this for us, so the leak will + // only happens in corner case where one does a manual deletion of the container + // through e.g. runc. This should be handled by implementing a conmon monitoring + // routine that does the cgroup cleanup once conmon is terminated. + if err := control.Add(cgroups.Process{Pid: pid}); err != nil { + fmt.Errorf("Failed to add conmon to cgroupfs sandbox cgroup: %v", err) + } + } + return nil +} + +func sysProcAttrPlatform() *syscall.SysProcAttr { + return &syscall.SysProcAttr{ + Setpgid: true, + } +} + +// newPipe creates a unix socket pair for communication +func newPipe() (parent *os.File, child *os.File, err error) { + fds, err := unix.Socketpair(unix.AF_LOCAL, unix.SOCK_STREAM|unix.SOCK_CLOEXEC, 0) + if err != nil { + return nil, nil, err + } + return os.NewFile(uintptr(fds[1]), "parent"), os.NewFile(uintptr(fds[0]), "child"), nil +} diff --git a/oci/oci_unsupported.go b/oci/oci_unsupported.go new file mode 100644 index 00000000..4824d767 --- /dev/null +++ b/oci/oci_unsupported.go @@ -0,0 +1,20 @@ +// +build !linux + +package oci + +import ( + "os" + "syscall" +) + +func createContainerPlatform(c *Container, cgroupParent string, pid int) error { + return nil +} + +func sysProcAttrPlatform() *syscall.SysProcAttr { + return &syscall.SysProcAttr{} +} + +func newPipe() (parent *os.File, child *os.File, err error) { + return os.Pipe() +}