Offline: always transfer image if lookup fails, always download drivers

cmd/minikube/cmd/start.go

@@ -304,10 +304,8 @@ func runStart(cmd *cobra.Command, args []string) {
 	validateFlags(cmd, driverName)
 	validateUser(driverName)
 
-	// No need to install a driver in download-only mode
-	if !viper.GetBool(downloadOnly) {
-		updateDriver(driverName)
-	}
+	// Download & update the driver, even in --download-only mode
+	updateDriver(driverName)
 
 	k8sVersion, isUpgrade := getKubernetesVersion(existing)
 	config, err := generateCfgFromFlags(cmd, k8sVersion, driverName)

pkg/minikube/machine/cache_images.go

@@ -17,6 +17,7 @@ limitations under the License.
 package machine
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 	"os/exec"
@@ -109,34 +110,41 @@ func LoadImages(cc *config.MachineConfig, runner command.Runner, images []string
 	for _, image := range images {
 		image := image
 		g.Go(func() error {
-			ref, err := name.ParseReference(image, name.WeakValidation)
-			if err != nil {
-				return errors.Wrap(err, "image name reference")
-			}
-
-			img, err := retrieveImage(ref)
-			if err != nil {
-				return errors.Wrap(err, "fetching image")
-			}
-			cf, err := img.ConfigName()
-			hash := cf.Hex
-			if err != nil {
-				glog.Infof("error retrieving image manifest for %s to check if it already exists: %v", image, err)
-			} else if cr.ImageExists(image, hash) {
-				glog.Infof("skipping re-loading image %q because sha %q already exists ", image, hash)
+			err := needsTransfer(image, cr)
+			if err == nil {
 				return nil
 			}
-			if err := transferAndLoadImage(runner, cc.KubernetesConfig, image, cacheDir); err != nil {
-				glog.Warningf("Failed to load %s: %v", image, err)
-				return errors.Wrapf(err, "loading image %s", image)
-			}
-			return nil
+			glog.Infof("%q needs transfer: %v", image, err)
+			return transferAndLoadImage(runner, cc.KubernetesConfig, image, cacheDir)
 		})
 	}
 	if err := g.Wait(); err != nil {
 		return errors.Wrap(err, "loading cached images")
 	}
-	glog.Infoln("Successfully loaded all cached images.")
+	glog.Infoln("Successfully loaded all cached images")
+	return nil
+}
+
+// needsTransfer returns an error if an image needs to be retransfered
+func needsTransfer(image string, cr cruntime.Manager) error {
+	ref, err := name.ParseReference(image, name.WeakValidation)
+	if err != nil {
+		return errors.Wrap(err, "parse ref")
+	}
+
+	img, err := retrieveImage(ref)
+	if err != nil {
+		return errors.Wrap(err, "retrieve")
+	}
+
+	cf, err := img.ConfigName()
+	if err != nil {
+		return errors.Wrap(err, "image hash")
+	}
+
+	if !cr.ImageExists(image, cf.Hex) {
+		return fmt.Errorf("%q does not exist at hash %q in container runtime", image, cf.Hex)
+	}
 	return nil
 }
 
@@ -277,7 +285,7 @@ func transferAndLoadImage(cr command.Runner, k8s config.KubernetesConfig, imgNam
 		return errors.Wrapf(err, "%s load %s", r.Name(), dst)
 	}
 
-	glog.Infof("Successfully loaded image %s from cache", src)
+	glog.Infof("Transferred and loaded %s from cache", src)
 	return nil
 }
 
@@ -362,35 +370,39 @@ func CacheImage(image, dst string) error {
 
 	img, err := retrieveImage(ref)
 	if err != nil {
-		return errors.Wrap(err, "fetching image")
+		glog.Warningf("unable to retrieve image: %v", err)
 	}
 
 	glog.Infoln("OPENING: ", dstPath)
 	f, err := ioutil.TempFile(filepath.Dir(dstPath), filepath.Base(dstPath)+".*.tmp")
 	if err != nil {
 		return err
 	}
-	defer func() { // clean up temp files
-		err := os.Remove(f.Name())
-		if err != nil {
-			glog.Infof("Failed to clean up the temp file %s : %v", f.Name(), err)
+	defer func() {
+		// If we left behind a temp file, remove it.
+		_, err := os.Stat(f.Name())
+		if err == nil {
+			os.Remove(f.Name())
+			if err != nil {
+				glog.Warningf("Failed to clean up the temp file %s: %v", f.Name(), err)
+			}
 		}
 	}()
 	tag, err := name.NewTag(image, name.WeakValidation)
 	if err != nil {
-		return err
+		return errors.Wrap(err, "newtag")
 	}
 	err = tarball.Write(tag, img, &tarball.WriteOptions{}, f)
 	if err != nil {
-		return err
+		return errors.Wrap(err, "write")
 	}
 	err = f.Close()
 	if err != nil {
-		return err
+		return errors.Wrap(err, "close")
 	}
 	err = os.Rename(f.Name(), dstPath)
 	if err != nil {
-		return err
+		return errors.Wrap(err, "rename")
 	}
 	glog.Infof("%s exists", dst)
 	return nil
@@ -400,18 +412,20 @@ func retrieveImage(ref name.Reference) (v1.Image, error) {
 	glog.Infof("retrieving image: %+v", ref)
 	img, err := daemon.Image(ref)
 	if err == nil {
-		glog.Infof("found %s locally; caching", ref.Name())
-		return img, err
+		glog.Infof("found %s locally: %+v", ref.Name(), img)
+		return img, nil
+	}
+	// reference does not exist in the local daemon
+	if err != nil {
+		glog.Infof("daemon lookup for %+v: %v", ref, err)
 	}
-	glog.Infof("daemon image for %+v: %v", img, err)
+
 	img, err = remote.Image(ref, remote.WithAuthFromKeychain(authn.DefaultKeychain))
 	if err == nil {
-		return img, err
+		return img, nil
 	}
-	glog.Warningf("failed authn download for %+v (trying anon): %+v", ref, err)
+
+	glog.Warningf("authn lookup for %+v (trying anon): %+v", ref, err)
 	img, err = remote.Image(ref)
-	if err != nil {
-		glog.Warningf("failed anon download for %+v: %+v", ref, err)
-	}
 	return img, err
 }

test/integration/aab_offline_test.go

@@ -0,0 +1,62 @@
+// +build integration
+
+/*
+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 integration
+
+import (
+	"context"
+	"fmt"
+	"os"
+	"os/exec"
+	"testing"
+	"time"
+)
+
+func TestOffline(t *testing.T) {
+	t.Run("group", func(t *testing.T) {
+		for _, runtime := range []string{"docker", "crio", "containerd"} {
+			t.Run(runtime, func(t *testing.T) {
+				MaybeParallel(t)
+				WaitForStartSlot(t)
+
+				if runtime != "docker" && NoneDriver() {
+					t.Skipf("skipping %s - incompatible with none driver", t.Name())
+				}
+
+				profile := UniqueProfileName(fmt.Sprintf("offline-%s", runtime))
+				ctx, cancel := context.WithTimeout(context.Background(), 15*time.Minute)
+				defer CleanupWithLogs(t, profile, cancel)
+
+				startArgs := []string{"start", "-p", profile, "--alsologtostderr", "-v=1", "--wait=true", "--container-runtime", runtime}
+				startArgs = append(startArgs, StartArgs()...)
+				c := exec.CommandContext(ctx, Target(), startArgs...)
+				env := os.Environ()
+				// RFC1918 address that unlikely to host working a proxy server
+				env = append(env, "HTTP_PROXY=172.16.1.1:1")
+				env = append(env, "HTTP_PROXYS=172.16.1.1:1")
+				env = append(env, "DOCKER_HOST=172.16.1.1:1")
+				c.Env = env
+				rr, err := Run(t, c)
+				if err != nil {
+					// Fatal so that we may collect logs before stop/delete steps
+					t.Fatalf("%s failed: %v", rr.Args, err)
+				}
+			})
+		}
+	})
+}