Merge pull request #29806 from Random-Liu/improve-node-e2e-test-framework

test/e2e/configmap.go

@@ -195,9 +195,9 @@ var _ = framework.KubeDescribe("ConfigMap", func() {
 			},
 		}
 
-		framework.TestContainerOutput("consume configMaps", f.Client, pod, 0, []string{
+		f.TestContainerOutput("consume configMaps", pod, 0, []string{
 			"CONFIG_DATA_1=value-1",
-		}, f.Namespace.Name)
+		})
 	})
 
 	It("should be consumable in multiple volumes in the same pod", func() {
@@ -272,9 +272,9 @@ var _ = framework.KubeDescribe("ConfigMap", func() {
 			},
 		}
 
-		framework.TestContainerOutput("consume configMaps", f.Client, pod, 0, []string{
+		f.TestContainerOutput("consume configMaps", pod, 0, []string{
 			"content of file \"/etc/configmap-volume/data-1\": value-1",
-		}, f.Namespace.Name)
+		})
 
 	})
 })
@@ -357,9 +357,9 @@ func doConfigMapE2EWithoutMappings(f *framework.Framework, uid, fsGroup int64) {
 		pod.Spec.SecurityContext.FSGroup = &fsGroup
 	}
 
-	framework.TestContainerOutput("consume configMaps", f.Client, pod, 0, []string{
+	f.TestContainerOutput("consume configMaps", pod, 0, []string{
 		"content of file \"/etc/configmap-volume/data-1\": value-1",
-	}, f.Namespace.Name)
+	})
 
 }
 
@@ -433,7 +433,7 @@ func doConfigMapE2EWithMappings(f *framework.Framework, uid, fsGroup int64) {
 		pod.Spec.SecurityContext.FSGroup = &fsGroup
 	}
 
-	framework.TestContainerOutput("consume configMaps", f.Client, pod, 0, []string{
+	f.TestContainerOutput("consume configMaps", pod, 0, []string{
 		"content of file \"/etc/configmap-volume/path/to/data-2\": value-2",
-	}, f.Namespace.Name)
+	})
 }

test/e2e/docker_containers.go

