Merge pull request #8373 from priyawadhwa/refactor

Refactor downloading kic base image

pkg/drivers/kic/types.go

@@ -36,14 +36,17 @@ var (
 	// BaseImage is the base image is used to spin up kic containers. it uses same base-image as kind.
 	BaseImage = fmt.Sprintf("gcr.io/k8s-minikube/kicbase:%s@sha256:%s", Version, baseImageSHA)
 
-	// BaseImageFallBack1 the fall back of BaseImage in case gcr.io is not available. stored in docker hub
-	// same image is push to https://github.com/kicbase/stable
-	BaseImageFallBack1 = fmt.Sprintf("kicbase/stable:%s@sha256:%s", Version, baseImageSHA)
-
-	// BaseImageFallBack2 the fall back of BaseImage in case gcr.io is not available. stored in github packages https://github.com/kubernetes/minikube/packages/206071
-	// github packages docker does _NOT_ support pulling by sha as mentioned in the docs:
-	// https://help.github.com/en/packages/using-github-packages-with-your-projects-ecosystem/configuring-docker-for-use-with-github-packages
-	BaseImageFallBack2 = fmt.Sprintf("docker.pkg.github.com/kubernetes/minikube/kicbase:%s", Version)
+	// FallbackImages are backup base images in case gcr isn't available
+	FallbackImages = []string{
+		// the fallback of BaseImage in case gcr.io is not available. stored in docker hub
+		// same image is push to https://github.com/kicbase/stable
+		fmt.Sprintf("kicbase/stable:%s@sha256:%s", Version, baseImageSHA),
+
+		// the fallback of BaseImage in case gcr.io is not available. stored in github packages https://github.com/kubernetes/minikube/packages/206071
+		// github packages docker does _NOT_ support pulling by sha as mentioned in the docs:
+		// https://help.github.com/en/packages/using-github-packages-with-your-projects-ecosystem/configuring-docker-for-use-with-github-packages
+		fmt.Sprintf("docker.pkg.github.com/kubernetes/minikube/kicbase:%s", Version),
+	}
 )
 
 // Config is configuration for the kic driver used by registry

pkg/minikube/image/image.go

