[lxc-devel] [crio-lxc/master] fix created status

[mikemccracken on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 737 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200129/59558f5f/attachment.bin>
-------------- next part --------------
From 09a48901b62ed46ffdc5ce15de343dfb828a7dbc Mon Sep 17 00:00:00 2001
From: Michael McCracken <mikmccra at cisco.com>
Date: Wed, 29 Jan 2020 14:07:40 -0800
Subject: [PATCH 1/2] state: support 'created' status for containers waiting on
 fifo

per the spec, a container that has been created but has not been
started by a separate call, should return status 'created', not
'running', despite its actual underlying container process being up
and running.

Do this by getting the child PID of the init task and reading its
cmdline. if the cmdline is our fifo-waiter, we're 'created'.

Signed-off-by: Michael McCracken <mikmccra at cisco.com>
---
 cmd/state.go     | 44 ++++++++++++++++++++++++++++++++++++++------
 test/manual.bats |  8 ++++++++
 2 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/cmd/state.go b/cmd/state.go
index 6bea455..6494752 100644
--- a/cmd/state.go
+++ b/cmd/state.go
@@ -3,10 +3,13 @@ package main
 import (
 	"encoding/json"
 	"fmt"
+	"io/ioutil"
 	"os"
 	"path/filepath"
+	"strconv"
+	"strings"
 
-	//	"github.com/apex/log"
+	// "github.com/apex/log"
 	"github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
 	"github.com/urfave/cli"
@@ -51,14 +54,43 @@ func doState(ctx *cli.Context) error {
 
 	}
 
-	// TODO need to detect 'created' per
-	// https://github.com/opencontainers/runtime-spec/blob/v1.0.0-rc4/runtime.md#state
-	// it means "the container process has neither exited nor executed the user-specified program"
 	status := "stopped"
 	pid := 0
-	if c.Running() && checkHackyPreStart(c) == "started" {
-		status = "running"
+	if c.Running() {
+		if checkHackyPreStart(c) == "started" {
+			status = "running"
+		}
 		pid = c.InitPid()
+
+		// need to detect 'created' per
+		// https://github.com/opencontainers/runtime-spec/blob/v1.0.0-rc4/runtime.md#state
+		// it means "the container process has neither exited nor executed the user-specified program"
+
+		// if cmd name of the child of the init pid starts with "/bin/sh /fifo-wait" then we can say it's 'created'
+
+		procChildrenFilename := fmt.Sprintf("/proc/%d/task/%d/children", pid, pid)
+		childrenStr, err := ioutil.ReadFile(procChildrenFilename)
+		if err != nil {
+			return errors.Wrapf(err, "failed to read children from %s", procChildrenFilename)
+		}
+		children := strings.Split(strings.TrimSpace(string(childrenStr)), " ")
+
+		if len(children) == 1 {
+			childPid, err := strconv.Atoi(children[0])
+			if err != nil {
+				return errors.Wrapf(err, "failed to convert child pid")
+			}
+			procCmdlineFilename := fmt.Sprintf("/proc/%d/cmdline", childPid)
+			cmdline, err := ioutil.ReadFile(procCmdlineFilename)
+			if err != nil {
+				return errors.Wrapf(err, "failed to read cmdline from %s", procCmdlineFilename)
+			}
+
+			cmdArgv := strings.Split(string(cmdline), "\x00")
+			if len(cmdArgv) > 2 && cmdArgv[0] == "/bin/sh" && cmdArgv[1] == "/fifo-wait" {
+				status = "created"
+			}
+		}
 	}
 	// bundlePath is the enclosing directory of the rootfs:
 	// https://github.com/opencontainers/runtime-spec/blob/v1.0.0-rc4/bundle.md
diff --git a/test/manual.bats b/test/manual.bats
index 265e1fb..c4f9ae0 100644
--- a/test/manual.bats
+++ b/test/manual.bats
@@ -16,7 +16,15 @@ function teardown() {
 
 @test "manual invocation" {
     crio-lxc --debug --log-level trace --log-file "$TEMP_DIR/log" create --bundle "$TEMP_DIR/dest" --pid-file "$TEMP_DIR/pid" alpine
+
+    status=$(crio-lxc --debug --log-level trace --log-file "$TEMP_DIR/log" state alpine | jq -r .status)
+    [ $status = "created" ]
+
     crio-lxc --debug --log-level trace --log-file "$TEMP_DIR/log" start alpine
+
+    status=$(crio-lxc --debug --log-level trace --log-file "$TEMP_DIR/log" state alpine | jq -r .status)
+    [ $status = "running" ]
+
     pid1ipcnsinode=$(stat -L -c%i /proc/1/ns/ipc)
     mypid=$(<"$TEMP_DIR/pid")
     mypidipcnsinode=$(stat -L -c%i "/proc/$mypid/ns/ipc")

From 4d97b140a3f5fd5c033d1a72bbf5c9eeceab24a6 Mon Sep 17 00:00:00 2001
From: Michael McCracken <mikmccra at cisco.com>
Date: Wed, 29 Jan 2020 14:10:00 -0800
Subject: [PATCH 2/2] test helper: support flag file for keeping temp dirs
 while debugging

Signed-off-by: Michael McCracken <mikmccra at cisco.com>
---
 test/helpers.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/helpers.bash b/test/helpers.bash
index a6e2cf7..2bba1b2 100644
--- a/test/helpers.bash
+++ b/test/helpers.bash
@@ -47,7 +47,7 @@ function cleanup_crio {
 }
 
 function cleanup_tempdir {
-    rm -rf "$TEMP_DIR" || true
+    [ -f .keeptempdirs ] || rm -rf "$TEMP_DIR" || true
 }
 
 function crictl {