Merge pull request #10219 from sharifelgamal/fix-download-test

refactor download only test for better visibility

test/integration/aaa_download_only_test.go

@@ -42,7 +42,6 @@ import (
 func TestDownloadOnly(t *testing.T) {
 	// Stores the startup run result for later error messages
 	var rrr *RunResult
-
 	profile := UniqueProfileName("download-only")
 	ctx, cancel := context.WithTimeout(context.Background(), Minutes(30))
 	defer Cleanup(t, profile, cancel)
@@ -54,26 +53,31 @@ func TestDownloadOnly(t *testing.T) {
 		constants.NewestKubernetesVersion,
 	}
 
+	// Small optimization, don't run the exact same set of tests twice
+	if constants.DefaultKubernetesVersion == constants.NewestKubernetesVersion {
+		versions = versions[:len(versions)-1]
+	}
+
 	for _, v := range versions {
 		t.Run(v, func(t *testing.T) {
 			defer PostMortemLogs(t, profile)
 
-			// --force to avoid uid check
-			args := append([]string{"start", "-o=json", "--download-only", "-p", profile, "--force", "--alsologtostderr", fmt.Sprintf("--kubernetes-version=%s", v)}, StartArgs()...)
+			t.Run("json-events", func(t *testing.T) {
+				// --force to avoid uid check
+				args := append([]string{"start", "-o=json", "--download-only", "-p", profile, "--force", "--alsologtostderr", fmt.Sprintf("--kubernetes-version=%s", v), fmt.Sprintf("--container-runtime=%s", containerRuntime)}, StartArgs()...)
+				rt, err := Run(t, exec.CommandContext(ctx, Target(), args...))
+				if rrr == nil {
+					// Preserve the initial run-result for debugging
+					rrr = rt
+				}
+				if err != nil {
+					t.Errorf("failed to download only. args: %q %v", args, err)
+				}
 
-			rt, err := Run(t, exec.CommandContext(ctx, Target(), args...))
-			if rrr == nil {
-				// Preserve the initial run-result for debugging
-				rrr = rt
-			}
-			if err != nil {
-				t.Errorf("failed to download only. args: %q %v", args, err)
-			}
-			t.Run("check json events", func(t *testing.T) {
 				s := bufio.NewScanner(bytes.NewReader(rt.Stdout.Bytes()))
 				for s.Scan() {
 					var rtObj map[string]interface{}
-					err = json.Unmarshal(s.Bytes(), &rtObj)
+					err := json.Unmarshal(s.Bytes(), &rtObj)
 					if err != nil {
 						t.Errorf("failed to parse output: %v", err)
 					} else if step, ok := rtObj["data"]; ok {
@@ -86,23 +90,36 @@ func TestDownloadOnly(t *testing.T) {
 				}
 			})
 
-			// skip for none, as none driver does not have preload feature.
-			if !NoneDriver() {
+			preloadExists := false
+			t.Run("preload-exists", func(t *testing.T) {
+				// skip for none, as none driver does not have preload feature.
+				if NoneDriver() {
+					t.Skip("None driver does not have preload")
+				}
 				if download.PreloadExists(v, containerRuntime, true) {
 					// Just make sure the tarball path exists
 					if _, err := os.Stat(download.TarballPath(v, containerRuntime)); err != nil {
 						t.Errorf("failed to verify preloaded tarball file exists: %v", err)
 					}
-					return
+					preloadExists = true
+				} else {
+					t.Skip("No preload image")
+				}
+			})
+
+			t.Run("cached-images", func(t *testing.T) {
+				// skip verify for cache images if --driver=none
+				if NoneDriver() {
+					t.Skip("None driver has no cache")
+				}
+				if preloadExists {
+					t.Skip("Preload exists, images won't be cached")
+				}
+				imgs, err := images.Kubeadm("", v)
+				if err != nil {
+					t.Errorf("failed to get kubeadm images for %v: %+v", v, err)
 				}
-			}
-			imgs, err := images.Kubeadm("", v)
-			if err != nil {
-				t.Errorf("failed to get kubeadm images for %v: %+v", v, err)
-			}
 
-			// skip verify for cache images if --driver=none
-			if !NoneDriver() {
 				for _, img := range imgs {
 					img = strings.Replace(img, ":", "_", 1) // for example kube-scheduler:v1.15.2 --> kube-scheduler_v1.15.2
 					fp := filepath.Join(localpath.MiniPath(), "cache", "images", img)
@@ -111,30 +128,35 @@ func TestDownloadOnly(t *testing.T) {
 						t.Errorf("expected image file exist at %q but got error: %v", fp, err)
 					}
 				}
-			}
+			})
 
-			// checking binaries downloaded (kubelet,kubeadm)
-			for _, bin := range constants.KubernetesReleaseBinaries {
-				fp := filepath.Join(localpath.MiniPath(), "cache", "linux", v, bin)
-				_, err := os.Stat(fp)
-				if err != nil {
+			t.Run("binaries", func(t *testing.T) {
+				// checking binaries downloaded (kubelet,kubeadm)
+				for _, bin := range constants.KubernetesReleaseBinaries {
+					fp := filepath.Join(localpath.MiniPath(), "cache", "linux", v, bin)
+					_, err := os.Stat(fp)
+					if err != nil {
+						t.Errorf("expected the file for binary exist at %q but got error %v", fp, err)
+					}
+				}
+			})
+
+			t.Run("kubectl", func(t *testing.T) {
+				// If we are on darwin/windows, check to make sure OS specific kubectl has been downloaded
+				// as well for the `minikube kubectl` command
+				if runtime.GOOS == "linux" {
+					t.Skip("Test for darwin and windows")
+				}
+				binary := "kubectl"
+				if runtime.GOOS == "windows" {
+					binary = "kubectl.exe"
+				}
+				fp := filepath.Join(localpath.MiniPath(), "cache", runtime.GOOS, v, binary)
+				if _, err := os.Stat(fp); err != nil {
 					t.Errorf("expected the file for binary exist at %q but got error %v", fp, err)
 				}
-			}
+			})
 
-			// If we are on darwin/windows, check to make sure OS specific kubectl has been downloaded
-			// as well for the `minikube kubectl` command
-			if runtime.GOOS == "linux" {
-				return
-			}
-			binary := "kubectl"
-			if runtime.GOOS == "windows" {
-				binary = "kubectl.exe"
-			}
-			fp := filepath.Join(localpath.MiniPath(), "cache", runtime.GOOS, v, binary)
-			if _, err := os.Stat(fp); err != nil {
-				t.Errorf("expected the file for binary exist at %q but got error %v", fp, err)
-			}
 		})
 	}