@@ -18,6 +18,7 @@ package image
 
 import (
 	"context"
+	"fmt"
 	"io/ioutil"
 	"os"
 	"os/exec"
@@ -93,6 +94,18 @@ func ExistsImageInDaemon(img string) bool {
 	return false
 }
 
+// Tag returns just the image with the tag
+// eg image:tag@sha256:digest -> image:tag if there is an associated tag
+// if not possible, just return the initial img
+func Tag(img string) string {
+	split := strings.Split(img, ":")
+	if len(split) == 3 {
+		tag := strings.Split(split[1], "@")[0]
+		return fmt.Sprintf("%s:%s", split[0], tag)
+	}
+	return img
+}
+
 // WriteImageToDaemon write img to the local docker daemon
 func WriteImageToDaemon(img string) error {
 	glog.Infof("Writing %s to local daemon", img)

pkg/minikube/image/image_test.go

+/*
+Copyright 2020 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 image
+
+import "testing"
+
+func TestTag(t *testing.T) {
+	tcs := []struct {
+		image    string
+		expected string
+	}{
+		{
+			image:    "image:tag@sha256:digest",
+			expected: "image:tag",
+		}, {
+			image:    "image:tag",
+			expected: "image:tag",
+		}, {
+			image:    "image@sha256:digest",
+			expected: "image@sha256:digest",
+		}, {
+			image:    "image",
+			expected: "image",
+		},
+	}
+	for _, tc := range tcs {
+		t.Run(tc.image, func(t *testing.T) {
+			actual := Tag(tc.image)
+			if actual != tc.expected {
+				t.Errorf("actual does not match expected\nActual:%v\nExpected:%v\n", actual, tc.expected)
+			}
+		})
+	}
+}

pkg/minikube/node/cache.go

@@ -17,6 +17,7 @@ limitations under the License.
 package node
 
 import (
+	"fmt"
 	"os"
 	"runtime"
 	"strings"
@@ -77,7 +78,7 @@ func handleDownloadOnly(cacheGroup, kicGroup *errgroup.Group, k8sVersion string)
 		exit.WithError("Failed to cache kubectl", err)
 	}
 	waitCacheRequiredImages(cacheGroup)
-	waitDownloadKicArtifacts(kicGroup)
+	waitDownloadKicBaseImage(kicGroup)
 	if err := saveImagesToTarFromConfig(); err != nil {
 		exit.WithError("Failed to cache images to tar", err)
 	}
@@ -100,36 +101,41 @@ func doCacheBinaries(k8sVersion string) error {
 	return machine.CacheBinariesForBootstrapper(k8sVersion, viper.GetString(cmdcfg.Bootstrapper))
 }
 
-// BeginDownloadKicArtifacts downloads the kic image + preload tarball, returns true if preload is available
-func beginDownloadKicArtifacts(g *errgroup.Group, cc *config.ClusterConfig) {
-	glog.Infof("Beginning downloading kic artifacts for %s with %s", cc.Driver, cc.KubernetesConfig.ContainerRuntime)
-	if cc.Driver == "docker" {
-		if !image.ExistsImageInDaemon(cc.KicBaseImage) {
-			out.T(out.Pulling, "Pulling base image ...")
-			g.Go(func() error {
-				// TODO #8004 : make base-image respect --image-repository
-				glog.Infof("Downloading %s to local daemon", cc.KicBaseImage)
-				err := image.WriteImageToDaemon(cc.KicBaseImage)
-				if err != nil {
-					glog.Infof("failed to download base-image %q will try to download the fallback base-image %q instead.", cc.KicBaseImage, kic.BaseImageFallBack1)
-					cc.KicBaseImage = kic.BaseImageFallBack1
-					if err := image.WriteImageToDaemon(kic.BaseImageFallBack1); err != nil {
-						cc.KicBaseImage = kic.BaseImageFallBack2
-						glog.Infof("failed to docker hub base-image %q will try to download the github packages base-image %q instead.", cc.KicBaseImage, kic.BaseImageFallBack2)
-						return image.WriteImageToDaemon(kic.BaseImageFallBack2)
-					}
+// beginDownloadKicBaseImage downloads the kic image
+func beginDownloadKicBaseImage(g *errgroup.Group, cc *config.ClusterConfig) {
+	if cc.Driver != "docker" {
+		// TODO: driver == "podman"
+		glog.Info("Driver isn't docker, skipping base image download")
+		return
+	}
+	if image.ExistsImageInDaemon(cc.KicBaseImage) {
+		glog.Infof("%s exists in daemon, skipping pull", cc.KicBaseImage)
+		return
+	}
+
+	glog.Infof("Beginning downloading kic base image for %s with %s", cc.Driver, cc.KubernetesConfig.ContainerRuntime)
+	out.T(out.Pulling, "Pulling base image ...")
+	g.Go(func() error {
+		// TODO #8004 : make base-image respect --image-repository
+		for _, img := range append([]string{cc.KicBaseImage}, kic.FallbackImages...) {
+			glog.Infof("Downloading %s to local daemon", img)
+			err := image.WriteImageToDaemon(img)
+			if err == nil {
+				if img != cc.KicBaseImage {
+					out.WarningT(fmt.Sprintf("minikube was unable to download %s, but successfully downloaded %s\n minikube will use %s as a fallback image", image.Tag(cc.KicBaseImage), image.Tag(img), image.Tag(img)))
+					cc.KicBaseImage = img
 				}
+				glog.Infof("successfully downloaded %s", img)
 				return nil
-			})
+			}
+			glog.Infof("failed to download %s, will try fallback image if available: %v", img, err)
 		}
-	} else {
-		// TODO: driver == "podman"
-		glog.Info("Driver isn't docker, skipping base-image download")
-	}
+		return fmt.Errorf("failed to download kic base image or any fallback image")
+	})
 }
 
-// WaitDownloadKicArtifacts blocks until the required artifacts for KIC are downloaded.
-func waitDownloadKicArtifacts(g *errgroup.Group) {
+// waitDownloadKicBaseImage blocks until the base image for KIC is downloaded.
+func waitDownloadKicBaseImage(g *errgroup.Group) {
 	if err := g.Wait(); err != nil {
 		if err != nil {
 			if errors.Is(err, image.ErrGithubNeedsLogin) {

pkg/minikube/node/start.go

@@ -195,7 +195,7 @@ func Provision(cc *config.ClusterConfig, n *config.Node, apiServer bool) (comman
 	}
 
 	if driver.IsKIC(cc.Driver) {
-		beginDownloadKicArtifacts(&kicGroup, cc)
+		beginDownloadKicBaseImage(&kicGroup, cc)
 	}
 
 	if !driver.BareMetal(cc.Driver) {
@@ -209,7 +209,7 @@ func Provision(cc *config.ClusterConfig, n *config.Node, apiServer bool) (comman
 	}
 
 	handleDownloadOnly(&cacheGroup, &kicGroup, n.KubernetesVersion)
-	waitDownloadKicArtifacts(&kicGroup)
+	waitDownloadKicBaseImage(&kicGroup)
 
 	return startMachine(cc, n)