Merge pull request #535 from mrunalp/oom_handling

OOM handling

cmd/crioctl/container.go

@@ -460,6 +460,7 @@ func ContainerStatus(client pb.RuntimeServiceClient, ID string) error {
 	ftm := time.Unix(0, r.Status.FinishedAt)
 	fmt.Printf("Finished: %v\n", ftm)
 	fmt.Printf("Exit Code: %v\n", r.Status.ExitCode)
+	fmt.Printf("Reason: %v\n", r.Status.Reason)
 	if r.Status.Image != nil {
 		fmt.Printf("Image: %v\n", r.Status.Image.Image)
 	}

conmon/conmon.c

@@ -13,6 +13,7 @@
 #include <sys/types.h>
 #include <sys/un.h>
 #include <sys/wait.h>
+#include <sys/eventfd.h>
 #include <syslog.h>
 #include <unistd.h>
 
@@ -59,6 +60,12 @@ static inline void closep(int *fd)
 	*fd = -1;
 }
 
+static inline void fclosep(FILE **fp) {
+	if (*fp)
+		fclose(*fp);
+	*fp = NULL;
+}
+
 static inline void gstring_free_cleanup(GString **string)
 {
 	if (*string)
@@ -67,6 +74,7 @@ static inline void gstring_free_cleanup(GString **string)
 
 #define _cleanup_free_ _cleanup_(freep)
 #define _cleanup_close_ _cleanup_(closep)
+#define _cleanup_fclose_ _cleanup_(fclosep)
 #define _cleanup_gstring_ _cleanup_(gstring_free_cleanup)
 
 #define BUF_SIZE 256
@@ -97,6 +105,8 @@ static GOptionEntry entries[] =
 /* strlen("1997-03-25T13:20:42.999999999+01:00") + 1 */
 #define TSBUFLEN 36
 
+#define CGROUP_ROOT "/sys/fs/cgroup"
+
 int set_k8s_timestamp(char *buf, ssize_t buflen)
 {
 	struct tm *tm;
@@ -226,6 +236,66 @@ next:
 	return 0;
 }
 
+/*
+ * Returns the path for specified controller name for a pid.
+ * Returns NULL on error.
+ */
+static char *process_cgroup_subsystem_path(int pid, const char *subsystem) {
+	_cleanup_free_ char *cgroups_file_path = NULL;
+	int rc;
+	rc = asprintf(&cgroups_file_path, "/proc/%d/cgroup", pid);
+	if (rc < 0) {
+		nwarn("Failed to allocate memory for cgroups file path");
+		return NULL;
+	}
+
+	_cleanup_fclose_ FILE *fp = NULL;
+	fp = fopen(cgroups_file_path, "r");
+	if (fp == NULL) {
+		nwarn("Failed to open cgroups file: %s", cgroups_file_path);
+		return NULL;
+	}
+
+	_cleanup_free_ char *line = NULL;
+	ssize_t read;
+	size_t len = 0;
+	char *ptr;
+	char *subsystem_path = NULL;
+	while ((read = getline(&line, &len, fp)) != -1) {
+		ptr = strchr(line, ':');
+		if (ptr == NULL) {
+			nwarn("Error parsing cgroup, ':' not found: %s", line);
+			return NULL;
+		}
+		ptr++;
+		if (!strncmp(ptr, subsystem, strlen(subsystem))) {
+			char *path = strchr(ptr, '/');
+			if (path == NULL) {
+				nwarn("Error finding path in cgroup: %s", line);
+				return NULL;
+			}
+			ninfo("PATH: %s", path);
+			const char *subpath = strchr(subsystem, '=');
+			if (subpath == NULL) {
+				subpath = subsystem;
+			} else {
+				subpath++;
+			}
+
+			rc = asprintf(&subsystem_path, "%s/%s%s", CGROUP_ROOT, subpath, path);
+			if (rc < 0) {
+				nwarn("Failed to allocate memory for subsystemd path");
+				return NULL;
+			}
+			ninfo("SUBSYSTEM_PATH: %s", subsystem_path);
+			subsystem_path[strlen(subsystem_path) - 1] = '\0';
+			return subsystem_path;
+		}
+	}
+
+	return NULL;
+}
+
 int main(int argc, char *argv[])
 {
 	int ret, runtime_status;
@@ -257,6 +327,14 @@ int main(int argc, char *argv[])
 	GOptionContext *context;
 	_cleanup_gstring_ GString *cmd = NULL;
 
+	/* Used for OOM notification API */
+	_cleanup_close_ int efd = -1;
+	_cleanup_close_ int cfd = -1;
+	_cleanup_close_ int ofd = -1;
+	_cleanup_free_ char *memory_cgroup_path = NULL;
+	int wb;
+	uint64_t oom_event;
+
 	/* Command line parameters */
 	context = g_option_context_new("- conmon utility");
 	g_option_context_add_main_entries(context, entries, "conmon");
@@ -545,6 +623,33 @@ int main(int argc, char *argv[])
 		}
 	}
 
