Merge pull request #42273 from smarterclayton/evaluate_probes

Automatic merge from submit-queue (batch tested with PRs 41672, 42084, 42233, 42165, 42273)

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.

Improves the state of #40846, will spawn a follow up issue for future refactoring after CRI settles down

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))
 

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",

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
 }
 

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))

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)
+				}
+			}
 		}
 	}
 }

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.