@@ -18,7 +18,6 @@ package e2e
 
 import (
 	"k8s.io/kubernetes/pkg/api"
-	client "k8s.io/kubernetes/pkg/client/unversioned"
 	"k8s.io/kubernetes/pkg/util/uuid"
 	"k8s.io/kubernetes/test/e2e/framework"
 
@@ -27,27 +26,20 @@ import (
 
 var _ = framework.KubeDescribe("Docker Containers", func() {
 	f := framework.NewDefaultFramework("containers")
-	var c *client.Client
-	var ns string
-
-	BeforeEach(func() {
-		c = f.Client
-		ns = f.Namespace.Name
-	})
 
 	It("should use the image defaults if command and args are blank [Conformance]", func() {
-		framework.TestContainerOutput("use defaults", c, entrypointTestPod(), 0, []string{
+		f.TestContainerOutput("use defaults", entrypointTestPod(), 0, []string{
 			"[/ep default arguments]",
-		}, ns)
+		})
 	})
 
 	It("should be able to override the image's default arguments (docker cmd) [Conformance]", func() {
 		pod := entrypointTestPod()
 		pod.Spec.Containers[0].Args = []string{"override", "arguments"}
 
-		framework.TestContainerOutput("override arguments", c, pod, 0, []string{
+		f.TestContainerOutput("override arguments", pod, 0, []string{
 			"[/ep override arguments]",
-		}, ns)
+		})
 	})
 
 	// Note: when you override the entrypoint, the image's arguments (docker cmd)
@@ -56,9 +48,9 @@ var _ = framework.KubeDescribe("Docker Containers", func() {
 		pod := entrypointTestPod()
 		pod.Spec.Containers[0].Command = []string{"/ep-2"}
 
-		framework.TestContainerOutput("override command", c, pod, 0, []string{
+		f.TestContainerOutput("override command", pod, 0, []string{
 			"[/ep-2]",
-		}, ns)
+		})
 	})
 
 	It("should be able to override the image's default command and arguments [Conformance]", func() {
@@ -66,9 +58,9 @@ var _ = framework.KubeDescribe("Docker Containers", func() {
 		pod.Spec.Containers[0].Command = []string{"/ep-2"}
 		pod.Spec.Containers[0].Args = []string{"override", "arguments"}
 
-		framework.TestContainerOutput("override all", c, pod, 0, []string{
+		f.TestContainerOutput("override all", pod, 0, []string{
 			"[/ep-2 override arguments]",
-		}, ns)
+		})
 	})
 })
 

test/e2e/downwardapi_volume.go

@@ -38,9 +38,9 @@ var _ = framework.KubeDescribe("Downward API volume", func() {
 		podName := "downwardapi-volume-" + string(uuid.NewUUID())
 		pod := downwardAPIVolumePodForSimpleTest(podName, "/etc/podname")
 
-		framework.TestContainerOutput("downward API volume plugin", f.Client, pod, 0, []string{
+		f.TestContainerOutput("downward API volume plugin", pod, 0, []string{
 			fmt.Sprintf("%s\n", podName),
-		}, f.Namespace.Name)
+		})
 	})
 
 	It("should provide podname as non-root with fsgroup [Feature:FSGroup]", func() {
@@ -52,9 +52,9 @@ var _ = framework.KubeDescribe("Downward API volume", func() {
 			RunAsUser: &uid,
 			FSGroup:   &gid,
 		}
-		framework.TestContainerOutput("downward API volume plugin", f.Client, pod, 0, []string{
+		f.TestContainerOutput("downward API volume plugin", pod, 0, []string{
 			fmt.Sprintf("%s\n", podName),
-		}, f.Namespace.Name)
+		})
 	})
 
 	It("should update labels on modification [Conformance]", func() {
@@ -130,36 +130,36 @@ var _ = framework.KubeDescribe("Downward API volume", func() {
 		podName := "downwardapi-volume-" + string(uuid.NewUUID())
 		pod := downwardAPIVolumeForContainerResources(podName, "/etc/cpu_limit")
 
-		framework.TestContainerOutput("downward API volume plugin", f.Client, pod, 0, []string{
+		f.TestContainerOutput("downward API volume plugin", pod, 0, []string{
 			fmt.Sprintf("2\n"),
-		}, f.Namespace.Name)
+		})
 	})
 
 	It("should provide container's memory limit", func() {
 		podName := "downwardapi-volume-" + string(uuid.NewUUID())
 		pod := downwardAPIVolumeForContainerResources(podName, "/etc/memory_limit")
 
-		framework.TestContainerOutput("downward API volume plugin", f.Client, pod, 0, []string{
+		f.TestContainerOutput("downward API volume plugin", pod, 0, []string{
 			fmt.Sprintf("67108864\n"),
-		}, f.Namespace.Name)
+		})
 	})
 
 	It("should provide container's cpu request", func() {
 		podName := "downwardapi-volume-" + string(uuid.NewUUID())
 		pod := downwardAPIVolumeForContainerResources(podName, "/etc/cpu_request")
 
-		framework.TestContainerOutput("downward API volume plugin", f.Client, pod, 0, []string{
+		f.TestContainerOutput("downward API volume plugin", pod, 0, []string{
 			fmt.Sprintf("1\n"),
-		}, f.Namespace.Name)
+		})
 	})
 
 	It("should provide container's memory request", func() {
 		podName := "downwardapi-volume-" + string(uuid.NewUUID())
 		pod := downwardAPIVolumeForContainerResources(podName, "/etc/memory_request")
 
-		framework.TestContainerOutput("downward API volume plugin", f.Client, pod, 0, []string{
+		f.TestContainerOutput("downward API volume plugin", pod, 0, []string{
 			fmt.Sprintf("33554432\n"),
-		}, f.Namespace.Name)
+		})
 	})
 
 })

test/e2e/framework/framework.go

@@ -373,14 +373,18 @@ func (f *Framework) WaitForPodNoLongerRunning(podName string) error {
 	return WaitForPodNoLongerRunningInNamespace(f.Client, podName, f.Namespace.Name, "")
 }
 
-// Runs the given pod and verifies that the output of exact container matches the desired output.
+// TestContainerOutput runs the given pod in the given namespace and waits
+// for all of the containers in the podSpec to move into the 'Success' status, and tests
+// the specified container log against the given expected output using a substring matcher.
 func (f *Framework) TestContainerOutput(scenarioName string, pod *api.Pod, containerIndex int, expectedOutput []string) {
-	TestContainerOutput(scenarioName, f.Client, pod, containerIndex, expectedOutput, f.Namespace.Name)
+	f.testContainerOutputMatcher(scenarioName, pod, containerIndex, expectedOutput, ContainSubstring)
 }
 
-// Runs the given pod and verifies that the output of exact container matches the desired regexps.
+// TestContainerOutputRegexp runs the given pod in the given namespace and waits
+// for all of the containers in the podSpec to move into the 'Success' status, and tests
+// the specified container log against the given expected output using a regexp matcher.
 func (f *Framework) TestContainerOutputRegexp(scenarioName string, pod *api.Pod, containerIndex int, expectedOutput []string) {
-	testContainerOutputRegexp(scenarioName, f.Client, pod, containerIndex, expectedOutput, f.Namespace.Name)
+	f.testContainerOutputMatcher(scenarioName, pod, containerIndex, expectedOutput, MatchRegexp)
 }
 
 // WaitForAnEndpoint waits for at least one endpoint to become available in the

test/e2e/framework/pods.go

@@ -45,7 +45,7 @@ type PodClient struct {
 
 // Create creates a new pod according to the framework specifications (don't wait for it to start).
 func (c *PodClient) Create(pod *api.Pod) *api.Pod {
-	c.MungeSpec(pod)
+	c.mungeSpec(pod)
 	p, err := c.PodInterface.Create(pod)
 	ExpectNoError(err, "Error creating Pod")
 	return p
@@ -77,16 +77,6 @@ func (c *PodClient) CreateBatch(pods []*api.Pod) []*api.Pod {
 	return ps
 }
 
-// MungeSpec apply test-suite specific transformations to the pod spec.
-// TODO: Refactor the framework to always use PodClient so that we can completely hide the munge logic
-// in the PodClient.
-func (c *PodClient) MungeSpec(pod *api.Pod) {
-	if TestContext.NodeName != "" {
-		Expect(pod.Spec.NodeName).To(Or(BeZero(), Equal(TestContext.NodeName)), "Test misconfigured")
-		pod.Spec.NodeName = TestContext.NodeName
-	}
-}
-
 // Update updates the pod object. It retries if there is a conflict, throw out error if
 // there is any other errors. name is the pod name, updateFn is the function updating the
 // pod object.
@@ -110,4 +100,12 @@ func (c *PodClient) Update(name string, updateFn func(pod *api.Pod)) {
 	}))
 }
 
+// mungeSpec apply test-suite specific transformations to the pod spec.
+func (c *PodClient) mungeSpec(pod *api.Pod) {
+	if TestContext.NodeName != "" {
+		Expect(pod.Spec.NodeName).To(Or(BeZero(), Equal(TestContext.NodeName)), "Test misconfigured")
+		pod.Spec.NodeName = TestContext.NodeName
+	}
+}
+
 // TODO(random-liu): Move pod wait function into this file

test/e2e/framework/util.go

@@ -2083,40 +2083,25 @@ func TryKill(cmd *exec.Cmd) {
 	}
 }
 
-// TestContainerOutput runs the given pod in the given namespace and waits
-// for all of the containers in the podSpec to move into the 'Success' status, and tests
-// the specified container log against the given expected output using a substring matcher.
-func TestContainerOutput(scenarioName string, c *client.Client, pod *api.Pod, containerIndex int, expectedOutput []string, ns string) {
-	testContainerOutputMatcher(scenarioName, c, pod, containerIndex, expectedOutput, ns, ContainSubstring)
-}
-
-// testContainerOutputRegexp runs the given pod in the given namespace and waits
-// for all of the containers in the podSpec to move into the 'Success' status, and tests
-// the specified container log against the given expected output using a regexp matcher.
-func testContainerOutputRegexp(scenarioName string, c *client.Client, pod *api.Pod, containerIndex int, expectedOutput []string, ns string) {
-	testContainerOutputMatcher(scenarioName, c, pod, containerIndex, expectedOutput, ns, MatchRegexp)
-}
-
 // testContainerOutputMatcher runs the given pod in the given namespace and waits
 // for all of the containers in the podSpec to move into the 'Success' status, and tests
 // the specified container log against the given expected output using the given matcher.
-func testContainerOutputMatcher(scenarioName string,
-	c *client.Client,
+func (f *Framework) testContainerOutputMatcher(scenarioName string,
 	pod *api.Pod,
 	containerIndex int,
-	expectedOutput []string, ns string,
+	expectedOutput []string,
 	matcher func(string, ...interface{}) gomegatypes.GomegaMatcher) {
 	By(fmt.Sprintf("Creating a pod to test %v", scenarioName))
+	podClient := f.PodClient()
+	ns := f.Namespace.Name
 
-	defer c.Pods(ns).Delete(pod.Name, api.NewDeleteOptions(0))
-	if _, err := c.Pods(ns).Create(pod); err != nil {
-		Failf("Failed to create pod: %v", err)
-	}
+	defer podClient.Delete(pod.Name, api.NewDeleteOptions(0))
+	podClient.Create(pod)
 
 	// Wait for client pod to complete.
 	var containerName string
 	for id, container := range pod.Spec.Containers {
-		ExpectNoError(WaitForPodSuccessInNamespace(c, pod.Name, container.Name, ns))
+		ExpectNoError(WaitForPodSuccessInNamespace(f.Client, pod.Name, container.Name, ns))
 		if id == containerIndex {
 			containerName = container.Name
 		}
@@ -2126,7 +2111,7 @@ func testContainerOutputMatcher(scenarioName string,
 	}
 
 	// Grab its logs.  Get host first.
-	podStatus, err := c.Pods(ns).Get(pod.Name)
+	podStatus, err := podClient.Get(pod.Name)
 	if err != nil {
 		Failf("Failed to get pod status: %v", err)
 	}
@@ -2139,7 +2124,7 @@ func testContainerOutputMatcher(scenarioName string,
 	// Sometimes the actual containers take a second to get started, try to get logs for 60s
 	for time.Now().Sub(start) < (60 * time.Second) {
 		err = nil
-		logs, err = GetPodLogs(c, ns, pod.Name, containerName)
+		logs, err = GetPodLogs(f.Client, ns, pod.Name, containerName)
 		if err != nil {
 			By(fmt.Sprintf("Warning: Failed to get logs from node %q pod %q container %q. %v",
 				podStatus.Spec.NodeName, podStatus.Name, containerName, err))

test/e2e/host_path.go

@@ -56,10 +56,9 @@ var _ = framework.KubeDescribe("hostPath", func() {
 			fmt.Sprintf("--fs_type=%v", volumePath),
 			fmt.Sprintf("--file_mode=%v", volumePath),
 		}
-		framework.TestContainerOutput("hostPath mode", c, pod, 0, []string{
+		f.TestContainerOutput("hostPath mode", pod, 0, []string{
 			"mode of file \"/test-volume\": dtrwxrwxrwx", // we expect the sticky bit (mode flag t) to be set for the dir
-		},
-			namespace.Name)
+		})
 	})
 
 	// This test requires mounting a folder into a container with write privileges.
@@ -83,10 +82,9 @@ var _ = framework.KubeDescribe("hostPath", func() {
 		}
 		//Read the content of the file with the second container to
 		//verify volumes  being shared properly among containers within the pod.
-		framework.TestContainerOutput("hostPath r/w", c, pod, 1, []string{
+		f.TestContainerOutput("hostPath r/w", pod, 1, []string{
 			"content of file \"/test-volume/test-file\": mount-tester new file",
-		}, namespace.Name,
-		)
+		})
 	})
 
 	It("should support subPath [Conformance]", func() {
@@ -115,9 +113,9 @@ var _ = framework.KubeDescribe("hostPath", func() {
 			fmt.Sprintf("--retry_time=%d", retryDuration),
 		}
 
-		framework.TestContainerOutput("hostPath subPath", c, pod, 1, []string{
+		f.TestContainerOutput("hostPath subPath", pod, 1, []string{
 			"content of file \"" + filePathInReader + "\": mount-tester new file",
-		}, namespace.Name)
+		})
 	})
 })
 

test/e2e/secrets.go

@@ -82,10 +82,10 @@ var _ = framework.KubeDescribe("Secrets", func() {
 			},
 		}
 
-		framework.TestContainerOutput("consume secrets", f.Client, pod, 0, []string{
+		f.TestContainerOutput("consume secrets", pod, 0, []string{
 			"content of file \"/etc/secret-volume/data-1\": value-1",
 			"mode of file \"/etc/secret-volume/data-1\": -rw-r--r--",
-		}, f.Namespace.Name)
+		})
 	})
 
 	It("should be consumable in multiple volumes in a pod [Conformance]", func() {
@@ -161,10 +161,10 @@ var _ = framework.KubeDescribe("Secrets", func() {
 			},
 		}
 
-		framework.TestContainerOutput("consume secrets", f.Client, pod, 0, []string{
+		f.TestContainerOutput("consume secrets", pod, 0, []string{
 			"content of file \"/etc/secret-volume/data-1\": value-1",
 			"mode of file \"/etc/secret-volume/data-1\": -rw-r--r--",
-		}, f.Namespace.Name)
+		})
 	})
 
 	It("should be consumable from pods in env vars [Conformance]", func() {
@@ -212,9 +212,9 @@ var _ = framework.KubeDescribe("Secrets", func() {
 			},
 		}
 
-		framework.TestContainerOutput("consume secrets", f.Client, pod, 0, []string{
+		f.TestContainerOutput("consume secrets", pod, 0, []string{
 			"SECRET_DATA=value-1",
-		}, f.Namespace.Name)
+		})
 	})
 })
 

test/e2e_node/configmap_test.go

@@ -179,12 +179,9 @@ var _ = framework.KubeDescribe("ConfigMap", func() {
 			},
 		}
 
-		// TODO(random-liu): Change TestContainerOutput to use PodClient and avoid MungeSpec explicitly
-		f.PodClient().MungeSpec(pod)
-
-		framework.TestContainerOutput("consume configMaps", f.Client, pod, 0, []string{
+		f.TestContainerOutput("consume configMaps", pod, 0, []string{
 			"CONFIG_DATA_1=value-1",
-		}, f.Namespace.Name)
+		})
 	})
 })
 
