diff --git a/oci/oci.go b/oci/oci.go index 6a7ceefd..e9ca02ae 100644 --- a/oci/oci.go +++ b/oci/oci.go @@ -423,7 +423,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, @@ -448,7 +448,6 @@ func (r *Runtime) ExecSync(c *Container, command []string, timeout int64) (resp args = append(args, "--socket-dir-path", ContainerAttachSocketDir) pspec := c.Spec().Process - pspec.Env = append(pspec.Env, r.conmonEnv...) pspec.Args = command processJSON, err := json.Marshal(pspec) if err != nil { diff --git a/server/container_create.go b/server/container_create.go index a59bca63..0f581c00 100644 --- a/server/container_create.go +++ b/server/container_create.go @@ -1115,30 +1115,10 @@ 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 == "" { - continue - } - specgen.AddProcessEnv(key, value) - } - } - 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]) - } + envs := mergeEnvs(containerImageConfig, containerConfig.GetEnvs()) + for _, e := range envs { + parts := strings.SplitN(e, "=", 2) + specgen.AddProcessEnv(parts[0], parts[1]) } // Set working directory diff --git a/server/container_exec.go b/server/container_exec.go index 04b82306..4c82c623 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 { diff --git a/server/utils.go b/server/utils.go index 2a15ab42..66c57757 100644 --- a/server/utils.go +++ b/server/utils.go @@ -10,8 +10,10 @@ import ( "github.com/cri-o/ocicni/pkg/ocicni" "github.com/kubernetes-incubator/cri-o/libkpod/sandbox" "github.com/kubernetes-incubator/cri-o/server/metrics" + "github.com/opencontainers/image-spec/specs-go/v1" "github.com/opencontainers/runtime-tools/validate" "github.com/syndtr/gocapability/capability" + pb "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" ) const ( @@ -210,3 +212,44 @@ func validateLabels(labels map[string]string) error { } return nil } + +func mergeEnvs(imageConfig *v1.Image, kubeEnvs []*pb.KeyValue) []string { + envs := []string{} + if kubeEnvs == nil && imageConfig != nil { + envs = imageConfig.Config.Env + } else { + for _, item := range kubeEnvs { + if item.GetKey() == "" { + continue + } + envs = append(envs, item.GetKey()+"="+item.GetValue()) + } + if imageConfig != nil { + for _, imageEnv := range imageConfig.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) + } + } + } + } + return envs +} diff --git a/server/utils_test.go b/server/utils_test.go index b68f0f81..f943c2ea 100644 --- a/server/utils_test.go +++ b/server/utils_test.go @@ -4,6 +4,9 @@ import ( "io/ioutil" "os" "testing" + + "github.com/opencontainers/image-spec/specs-go/v1" + pb "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" ) const ( @@ -108,3 +111,33 @@ func TestSysctlsFromPodAnnotations(t *testing.T) { } } } + +func TestMergeEnvs(t *testing.T) { + configImage := &v1.Image{ + Config: v1.ImageConfig{ + Env: []string{"VAR1=1", "VAR2=2"}, + }, + } + + configKube := []*pb.KeyValue{ + { + Key: "VAR2", + Value: "3", + }, + { + Key: "VAR3", + Value: "3", + }, + } + + mergedEnvs := mergeEnvs(configImage, configKube) + + if len(mergedEnvs) != 3 { + t.Fatalf("Expected 3 env var, VAR1=1, VAR2=3 and VAR3=3, found %d", len(mergedEnvs)) + } + for _, env := range mergedEnvs { + if env != "VAR1=1" && env != "VAR2=3" && env != "VAR3=3" { + t.Fatalf("Expected VAR1=1 or VAR2=3 or VAR3=3, found %s", env) + } + } +} diff --git a/test/ctr.bats b/test/ctr.bats index 0459897b..a88f2a12 100644 --- a/test/ctr.bats +++ b/test/ctr.bats @@ -1032,3 +1032,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" + ] + } + } + } +}