Merge pull request #25944 from pwittrock/fix-24905

Automatic merge from submit-queue

Pre-pull images in node e2e-tests.  Possible resolution for #24905



[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/.github/PULL_REQUEST_TEMPLATE.md?pixel)]()

hack/verify-flags/known-flags.txt

@@ -322,6 +322,7 @@ pod-running
 policy-config-file
 poll-interval
 portal-net
+prepull-images
 private-mountns
 prom-push-gateway
 proto-import

test/e2e_node/container_list.go

@@ -0,0 +1,61 @@
+/*
+Copyright 2016 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 e2e_node
+
+import (
+	"github.com/golang/glog"
+	"os/exec"
+)
+
+const (
+	busyBoxImage = iota
+
+	hostExecImage
+	netExecImage
+	nginxImage
+	pauseImage
+
+	// Images just used for explicitly testing pulling of images
+	pullTestExecHealthz
+	pullTestAlpineWithBash
+)
+
+var ImageRegistry = map[int]string{
+	busyBoxImage:  "gcr.io/google_containers/busybox:1.24",
+	hostExecImage: "gcr.io/google_containers/hostexec:1.2",
+	netExecImage:  "gcr.io/google_containers/netexec:1.4",
+	nginxImage:    "gcr.io/google_containers/nginx:1.7.9",
+	pauseImage:    "gcr.io/google_containers/pause-amd64:3.0",
+}
+
+// These are used by tests that explicitly test the ability to pull images
+var NoPullImagRegistry = map[int]string{
+	pullTestAlpineWithBash: "gcr.io/google_containers/alpine-with-bash:1.0",
+	pullTestExecHealthz:    "gcr.io/google_containers/exechealthz:1.0",
+}
+
+// Pre-fetch all images tests depend on so that we don't fail in an actual test
+func PrePullAllImages() error {
+	for _, image := range ImageRegistry {
+		output, err := exec.Command("docker", "pull", image).CombinedOutput()
+		if err != nil {
+			glog.Warning("Could not pre-pull image %s %v output:  %s", image, err, output)
+			return err
+		}
+	}
+	return nil
+}

test/e2e_node/container_manager_test.go

@@ -55,7 +55,7 @@ var _ = Describe("Kubelet Container Manager", func() {
 						RestartPolicy: api.RestartPolicyNever,
 						Containers: []api.Container{
 							{
-								Image:   "gcr.io/google_containers/busybox:1.24",
+								Image:   ImageRegistry[busyBoxImage],
 								Name:    podName,
 								Command: []string{"/bin/false"},
 							},

test/e2e_node/e2e_node_suite_test.go

@@ -38,6 +38,8 @@ import (
 
 var e2es *e2eService
 
+var prePullImages = flag.Bool("prepull-images", true, "If true, prepull images so image pull failures do not cause test failures.")
+
 func TestE2eNode(t *testing.T) {
 	flag.Parse()
 	rand.Seed(time.Now().UTC().UnixNano())
@@ -59,6 +61,14 @@ var _ = BeforeSuite(func() {
 		*nodeName = strings.TrimSpace(fmt.Sprintf("%s", output))
 	}
 
+	// Pre-pull the images tests depend on so we can fail immediately if there is an image pull issue
+	// This helps with debugging test flakes since it is hard to tell when a test failure is due to image pulling.
+	if *prePullImages {
+		glog.Infof("Pre-pulling images so that they are cached for the tests.")
+		err := PrePullAllImages()
+		Expect(err).ShouldNot(HaveOccurred())
+	}
+
 	// TODO(yifan): Temporary workaround to disable coreos from auto restart
 	// by masking the locksmithd.
 	// We should mask locksmithd when provisioning the machine.

test/e2e_node/image_conformance_test.go

@@ -27,8 +27,10 @@ import (
 )
 
 const (
-	imageRetryTimeout = time.Minute * 2
+	imageRetryTimeout         = time.Minute * 10 // Image pulls can take a long time and shouldn't cause flakes
-	imagePullInterval = time.Second * 15
+	imagePullInterval         = time.Second * 15
+	imageConsistentlyTimeout  = time.Second * 30
+	imageConsistentlyInterval = time.Second * 5
 )
 
 var _ = Describe("Image Container Conformance Test", func() {
@@ -44,8 +46,8 @@ var _ = Describe("Image Container Conformance Test", func() {
 			var conformImages []ConformanceImage
 			BeforeEach(func() {
 				existImageTags := []string{
-					"gcr.io/google_containers/busybox:1.24",
+					NoPullImagRegistry[pullTestExecHealthz],
-					"gcr.io/google_containers/mounttest:0.2",
+					NoPullImagRegistry[pullTestAlpineWithBash],
 				}
 				for _, existImageTag := range existImageTags {
 					conformImage, _ := NewConformanceImage("docker", existImageTag)
@@ -84,55 +86,45 @@ var _ = Describe("Image Container Conformance Test", func() {
 		})
 		Context("when testing image that does not exist", func() {
 			var conformImages []ConformanceImage
-			BeforeEach(func() {
+			invalidImageTags := []string{
-				invalidImageTags := []string{
+				// nonexistent image registry
-					// nonexistent image registry
+				"foo.com/foo/fooimage",
-					"foo.com/foo/fooimage",
+				// nonexistent image
-					// nonexistent image
+				"gcr.io/google_containers/not_exist",
-					"gcr.io/google_containers/not_exist",
+				// TODO(random-liu): Add test for image pulling credential
-					// TODO(random-liu): Add test for image pulling credential
+			}
-				}
+			It("should ignore pull failures", func() {
 				for _, invalidImageTag := range invalidImageTags {
 					conformImage, _ := NewConformanceImage("docker", invalidImageTag)
 					// Pulling images from gcr.io is flaky, so retry to make sure failure is not caused by flaky.
-					Consistently(func() error {
+					Expect(conformImage.Pull()).Should(HaveOccurred())
-						return conformImage.Pull()
-					}, imageRetryTimeout, imagePullInterval).Should(HaveOccurred())
 					conformImages = append(conformImages, conformImage)
 				}
-			})
 
-			It("it should not present images [Conformance]", func() {
+				By("not presenting images [Conformance]", func() {
-				for _, conformImage := range conformImages {
+					for _, conformImage := range conformImages {
-					present, err := conformImage.Present()
+						present, err := conformImage.Present()
+						Expect(err).ShouldNot(HaveOccurred())
+						Expect(present).To(BeFalse())
+					}
+				})
+
+				By("not listing pulled images [Conformance]", func() {
+					image, _ := NewConformanceImage("docker", "")
+					tags, err := image.List()
 					Expect(err).ShouldNot(HaveOccurred())
-					Expect(present).To(BeFalse())
+					for _, conformImage := range conformImages {
-				}
+						Expect(tags).NotTo(ContainElement(conformImage.GetTag()))
-			})
+					}
+				})
 
-			It("it should not list pulled images [Conformance]", func() {
+				By("not removing non-exist images [Conformance]", func() {
-				image, _ := NewConformanceImage("docker", "")
+					for _, conformImage := range conformImages {
-				tags, err := image.List()
+						err := conformImage.Remove()
-				Expect(err).ShouldNot(HaveOccurred())
+						Expect(err).Should(HaveOccurred())
-				for _, conformImage := range conformImages {
+					}
-					Expect(tags).NotTo(ContainElement(conformImage.GetTag()))
+				})
-				}
 			})
-
-			It("it should not remove non-exist images [Conformance]", func() {
-				for _, conformImage := range conformImages {
-					err := conformImage.Remove()
-					Expect(err).Should(HaveOccurred())
-				}
-			})
-
-			AfterEach(func() {
-				for _, conformImage := range conformImages {
-					conformImage.Remove()
-				}
-				conformImages = []ConformanceImage{}
-			})
-
 		})
 	})
 })

test/e2e_node/kubelet_test.go

@@ -55,7 +55,7 @@ var _ = framework.KubeDescribe("Kubelet", func() {
 					RestartPolicy: api.RestartPolicyNever,
 					Containers: []api.Container{
 						{
-							Image:   "gcr.io/google_containers/busybox",
+							Image:   ImageRegistry[busyBoxImage],
 							Name:    podName,
 							Command: []string{"sh", "-c", "echo 'Hello World' ; sleep 240"},
 						},
@@ -97,7 +97,7 @@ var _ = framework.KubeDescribe("Kubelet", func() {
 					RestartPolicy: api.RestartPolicyNever,
 					Containers: []api.Container{
 						{
-							Image:   "gcr.io/google_containers/busybox",
+							Image:   ImageRegistry[busyBoxImage],
 							Name:    podName,
 							Command: []string{"sh", "-c", "echo test > /file; sleep 240"},
 							SecurityContext: &api.SecurityContext{
@@ -179,7 +179,7 @@ func createSummaryTestPods(f *framework.Framework, podNamePrefix string, count i
 	for _, podName := range podNames.List() {
 		createPod(f, podName, []api.Container{
 			{
-				Image:   "gcr.io/google_containers/busybox",
+				Image:   ImageRegistry[busyBoxImage],
 				Command: []string{"sh", "-c", "while true; do echo 'hello world' | tee ~/file | tee /test-empty-dir-mnt ; sleep 1; done"},
 				Name:    podName + containerSuffix,
 				VolumeMounts: []api.VolumeMount{

test/e2e_node/mirror_pod_test.go

@@ -47,7 +47,7 @@ var _ = Describe("MirrorPod", func() {
 			mirrorPodName = staticPodName + "-" + e2es.nodeName
 
 			By("create the static pod")
-			err := createStaticPod(e2es.kubeletStaticPodDir, staticPodName, ns, "gcr.io/google_containers/nginx:1.7.9", api.RestartPolicyAlways)
+			err := createStaticPod(e2es.kubeletStaticPodDir, staticPodName, ns, ImageRegistry[nginxImage], api.RestartPolicyAlways)
 			Expect(err).ShouldNot(HaveOccurred())
 
 			By("wait for the mirror pod to be running")
@@ -62,7 +62,7 @@ var _ = Describe("MirrorPod", func() {
 			uid := pod.UID
 
 			By("update the static pod container image")
-			image := "gcr.io/google_containers/pause-amd64:3.0"
+			image := ImageRegistry[pauseImage]
 			err = createStaticPod(e2es.kubeletStaticPodDir, staticPodName, ns, image, api.RestartPolicyAlways)
 			Expect(err).ShouldNot(HaveOccurred())
 

test/e2e_node/privileged_test.go

@@ -42,7 +42,6 @@ const (
 	notPrivilegedHttpPort      = 9090
 	notPrivilegedUdpPort       = 9091
 	notPrivilegedContainerName = "not-privileged-container"
-	privilegedContainerImage   = "gcr.io/google_containers/netexec:1.4"
 	privilegedCommand          = "ip link add dummy1 type dummy"
 )
 
@@ -98,7 +97,7 @@ func (config *PrivilegedPodTestConfig) createPrivilegedPodSpec() *api.Pod {
 			Containers: []api.Container{
 				{
 					Name:            privilegedContainerName,
-					Image:           privilegedContainerImage,
+					Image:           ImageRegistry[netExecImage],
 					ImagePullPolicy: api.PullIfNotPresent,
 					SecurityContext: &api.SecurityContext{Privileged: &isPrivileged},
 					Command: []string{
@@ -109,7 +108,7 @@ func (config *PrivilegedPodTestConfig) createPrivilegedPodSpec() *api.Pod {
 				},
 				{
 					Name:            notPrivilegedContainerName,
-					Image:           privilegedContainerImage,
+					Image:           ImageRegistry[netExecImage],
 					ImagePullPolicy: api.PullIfNotPresent,
 					SecurityContext: &api.SecurityContext{Privileged: &notPrivileged},
 					Command: []string{
@@ -168,7 +167,7 @@ func newHostExecPodSpec(ns, name string) *api.Pod {
 			Containers: []api.Container{
 				{
 					Name:            "hostexec",
-					Image:           "gcr.io/google_containers/hostexec:1.2",
+					Image:           ImageRegistry[hostExecImage],
 					ImagePullPolicy: api.PullIfNotPresent,
 				},
 			},

test/e2e_node/runtime_conformance_test.go

@@ -32,7 +32,7 @@ import (
 )
 
 const (
-	consistentCheckTimeout = time.Second * 20
+	consistentCheckTimeout = time.Second * 10
 	retryTimeout           = time.Minute * 5
 	pollInterval           = time.Second * 5
 )
@@ -66,7 +66,7 @@ var _ = Describe("Container runtime Conformance Test", func() {
 		Context("when start a container that exits successfully", func() {
 			It("it should run with the expected status [Conformance]", func() {
 				testContainer := api.Container{
-					Image: "gcr.io/google_containers/busybox",
+					Image: ImageRegistry[busyBoxImage],
 					VolumeMounts: []api.VolumeMount{
 						{
 							MountPath: "/restart-count",
@@ -145,7 +145,7 @@ var _ = Describe("Container runtime Conformance Test", func() {
 		Context("when start a container that keeps running", func() {
 			It("it should run with the expected status [Conformance]", func() {
 				testContainer := api.Container{
-					Image:           "gcr.io/google_containers/busybox",
+					Image:           ImageRegistry[busyBoxImage],
 					Command:         []string{"sh", "-c", "while true; do echo hello; sleep 1; done"},
 					ImagePullPolicy: api.PullIfNotPresent,
 				}
@@ -203,7 +203,7 @@ var _ = Describe("Container runtime Conformance Test", func() {
 		Context("when start a container that exits failure", func() {
 			It("it should run with the expected status [Conformance]", func() {
 				testContainer := api.Container{
-					Image:           "gcr.io/google_containers/busybox",
+					Image:           ImageRegistry[busyBoxImage],
 					Command:         []string{"false"},
 					ImagePullPolicy: api.PullIfNotPresent,
 				}