Merge pull request #11461 from andriyDev/RepeatBinaryDownloads

Prevent downloading duplicate binaries already present in preload

pkg/minikube/download/preload.go

@@ -47,6 +47,17 @@ const (
 	PreloadBucket = "minikube-preloaded-volume-tarballs"
 )
 
+// Enumeration for preload existence cache.
+const (
+	preloadUnknown = iota // Value when preload status has not been checked.
+	preloadMissing        // Value when preload has been checked and is missing.
+	preloadPresent        // Value when preload has been checked and is present.
+)
+
+var (
+	preloadState int = preloadUnknown
+)
+
 // TarballName returns name of the tarball
 func TarballName(k8sVersion, containerRuntime string) string {
 	if containerRuntime == "crio" {
@@ -100,27 +111,36 @@ func PreloadExists(k8sVersion, containerRuntime string, forcePreload ...bool) bo
 		return false
 	}
 
+	// If the preload existence is cached, just return that value.
+	if preloadState != preloadUnknown {
+		return preloadState == preloadPresent
+	}
+
 	// Omit remote check if tarball exists locally
 	targetPath := TarballPath(k8sVersion, containerRuntime)
 	if _, err := checkCache(targetPath); err == nil {
 		klog.Infof("Found local preload: %s", targetPath)
+		preloadState = preloadPresent
 		return true
 	}
 
 	url := remoteTarballURL(k8sVersion, containerRuntime)
 	resp, err := http.Head(url)
 	if err != nil {
 		klog.Warningf("%s fetch error: %v", url, err)
+		preloadState = preloadMissing
 		return false
 	}
 
 	// note: err won't be set if it's a 404
 	if resp.StatusCode != 200 {
 		klog.Warningf("%s status code: %d", url, resp.StatusCode)
+		preloadState = preloadMissing
 		return false
 	}
 
 	klog.Infof("Found remote preload: %s", url)
+	preloadState = preloadPresent
 	return true
 }
 
@@ -184,6 +204,8 @@ func Preload(k8sVersion, containerRuntime string) error {
 		}
 	}
 
+	// If the download was successful, mark off that the preload exists in the cache.
+	preloadState = preloadPresent
 	return nil
 }
 

pkg/minikube/machine/cache_binaries.go

@@ -29,12 +29,28 @@ import (
 	"k8s.io/minikube/pkg/minikube/download"
 )
 
+// isExcluded returns whether `binary` is expected to be excluded, based on `excludedBinaries`.
+func isExcluded(binary string, excludedBinaries []string) bool {
+	if excludedBinaries == nil {
+		return false
+	}
+	for _, excludedBinary := range excludedBinaries {
+		if binary == excludedBinary {
+			return true
+		}
+	}
+	return false
+}
+
 // CacheBinariesForBootstrapper will cache binaries for a bootstrapper
