diff --git a/conmon/conmon.c b/conmon/conmon.c index c15f554f..25c682a5 100644 --- a/conmon/conmon.c +++ b/conmon/conmon.c @@ -87,6 +87,7 @@ static char *runtime_path = NULL; static char *bundle_path = NULL; static char *pid_file = NULL; static bool systemd_cgroup = false; +static char *exec_process_spec = NULL; static bool exec = false; static char *log_path = NULL; static GOptionEntry entries[] = @@ -98,6 +99,7 @@ static GOptionEntry entries[] = { "pidfile", 'p', 0, G_OPTION_ARG_STRING, &pid_file, "PID file", NULL }, { "systemd-cgroup", 's', 0, G_OPTION_ARG_NONE, &systemd_cgroup, "Enable systemd cgroup manager", NULL }, { "exec", 'e', 0, G_OPTION_ARG_NONE, &exec, "Exec a command in a running container", NULL }, + { "exec-process-spec", 0, 0, G_OPTION_ARG_STRING, &exec_process_spec, "Path to the process spec for exec", NULL }, { "log-path", 'l', 0, G_OPTION_ARG_STRING, &log_path, "Log file path", NULL }, { NULL } }; @@ -356,6 +358,10 @@ int main(int argc, char *argv[]) bundle_path = cwd; } + if (exec && exec_process_spec == NULL) { + nexit("Exec process spec path not provided. Use --exec-process-spec"); + } + if (pid_file == NULL) { if (snprintf(default_pid_file, sizeof(default_pid_file), "%s/pidfile-%s", cwd, cid) < 0) { @@ -458,23 +464,14 @@ int main(int argc, char *argv[]) if (terminal) g_string_append_printf(cmd, " --console-socket %s", csname); - /* Container name comes last. */ - g_string_append_printf(cmd, " %s", cid); - /* Set the exec arguments. */ if (exec) { - /* - * FIXME: This code is broken if argv[1] contains spaces or other - * similar characters that shells don't like. It's a bit silly - * that we're doing things inside a shell at all -- this should - * all be done in arrays. - */ - - int i; - for (i = 1; i < argc; i++) - g_string_append_printf(cmd, " %s", argv[i]); + g_string_append_printf(cmd, " --process %s", exec_process_spec); } + /* Container name comes last. */ + g_string_append_printf(cmd, " %s", cid); + /* * We have to fork here because the current runC API dups the stdio of the * calling process over the container's fds. This is actually *very bad* diff --git a/oci/oci.go b/oci/oci.go index c099bc3c..21c8f418 100644 --- a/oci/oci.go +++ b/oci/oci.go @@ -15,6 +15,7 @@ import ( "github.com/Sirupsen/logrus" "github.com/kubernetes-incubator/cri-o/utils" + rspec "github.com/opencontainers/runtime-spec/specs-go" "golang.org/x/sys/unix" ) @@ -329,6 +330,15 @@ func (r *Runtime) ExecSync(c *Container, command []string, timeout int64) (resp os.RemoveAll(logPath) }() + f, err := ioutil.TempFile("", "exec-process") + if err != nil { + return nil, ExecSyncError{ + ExitCode: -1, + Err: err, + } + } + defer os.RemoveAll(f.Name()) + var args []string args = append(args, "-c", c.name) args = append(args, "-r", r.Path(c)) @@ -339,7 +349,27 @@ func (r *Runtime) ExecSync(c *Container, command []string, timeout int64) (resp } args = append(args, "-l", logPath) - args = append(args, command...) + pspec := rspec.Process{ + Env: r.conmonEnv, + Args: command, + Cwd: "/", + } + processJSON, err := json.Marshal(pspec) + if err != nil { + return nil, ExecSyncError{ + ExitCode: -1, + Err: err, + } + } + + if err := ioutil.WriteFile(f.Name(), processJSON, 0644); err != nil { + return nil, ExecSyncError{ + ExitCode: -1, + Err: err, + } + } + + args = append(args, "--exec-process-spec", f.Name()) cmd := exec.Command(r.conmonPath, args...) diff --git a/test/ctr.bats b/test/ctr.bats index 30000e8e..675a83cf 100644 --- a/test/ctr.bats +++ b/test/ctr.bats @@ -401,6 +401,28 @@ function teardown() { stop_crio } +@test "ctr execsync conflicting with conmon flags parsing" { + start_crio + run crioctl pod run --config "$TESTDATA"/sandbox_config.json + echo "$output" + [ "$status" -eq 0 ] + pod_id="$output" + run crioctl ctr create --config "$TESTDATA"/container_redis.json --pod "$pod_id" + echo "$output" + [ "$status" -eq 0 ] + ctr_id="$output" + run crioctl ctr start --id "$ctr_id" + echo "$output" + [ "$status" -eq 0 ] + run crioctl ctr execsync --id "$ctr_id" sh -c "echo hello world" + echo "$output" + [ "$status" -eq 0 ] + [[ "$output" =~ "hello world" ]] + cleanup_ctrs + cleanup_pods + stop_crio +} + @test "ctr execsync" { start_crio run crioctl pod run --config "$TESTDATA"/sandbox_config.json @@ -511,19 +533,22 @@ function teardown() { run crioctl ctr start --id "$ctr_id" echo "$output" [ "$status" -eq 0 ] - run crioctl ctr execsync --id "$ctr_id" "echo hello0 stdout" + run crioctl ctr execsync --id "$ctr_id" echo hello0 stdout echo "$output" [ "$status" -eq 0 ] [[ "$output" == *"$(printf "Stdout:\nhello0 stdout")"* ]] - run crioctl ctr execsync --id "$ctr_id" "echo hello1 stderr >&2" + + stderrconfig=$(cat "$TESTDATA"/container_config.json | python -c 'import json,sys;obj=json.load(sys.stdin);obj["image"]["image"] = "runcom/stderr-test"; json.dump(obj, sys.stdout)') + echo "$stderrconfig" > "$TESTDIR"/container_config_stderr.json + run crioctl ctr create --config "$TESTDIR"/container_config_stderr.json --pod "$pod_id" echo "$output" [ "$status" -eq 0 ] - [[ "$output" == *"$(printf "Stderr:\nhello1 stderr")"* ]] - run crioctl ctr execsync --id "$ctr_id" "echo hello2 stderr >&2; echo hello3 stdout" + ctr_id="$output" + run crioctl ctr execsync --id "$ctr_id" stderr echo "$output" [ "$status" -eq 0 ] - [[ "$output" == *"$(printf "Stderr:\nhello2 stderr")"* ]] - [[ "$output" == *"$(printf "Stdout:\nhello3 stdout")"* ]] + [[ "$output" == *"$(printf "Stderr:\nthis goes to stderr")"* ]] + run crioctl pod remove --id "$pod_id" echo "$output" [ "$status" -eq 0 ] diff --git a/test/helpers.bash b/test/helpers.bash index 9b7fef0f..46e32db7 100644 --- a/test/helpers.bash +++ b/test/helpers.bash @@ -77,6 +77,16 @@ if ! [ -d "$ARTIFACTS_PATH"/redis-image ]; then fi fi +# Make sure we have a copy of the runcom/stderr-test image. +if ! [ -d "$ARTIFACTS_PATH"/stderr-test ]; then + mkdir -p "$ARTIFACTS_PATH"/stderr-test + if ! "$COPYIMG_BINARY" --import-from=docker://runcom/stderr-test:latest --export-to=dir:"$ARTIFACTS_PATH"/stderr-test --signature-policy="$INTEGRATION_ROOT"/policy.json ; then + echo "Error pulling docker://stderr-test" + rm -fr "$ARTIFACTS_PATH"/stderr-test + exit 1 + fi +fi + # Make sure we have a copy of the busybox:latest image. if ! [ -d "$ARTIFACTS_PATH"/busybox-image ]; then mkdir -p "$ARTIFACTS_PATH"/busybox-image @@ -159,6 +169,8 @@ function start_crio() { 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 + "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTS --runroot "$TESTDIR/crio-run" --image-name=busybox:latest --import-from=dir:"$ARTIFACTS_PATH"/busybox-image --add-name=docker.io/library/busybox:latest --signature-policy="$INTEGRATION_ROOT"/policy.json + "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTS --runroot "$TESTDIR/crio-run" --image-name=runcom/stderr-test:latest --import-from=dir:"$ARTIFACTS_PATH"/stderr-test --add-name=docker.io/runcom/stderr-test:latest --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 @@ -177,15 +189,10 @@ function start_crio() { crioctl image pull redis:alpine fi REDIS_IMAGEID=$(crioctl image status --id=redis:alpine | head -1 | sed -e "s/ID: //g") - run crioctl image status --id=busybox - 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 - # # # @@ -204,6 +211,15 @@ function start_crio() { # # # + run crioctl image status --id=runcom/stderr-test + if [ "$status" -ne 0 ] ; then + crioctl image pull runcom/stderr-test:latest + fi + STDERR_IMAGEID=$(crioctl image status --id=runcom/stderr-test | head -1 | sed -e "s/ID: //g") + run crioctl image status --id=busybox + if [ "$status" -ne 0 ] ; then + crioctl image pull busybox:latest + fi BUSYBOX_IMAGEID=$(crioctl image status --id=busybox | head -1 | sed -e "s/ID: //g") }