diff --git a/docs/design/expansion.md b/docs/design/expansion.md index 00c32797b4f..d15f2501dc0 100644 --- a/docs/design/expansion.md +++ b/docs/design/expansion.md @@ -207,9 +207,8 @@ The necessary changes to implement this functionality are: 2. Introduce `third_party/golang/expansion` package that provides: 1. An `Expand(string, func(string) string) string` function 2. A `MappingFuncFor(ObjectEventRecorder, ...map[string]string) string` function -3. Add a new EnvVarSource for expansions and associated tests -4. Make the kubelet expand environment correctly -5. Make the kubelet expand command correctly +3. Make the kubelet expand environment correctly +4. Make the kubelet expand command correctly #### Event Recording @@ -277,31 +276,17 @@ func Expand(input string, mapping func(string) string) string { } ``` -#### Expansion `EnvVarSource` - -In order to avoid changing the existing behavior of the `EnvVar.Value` field, there should be a new -`EnvVarSource` that represents a variable expansion that an env var's value should come from: - -```go -// EnvVarSource represents a source for the value of an EnvVar. -type EnvVarSource struct { - // Other fields omitted - - Expansion *EnvVarExpansion -} - -type EnvVarExpansion struct { - // The input string to be expanded - Expand string -} -``` - #### Kubelet changes -The Kubelet should change to: +The Kubelet should be made to correctly expand variables references in a container's environment, +command, and args. Changes will need to be made to: -1. Correctly expand environment variables with `Expansion` sources -2. Correctly expand references in the Command and Args +1. The `makeEnvironmentVariables` function in the kubelet; this is used by + `GenerateRunContainerOptions`, which is used by both the docker and rkt container runtimes +2. The docker manager `setEntrypointAndCommand` func has to be changed to perform variable + expansion +3. The rkt runtime should be made to support expansion in command and args when support for it is + implemented ### Examples diff --git a/pkg/api/types.go b/pkg/api/types.go index 83ffbc7e970..c1aae88af4d 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -541,7 +541,13 @@ type EnvVar struct { // Required: This must be a C_IDENTIFIER. Name string `json:"name"` // Optional: no more than one of the following may be specified. - // Optional: Defaults to "". + // Optional: Defaults to ""; variable references $(VAR_NAME) are expanded + // using the previous defined environment variables in the container and + // any service environment variables. If a variable cannot be resolved, + // the reference in the input string will be unchanged. The $(VAR_NAME) + // syntax can be escaped with a double $$, ie: $$(VAR_NAME). Escaped + // references will never be expanded, regardless of whether the variable + // exists or not. Value string `json:"value,omitempty"` // Optional: Specifies a source the value of this var should come from. ValueFrom *EnvVarSource `json:"valueFrom,omitempty"` @@ -640,8 +646,16 @@ type Container struct { // Required. Image string `json:"image"` // Optional: The docker image's entrypoint is used if this is not provided; cannot be updated. + // Variable references $(VAR_NAME) are expanded using the container's environment. If a variable + // cannot be resolved, the reference in the input string will be unchanged. The $(VAR_NAME) syntax + // can be escaped with a double $$, ie: $$(VAR_NAME). Escaped references will never be expanded, + // regardless of whether the variable exists or not. Command []string `json:"command,omitempty"` // Optional: The docker image's cmd is used if this is not provided; cannot be updated. + // Variable references $(VAR_NAME) are expanded using the container's environment. If a variable + // cannot be resolved, the reference in the input string will be unchanged. The $(VAR_NAME) syntax + // can be escaped with a double $$, ie: $$(VAR_NAME). Escaped references will never be expanded, + // regardless of whether the variable exists or not. Args []string `json:"args,omitempty"` // Optional: Defaults to Docker's default. WorkingDir string `json:"workingDir,omitempty"` diff --git a/pkg/api/v1/types.go b/pkg/api/v1/types.go index 2a9d38cc93b..987424efe0e 100644 --- a/pkg/api/v1/types.go +++ b/pkg/api/v1/types.go @@ -550,9 +550,15 @@ type VolumeMount struct { type EnvVar struct { // Required: This must be a C_IDENTIFIER. Name string `json:"name" description:"name of the environment variable; must be a C_IDENTIFIER"` - // Optional: No more than one of the following may be set. - // Optional: Defaults to "" - Value string `json:"value,omitempty" description:"value of the environment variable; defaults to empty string"` + // Optional: no more than one of the following may be specified. + // Optional: Defaults to ""; variable references $(VAR_NAME) are expanded + // using the previous defined environment variables in the container and + // any service environment variables. If a variable cannot be resolved, + // the reference in the input string will be unchanged. The $(VAR_NAME) + // syntax can be escaped with a double $$, ie: $$(VAR_NAME). Escaped + // references will never be expanded, regardless of whether the variable + // exists or not. + Value string `json:"value,omitempty" description:"value of the environment variable; defaults to empty string; variable references $(VAR_NAME) are expanded using the previously defined environment varibles in the container and any service environment variables; if a variable cannot be resolved, the reference in the input string will be unchanged; the $(VAR_NAME) syntax can be escaped with a double $$, ie: $$(VAR_NAME) ; escaped references will never be expanded, regardless of whether the variable exists or not"` // Optional: Specifies a source the value of this var should come from. ValueFrom *EnvVarSource `json:"valueFrom,omitempty" description:"source for the environment variable's value; cannot be used if value is not empty"` } @@ -653,9 +659,17 @@ type Container struct { // Required. Image string `json:"image" description:"Docker image name"` // Optional: The docker image's entrypoint is used if this is not provided; cannot be updated. - Command []string `json:"command,omitempty" description:"entrypoint array; not executed within a shell; the docker image's entrypoint is used if this is not provided; cannot be updated"` + // Variable references $(VAR_NAME) are expanded using the container's environment. If a variable + // cannot be resolved, the reference in the input string will be unchanged. The $(VAR_NAME) syntax + // can be escaped with a double $$, ie: $$(VAR_NAME). Escaped references will never be expanded, + // regardless of whether the variable exists or not. + Command []string `json:"command,omitempty" description:"entrypoint array; not executed within a shell; the docker image's entrypoint is used if this is not provided; cannot be updated; variable references $(VAR_NAME) are expanded using the container's environment variables; if a variable cannot be resolved, the reference in the input string will be unchanged; the $(VAR_NAME) syntax can be escaped with a double $$, ie: $$(VAR_NAME) ; escaped references will never be expanded, regardless of whether the variable exists or not"` // Optional: The docker image's cmd is used if this is not provided; cannot be updated. - Args []string `json:"args,omitempty" description:"command array; the docker image's cmd is used if this is not provided; arguments to the entrypoint; cannot be updated"` + // Variable references $(VAR_NAME) are expanded using the container's environment. If a variable + // cannot be resolved, the reference in the input string will be unchanged. The $(VAR_NAME) syntax + // can be escaped with a double $$, ie: $$(VAR_NAME). Escaped references will never be expanded, + // regardless of whether the variable exists or not. + Args []string `json:"args,omitempty" description:"command array; the docker image's cmd is used if this is not provided; arguments to the entrypoint; cannot be updated; variable references $(VAR_NAME) are expanded using the container's environment variables; if a variable cannot be resolved, the reference in the input string will be unchanged; the $(VAR_NAME) syntax can be escaped with a double $$, ie: $$(VAR_NAME) ; escaped references will never be expanded, regardless of whether the variable exists or not"` // Optional: Defaults to Docker's default. WorkingDir string `json:"workingDir,omitempty" description:"container's working directory; defaults to image's default; cannot be updated"` Ports []ContainerPort `json:"ports,omitempty" description:"list of ports to expose from the container; cannot be updated" patchStrategy:"merge" patchMergeKey:"containerPort"` diff --git a/pkg/api/v1beta1/types.go b/pkg/api/v1beta1/types.go index 28efed20e50..fbad36cbf05 100644 --- a/pkg/api/v1beta1/types.go +++ b/pkg/api/v1beta1/types.go @@ -439,9 +439,15 @@ type EnvVar struct { // DEPRECATED: EnvVar.Key will be removed in a future version of the API. Name string `json:"name" description:"name of the environment variable; must be a C_IDENTIFIER"` Key string `json:"key,omitempty" description:"name of the environment variable; must be a C_IDENTIFIER; deprecated - use name instead"` - // Optional: No more than one of the following may be set. - // Optional: Defaults to "" - Value string `json:"value,omitempty" description:"value of the environment variable; defaults to empty string"` + // Optional: no more than one of the following may be specified. + // Optional: Defaults to ""; variable references $(VAR_NAME) are expanded + // using the previous defined environment variables in the container and + // any service environment variables. If a variable cannot be resolved, + // the reference in the input string will be unchanged. The $(VAR_NAME) + // syntax can be escaped with a double $$, ie: $$(VAR_NAME). Escaped + // references will never be expanded, regardless of whether the variable + // exists or not. + Value string `json:"value,omitempty" description:"value of the environment variable; defaults to empty string; variable references $(VAR_NAME) are expanded using the previously defined environment varibles in the container and any service environment variables; if a variable cannot be resolved, the reference in the input string will be unchanged; the $(VAR_NAME) syntax can be escaped with a double $$, ie: $$(VAR_NAME) ; escaped references will never be expanded, regardless of whether the variable exists or not"` // Optional: Specifies a source the value of this var should come from. ValueFrom *EnvVarSource `json:"valueFrom,omitempty" description:"source for the environment variable's value; cannot be used if value is not empty"` } @@ -542,9 +548,17 @@ type Container struct { // Required. Image string `json:"image" description:"Docker image name"` // Optional: The image's entrypoint is used if this is not provided; cannot be updated. - Entrypoint []string `json:"entrypoint,omitempty" description:"entrypoint array; not executed within a shell; the image's entrypoint is used if this is not provided; cannot be updated"` + // Variable references $(VAR_NAME) are expanded using the container's environment. If a variable + // cannot be resolved, the reference in the input string will be unchanged. The $(VAR_NAME) syntax + // can be escaped with a double $$, ie: $$(VAR_NAME). Escaped references will never be expanded, + // regardless of whether the variable exists or not. + Entrypoint []string `json:"entrypoint,omitempty" description:"entrypoint array; not executed within a shell; the image's entrypoint is used if this is not provided; cannot be updated; variable references $(VAR_NAME) are expanded using the container's environment variables; if a variable cannot be resolved, the reference in the input string will be unchanged; the $(VAR_NAME) syntax can be escaped with a double $$, ie: $$(VAR_NAME) ; escaped references will never be expanded, regardless of whether the variable exists or not"` // Optional: The image's cmd is used if this is not provided; cannot be updated. - Command []string `json:"command,omitempty" description:"command argv array; not executed within a shell; the image's cmd is used if this is not provided; cannot be updated"` + // Variable references $(VAR_NAME) are expanded using the container's environment. If a variable + // cannot be resolved, the reference in the input string will be unchanged. The $(VAR_NAME) syntax + // can be escaped with a double $$, ie: $$(VAR_NAME). Escaped references will never be expanded, + // regardless of whether the variable exists or not. + Command []string `json:"command,omitempty" description:"command argv array; not executed within a shell; the image's cmd is used if this is not provided; cannot be updated; variable references $(VAR_NAME) are expanded using the container's environment variables; if a variable cannot be resolved, the reference in the input string will be unchanged; the $(VAR_NAME) syntax can be escaped with a double $$, ie: $$(VAR_NAME) ; escaped references will never be expanded, regardless of whether the variable exists or not"` // Optional: Docker's default is used if this is not provided. WorkingDir string `json:"workingDir,omitempty" description:"container's working directory; defaults to image's default; cannot be updated"` Ports []ContainerPort `json:"ports,omitempty" description:"list of ports to expose from the container; cannot be updated"` diff --git a/pkg/api/v1beta2/types.go b/pkg/api/v1beta2/types.go index 4bfc4ec46c3..1715823925d 100644 --- a/pkg/api/v1beta2/types.go +++ b/pkg/api/v1beta2/types.go @@ -413,9 +413,15 @@ type VolumeMount struct { type EnvVar struct { // Required: This must be a C_IDENTIFIER. Name string `json:"name" description:"name of the environment variable; must be a C_IDENTIFIER"` - // Optional: No more than one of the following may be set. - // Optional: Defaults to "" - Value string `json:"value,omitempty" description:"value of the environment variable; defaults to empty string"` + // Optional: no more than one of the following may be specified. + // Optional: Defaults to ""; variable references $(VAR_NAME) are expanded + // using the previous defined environment variables in the container and + // any service environment variables. If a variable cannot be resolved, + // the reference in the input string will be unchanged. The $(VAR_NAME) + // syntax can be escaped with a double $$, ie: $$(VAR_NAME). Escaped + // references will never be expanded, regardless of whether the variable + // exists or not. + Value string `json:"value,omitempty" description:"value of the environment variable; defaults to empty string; variable references $(VAR_NAME) are expanded using the previously defined environment varibles in the container and any service environment variables; if a variable cannot be resolved, the reference in the input string will be unchanged; the $(VAR_NAME) syntax can be escaped with a double $$, ie: $$(VAR_NAME) ; escaped references will never be expanded, regardless of whether the variable exists or not"` // Optional: Specifies a source the value of this var should come from. ValueFrom *EnvVarSource `json:"valueFrom,omitempty" description:"source for the environment variable's value; cannot be used if value is not empty"` } @@ -527,9 +533,17 @@ type Container struct { // Required. Image string `json:"image" description:"Docker image name"` // Optional: The image's entrypoint is used if this is not provided; cannot be updated. - Entrypoint []string `json:"entrypoint,omitempty" description:"entrypoint array; not executed within a shell; the image's entrypoint is used if this is not provided; cannot be updated"` + // Variable references $(VAR_NAME) are expanded using the container's environment. If a variable + // cannot be resolved, the reference in the input string will be unchanged. The $(VAR_NAME) syntax + // can be escaped with a double $$, ie: $$(VAR_NAME). Escaped references will never be expanded, + // regardless of whether the variable exists or not. + Entrypoint []string `json:"entrypoint,omitempty" description:"entrypoint array; not executed within a shell; the image's entrypoint is used if this is not provided; cannot be updated; variable references $(VAR_NAME) are expanded using the container's environment variables; if a variable cannot be resolved, the reference in the input string will be unchanged; the $(VAR_NAME) syntax can be escaped with a double $$, ie: $$(VAR_NAME) ; escaped references will never be expanded, regardless of whether the variable exists or not"` // Optional: The image's cmd is used if this is not provided; cannot be updated. - Command []string `json:"command,omitempty" description:"command argv array; not executed within a shell; the image's cmd is used if this is not provided; cannot be updated"` + // Variable references $(VAR_NAME) are expanded using the container's environment. If a variable + // cannot be resolved, the reference in the input string will be unchanged. The $(VAR_NAME) syntax + // can be escaped with a double $$, ie: $$(VAR_NAME). Escaped references will never be expanded, + // regardless of whether the variable exists or not. + Command []string `json:"command,omitempty" description:"command argv array; not executed within a shell; the image's cmd is used if this is not provided; cannot be updated; variable references $(VAR_NAME) are expanded using the container's environment variables; if a variable cannot be resolved, the reference in the input string will be unchanged; the $(VAR_NAME) syntax can be escaped with a double $$, ie: $$(VAR_NAME) ; escaped references will never be expanded, regardless of whether the variable exists or not"` // Optional: Docker's default is used if this is not provided. WorkingDir string `json:"workingDir,omitempty" description:"container's working directory; defaults to image's default; cannot be updated"` Ports []ContainerPort `json:"ports,omitempty" description:"list of ports to expose from the container; cannot be updated"` diff --git a/pkg/api/v1beta3/types.go b/pkg/api/v1beta3/types.go index 1f28ef0893d..f4e467557b6 100644 --- a/pkg/api/v1beta3/types.go +++ b/pkg/api/v1beta3/types.go @@ -550,9 +550,15 @@ type VolumeMount struct { type EnvVar struct { // Required: This must be a C_IDENTIFIER. Name string `json:"name" description:"name of the environment variable; must be a C_IDENTIFIER"` - // Optional: No more than one of the following may be set. - // Optional: Defaults to "" - Value string `json:"value,omitempty" description:"value of the environment variable; defaults to empty string"` + // Optional: no more than one of the following may be specified. + // Optional: Defaults to ""; variable references $(VAR_NAME) are expanded + // using the previous defined environment variables in the container and + // any service environment variables. If a variable cannot be resolved, + // the reference in the input string will be unchanged. The $(VAR_NAME) + // syntax can be escaped with a double $$, ie: $$(VAR_NAME). Escaped + // references will never be expanded, regardless of whether the variable + // exists or not. + Value string `json:"value,omitempty" description:"value of the environment variable; defaults to empty string; variable references $(VAR_NAME) are expanded using the previously defined environment varibles in the container and any service environment variables; if a variable cannot be resolved, the reference in the input string will be unchanged; the $(VAR_NAME) syntax can be escaped with a double $$, ie: $$(VAR_NAME) ; escaped references will never be expanded, regardless of whether the variable exists or not"` // Optional: Specifies a source the value of this var should come from. ValueFrom *EnvVarSource `json:"valueFrom,omitempty" description:"source for the environment variable's value; cannot be used if value is not empty"` } @@ -653,9 +659,17 @@ type Container struct { // Required. Image string `json:"image" description:"Docker image name"` // Optional: The docker image's entrypoint is used if this is not provided; cannot be updated. - Command []string `json:"command,omitempty" description:"entrypoint array; not executed within a shell; the docker image's entrypoint is used if this is not provided; cannot be updated"` + // Variable references $(VAR_NAME) are expanded using the container's environment. If a variable + // cannot be resolved, the reference in the input string will be unchanged. The $(VAR_NAME) syntax + // can be escaped with a double $$, ie: $$(VAR_NAME). Escaped references will never be expanded, + // regardless of whether the variable exists or not. + Command []string `json:"command,omitempty" description:"entrypoint array; not executed within a shell; the docker image's entrypoint is used if this is not provided; cannot be updated; variable references $(VAR_NAME) are expanded using the container's environment variables; if a variable cannot be resolved, the reference in the input string will be unchanged; the $(VAR_NAME) syntax can be escaped with a double $$, ie: $$(VAR_NAME) ; escaped references will never be expanded, regardless of whether the variable exists or not"` // Optional: The docker image's cmd is used if this is not provided; cannot be updated. - Args []string `json:"args,omitempty" description:"command array; the docker image's cmd is used if this is not provided; arguments to the entrypoint; cannot be updated"` + // Variable references $(VAR_NAME) are expanded using the container's environment. If a variable + // cannot be resolved, the reference in the input string will be unchanged. The $(VAR_NAME) syntax + // can be escaped with a double $$, ie: $$(VAR_NAME). Escaped references will never be expanded, + // regardless of whether the variable exists or not. + Args []string `json:"args,omitempty" description:"command array; the docker image's cmd is used if this is not provided; arguments to the entrypoint; cannot be updated; variable references $(VAR_NAME) are expanded using the container's environment variables; if a variable cannot be resolved, the reference in the input string will be unchanged; the $(VAR_NAME) syntax can be escaped with a double $$, ie: $$(VAR_NAME) ; escaped references will never be expanded, regardless of whether the variable exists or not"` // Optional: Defaults to Docker's default. WorkingDir string `json:"workingDir,omitempty" description:"container's working directory; defaults to image's default; cannot be updated"` Ports []ContainerPort `json:"ports,omitempty" description:"list of ports to expose from the container; cannot be updated" patchStrategy:"merge" patchMergeKey:"containerPort"` diff --git a/pkg/kubelet/container/helpers.go b/pkg/kubelet/container/helpers.go index dbce6d08ce4..d6272e039f8 100644 --- 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 +} diff --git a/pkg/kubelet/container/helpers_test.go b/pkg/kubelet/container/helpers_test.go new file mode 100644 index 00000000000..19f34a7c005 --- /dev/null +++ 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) + } + + } +} diff --git a/pkg/kubelet/dockertools/manager.go b/pkg/kubelet/dockertools/manager.go index 7baae160f65..80f8450ea99 100644 --- 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 diff --git a/pkg/kubelet/dockertools/manager_test.go b/pkg/kubelet/dockertools/manager_test.go index 6dc661cd629..311982e84b2 100644 --- 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) diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index e46e4316a9b..db752463a5e 100644 --- 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 { diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index d36662babec..c915bb881c8 100644 --- 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 { diff --git a/test/e2e/dns.go b/test/e2e/dns.go index 13946002919..2f133e9337c 100644 --- a/test/e2e/dns.go +++ b/test/e2e/dns.go @@ -74,8 +74,8 @@ var _ = Describe("DNS", func() { probeCmd := "for i in `seq 1 600`; do " for _, name := range namesToResolve { // Resolve by TCP and UDP DNS. - probeCmd += fmt.Sprintf(`test -n "$(dig +notcp +noall +answer +search %s)" && echo OK > /results/udp@%s;`, name, name) - probeCmd += fmt.Sprintf(`test -n "$(dig +tcp +noall +answer +search %s)" && echo OK > /results/tcp@%s;`, name, name) + probeCmd += fmt.Sprintf(`test -n "$$(dig +notcp +noall +answer +search %s)" && echo OK > /results/udp@%s;`, name, name) + probeCmd += fmt.Sprintf(`test -n "$$(dig +tcp +noall +answer +search %s)" && echo OK > /results/tcp@%s;`, name, name) } probeCmd += "sleep 1; done" diff --git a/test/e2e/es_cluster_logging.go b/test/e2e/es_cluster_logging.go index b574b7aaead..c505dce648c 100644 --- a/test/e2e/es_cluster_logging.go +++ b/test/e2e/es_cluster_logging.go @@ -197,9 +197,10 @@ func ClusterLevelLoggingWithElasticsearch(c *client.Client) { Spec: api.PodSpec{ Containers: []api.Container{ { - Name: "synth-logger", - Image: "gcr.io/google_containers/ubuntu:14.04", - Command: []string{"bash", "-c", fmt.Sprintf("i=0; while ((i < %d)); do echo \"%d %s $i %s\"; i=$(($i+1)); done", countTo, i, taintName, podName)}, + Name: "synth-logger", + Image: "gcr.io/google_containers/ubuntu:14.04", + // notice: the subshell syntax is escaped with `$$` + Command: []string{"bash", "-c", fmt.Sprintf("i=0; while ((i < %d)); do echo \"%d %s $i %s\"; i=$$(($i+1)); done", countTo, i, taintName, podName)}, }, }, Host: node.Name,