@@ -261,11 +258,9 @@ func doConfigMapE2EWithoutMappings(f *framework.Framework, uid, fsGroup int64) {
 		pod.Spec.SecurityContext.FSGroup = &fsGroup
 	}
 
-	f.PodClient().MungeSpec(pod)
-
-	framework.TestContainerOutput("consume configMaps", f.Client, pod, 0, []string{
+	f.TestContainerOutput("consume configMaps", pod, 0, []string{
 		"content of file \"/etc/configmap-volume/data-1\": value-1",
-	}, f.Namespace.Name)
+	})
 
 }
 
@@ -334,9 +329,7 @@ func doConfigMapE2EWithMappings(f *framework.Framework, uid, fsGroup int64) {
 		pod.Spec.SecurityContext.FSGroup = &fsGroup
 	}
 
-	f.PodClient().MungeSpec(pod)
-
-	framework.TestContainerOutput("consume configMaps", f.Client, pod, 0, []string{
+	f.TestContainerOutput("consume configMaps", pod, 0, []string{
 		"content of file \"/etc/configmap-volume/path/to/data-2\": value-2",
-	}, f.Namespace.Name)
+	})
 }

test/e2e_node/downward_api_test.go

@@ -158,8 +158,5 @@ func testDownwardAPI(f *framework.Framework, podName string, env []api.EnvVar, e
 			RestartPolicy: api.RestartPolicyNever,
 		},
 	}
-	// TODO(random-liu): Change TestContainerOutputRegexp to use PodClient and avoid MungeSpec explicitly
-	f.PodClient().MungeSpec(pod)
-
 	f.TestContainerOutputRegexp("downward api env vars", pod, 0, expectations)
 }