-func CacheBinariesForBootstrapper(version string, clusterBootstrapper string) error {
+func CacheBinariesForBootstrapper(version string, clusterBootstrapper string, excludeBinaries []string) error {
 	binaries := bootstrapper.GetCachedBinaryList(clusterBootstrapper)
 
 	var g errgroup.Group
 	for _, bin := range binaries {
+		if isExcluded(bin, excludeBinaries) {
+			continue
+		}
 		bin := bin // https://golang.org/doc/faq#closures_and_goroutines
 		g.Go(func() error {
 			if _, err := download.Binary(bin, version, "linux", runtime.GOARCH); err != nil {

pkg/minikube/machine/cache_binaries_test.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+	"strings"
 	"testing"
 
 	"k8s.io/minikube/pkg/minikube/assets"
@@ -121,7 +122,7 @@ func TestCacheBinariesForBootstrapper(t *testing.T) {
 	for _, test := range tc {
 		t.Run(test.version, func(t *testing.T) {
 			os.Setenv("MINIKUBE_HOME", test.minikubeHome)
-			err := CacheBinariesForBootstrapper(test.version, test.clusterBootstrapper)
+			err := CacheBinariesForBootstrapper(test.version, test.clusterBootstrapper, nil)
 			if err != nil && !test.err {
 				t.Fatalf("Got unexpected error %v", err)
 			}
@@ -131,3 +132,36 @@ func TestCacheBinariesForBootstrapper(t *testing.T) {
 		})
 	}
 }
+
+func TestExcludedBinariesNotDownloaded(t *testing.T) {
+	clusterBootstrapper := bootstrapper.Kubeadm
+	binaryList := bootstrapper.GetCachedBinaryList(clusterBootstrapper)
+	binaryToExclude := binaryList[0]
+
+	download.DownloadMock = func(src, dst string) error {
+		if strings.Contains(src, binaryToExclude) {
+			t.Errorf("Excluded binary was downloaded! Binary to exclude: %s", binaryToExclude)
+		}
+		return download.CreateDstDownloadMock(src, dst)
+	}
+
+	oldMinikubeHome := os.Getenv("MINIKUBE_HOME")
+	defer os.Setenv("MINIKUBE_HOME", oldMinikubeHome)
+
+	minikubeHome, err := ioutil.TempDir("/tmp", "")
+	if err != nil {
+		t.Fatalf("error during creating tmp dir: %v", err)
+	}
+	os.Setenv("MINIKUBE_HOME", minikubeHome)
+
+	defer func() { // clean up tempdir
+		err := os.RemoveAll(minikubeHome)
+		if err != nil {
+			t.Errorf("failed to clean up temp folder  %q", minikubeHome)
+		}
+	}()
+
+	if err := CacheBinariesForBootstrapper("v1.16.0", clusterBootstrapper, []string{binaryToExclude}); err != nil {
+		t.Errorf("Failed to cache binaries: %v", err)
+	}
+}

pkg/minikube/node/cache.go

@@ -69,13 +69,13 @@ func beginCacheKubernetesImages(g *errgroup.Group, imageRepository string, k8sVe
 	})
 }
 
-// HandleDownloadOnly caches appropariate binaries and images
-func handleDownloadOnly(cacheGroup, kicGroup *errgroup.Group, k8sVersion string) {
+// handleDownloadOnly caches appropariate binaries and images
+func handleDownloadOnly(cacheGroup, kicGroup *errgroup.Group, k8sVersion, containerRuntime string) {
 	// If --download-only, complete the remaining downloads and exit.
 	if !viper.GetBool("download-only") {
 		return
 	}
-	if err := doCacheBinaries(k8sVersion); err != nil {
+	if err := doCacheBinaries(k8sVersion, containerRuntime); err != nil {
 		exit.Error(reason.InetCacheBinaries, "Failed to cache binaries", err)
 	}
 	if _, err := CacheKubectlBinary(k8sVersion); err != nil {
@@ -101,8 +101,12 @@ func CacheKubectlBinary(k8sVersion string) (string, error) {
 }
 
 // doCacheBinaries caches Kubernetes binaries in the foreground
-func doCacheBinaries(k8sVersion string) error {
-	return machine.CacheBinariesForBootstrapper(k8sVersion, viper.GetString(cmdcfg.Bootstrapper))
+func doCacheBinaries(k8sVersion, containerRuntime string) error {
+	existingBinaries := constants.KubernetesReleaseBinaries
+	if !download.PreloadExists(k8sVersion, containerRuntime) {
+		existingBinaries = nil
+	}
+	return machine.CacheBinariesForBootstrapper(k8sVersion, viper.GetString(cmdcfg.Bootstrapper), existingBinaries)
 }
 
 // beginDownloadKicBaseImage downloads the kic image
@@ -191,7 +195,7 @@ func waitDownloadKicBaseImage(g *errgroup.Group) {
 	klog.Info("Successfully downloaded all kic artifacts")
 }
 
-// WaitCacheRequiredImages blocks until the required images are all cached.
+// waitCacheRequiredImages blocks until the required images are all cached.
 func waitCacheRequiredImages(g *errgroup.Group) {
 	if !viper.GetBool(cacheImages) {
 		return

pkg/minikube/node/start.go

@@ -289,7 +289,7 @@ func Provision(cc *config.ClusterConfig, n *config.Node, apiServer bool, delOnFa
 		return nil, false, nil, nil, errors.Wrap(err, "Failed to save config")
 	}
 
-	handleDownloadOnly(&cacheGroup, &kicGroup, n.KubernetesVersion)
+	handleDownloadOnly(&cacheGroup, &kicGroup, n.KubernetesVersion, cc.KubernetesConfig.ContainerRuntime)
 	waitDownloadKicBaseImage(&kicGroup)
 
 	return startMachine(cc, n, delOnFail)

test/integration/aaa_download_only_test.go

@@ -132,6 +132,9 @@ func TestDownloadOnly(t *testing.T) {
 			})
 
 			t.Run("binaries", func(t *testing.T) {
+				if preloadExists {
+					t.Skip("Preload exists, binaries are present within.")
+				}
 				// checking binaries downloaded (kubelet,kubeadm)
 				for _, bin := range constants.KubernetesReleaseBinaries {
 					fp := filepath.Join(localpath.MiniPath(), "cache", "linux", v, bin)