From 8f735a581dfea491b6a926a7fb3315d46a9cd503 Mon Sep 17 00:00:00 2001 From: Mrunal Patel Date: Wed, 9 Aug 2017 14:17:33 -0400 Subject: [PATCH 1/2] container: Don't call runtime state on every container on list Signed-off-by: Mrunal Patel --- server/container_list.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/server/container_list.go b/server/container_list.go index 3589ffa5..a6ddd81c 100644 --- a/server/container_list.go +++ b/server/container_list.go @@ -66,10 +66,6 @@ func (s *Server) ListContainers(ctx context.Context, req *pb.ListContainersReque } for _, ctr := range ctrList { - if err := s.Runtime().UpdateStatus(ctr); err != nil { - return nil, err - } - podSandboxID := ctr.Sandbox() cState := s.Runtime().ContainerStatus(ctr) created := cState.Created.UnixNano() From 30ded8309609a27af2f70558706f7e147448a326 Mon Sep 17 00:00:00 2001 From: Mrunal Patel Date: Sat, 12 Aug 2017 18:35:55 -0700 Subject: [PATCH 2/2] Add inotify watcher for container exits This allows the container list API to return updated status for exited container without having to call container status first. Signed-off-by: Mrunal Patel --- cmd/crio/main.go | 4 +++ conmon/conmon.c | 8 +++++- libkpod/config.go | 16 ++++++++---- libkpod/container_server.go | 2 +- oci/oci.go | 35 ++++++++++++++------------ server/container_remove.go | 6 +++++ server/server.go | 49 +++++++++++++++++++++++++++++++++++++ 7 files changed, 97 insertions(+), 23 deletions(-) diff --git a/cmd/crio/main.go b/cmd/crio/main.go index 71d83d5a..4d4c3c2f 100644 --- a/cmd/crio/main.go +++ b/cmd/crio/main.go @@ -408,6 +408,10 @@ func main() { // after the daemon is done setting up we can notify systemd api notifySystem() + go func() { + service.StartExitMonitor() + }() + err = s.Serve(lis) if graceful && strings.Contains(strings.ToLower(err.Error()), "use of closed network connection") { err = nil diff --git a/conmon/conmon.c b/conmon/conmon.c index 2009bf49..006ba141 100644 --- a/conmon/conmon.c +++ b/conmon/conmon.c @@ -105,6 +105,7 @@ static bool opt_systemd_cgroup = false; static char *opt_exec_process_spec = NULL; static bool opt_exec = false; static char *opt_log_path = NULL; +static char *opt_exit_dir = NULL; static int opt_timeout = 0; static GOptionEntry opt_entries[] = { @@ -118,6 +119,7 @@ static GOptionEntry opt_entries[] = { "systemd-cgroup", 's', 0, G_OPTION_ARG_NONE, &opt_systemd_cgroup, "Enable systemd cgroup manager", NULL }, { "exec", 'e', 0, G_OPTION_ARG_NONE, &opt_exec, "Exec a command in a running container", NULL }, { "exec-process-spec", 0, 0, G_OPTION_ARG_STRING, &opt_exec_process_spec, "Path to the process spec for exec", NULL }, + { "exit-dir", 0, 0, G_OPTION_ARG_STRING, &opt_exit_dir, "Path to the directory where exit files are written", NULL }, { "log-path", 'l', 0, G_OPTION_ARG_STRING, &opt_log_path, "Log file path", NULL }, { "timeout", 'T', 0, G_OPTION_ARG_INT, &opt_timeout, "Timeout in seconds", NULL }, { NULL } @@ -1067,6 +1069,9 @@ int main(int argc, char *argv[]) if (opt_runtime_path == NULL) nexit("Runtime path not provided. Use --runtime"); + if (!opt_exec && opt_exit_dir == NULL) + nexit("Container exit directory not provided. Use --exit-dir"); + if (opt_bundle_path == NULL && !opt_exec) { if (getcwd(cwd, sizeof(cwd)) == NULL) { nexit("Failed to get working directory"); @@ -1383,7 +1388,8 @@ int main(int argc, char *argv[]) if (!opt_exec) { _cleanup_free_ char *status_str = g_strdup_printf("%d", exit_status); - if (!g_file_set_contents("exit", status_str, -1, &err)) + _cleanup_free_ char *exit_file_path = g_build_filename(opt_exit_dir, opt_cid, NULL); + if (!g_file_set_contents(exit_file_path, status_str, -1, &err)) nexit("Failed to write %s to exit file: %s\n", status_str, err->message); } else { diff --git a/libkpod/config.go b/libkpod/config.go index a6b276d3..c71aa314 100644 --- a/libkpod/config.go +++ b/libkpod/config.go @@ -22,6 +22,7 @@ const ( cniBinDir = "/opt/cni/bin/" cgroupManager = "cgroupfs" lockPath = "/run/crio.lock" + containerExitsDir = "/var/run/kpod/exits" ) // Config represents the entire set of configuration values that can be set for @@ -136,6 +137,10 @@ type RuntimeConfig struct { // PidsLimit is the number of processes each container is restricted to // by the cgroup process number controller. PidsLimit int64 `toml:"pids_limit"` + + // ContainerExitsDir is the directory in which container exit files are + // written to by conmon. + ContainerExitsDir string `toml:"container_exits_dir"` } // ImageConfig represents the "crio.image" TOML config table. @@ -255,11 +260,12 @@ func DefaultConfig() *Config { ConmonEnv: []string{ "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", }, - SELinux: selinux.GetEnabled(), - SeccompProfile: seccompProfilePath, - ApparmorProfile: apparmorProfileName, - CgroupManager: cgroupManager, - PidsLimit: DefaultPidsLimit, + SELinux: selinux.GetEnabled(), + SeccompProfile: seccompProfilePath, + ApparmorProfile: apparmorProfileName, + CgroupManager: cgroupManager, + PidsLimit: DefaultPidsLimit, + ContainerExitsDir: containerExitsDir, }, ImageConfig: ImageConfig{ DefaultTransport: defaultTransport, diff --git a/libkpod/container_server.go b/libkpod/container_server.go index c3535e9b..b4c333c3 100644 --- a/libkpod/container_server.go +++ b/libkpod/container_server.go @@ -114,7 +114,7 @@ func New(config *Config) (*ContainerServer, error) { return nil, err } - runtime, err := oci.New(config.Runtime, config.RuntimeUntrustedWorkload, config.DefaultWorkloadTrust, config.Conmon, config.ConmonEnv, config.CgroupManager) + runtime, err := oci.New(config.Runtime, config.RuntimeUntrustedWorkload, config.DefaultWorkloadTrust, config.Conmon, config.ConmonEnv, config.CgroupManager, config.ContainerExitsDir) if err != nil { return nil, err } diff --git a/oci/oci.go b/oci/oci.go index 2be42b78..2eb14f0c 100644 --- a/oci/oci.go +++ b/oci/oci.go @@ -31,28 +31,30 @@ const ( ) // New creates a new Runtime with options provided -func New(runtimeTrustedPath string, runtimeUntrustedPath string, trustLevel string, conmonPath string, conmonEnv []string, cgroupManager string) (*Runtime, error) { +func New(runtimeTrustedPath string, runtimeUntrustedPath string, trustLevel string, conmonPath string, conmonEnv []string, cgroupManager string, containerExitsDir string) (*Runtime, error) { r := &Runtime{ - name: filepath.Base(runtimeTrustedPath), - trustedPath: runtimeTrustedPath, - untrustedPath: runtimeUntrustedPath, - trustLevel: trustLevel, - conmonPath: conmonPath, - conmonEnv: conmonEnv, - cgroupManager: cgroupManager, + name: filepath.Base(runtimeTrustedPath), + trustedPath: runtimeTrustedPath, + untrustedPath: runtimeUntrustedPath, + trustLevel: trustLevel, + conmonPath: conmonPath, + conmonEnv: conmonEnv, + cgroupManager: cgroupManager, + containerExitsDir: containerExitsDir, } return r, nil } // Runtime stores the information about a oci runtime type Runtime struct { - name string - trustedPath string - untrustedPath string - trustLevel string - conmonPath string - conmonEnv []string - cgroupManager string + name string + trustedPath string + untrustedPath string + trustLevel string + conmonPath string + conmonEnv []string + cgroupManager string + containerExitsDir string } // syncInfo is used to return data from monitor process to daemon @@ -146,6 +148,7 @@ func (r *Runtime) CreateContainer(c *Container, cgroupParent string) error { args = append(args, "-b", c.bundlePath) args = append(args, "-p", filepath.Join(c.bundlePath, "pidfile")) args = append(args, "-l", c.logPath) + args = append(args, "--exit-dir", r.containerExitsDir) if c.terminal { args = append(args, "-t") } else if c.stdin { @@ -579,7 +582,7 @@ func (r *Runtime) UpdateStatus(c *Container) error { } if c.state.Status == ContainerStateStopped { - exitFilePath := filepath.Join(c.bundlePath, "exit") + exitFilePath := filepath.Join(r.containerExitsDir, c.id) fi, err := os.Stat(exitFilePath) if err != nil { logrus.Warnf("failed to find container exit file: %v", err) diff --git a/server/container_remove.go b/server/container_remove.go index 86884c75..08fe8e40 100644 --- a/server/container_remove.go +++ b/server/container_remove.go @@ -2,6 +2,8 @@ package server import ( "fmt" + "os" + "path/filepath" "github.com/kubernetes-incubator/cri-o/oci" "github.com/sirupsen/logrus" @@ -36,6 +38,10 @@ func (s *Server) RemoveContainer(ctx context.Context, req *pb.RemoveContainerReq return nil, fmt.Errorf("failed to delete container %s: %v", c.ID(), err) } + if err := os.Remove(filepath.Join(s.config.ContainerExitsDir, c.ID())); err != nil { + return nil, fmt.Errorf("failed to remove container exit file %s: %v", c.ID(), err) + } + s.removeContainer(c) if err := s.StorageRuntimeServer().DeleteContainer(c.ID()); err != nil { diff --git a/server/server.go b/server/server.go index 482d67e3..141eccbc 100644 --- a/server/server.go +++ b/server/server.go @@ -7,8 +7,10 @@ import ( "net" "net/http" "os" + "path/filepath" "sync" + "github.com/fsnotify/fsnotify" "github.com/kubernetes-incubator/cri-o/libkpod" "github.com/kubernetes-incubator/cri-o/libkpod/sandbox" "github.com/kubernetes-incubator/cri-o/oci" @@ -148,6 +150,13 @@ func New(config *Config) (*Server, error) { if err := os.MkdirAll("/var/run/crio", 0755); err != nil { return nil, err } + + config.ContainerExitsDir = "/var/run/crio/exits" + + // This is used to monitor container exits using inotify + if err := os.MkdirAll(config.ContainerExitsDir, 0755); err != nil { + return nil, err + } containerServer, err := libkpod.New(&config.Config) if err != nil { return nil, err @@ -286,3 +295,43 @@ func (s *Server) CreateMetricsEndpoint() (*http.ServeMux, error) { mux.Handle("/metrics", prometheus.Handler()) return mux, nil } + +// StartExitMonitor start a routine that monitors container exits +// and updates the container status +func (s *Server) StartExitMonitor() { + watcher, err := fsnotify.NewWatcher() + if err != nil { + logrus.Fatalf("Failed to create new watch: %v", err) + } + defer watcher.Close() + + done := make(chan bool) + go func() { + for { + select { + case event := <-watcher.Events: + logrus.Debugf("event: %v", event) + if event.Op&fsnotify.Create == fsnotify.Create { + containerID := filepath.Base(event.Name) + logrus.Debugf("container exited: %v", containerID) + c := s.GetContainer(containerID) + if c != nil { + err := s.Runtime().UpdateStatus(c) + if err != nil { + logrus.Warnf("Failed to update container status %s: %v", c, err) + } else { + s.ContainerStateToDisk(c) + } + } + } + case err := <-watcher.Errors: + logrus.Debugf("watch error: %v", err) + done <- true + } + } + }() + if err := watcher.Add(s.config.ContainerExitsDir); err != nil { + logrus.Fatalf("watcher.Add(%q) failed: %s", s.config.ContainerExitsDir, err) + } + <-done +}