Improve image test and make test run in parallel.

Signed-off-by: Lantao Liu <[email protected]>

hack/run-critest.sh

@@ -31,7 +31,7 @@ sleep 10
 
 # Run e2e test cases
 # Skip reopen container log test because docker doesn't support it.
-critest --skip="runtime should support reopening container log" v
+critest --skip="runtime should support reopening container log" --ginkgo-flags=--nodes=8 v
 
 # Run benchmark test cases
 critest b

images/image-test/Dockerfile

@@ -0,0 +1,17 @@
+# Copyright 2018 The Kubernetes Authors.
+#
+# 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.
+
+FROM busybox
+ARG TEST
+RUN touch ${TEST}

images/image-test/Makefile

@@ -0,0 +1,30 @@
+# Copyright 2018 The Kubernetes Authors.
+#
+# 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.
+
+.PHONY: all test-image-list test-image-tags
+
+all: test-image-list test-image-tags
+
+IMAGES_LIST = test-image-1 test-image-2 test-image-3 test-image-latest test-image-digest test-image-tag:test
+test-image-list:
+	$(foreach name,$(IMAGES_LIST),docker build . -t gcr.io/cri-tools/$(name) --build-arg TEST=$(name);)
+	$(foreach name,$(IMAGES_LIST),gcloud docker -- push gcr.io/cri-tools/$(name);)
+
+test-image-tags:
+	docker build . -t gcr.io/cri-tools/$@:1 --build-arg TEST=$@
+	docker tag gcr.io/cri-tools/$@:1 gcr.io/cri-tools/$@:2
+	docker tag gcr.io/cri-tools/$@:1 gcr.io/cri-tools/$@:3
+	gcloud docker -- push gcr.io/cri-tools/$@:1
+	gcloud docker -- push gcr.io/cri-tools/$@:2
+	gcloud docker -- push gcr.io/cri-tools/$@:3

pkg/framework/util.go

@@ -229,7 +229,7 @@ func ListImage(c internalapi.ImageManagerService, filter *runtimeapi.ImageFilter
 }
 
 // PullPublicImage pulls the public image named imageName.
