Merge pull request #3990 from tstromberg/no-infinite-loops

Block on cache downloads before using the results
kubernetes · Mar 27, 2019 · 58a7045 · 58a7045
2 parents b927af4 + e98f2d7
commit 58a7045
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 19 deletions.
diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go
@@ -201,8 +201,16 @@ func runStart(cmd *cobra.Command, args []string) {
 	}
 
 	cr := configureRuntimes(host, runner)
+
+	// prepareHostEnvironment uses the downloaded images, so we need to wait for background task completion.
+	if viper.GetBool(cacheImages) {
+		console.OutStyle("waiting", "Waiting for image downloads to complete ...")
+		if err := cacheGroup.Wait(); err != nil {
+			glog.Errorln("Error caching images: ", err)
+		}
+	}
+
 	bs := prepareHostEnvironment(m, config.KubernetesConfig)
-	waitCacheImages(&cacheGroup)
 
 	// The kube config must be update must come before bootstrapping, otherwise health checks may use a stale IP
 	kubeconfig := updateKubeConfig(host, &config)
@@ -511,17 +519,6 @@ func configureRuntimes(h *host.Host, runner bootstrapper.CommandRunner) cruntime
 	return cr
 }
 
-// waitCacheImages blocks until the image cache jobs complete
-func waitCacheImages(g *errgroup.Group) {
-	if !viper.GetBool(cacheImages) {
-		return
-	}
-	console.OutStyle("waiting", "Waiting for image downloads to complete ...")
-	if err := g.Wait(); err != nil {
-		glog.Errorln("Error caching images: ", err)
-	}
-}
-
 // bootstrapCluster starts Kubernetes using the chosen bootstrapper
 func bootstrapCluster(bs bootstrapper.Bootstrapper, r cruntime.Manager, runner bootstrapper.CommandRunner, kc cfg.KubernetesConfig, preexisting bool) {
 	console.OutStyle("pulling", "Pulling images required by Kubernetes %s ...", kc.KubernetesVersion)

diff --git a/pkg/minikube/machine/cache_images.go b/pkg/minikube/machine/cache_images.go
@@ -97,7 +97,7 @@ func LoadImages(cmd bootstrapper.CommandRunner, images []string, cacheDir string
 		g.Go(func() error {
 			src := filepath.Join(cacheDir, image)
 			src = sanitizeCacheDir(src)
-			if err := LoadFromCacheBlocking(cmd, cc.KubernetesConfig, src); err != nil {
+			if err := loadImageFromCache(cmd, cc.KubernetesConfig, src); err != nil {
 				return errors.Wrapf(err, "loading image %s", src)
 			}
 			return nil
@@ -198,14 +198,12 @@ func getWindowsVolumeNameCmd(d string) (string, error) {
 	return vname, nil
 }
 
-// LoadFromCacheBlocking loads images from cache, blocking until loaded
-func LoadFromCacheBlocking(cr bootstrapper.CommandRunner, k8s config.KubernetesConfig, src string) error {
+// loadImageFromCache loads a single image from the cache
+func loadImageFromCache(cr bootstrapper.CommandRunner, k8s config.KubernetesConfig, src string) error {
 	glog.Infoln("Loading image from cache at ", src)
 	filename := filepath.Base(src)
-	for {
-		if _, err := os.Stat(src); err == nil {
-			break
-		}
+	if _, err := os.Stat(src); err == nil {
+		return err
 	}
 	dst := path.Join(tempLoadDir, filename)
 	f, err := assets.NewFileAsset(src, tempLoadDir, filename, "0777")