From ce62f3d4a06680727324e6496df442f285ba2d92 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Tue, 28 Feb 2017 13:47:25 -0500 Subject: [PATCH] ExecProbes should be able to do simple env var substitution For containers that don't have bash, we should support env substitution like we do on command and args. However, without major refactoring valueFrom is not supportable from inside the prober. For now, implement substitution based on hardcoded env and leave TODOs for future work. --- pkg/kubelet/container/helpers.go | 23 ++++++++++++++ pkg/kubelet/prober/BUILD | 1 + pkg/kubelet/prober/common_test.go | 2 +- pkg/kubelet/prober/prober.go | 3 +- pkg/kubelet/prober/prober_test.go | 53 ++++++++++++++++++++++++++++--- pkg/kubelet/prober/worker.go | 3 ++ 6 files changed, 78 insertions(+), 7 deletions(-) diff --git a/pkg/kubelet/container/helpers.go b/pkg/kubelet/container/helpers.go index 12a742a961a..b224354d9e5 100644 --- a/pkg/kubelet/container/helpers.go +++ b/pkg/kubelet/container/helpers.go @@ -119,10 +119,33 @@ func EnvVarsToMap(envs []EnvVar) map[string]string { for _, env := range envs { result[env.Name] = env.Value } + return result +} + +// V1EnvVarsToMap constructs a map of environment name to value from a slice +// of env vars. +func V1EnvVarsToMap(envs []v1.EnvVar) map[string]string { + result := map[string]string{} + for _, env := range envs { + result[env.Name] = env.Value + } return result } +// ExpandContainerCommandOnlyStatic substitutes only static environment variable values from the +// container environment definitions. This does *not* include valueFrom substitutions. +// TODO: callers should use ExpandContainerCommandAndArgs with a fully resolved list of environment. +func ExpandContainerCommandOnlyStatic(containerCommand []string, envs []v1.EnvVar) (command []string) { + mapping := expansion.MappingFuncFor(V1EnvVarsToMap(envs)) + if len(containerCommand) != 0 { + for _, cmd := range containerCommand { + command = append(command, expansion.Expand(cmd, mapping)) + } + } + return command +} + func ExpandContainerCommandAndArgs(container *v1.Container, envs []EnvVar) (command []string, args []string) { mapping := expansion.MappingFuncFor(EnvVarsToMap(envs)) diff --git a/pkg/kubelet/prober/BUILD b/pkg/kubelet/prober/BUILD index 7b0e9369081..961b011e1bd 100644 --- a/pkg/kubelet/prober/BUILD +++ b/pkg/kubelet/prober/BUILD @@ -58,6 +58,7 @@ go_test( "//pkg/kubelet/status:go_default_library", "//pkg/kubelet/status/testing:go_default_library", "//pkg/probe:go_default_library", + "//pkg/probe/exec:go_default_library", "//pkg/util/exec:go_default_library", "//vendor:github.com/golang/glog", "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1", diff --git a/pkg/kubelet/prober/common_test.go b/pkg/kubelet/prober/common_test.go index 6044cfe084c..409dbdf4236 100644 --- a/pkg/kubelet/prober/common_test.go +++ b/pkg/kubelet/prober/common_test.go @@ -125,7 +125,7 @@ type fakeExecProber struct { err error } -func (p fakeExecProber) Probe(_ exec.Cmd) (probe.Result, string, error) { +func (p fakeExecProber) Probe(c exec.Cmd) (probe.Result, string, error) { return p.result, "", p.err } diff --git a/pkg/kubelet/prober/prober.go b/pkg/kubelet/prober/prober.go index 323e6c88d5d..47f85b9824c 100644 --- a/pkg/kubelet/prober/prober.go +++ b/pkg/kubelet/prober/prober.go @@ -143,7 +143,8 @@ func (pb *prober) runProbe(p *v1.Probe, pod *v1.Pod, status v1.PodStatus, contai timeout := time.Duration(p.TimeoutSeconds) * time.Second if p.Exec != nil { glog.V(4).Infof("Exec-Probe Pod: %v, Container: %v, Command: %v", pod, container, p.Exec.Command) - return pb.exec.Probe(pb.newExecInContainer(container, containerID, p.Exec.Command, timeout)) + command := kubecontainer.ExpandContainerCommandOnlyStatic(p.Exec.Command, container.Env) + return pb.exec.Probe(pb.newExecInContainer(container, containerID, command, timeout)) } if p.HTTPGet != nil { scheme := strings.ToLower(string(p.HTTPGet.Scheme)) diff --git a/pkg/kubelet/prober/prober_test.go b/pkg/kubelet/prober/prober_test.go index 5f4393ded77..a8c784cc6bd 100644 --- a/pkg/kubelet/prober/prober_test.go +++ b/pkg/kubelet/prober/prober_test.go @@ -30,6 +30,7 @@ import ( containertest "k8s.io/kubernetes/pkg/kubelet/container/testing" "k8s.io/kubernetes/pkg/kubelet/prober/results" "k8s.io/kubernetes/pkg/probe" + execprobe "k8s.io/kubernetes/pkg/probe/exec" ) func TestFormatURL(t *testing.T) { @@ -197,10 +198,6 @@ func TestHTTPHeaders(t *testing.T) { } func TestProbe(t *testing.T) { - prober := &prober{ - refManager: kubecontainer.NewRefManager(), - recorder: &record.FakeRecorder{}, - } containerID := kubecontainer.ContainerID{Type: "test", ID: "foobar"} execProbe := &v1.Probe{ @@ -210,10 +207,12 @@ func TestProbe(t *testing.T) { } tests := []struct { probe *v1.Probe + env []v1.EnvVar execError bool expectError bool execResult probe.Result expectedResult results.Result + expectCommand []string }{ { // No probe probe: nil, @@ -246,12 +245,43 @@ func TestProbe(t *testing.T) { execResult: probe.Unknown, expectedResult: results.Failure, }, + { // Probe arguments are passed through + probe: &v1.Probe{ + Handler: v1.Handler{ + Exec: &v1.ExecAction{ + Command: []string{"/bin/bash", "-c", "some script"}, + }, + }, + }, + expectCommand: []string{"/bin/bash", "-c", "some script"}, + execResult: probe.Success, + expectedResult: results.Success, + }, + { // Probe arguments are passed through + probe: &v1.Probe{ + Handler: v1.Handler{ + Exec: &v1.ExecAction{ + Command: []string{"/bin/bash", "-c", "some $(A) $(B)"}, + }, + }, + }, + env: []v1.EnvVar{ + {Name: "A", Value: "script"}, + }, + expectCommand: []string{"/bin/bash", "-c", "some script $(B)"}, + execResult: probe.Success, + expectedResult: results.Success, + }, } for i, test := range tests { for _, probeType := range [...]probeType{liveness, readiness} { + prober := &prober{ + refManager: kubecontainer.NewRefManager(), + recorder: &record.FakeRecorder{}, + } testID := fmt.Sprintf("%d-%s", i, probeType) - testContainer := v1.Container{} + testContainer := v1.Container{Env: test.env} switch probeType { case liveness: testContainer.LivenessProbe = test.probe @@ -274,6 +304,19 @@ func TestProbe(t *testing.T) { if test.expectedResult != result { t.Errorf("[%s] Expected result to be %v but was %v", testID, test.expectedResult, result) } + + if len(test.expectCommand) > 0 { + prober.exec = execprobe.New() + prober.runner = &containertest.FakeContainerCommandRunner{} + _, err := prober.probe(probeType, &v1.Pod{}, v1.PodStatus{}, testContainer, containerID) + if err != nil { + t.Errorf("[%s] Didn't expect probe error but got: %v", testID, err) + continue + } + if !reflect.DeepEqual(test.expectCommand, prober.runner.(*containertest.FakeContainerCommandRunner).Cmd) { + t.Errorf("[%s] unexpected probe arguments: %v", testID, prober.runner.(*containertest.FakeContainerCommandRunner).Cmd) + } + } } } } diff --git a/pkg/kubelet/prober/worker.go b/pkg/kubelet/prober/worker.go index 1cf52680321..6f55fb86dcb 100644 --- a/pkg/kubelet/prober/worker.go +++ b/pkg/kubelet/prober/worker.go @@ -193,6 +193,9 @@ func (w *worker) doProbe() (keepGoing bool) { return true } + // TODO: in order for exec probes to correctly handle downward API env, we must be able to reconstruct + // the full container environment here, OR we must make a call to the CRI in order to get those environment + // values from the running container. result, err := w.probeManager.prober.probe(w.probeType, w.pod, status, w.container, w.containerID) if err != nil { // Prober error, throw away the result.