From 3a36f553a4409f8c050714a2f803dc3870426562 Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Thu, 30 Nov 2017 10:43:35 +0100 Subject: [PATCH 1/4] container_exec: use process file with runc exec Signed-off-by: Antonio Murdaca --- server/container_exec.go | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/server/container_exec.go b/server/container_exec.go index 01d6e7c4..857e6e27 100644 --- a/server/container_exec.go +++ b/server/container_exec.go @@ -1,8 +1,10 @@ package server import ( + "encoding/json" "fmt" "io" + "io/ioutil" "os" "os/exec" "time" @@ -53,12 +55,29 @@ func (ss streamService) Exec(containerID string, cmd []string, stdin io.Reader, return fmt.Errorf("container is not created or running") } + f, err := ioutil.TempFile("", "exec-process") + if err != nil { + return err + } + defer os.RemoveAll(f.Name()) + + pspec := c.Spec().Process + pspec.Args = cmd + processJSON, err := json.Marshal(pspec) + if err != nil { + return err + } + + if err := ioutil.WriteFile(f.Name(), processJSON, 0644); err != nil { + return err + } + args := []string{"exec"} if tty { args = append(args, "-t") } + args = append(args, "-p", f.Name()) args = append(args, c.ID()) - args = append(args, cmd...) execCmd := exec.Command(ss.runtimeServer.Runtime().Path(c), args...) var cmdErr error if tty { From e4470612a2569f79089e2a91719a6fa1e3bfe684 Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Thu, 30 Nov 2017 10:44:20 +0100 Subject: [PATCH 2/4] oci: do not append conmon env to container process Signed-off-by: Antonio Murdaca --- oci/oci.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/oci/oci.go b/oci/oci.go index fba80c6a..114c0935 100644 --- a/oci/oci.go +++ b/oci/oci.go @@ -412,7 +412,7 @@ func (r *Runtime) ExecSync(c *Container, command []string, timeout int64) (resp os.RemoveAll(logPath) }() - f, err := ioutil.TempFile("", "exec-process") + f, err := ioutil.TempFile("", "exec-sync-process") if err != nil { return nil, ExecSyncError{ ExitCode: -1, @@ -436,7 +436,6 @@ func (r *Runtime) ExecSync(c *Container, command []string, timeout int64) (resp args = append(args, "-l", logPath) pspec := c.Spec().Process - pspec.Env = append(pspec.Env, r.conmonEnv...) pspec.Args = command processJSON, err := json.Marshal(pspec) if err != nil { From cc0f78dfc461d959e800f8a8edb841f4948a3969 Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Thu, 30 Nov 2017 11:52:30 +0100 Subject: [PATCH 3/4] container_create: correctly set image and kube envs Signed-off-by: Antonio Murdaca --- server/container_create.go | 56 ++++++++++++++++++++++++-------------- 1 file changed, 36 insertions(+), 20 deletions(-) diff --git a/server/container_create.go b/server/container_create.go index ddbabf6f..f10d5c21 100644 --- a/server/container_create.go +++ b/server/container_create.go @@ -1006,30 +1006,46 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string, } specgen.SetProcessArgs(processArgs) - // Add environment variables from CRI and image config - envs := containerConfig.GetEnvs() - if envs != nil { - for _, item := range envs { - key := item.Key - value := item.Value - if key == "" { + envs := []string{} + if containerConfig.GetEnvs() == nil && containerImageConfig != nil { + envs = containerImageConfig.Config.Env + } else { + for _, item := range containerConfig.GetEnvs() { + if item.GetKey() == "" { continue } - specgen.AddProcessEnv(key, value) + envs = append(envs, item.GetKey()+"="+item.GetValue()) + } + if containerImageConfig != nil { + for _, imageEnv := range containerImageConfig.Config.Env { + var found bool + parts := strings.SplitN(imageEnv, "=", 2) + if len(parts) != 2 { + continue + } + imageEnvKey := parts[0] + if imageEnvKey == "" { + continue + } + for _, kubeEnv := range envs { + kubeEnvKey := strings.SplitN(kubeEnv, "=", 2)[0] + if kubeEnvKey == "" { + continue + } + if imageEnvKey == kubeEnvKey { + found = true + break + } + } + if !found { + envs = append(envs, imageEnv) + } + } } } - if containerImageConfig != nil { - for _, item := range containerImageConfig.Config.Env { - parts := strings.SplitN(item, "=", 2) - if len(parts) != 2 { - return nil, fmt.Errorf("invalid env from image: %s", item) - } - - if parts[0] == "" { - continue - } - specgen.AddProcessEnv(parts[0], parts[1]) - } + for _, e := range envs { + parts := strings.SplitN(e, "=", 2) + specgen.AddProcessEnv(parts[0], parts[1]) } // Set working directory From 762cb4cca580e8cd2b9c40a0b03b3f9c73f48924 Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Thu, 30 Nov 2017 11:24:03 +0100 Subject: [PATCH 4/4] test: add exec/execsync env conflict test Signed-off-by: Antonio Murdaca --- test/ctr.bats | 27 ++++++++ test/testdata/container_redis_env_custom.json | 62 +++++++++++++++++++ 2 files changed, 89 insertions(+) create mode 100644 test/testdata/container_redis_env_custom.json diff --git a/test/ctr.bats b/test/ctr.bats index fecd5860..a577a9c6 100644 --- a/test/ctr.bats +++ b/test/ctr.bats @@ -925,3 +925,30 @@ function teardown() { cleanup_pods stop_crio } + +@test "ctr execsync conflicting with conmon env" { + start_crio + run crictl runs "$TESTDATA"/sandbox_config.json + echo "$output" + [ "$status" -eq 0 ] + pod_id="$output" + run crictl create "$pod_id" "$TESTDATA"/container_redis_env_custom.json "$TESTDATA"/sandbox_config.json + echo "$output" + [ "$status" -eq 0 ] + ctr_id="$output" + run crictl start "$ctr_id" + echo "$output" + [ "$status" -eq 0 ] + run crictl exec "$ctr_id" env + echo "$output" + echo "$status" + [ "$status" -eq 0 ] + [[ "$output" =~ "acustompathinpath" ]] + run crictl exec --sync "$ctr_id" env + echo "$output" + [ "$status" -eq 0 ] + [[ "$output" =~ "acustompathinpath" ]] + cleanup_ctrs + cleanup_pods + stop_crio +} diff --git a/test/testdata/container_redis_env_custom.json b/test/testdata/container_redis_env_custom.json new file mode 100644 index 00000000..3ec41001 --- /dev/null +++ b/test/testdata/container_redis_env_custom.json @@ -0,0 +1,62 @@ +{ + "metadata": { + "name": "podsandbox1-redis" + }, + "image": { + "image": "redis:alpine" + }, + "args": [ + "docker-entrypoint.sh", + "redis-server" + ], + "working_dir": "/data", + "envs": [ + { + "key": "PATH", + "value": "/acustompathinpath:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" + }, + { + "key": "TERM", + "value": "xterm" + }, + { + "key": "REDIS_VERSION", + "value": "3.2.3" + }, + { + "key": "REDIS_DOWNLOAD_URL", + "value": "http://download.redis.io/releases/redis-3.2.3.tar.gz" + }, + { + "key": "REDIS_DOWNLOAD_SHA1", + "value": "92d6d93ef2efc91e595c8bf578bf72baff397507" + } + ], + "labels": { + "tier": "backend" + }, + "annotations": { + "pod": "podsandbox1" + }, + "readonly_rootfs": false, + "log_path": "", + "stdin": false, + "stdin_once": false, + "tty": false, + "linux": { + "resources": { + "memory_limit_in_bytes": 209715200, + "cpu_period": 10000, + "cpu_quota": 20000, + "cpu_shares": 512, + "oom_score_adj": 30 + }, + "security_context": { + "capabilities": { + "add_capabilities": [ + "sys_admin" + ] + } + } + } +}