From 72a26aed7d4b47749fd3c6b02fe17504be86b8ef Mon Sep 17 00:00:00 2001 From: Claudiu Belu Date: Thu, 30 Jul 2020 13:49:35 -0700 Subject: [PATCH 1/2] tests: Fixes variable expansion false positive test Some of the tests are negative test cases which are supposed to ensure that those invalid usecases are handled properly. However, some of the tests are false positives, they can pass due to various reasons. One such example is: "should fail substituting values in a volume subpath with absolute path". This test can pass if: - the Pod cannot start due to various reasons (e.g.: the container image cannot be pulled or does not exist). - the Pod ran to completion, even though the container was not supposed to start in the first place. --- test/e2e/common/expansion.go | 4 ++-- test/e2e/framework/pod/resource.go | 23 +++++++++++++++++++++++ test/e2e/framework/pod/wait.go | 7 +++++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/test/e2e/common/expansion.go b/test/e2e/common/expansion.go index 2513401792c..1723a969f4f 100644 --- a/test/e2e/common/expansion.go +++ b/test/e2e/common/expansion.go @@ -367,8 +367,8 @@ func testPodFailSubpath(f *framework.Framework, pod *v1.Pod) { e2epod.DeletePodWithWait(f.ClientSet, pod) }() - err := e2epod.WaitTimeoutForPodRunningInNamespace(f.ClientSet, pod.Name, pod.Namespace, framework.PodStartShortTimeout) - framework.ExpectError(err, "while waiting for pod to be running") + err := e2epod.WaitForPodContainerToFail(f.ClientSet, pod.Namespace, pod.Name, 0, "CreateContainerConfigError", framework.PodStartShortTimeout) + framework.ExpectNoError(err, "while waiting for the pod container to fail") } func newPod(command []string, envVars []v1.EnvVar, mounts []v1.VolumeMount, volumes []v1.Volume) *v1.Pod { diff --git a/test/e2e/framework/pod/resource.go b/test/e2e/framework/pod/resource.go index 155a4e74984..f74ead41179 100644 --- a/test/e2e/framework/pod/resource.go +++ b/test/e2e/framework/pod/resource.go @@ -298,6 +298,29 @@ func podsRunning(c clientset.Interface, pods *v1.PodList) []error { return e } +func podContainerFailed(c clientset.Interface, namespace, podName string, containerIndex int, reason string) wait.ConditionFunc { + return func() (bool, error) { + pod, err := c.CoreV1().Pods(namespace).Get(context.TODO(), podName, metav1.GetOptions{}) + if err != nil { + return false, err + } + switch pod.Status.Phase { + case v1.PodPending: + if len(pod.Status.ContainerStatuses) == 0 { + return false, nil + } + containerStatus := pod.Status.ContainerStatuses[containerIndex] + if containerStatus.State.Waiting != nil && containerStatus.State.Waiting.Reason == reason { + return true, nil + } + return false, nil + case v1.PodFailed, v1.PodRunning, v1.PodSucceeded: + return false, fmt.Errorf("pod was expected to be pending, but it is in the state: %s", pod.Status.Phase) + } + return false, nil + } +} + // LogPodStates logs basic info of provided pods for debugging. func LogPodStates(pods []v1.Pod) { // Find maximum widths for pod, node, and phase strings for column printing. diff --git a/test/e2e/framework/pod/wait.go b/test/e2e/framework/pod/wait.go index 8f1e8be6f8c..9ab2928f86e 100644 --- a/test/e2e/framework/pod/wait.go +++ b/test/e2e/framework/pod/wait.go @@ -537,3 +537,10 @@ func WaitForNRestartablePods(ps *testutils.PodStore, expect int, timeout time.Du } return podNames, nil } + +// WaitForPodContainerToFail waits for the given Pod container to fail with the given reason, specifically due to +// invalid container configuration. In this case, the container will remain in a waiting state with a specific +// reason set, which should match the given reason. +func WaitForPodContainerToFail(c clientset.Interface, namespace, podName string, containerIndex int, reason string, timeout time.Duration) error { + return wait.PollImmediate(poll, timeout, podContainerFailed(c, namespace, podName, containerIndex, reason)) +} From 8ca08f1cedc13d5627c8cae1eff403e37ef8a8ab Mon Sep 17 00:00:00 2001 From: Claudiu Belu Date: Tue, 18 Aug 2020 13:35:48 -0700 Subject: [PATCH 2/2] tests: Use Windows absolute path on Windows pods The test "should fail substituting values in a volume subpath with absolute path" creates a pod with a variable expansion path which is set as an absolute path. However "/tmp" is not an absolute on Windows, it has to be prefixed with the drive letter (C:\tmp). But C:\tmp does not typically exist on Windows nodes, so we use C:\Users instead. --- test/e2e/common/expansion.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/e2e/common/expansion.go b/test/e2e/common/expansion.go index 1723a969f4f..a50206efb05 100644 --- a/test/e2e/common/expansion.go +++ b/test/e2e/common/expansion.go @@ -181,11 +181,16 @@ var _ = framework.KubeDescribe("Variable Expansion", func() { Description: Make sure a container's subpath can not be set using an expansion of environment variables when absolute path is supplied. */ framework.ConformanceIt("should fail substituting values in a volume subpath with absolute path [sig-storage][Slow]", func() { + absolutePath := "/tmp" + if framework.NodeOSDistroIs("windows") { + // Windows does not typically have a C:\tmp folder. + absolutePath = "C:\\Users" + } envVars := []v1.EnvVar{ { Name: "POD_NAME", - Value: "/tmp", + Value: absolutePath, }, } mounts := []v1.VolumeMount{