Merge pull request #1187 from runcom/fixups-env-1.7

[release-1.0] Fix env handling on exec

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 {

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

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 {

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
+}

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"
+				]
+			}
+		}
+	}
+}