+	/* Setup OOM notification for container process */
+	memory_cgroup_path = process_cgroup_subsystem_path(cpid, "memory");
+	if (!memory_cgroup_path) {
+		nexit("Failed to get memory cgroup path");
+	}
+
+	bool oom_handling_enabled = true;
+	char memory_cgroup_file_path[PATH_MAX];
+	snprintf(memory_cgroup_file_path, PATH_MAX, "%s/cgroup.event_control", memory_cgroup_path);
+	if ((cfd = open(memory_cgroup_file_path, O_WRONLY)) == -1) {
+		nwarn("Failed to open %s", memory_cgroup_file_path);
+		oom_handling_enabled = false;
+	}
+
+	if (oom_handling_enabled) {
+		snprintf(memory_cgroup_file_path, PATH_MAX, "%s/memory.oom_control", memory_cgroup_path);
+		if ((ofd = open(memory_cgroup_file_path, O_RDONLY)) == -1)
+			pexit("Failed to open %s", memory_cgroup_file_path);
+
+		if ((efd = eventfd(0, 0)) == -1)
+			pexit("Failed to create eventfd");
+
+		wb = snprintf(buf, BUF_SIZE, "%d %d", efd, ofd);
+		if (write(cfd, buf, wb) < 0)
+			pexit("Failed to write to cgroup.event_control");
+	}
+
 	/* Create epoll_ctl so that we can handle read/write events. */
 	/*
 	 * TODO: Switch to libuv so that we can also implement exec as well as
@@ -568,6 +673,13 @@ int main(int argc, char *argv[])
 		num_stdio_fds++;
 	}
 
+	/* Add the OOM event fd to epoll */
+	if (oom_handling_enabled) {
+		ev.data.fd = efd;
+		if (epoll_ctl(epfd, EPOLL_CTL_ADD, ev.data.fd, &ev) < 0)
+			pexit("Failed to add OOM eventfd to epoll");
+	}
+
 	/* Log all of the container's output. */
 	while (num_stdio_fds > 0) {
 		int ready = epoll_wait(epfd, evlist, MAX_EVENTS, -1);
@@ -582,18 +694,28 @@ int main(int argc, char *argv[])
 					pipe = STDOUT_PIPE;
 				else if (masterfd == masterfd_stderr)
 					pipe = STDERR_PIPE;
+				else if (oom_handling_enabled && masterfd == efd) {
+					if (read(efd, &oom_event, sizeof(uint64_t)) != sizeof(uint64_t))
+						nwarn("Failed to read event from eventfd");
+					ninfo("OOM received");
+					if (open("oom", O_CREAT, 0666) < 0) {
+						nwarn("Failed to write oom file");
+					}
+				}
 				else {
 					nwarn("unknown pipe fd");
 					goto out;
 				}
 
-				num_read = read(masterfd, buf, BUF_SIZE);
+				if (masterfd == masterfd_stdout || masterfd == masterfd_stderr) {
-				if (num_read <= 0)
+					num_read = read(masterfd, buf, BUF_SIZE);
-					goto out;
+					if (num_read <= 0)
+						goto out;
 
-				if (write_k8s_log(logfd, pipe, buf, num_read) < 0) {
+					if (write_k8s_log(logfd, pipe, buf, num_read) < 0) {
-					nwarn("write_k8s_log failed");
+						nwarn("write_k8s_log failed");
-					goto out;
+						goto out;
+					}
 				}
 			} else if (evlist[i].events & (EPOLLHUP | EPOLLERR)) {
 				printf("closing fd %d\n", evlist[i].data.fd);

oci/container.go

@@ -38,10 +38,11 @@ type Container struct {
 // ContainerState represents the status of a container.
 type ContainerState struct {
 	specs.State
-	Created  time.Time `json:"created"`
+	Created   time.Time `json:"created"`
-	Started  time.Time `json:"started,omitempty"`
+	Started   time.Time `json:"started,omitempty"`
-	Finished time.Time `json:"finished,omitempty"`
+	Finished  time.Time `json:"finished,omitempty"`
-	ExitCode int32     `json:"exitCode,omitempty"`
+	ExitCode  int32     `json:"exitCode,omitempty"`
+	OOMKilled bool      `json:"oomKilled,omitempty"`
 }
 
 // NewContainer creates a container object.

oci/oci.go

@@ -552,6 +552,11 @@ func (r *Runtime) UpdateStatus(c *Container) error {
 			}
 			c.state.ExitCode = int32(statusCode)
 		}
+
+		oomFilePath := filepath.Join(c.bundlePath, "oom")
+		if _, err = os.Stat(oomFilePath); err == nil {
+			c.state.OOMKilled = true
+		}
 	}
 
 	return nil

server/container_status.go

@@ -98,6 +98,9 @@ func (s *Server) ContainerStatus(ctx context.Context, req *pb.ContainerStatusReq
 		finished := cState.Finished.UnixNano()
 		resp.Status.FinishedAt = finished
 		resp.Status.ExitCode = cState.ExitCode
+		if cState.OOMKilled {
+			resp.Status.Reason = "OOMKilled"
+		}
 	}
 
 	resp.Status.State = rStatus

test/ctr.bats

@@ -593,3 +593,38 @@ function teardown() {
 	cleanup_pods
 	stop_crio
 }
+
+@test "ctr oom" {
+	if [[ "$TRAVIS" == "true" ]]; then
+		skip "travis container tests don't support testing OOM"
+	fi
+	start_crio
+	run crioctl pod run --config "$TESTDATA"/sandbox_config.json
+	echo "$output"
+	[ "$status" -eq 0 ]
+	pod_id="$output"
+	oomconfig=$(cat "$TESTDATA"/container_config.json | python -c 'import json,sys;obj=json.load(sys.stdin);obj["image"]["image"] = "mrunalp/oom"; obj["linux"]["resources"]["memory_limit_in_bytes"] = 512000; obj["command"] = ["/oom"]; json.dump(obj, sys.stdout)')
+	echo "$oomconfig" > "$TESTDIR"/container_config_oom.json
+	run crioctl ctr create --config "$TESTDIR"/container_config_oom.json --pod "$pod_id"
+	echo "$output"
+	[ "$status" -eq 0 ]
+	ctr_id="$output"
+	run crioctl ctr start --id "$ctr_id"
+	echo "$output"
+	[ "$status" -eq 0 ]
+        # Wait for container to OOM
+	run sleep 10
+	run crioctl ctr status --id "$ctr_id"
+	echo "$output"
+	[ "$status" -eq 0 ]
+	[[ "$output" =~ "OOMKilled" ]]
+	run crioctl pod stop --id "$pod_id"
+	echo "$output"
+	[ "$status" -eq 0 ]
+	run crioctl pod remove --id "$pod_id"
+	echo "$output"
+	[ "$status" -eq 0 ]
+	cleanup_ctrs
+	cleanup_pods
+	stop_crio
+}

test/helpers.bash

@@ -87,6 +87,16 @@ if ! [ -d "$ARTIFACTS_PATH"/busybox-image ]; then
     fi
 fi
 
+# Make sure we have a copy of the mrunalp/oom:latest image.
+if ! [ -d "$ARTIFACTS_PATH"/oom-image ]; then
+    mkdir -p "$ARTIFACTS_PATH"/oom-image
+    if ! "$COPYIMG_BINARY" --import-from=docker://mrunalp/oom --export-to=dir:"$ARTIFACTS_PATH"/oom-image --signature-policy="$INTEGRATION_ROOT"/policy.json ; then
+        echo "Error pulling docker://mrunalp/oom"
+        rm -fr "$ARTIFACTS_PATH"/oom-image
+        exit 1
+    fi
+fi
+
 # Run crio using the binary specified by $CRIO_BINARY.
 # This must ONLY be run on engines created with `start_crio`.
 function crio() {
@@ -148,6 +158,7 @@ function start_crio() {
 		"$BIN2IMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTS --runroot "$TESTDIR/crio-run" --source-binary "$PAUSE_BINARY"
 	fi
 	"$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTS --runroot "$TESTDIR/crio-run" --image-name=redis:alpine --import-from=dir:"$ARTIFACTS_PATH"/redis-image --add-name=docker.io/library/redis:alpine --signature-policy="$INTEGRATION_ROOT"/policy.json
+	"$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTS --runroot "$TESTDIR/crio-run" --image-name=mrunalp/oom --import-from=dir:"$ARTIFACTS_PATH"/oom-image --add-name=docker.io/library/mrunalp/oom --signature-policy="$INTEGRATION_ROOT"/policy.json
 	"$CRIO_BINARY" --conmon "$CONMON_BINARY" --listen "$CRIO_SOCKET" --cgroup-manager "$CGROUP_MANAGER" --runtime "$RUNTIME_BINARY" --root "$TESTDIR/crio" --runroot "$TESTDIR/crio-run" $STORAGE_OPTS --seccomp-profile "$seccomp" --apparmor-profile "$apparmor" --cni-config-dir "$CRIO_CNI_CONFIG" --signature-policy "$INTEGRATION_ROOT"/policy.json --config /dev/null config >$CRIO_CONFIG
 
 	# Prepare the CNI configuration files, we're running with non host networking by default
@@ -170,6 +181,11 @@ function start_crio() {
 	if [ "$status" -ne 0 ] ; then
 		crioctl image pull busybox:latest
 	fi
+	run crioctl image status --id=mrunalp/oom
+	if [ "$status" -ne 0 ] ; then
+		  crioctl image pull mrunalp/oom
+	fi
+
 	#
 	#
 	#