Merge pull request #1186 from runcom/fixups-env-master
Fix env handling on exec
This commit is contained in:
commit
40da5c2c16
7 changed files with 190 additions and 27 deletions
|
@ -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 {
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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 {
|
||||
|
|
|
@ -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
|
||||
}
|
||||
|
|
|
@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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
|
||||
}
|
||||
|
|
62
test/testdata/container_redis_env_custom.json
vendored
Normal file
62
test/testdata/container_redis_env_custom.json
vendored
Normal file
|
@ -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"
|
||||
]
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
Loading…
Reference in a new issue