-func PullPublicImage(c internalapi.ImageManagerService, imageName string) {
+func PullPublicImage(c internalapi.ImageManagerService, imageName string) string {
 	if !strings.Contains(imageName, ":") {
 		imageName = imageName + ":latest"
 		Logf("Use latest as default image tag.")
@@ -239,6 +239,7 @@ func PullPublicImage(c internalapi.ImageManagerService, imageName string) {
 	imageSpec := &runtimeapi.ImageSpec{
 		Image: imageName,
 	}
-	_, err := c.PullImage(imageSpec, nil)
+	id, err := c.PullImage(imageSpec, nil)
 	ExpectNoError(err, "failed to pull image: %v", err)
+	return id
 }

pkg/validate/image.go

@@ -17,7 +17,7 @@ limitations under the License.
 package validate
 
 import (
-	"strings"
+	"sort"
 
 	"github.com/kubernetes-incubator/cri-tools/pkg/framework"
 	internalapi "k8s.io/kubernetes/pkg/kubelet/apis/cri"
@@ -27,15 +27,15 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-var (
-	// image name for test image api
-	testImageName = "busybox"
+const (
+	// image reference without
+	testImageWithoutTag = "gcr.io/cri-tools/test-image-latest"
 
 	// name-tagged reference for test image
-	testImageRef = testImageName + ":1.26.2"
+	testImageWithTag = "gcr.io/cri-tools/test-image-tag:test"
 
-	// Digested reference for test image
-	busyboxDigestRef = testImageName + "@sha256:817a12c32a39bbe394944ba49de563e085f1d3c5266eb8e9723256bc4448680e"
+	// digested reference for test image
+	testImageWithDigest = "gcr.io/cri-tools/test-image-digest@sha256:9179135b4b4cc5a8721e09379244807553c318d92fa3111a65133241551ca343"
 )
 
 var _ = framework.KubeDescribe("Image Manager", func() {
@@ -48,88 +48,79 @@ var _ = framework.KubeDescribe("Image Manager", func() {
 	})
 
 	It("public image with tag should be pulled and removed [Conformance]", func() {
-		testPullPublicImage(c, testImageRef)
+		testPullPublicImage(c, testImageWithTag, func(s *runtimeapi.Image) {
+			Expect(s.RepoTags).To(Equal([]string{testImageWithTag}))
+		})
 	})
 
 	It("public image without tag should be pulled and removed [Conformance]", func() {
-		testPullPublicImage(c, testImageName)
+		testPullPublicImage(c, testImageWithoutTag, func(s *runtimeapi.Image) {
+			Expect(s.RepoTags).To(Equal([]string{testImageWithoutTag + ":latest"}))
+		})
 	})
 
 	It("public image with digest should be pulled and removed [Conformance]", func() {
-		testPullPublicImage(c, busyboxDigestRef)
-	})
-
-	It("image status get image fields should not be empty [Conformance]", func() {
-		framework.PullPublicImage(c, testImageRef)
-
-		defer removeImage(c, testImageRef)
-
-		status := framework.ImageStatus(c, testImageRef)
-		Expect(status.Id).NotTo(BeNil(), "Image Id should not be nil")
-		Expect(len(status.RepoTags)).NotTo(Equal(0), "Should have repoTags in image")
-		Expect(status.Size_).NotTo(BeNil(), "Image Size should not be nil")
+		testPullPublicImage(c, testImageWithDigest, func(s *runtimeapi.Image) {
+			Expect(s.RepoTags).To(BeEmpty())
+			Expect(s.RepoDigests).To(Equal([]string{testImageWithDigest}))
+		})
 	})
 
 	It("listImage should get exactly 3 image in the result list [Conformance]", func() {
 		// different tags refer to different images
 		testImageList := []string{
-			"busybox:1-uclibc",
-			"busybox:1-musl",
-			"busybox:1-glibc",
+			"gcr.io/cri-tools/test-image-1:latest",
+			"gcr.io/cri-tools/test-image-2:latest",
+			"gcr.io/cri-tools/test-image-3:latest",
 		}
 
-		pullImageList(c, testImageList)
+		// Make sure test image does not exist.
+		removeImageList(c, testImageList)
+		ids := pullImageList(c, testImageList)
+		ids = removeDuplicates(ids)
+		Expect(len(ids)).To(Equal(3), "3 image ids should be returned")
 
 		defer removeImageList(c, testImageList)
 
 		images := framework.ListImage(c, &runtimeapi.ImageFilter{})
 
-		count := 0
-		for _, imageName := range images {
-			for _, imagesTag := range imageName.RepoTags {
-				if strings.HasSuffix(imagesTag, "busybox:1-uclibc") {
-					count = count + 1
-				}
-				if strings.HasSuffix(imagesTag, "busybox:1-musl") {
-					count = count + 1
-				}
-				if strings.HasSuffix(imagesTag, "busybox:1-glibc") {
-					count = count + 1
+		for i, id := range ids {
+			for _, img := range images {
+				if img.Id == id {
+					Expect(len(img.RepoTags)).To(Equal(1), "Should only have 1 repo tag")
+					Expect(img.RepoTags[0]).To(Equal(testImageList[i]), "Repo tag should be correct")
+					break
 				}
 			}
 		}
-		Expect(count).To(Equal(3), "Should have the specified three images in list")
-
 	})
 
-	It("listImage should get exactly 2 repoTags in the result image [Conformance]", func() {
+	It("listImage should get exactly 3 repoTags in the result image [Conformance]", func() {
 		// different tags refer to the same image
 		testImageList := []string{
-			"busybox:1-uclibc",
-			"busybox:1",
+			"gcr.io/cri-tools/test-image-tags:1",
+			"gcr.io/cri-tools/test-image-tags:2",
+			"gcr.io/cri-tools/test-image-tags:3",
 		}
 
-		pullImageList(c, testImageList)
+		// Make sure test image does not exist.
+		removeImageList(c, testImageList)
+		ids := pullImageList(c, testImageList)
+		ids = removeDuplicates(ids)
+		Expect(len(ids)).To(Equal(1), "Only 1 image id should be returned")
 
 		defer removeImageList(c, testImageList)
 
 		images := framework.ListImage(c, &runtimeapi.ImageFilter{})
 
-		count := 0
-		for _, imageName := range images {
-			for _, imagesTag := range imageName.RepoTags {
-				if strings.HasSuffix(imagesTag, "busybox:1-uclibc") {
-					count = count + 1
-				}
-				if strings.HasSuffix(imagesTag, "busybox:1") {
-					count = count + 1
-				}
-			}
-			if count < 2 {
-				count = 0
+		sort.Strings(testImageList)
+		for _, img := range images {
+			if img.Id == ids[0] {
+				sort.Strings(img.RepoTags)
+				Expect(img.RepoTags).To(Equal(testImageList), "Should have 3 repoTags in single image")
+				break
 			}
 		}
-		Expect(count).To(Equal(2), "Should have two repoTags in single image in list")
 	})
 })
 
@@ -144,24 +135,31 @@ func testRemoveImage(c internalapi.ImageManagerService, imageName string) {
 }
 
 // testPullPublicImage pulls the image named imageName, make sure it success and remove the image.
-func testPullPublicImage(c internalapi.ImageManagerService, imageName string) {
-	if !strings.Contains(imageName, ":") {
-		imageName = imageName + ":latest"
-	}
+func testPullPublicImage(c internalapi.ImageManagerService, imageName string, statusCheck func(*runtimeapi.Image)) {
+	// Make sure image does not exist before testing.
+	removeImage(c, imageName)
+
 	framework.PullPublicImage(c, imageName)
 
 	By("Check image list to make sure pulling image success : " + imageName)
 	status := framework.ImageStatus(c, imageName)
 	Expect(status).NotTo(BeNil(), "Should have one image in list")
+	Expect(status.Id).NotTo(BeNil(), "Image Id should not be nil")
+	Expect(status.Size_).NotTo(BeNil(), "Image Size should not be nil")
+	if statusCheck != nil {
+		statusCheck(status)
+	}
 
 	testRemoveImage(c, imageName)
 }
 
 // pullImageList pulls the images listed in the imageList.
-func pullImageList(c internalapi.ImageManagerService, imageList []string) {
+func pullImageList(c internalapi.ImageManagerService, imageList []string) []string {
+	var ids []string
 	for _, imageName := range imageList {
-		framework.PullPublicImage(c, imageName)
+		ids = append(ids, framework.PullPublicImage(c, imageName))
 	}
+	return ids
 }
 
 // removeImageList removes the images listed in the imageList.
@@ -183,3 +181,17 @@ func removeImage(c internalapi.ImageManagerService, imageName string) {
 		framework.ExpectNoError(err, "failed to remove image: %v", err)
 	}
 }
+
+// removeDuplicates remove duplicates strings from a list
+func removeDuplicates(ss []string) []string {
+	encountered := map[string]bool{}
+	result := []string{}
+	for _, s := range ss {
+		if encountered[s] == true {
+			continue
+		}
+		encountered[s] = true
+		result = append(result, s)
+	}
+	return result
+}

pkg/validate/networking.go

@@ -30,13 +30,14 @@ import (
 )
 
 const (
-	defaultDNSServer   string = "10.10.10.10"
-	defaultDNSSearch   string = "google.com"
-	defaultDNSOption   string = "ndots:8"
-	resolvConfigPath   string = "/etc/resolv.conf"
-	nginxImage         string = "nginx"
-	nginxContainerPort int32  = 80
-	nginxHostPort      int32  = 8000
+	defaultDNSServer            string = "10.10.10.10"
+	defaultDNSSearch            string = "google.com"
+	defaultDNSOption            string = "ndots:8"
+	resolvConfigPath            string = "/etc/resolv.conf"
+	nginxImage                  string = "nginx"
+	nginxContainerPort          int32  = 80
+	nginxHostPortForPortMapping int32  = 8000
+	nginxHostPortForPortForward int32  = 8001
 )
 
 var _ = framework.KubeDescribe("Networking", func() {
@@ -97,7 +98,7 @@ var _ = framework.KubeDescribe("Networking", func() {
 			startContainer(rc, containerID)
 
 			By("check the port mapping with only container port")
-			checkNginxMainPage(rc, podID, false)
+			checkNginxMainPage(rc, podID, 0)
 		})
 
 		It("runtime should support port mapping with host port and container port [Conformance]", func() {
@@ -106,7 +107,7 @@ var _ = framework.KubeDescribe("Networking", func() {
 			portMappings := []*runtimeapi.PortMapping{
 				{
 					ContainerPort: nginxContainerPort,
-					HostPort:      nginxHostPort,
+					HostPort:      nginxHostPortForPortMapping,
 				},
 			}
 			podID, podConfig = createPodSandboxWithPortMapping(rc, portMappings)
@@ -118,7 +119,7 @@ var _ = framework.KubeDescribe("Networking", func() {
 			startContainer(rc, containerID)
 
 			By("check the port mapping with host port and container port")
-			checkNginxMainPage(rc, "", true)
+			checkNginxMainPage(rc, "", nginxHostPortForPortMapping)
 		})
 	})
 })
@@ -182,14 +183,14 @@ func createNginxContainer(rc internalapi.RuntimeService, ic internalapi.ImageMan
 }
 
 // checkNginxMainPage check if the we can get the main page of nginx via given IP:port.
-func checkNginxMainPage(c internalapi.RuntimeService, podID string, localHost bool) {
+func checkNginxMainPage(c internalapi.RuntimeService, podID string, hostPort int32) {
 	By("get the IP:port needed to be checked")
 	var err error
 	var resp *http.Response
 
 	url := "http://"
-	if localHost {
-		url += "127.0.0.1:" + strconv.Itoa(int(nginxHostPort))
+	if hostPort != 0 {
+		url += "127.0.0.1:" + strconv.Itoa(int(hostPort))
 	} else {
 		status := getPodSandboxStatus(c, podID)
 		Expect(status.GetNetwork()).NotTo(BeNil(), "The network in status should not be nil.")

pkg/validate/streaming.go

@@ -18,6 +18,7 @@ package validate
 
 import (
 	"bytes"
+	"fmt"
 	"io"
 	"net/http"
 	"net/url"
@@ -245,7 +246,7 @@ func checkPortForward(c internalapi.RuntimeService, portForwardSeverURL string)
 	framework.ExpectNoError(err, "failed to create spdy round tripper")
 	url := parseURL(c, portForwardSeverURL)
 	dialer := spdy.NewDialer(upgrader, &http.Client{Transport: transport}, "POST", url)
-	pf, err := portforward.New(dialer, []string{"8000:80"}, stopChan, readyChan, os.Stdout, os.Stderr)
+	pf, err := portforward.New(dialer, []string{fmt.Sprintf("%d:80", nginxHostPortForPortForward)}, stopChan, readyChan, os.Stdout, os.Stderr)
 	framework.ExpectNoError(err, "failed to create port forward for %q", portForwardSeverURL)
 
 	go func() {
@@ -255,7 +256,7 @@ func checkPortForward(c internalapi.RuntimeService, portForwardSeverURL string)
 		framework.ExpectNoError(err, "failed to start port forward for %q", portForwardSeverURL)
 	}()
 
-	By("check if we can get nginx main page via localhost:8000")
-	checkNginxMainPage(c, "", true)
+	By(fmt.Sprintf("check if we can get nginx main page via localhost:%d", nginxHostPortForPortForward))
+	checkNginxMainPage(c, "", nginxHostPortForPortForward)
 	framework.Logf("Check port forward url %q succeed", portForwardSeverURL)
 }