Make kubelet expand var refs in cmd, args, env

2015-05-22 18:21:03 -04:00
parent ee82d469c6
commit 8b338860aa
14 changed files with 513 additions and 84 deletions
--- a/pkg/kubelet/container/helpers.go
+++ b/pkg/kubelet/container/helpers.go
@@ -22,6 +22,8 @@ import (

 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
+	"github.com/GoogleCloudPlatform/kubernetes/third_party/golang/expansion"
+
 	"github.com/golang/glog"
 )

@@ -90,3 +92,32 @@ func HashContainer(container *api.Container) uint64 {
 	util.DeepHashObject(hash, *container)
 	return uint64(hash.Sum32())
 }
+
+// EnvVarsToMap constructs a map of environment name to value from a slice
+// of env vars.
+func EnvVarsToMap(envs []EnvVar) map[string]string {
+	result := map[string]string{}
+	for _, env := range envs {
+		result[env.Name] = env.Value
+	}
+
+	return result
+}
+
+func ExpandContainerCommandAndArgs(container *api.Container, envs []EnvVar) (command []string, args []string) {
+	mapping := expansion.MappingFuncFor(EnvVarsToMap(envs))
+
+	if len(container.Command) != 0 {
+		for _, cmd := range container.Command {
+			command = append(command, expansion.Expand(cmd, mapping))
+		}
+	}
+
+	if len(container.Args) != 0 {
+		for _, arg := range container.Args {
+			args = append(args, expansion.Expand(arg, mapping))
+		}
+	}
+
+	return command, args
+}
--- a/pkg/kubelet/container/helpers_test.go
+++ b/pkg/kubelet/container/helpers_test.go
@@ -0,0 +1,136 @@
+/*
+Copyright 2015 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package container
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+)
+
+func TestEnvVarsToMap(t *testing.T) {
+	vars := []EnvVar{
+		{
+			Name:  "foo",
+			Value: "bar",
+		},
+		{
+			Name:  "zoo",
+			Value: "baz",
+		},
+	}
+
+	varMap := EnvVarsToMap(vars)
+
+	if e, a := len(vars), len(varMap); e != a {
+		t.Error("Unexpected map length; expected: %v, got %v", e, a)
+	}
+
+	if a := varMap["foo"]; a != "bar" {
+		t.Errorf("Unexpected value of key 'foo': %v", a)
+	}
+
+	if a := varMap["zoo"]; a != "baz" {
+		t.Errorf("Unexpected value of key 'zoo': %v", a)
+	}
+}
+
+func TestExpandCommandAndArgs(t *testing.T) {
+	cases := []struct {
+		name            string
+		container       *api.Container
+		envs            []EnvVar
+		expectedCommand []string
+		expectedArgs    []string
+	}{
+		{
+			name:      "none",
+			container: &api.Container{},
+		},
+		{
+			name: "command expanded",
+			container: &api.Container{
+				Command: []string{"foo", "$(VAR_TEST)", "$(VAR_TEST2)"},
+			},
+			envs: []EnvVar{
+				{
+					Name:  "VAR_TEST",
+					Value: "zoo",
+				},
+				{
+					Name:  "VAR_TEST2",
+					Value: "boo",
+				},
+			},
+			expectedCommand: []string{"foo", "zoo", "boo"},
+		},
+		{
+			name: "args expanded",
+			container: &api.Container{
+				Args: []string{"zap", "$(VAR_TEST)", "$(VAR_TEST2)"},
+			},
+			envs: []EnvVar{
+				{
+					Name:  "VAR_TEST",
+					Value: "hap",
+				},
+				{
+					Name:  "VAR_TEST2",
+					Value: "trap",
+				},
+			},
+			expectedArgs: []string{"zap", "hap", "trap"},
+		},
+		{
+			name: "both expanded",
+			container: &api.Container{
+				Command: []string{"$(VAR_TEST2)--$(VAR_TEST)", "foo", "$(VAR_TEST3)"},
+				Args:    []string{"foo", "$(VAR_TEST)", "$(VAR_TEST2)"},
+			},
+			envs: []EnvVar{
+				{
+					Name:  "VAR_TEST",
+					Value: "zoo",
+				},
+				{
+					Name:  "VAR_TEST2",
+					Value: "boo",
+				},
+				{
+					Name:  "VAR_TEST3",
+					Value: "roo",
+				},
+			},
+			expectedCommand: []string{"boo--zoo", "foo", "roo"},
+			expectedArgs:    []string{"foo", "zoo", "boo"},
+		},
+	}
+
+	for _, tc := range cases {
+		actualCommand, actualArgs := ExpandContainerCommandAndArgs(tc.container, tc.envs)
+
+		if e, a := tc.expectedCommand, actualCommand; !reflect.DeepEqual(e, a) {
+			t.Errorf("%v: unexpected command; expected %v, got %v", tc.name, e, a)
+		}
+
+		if e, a := tc.expectedArgs, actualArgs; !reflect.DeepEqual(e, a) {
+			t.Errorf("%v: unexpected args; expected %v, got %v", tc.name, e, a)
+		}
+
+	}
+}
--- a/pkg/kubelet/dockertools/manager.go
+++ b/pkg/kubelet/dockertools/manager.go
@@ -590,7 +590,7 @@ func (dm *DockerManager) runContainer(
 		},
 	}

-	setEntrypointAndCommand(container, &dockerOpts)
+	setEntrypointAndCommand(container, opts, &dockerOpts)

 	glog.V(3).Infof("Container %v/%v/%v: setting entrypoint \"%v\" and command \"%v\"", pod.Namespace, pod.Name, container.Name, dockerOpts.Config.Entrypoint, dockerOpts.Config.Cmd)

@@ -661,13 +661,11 @@ func (dm *DockerManager) runContainer(
 	return dockerContainer.ID, nil
 }

-func setEntrypointAndCommand(container *api.Container, opts *docker.CreateContainerOptions) {
-	if len(container.Command) != 0 {
-		opts.Config.Entrypoint = container.Command
-	}
-	if len(container.Args) != 0 {
-		opts.Config.Cmd = container.Args
-	}
+func setEntrypointAndCommand(container *api.Container, opts *kubecontainer.RunContainerOptions, dockerOpts *docker.CreateContainerOptions) {
+	command, args := kubecontainer.ExpandContainerCommandAndArgs(container, opts.Envs)
+
+	dockerOpts.Config.Entrypoint = command
+	dockerOpts.Config.Cmd = args
 }

 // A helper function to get the KubeletContainerName and hash from a docker
--- a/pkg/kubelet/dockertools/manager_test.go
+++ b/pkg/kubelet/dockertools/manager_test.go
@@ -124,6 +124,7 @@ func TestSetEntrypointAndCommand(t *testing.T) {
 	cases := []struct {
 		name      string
 		container *api.Container
+		envs      []kubecontainer.EnvVar
 		expected  *docker.CreateContainerOptions
 	}{
 		{
@@ -144,6 +145,27 @@ func TestSetEntrypointAndCommand(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "command expanded",
+			container: &api.Container{
+				Command: []string{"foo", "$(VAR_TEST)", "$(VAR_TEST2)"},
+			},
+			envs: []kubecontainer.EnvVar{
+				{
+					Name:  "VAR_TEST",
+					Value: "zoo",
+				},
+				{
+					Name:  "VAR_TEST2",
+					Value: "boo",
+				},
+			},
+			expected: &docker.CreateContainerOptions{
+				Config: &docker.Config{
+					Entrypoint: []string{"foo", "zoo", "boo"},
+				},
+			},
+		},
 		{
 			name: "args",
 			container: &api.Container{
@@ -155,6 +177,27 @@ func TestSetEntrypointAndCommand(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "args expanded",
+			container: &api.Container{
+				Args: []string{"zap", "$(VAR_TEST)", "$(VAR_TEST2)"},
+			},
+			envs: []kubecontainer.EnvVar{
+				{
+					Name:  "VAR_TEST",
+					Value: "hap",
+				},
+				{
+					Name:  "VAR_TEST2",
+					Value: "trap",
+				},
+			},
+			expected: &docker.CreateContainerOptions{
+				Config: &docker.Config{
+					Cmd: []string{"zap", "hap", "trap"},
+				},
+			},
+		},
 		{
 			name: "both",
 			container: &api.Container{
@@ -168,13 +211,44 @@ func TestSetEntrypointAndCommand(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "both expanded",
+			container: &api.Container{
+				Command: []string{"$(VAR_TEST2)--$(VAR_TEST)", "foo", "$(VAR_TEST3)"},
+				Args:    []string{"foo", "$(VAR_TEST)", "$(VAR_TEST2)"},
+			},
+			envs: []kubecontainer.EnvVar{
+				{
+					Name:  "VAR_TEST",
+					Value: "zoo",
+				},
+				{
+					Name:  "VAR_TEST2",
+					Value: "boo",
+				},
+				{
+					Name:  "VAR_TEST3",
+					Value: "roo",
+				},
+			},
+			expected: &docker.CreateContainerOptions{
+				Config: &docker.Config{
+					Entrypoint: []string{"boo--zoo", "foo", "roo"},
+					Cmd:        []string{"foo", "zoo", "boo"},
+				},
+			},
+		},
 	}

 	for _, tc := range cases {
+		opts := &kubecontainer.RunContainerOptions{
+			Envs: tc.envs,
+		}
+
 		actualOpts := &docker.CreateContainerOptions{
 			Config: &docker.Config{},
 		}
-		setEntrypointAndCommand(tc.container, actualOpts)
+		setEntrypointAndCommand(tc.container, opts, actualOpts)

 		if e, a := tc.expected.Config.Entrypoint, actualOpts.Config.Entrypoint; !api.Semantic.DeepEqual(e, a) {
 			t.Errorf("%v: unexpected entrypoint: expected %v, got %v", tc.name, e, a)
--- a/pkg/kubelet/kubelet.go
+++ b/pkg/kubelet/kubelet.go
@@ -58,6 +58,7 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/volume"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/watch"
 	"github.com/GoogleCloudPlatform/kubernetes/plugin/pkg/scheduler/algorithm/predicates"
+	"github.com/GoogleCloudPlatform/kubernetes/third_party/golang/expansion"
 	"github.com/golang/glog"

 	cadvisorApi "github.com/google/cadvisor/info/v1"
@@ -926,20 +927,40 @@ func (kl *Kubelet) makeEnvironmentVariables(pod *api.Pod, container *api.Contain
 		return result, err
 	}

-	for _, value := range container.Env {
+	// Determine the final values of variables:
+	//
+	// 1.  Determine the final value of each variable:
+	//     a.  If the variable's Value is set, expand the `$(var)` references to other
+	//         variables in the .Value field; the sources of variables are the declared
+	//         variables of the container and the service environment variables
+	//     b.  If a source is defined for an environment variable, resolve the source
+	// 2.  Create the container's environment in the order variables are declared
+	// 3.  Add remaining service environment vars
+
+	tmpEnv := make(map[string]string)
+	mappingFunc := expansion.MappingFuncFor(tmpEnv, serviceEnv)
+	for _, envVar := range container.Env {
 		// Accesses apiserver+Pods.
 		// So, the master may set service env vars, or kubelet may.  In case both are doing
 		// it, we delete the key from the kubelet-generated ones so we don't have duplicate
 		// env vars.
 		// TODO: remove this net line once all platforms use apiserver+Pods.
-		delete(serviceEnv, value.Name)
+		delete(serviceEnv, envVar.Name)

-		runtimeValue, err := kl.runtimeEnvVarValue(value, pod)
-		if err != nil {
-			return result, err
+		runtimeVal := envVar.Value
+		if runtimeVal != "" {
+			// Step 1a: expand variable references
+			runtimeVal = expansion.Expand(runtimeVal, mappingFunc)
+		} else if envVar.ValueFrom != nil && envVar.ValueFrom.FieldRef != nil {
+			// Step 1b: resolve alternate env var sources
+			runtimeVal, err = kl.podFieldSelectorRuntimeValue(envVar.ValueFrom.FieldRef, pod)
+			if err != nil {
+				return result, err
+			}
 		}

-		result = append(result, kubecontainer.EnvVar{Name: value.Name, Value: runtimeValue})
+		tmpEnv[envVar.Name] = runtimeVal
+		result = append(result, kubecontainer.EnvVar{Name: envVar.Name, Value: tmpEnv[envVar.Name]})
 	}

 	// Append remaining service env vars.
@@ -949,24 +970,6 @@ func (kl *Kubelet) makeEnvironmentVariables(pod *api.Pod, container *api.Contain
 	return result, nil
 }

-// runtimeEnvVarValue determines the value that an env var should take when a container
-// is started.  If the value of the env var is the empty string, the source of the env var
-// is resolved, if one is specified.
-//
-// TODO: preliminary factoring; make better
-func (kl *Kubelet) runtimeEnvVarValue(envVar api.EnvVar, pod *api.Pod) (string, error) {
-	runtimeVal := envVar.Value
-	if runtimeVal != "" {
-		return runtimeVal, nil
-	}
-
-	if envVar.ValueFrom != nil && envVar.ValueFrom.FieldRef != nil {
-		return kl.podFieldSelectorRuntimeValue(envVar.ValueFrom.FieldRef, pod)
-	}
-
-	return runtimeVal, nil
-}
-
 func (kl *Kubelet) podFieldSelectorRuntimeValue(fs *api.ObjectFieldSelector, pod *api.Pod) (string, error) {
 	internalFieldPath, _, err := api.Scheme.ConvertFieldLabel(fs.APIVersion, "Pod", fs.FieldPath, "")
 	if err != nil {
--- a/pkg/kubelet/kubelet_test.go
+++ b/pkg/kubelet/kubelet_test.go
@@ -1734,6 +1734,137 @@ func TestMakeEnvironmentVariables(t *testing.T) {
 				{Name: "POD_NAMESPACE", Value: "downward-api"},
 			},
 		},
+		{
+			name: "env expansion",
+			ns:   "test1",
+			container: &api.Container{
+				Env: []api.EnvVar{
+					{
+						Name:  "TEST_LITERAL",
+						Value: "test-test-test",
+					},
+					{
+						Name: "POD_NAME",
+						ValueFrom: &api.EnvVarSource{
+							FieldRef: &api.ObjectFieldSelector{
+								APIVersion: "v1beta3",
+								FieldPath:  "metadata.name",
+							},
+						},
+					},
+					{
+						Name:  "OUT_OF_ORDER_TEST",
+						Value: "$(OUT_OF_ORDER_TARGET)",
+					},
+					{
+						Name:  "OUT_OF_ORDER_TARGET",
+						Value: "FOO",
+					},
+					{
+						Name: "EMPTY_VAR",
+					},
+					{
+						Name:  "EMPTY_TEST",
+						Value: "foo-$(EMPTY_VAR)",
+					},
+					{
+						Name:  "POD_NAME_TEST2",
+						Value: "test2-$(POD_NAME)",
+					},
+					{
+						Name:  "POD_NAME_TEST3",
+						Value: "$(POD_NAME_TEST2)-3",
+					},
+					{
+						Name:  "LITERAL_TEST",
+						Value: "literal-$(TEST_LITERAL)",
+					},
+					{
+						Name:  "SERVICE_VAR_TEST",
+						Value: "$(TEST_SERVICE_HOST):$(TEST_SERVICE_PORT)",
+					},
+					{
+						Name:  "TEST_UNDEFINED",
+						Value: "$(UNDEFINED_VAR)",
+					},
+				},
+			},
+			masterServiceNs: "nothing",
+			nilLister:       false,
+			expectedEnvs: []kubecontainer.EnvVar{
+				{
+					Name:  "TEST_LITERAL",
+					Value: "test-test-test",
+				},
+				{
+					Name:  "POD_NAME",
+					Value: "dapi-test-pod-name",
+				},
+				{
+					Name:  "POD_NAME_TEST2",
+					Value: "test2-dapi-test-pod-name",
+				},
+				{
+					Name:  "POD_NAME_TEST3",
+					Value: "test2-dapi-test-pod-name-3",
+				},
+				{
+					Name:  "LITERAL_TEST",
+					Value: "literal-test-test-test",
+				},
+				{
+					Name:  "TEST_SERVICE_HOST",
+					Value: "1.2.3.3",
+				},
+				{
+					Name:  "TEST_SERVICE_PORT",
+					Value: "8083",
+				},
+				{
+					Name:  "TEST_PORT",
+					Value: "tcp://1.2.3.3:8083",
+				},
+				{
+					Name:  "TEST_PORT_8083_TCP",
+					Value: "tcp://1.2.3.3:8083",
+				},
+				{
+					Name:  "TEST_PORT_8083_TCP_PROTO",
+					Value: "tcp",
+				},
+				{
+					Name:  "TEST_PORT_8083_TCP_PORT",
+					Value: "8083",
+				},
+				{
+					Name:  "TEST_PORT_8083_TCP_ADDR",
+					Value: "1.2.3.3",
+				},
+				{
+					Name:  "SERVICE_VAR_TEST",
+					Value: "1.2.3.3:8083",
+				},
+				{
+					Name:  "OUT_OF_ORDER_TEST",
+					Value: "$(OUT_OF_ORDER_TARGET)",
+				},
+				{
+					Name:  "OUT_OF_ORDER_TARGET",
+					Value: "FOO",
+				},
+				{
+					Name:  "TEST_UNDEFINED",
+					Value: "$(UNDEFINED_VAR)",
+				},
+				{
+					Name: "EMPTY_VAR",
+				},
+				{
+					Name:  "EMPTY_TEST",
+					Value: "foo-",
+				},
+			},
+		},
 	}

 	for i, tc := range testCases {