From 9fb9e940f1e1c0c6455cdc9b84ec5cd4c41ca0c7 Mon Sep 17 00:00:00 2001 From: Andriy Dzikh Date: Thu, 6 May 2021 15:12:31 -0700 Subject: [PATCH 1/8] Prevent `minikube start --download-only` from downloading binaries that are part of the Kubernetes release binaries. --- pkg/minikube/machine/cache_binaries.go | 18 +++++++++++++++++- pkg/minikube/machine/cache_binaries_test.go | 2 +- pkg/minikube/node/cache.go | 6 +++--- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/pkg/minikube/machine/cache_binaries.go b/pkg/minikube/machine/cache_binaries.go index 3bb9d4d998da..778ca693e35a 100644 --- a/pkg/minikube/machine/cache_binaries.go +++ b/pkg/minikube/machine/cache_binaries.go @@ -28,12 +28,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 { diff --git a/pkg/minikube/machine/cache_binaries_test.go b/pkg/minikube/machine/cache_binaries_test.go index 113d60132c04..274164c045a5 100644 --- a/pkg/minikube/machine/cache_binaries_test.go +++ b/pkg/minikube/machine/cache_binaries_test.go @@ -121,7 +121,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) } diff --git a/pkg/minikube/node/cache.go b/pkg/minikube/node/cache.go index 37b51aebe412..e100cc54106e 100644 --- a/pkg/minikube/node/cache.go +++ b/pkg/minikube/node/cache.go @@ -69,7 +69,7 @@ func beginCacheKubernetesImages(g *errgroup.Group, imageRepository string, k8sVe }) } -// HandleDownloadOnly caches appropariate binaries and images +// handleDownloadOnly caches appropariate binaries and images func handleDownloadOnly(cacheGroup, kicGroup *errgroup.Group, k8sVersion string) { // If --download-only, complete the remaining downloads and exit. if !viper.GetBool("download-only") { @@ -102,7 +102,7 @@ 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)) + return machine.CacheBinariesForBootstrapper(k8sVersion, viper.GetString(cmdcfg.Bootstrapper), constants.KubernetesReleaseBinaries) } // beginDownloadKicBaseImage downloads the kic image @@ -191,7 +191,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 From b441330f7e67864df00d90850ebb703d6db33e4b Mon Sep 17 00:00:00 2001 From: Andriy Dzikh Date: Tue, 18 May 2021 15:57:22 -0700 Subject: [PATCH 2/8] Create unit test for excluding binaries from caching. --- pkg/minikube/machine/cache_binaries_test.go | 34 +++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/pkg/minikube/machine/cache_binaries_test.go b/pkg/minikube/machine/cache_binaries_test.go index 274164c045a5..994a2a868fdb 100644 --- a/pkg/minikube/machine/cache_binaries_test.go +++ b/pkg/minikube/machine/cache_binaries_test.go @@ -20,6 +20,7 @@ import ( "fmt" "io/ioutil" "os" + "strings" "testing" "k8s.io/minikube/pkg/minikube/assets" @@ -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) + } +} From 8882de007b2f7d2d6709acbb6cb183a58aab3562 Mon Sep 17 00:00:00 2001 From: Andriy Dzikh Date: Thu, 20 May 2021 10:33:06 -0700 Subject: [PATCH 3/8] Add check to allow downloading of listed binaries if preload is missing. --- pkg/minikube/node/cache.go | 12 ++++++++---- pkg/minikube/node/start.go | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/pkg/minikube/node/cache.go b/pkg/minikube/node/cache.go index e100cc54106e..74082c2ed4f3 100644 --- a/pkg/minikube/node/cache.go +++ b/pkg/minikube/node/cache.go @@ -70,12 +70,12 @@ func beginCacheKubernetesImages(g *errgroup.Group, imageRepository string, k8sVe } // handleDownloadOnly caches appropariate binaries and images -func handleDownloadOnly(cacheGroup, kicGroup *errgroup.Group, k8sVersion string) { +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), constants.KubernetesReleaseBinaries) +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 diff --git a/pkg/minikube/node/start.go b/pkg/minikube/node/start.go index 7a9f1e304fcc..82542a43cc12 100644 --- a/pkg/minikube/node/start.go +++ b/pkg/minikube/node/start.go @@ -288,7 +288,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) From 9692cae4372d75f31a75e2aff9359cacdac76700 Mon Sep 17 00:00:00 2001 From: Andriy Dzikh Date: Thu, 20 May 2021 13:19:58 -0700 Subject: [PATCH 4/8] Add caching to PreloadExists to improve performance. --- pkg/minikube/download/preload.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/pkg/minikube/download/preload.go b/pkg/minikube/download/preload.go index 706537297083..7e4f9ab2a2a7 100644 --- a/pkg/minikube/download/preload.go +++ b/pkg/minikube/download/preload.go @@ -45,6 +45,15 @@ const ( PreloadVersion = "v11" // PreloadBucket is the name of the GCS bucket where preloaded volume tarballs exist PreloadBucket = "minikube-preloaded-volume-tarballs" + + // Enumeration for preload existence cache. + preloadExistsUNKNOWN = 0 + preloadExistsMISSING = 1 + preloadExistsEXISTS = 2 +) + +var ( + preloadExistsState int = preloadExistsUNKNOWN ) // TarballName returns name of the tarball @@ -100,10 +109,16 @@ func PreloadExists(k8sVersion, containerRuntime string, forcePreload ...bool) bo return false } + // If the preload existence is cached, just return that value. + if preloadExistsState != preloadExistsUNKNOWN { + return preloadExistsState == preloadExistsEXISTS + } + // Omit remote check if tarball exists locally targetPath := TarballPath(k8sVersion, containerRuntime) if _, err := checkCache(targetPath); err == nil { klog.Infof("Found local preload: %s", targetPath) + preloadExistsState = preloadExistsEXISTS return true } @@ -111,16 +126,19 @@ func PreloadExists(k8sVersion, containerRuntime string, forcePreload ...bool) bo resp, err := http.Head(url) if err != nil { klog.Warningf("%s fetch error: %v", url, err) + preloadExistsState = preloadExistsMISSING 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) + preloadExistsState = preloadExistsMISSING return false } klog.Infof("Found remote preload: %s", url) + preloadExistsState = preloadExistsEXISTS return true } @@ -183,6 +201,8 @@ func Preload(k8sVersion, containerRuntime string) error { } } + // If the download was successful, mark off that the preload exists in the cache. + preloadExistsState = preloadExistsEXISTS return nil } From 38a60356e06b55fa3757af4e990b3860482fa9b6 Mon Sep 17 00:00:00 2001 From: Andriy Dzikh Date: Tue, 25 May 2021 13:04:14 -0700 Subject: [PATCH 5/8] Add comments to each enum value. --- pkg/minikube/download/preload.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/minikube/download/preload.go b/pkg/minikube/download/preload.go index 7e4f9ab2a2a7..4d21fb8e1574 100644 --- a/pkg/minikube/download/preload.go +++ b/pkg/minikube/download/preload.go @@ -47,9 +47,9 @@ const ( PreloadBucket = "minikube-preloaded-volume-tarballs" // Enumeration for preload existence cache. - preloadExistsUNKNOWN = 0 - preloadExistsMISSING = 1 - preloadExistsEXISTS = 2 + preloadExistsUNKNOWN = 0 // Value when preload status has not been checked. + preloadExistsMISSING = 1 // Value when preload has been checked and is missing. + preloadExistsEXISTS = 2 // Value when preload has been checked and is present. ) var ( From 9638241aeb4ec130af99d28baf091f5ff2516d95 Mon Sep 17 00:00:00 2001 From: Andriy Dzikh Date: Tue, 25 May 2021 16:31:32 -0700 Subject: [PATCH 6/8] Rename preload enum and use iota for values. --- pkg/minikube/download/preload.go | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/pkg/minikube/download/preload.go b/pkg/minikube/download/preload.go index 4d21fb8e1574..43a89203cabe 100644 --- a/pkg/minikube/download/preload.go +++ b/pkg/minikube/download/preload.go @@ -45,15 +45,17 @@ const ( PreloadVersion = "v11" // PreloadBucket is the name of the GCS bucket where preloaded volume tarballs exist PreloadBucket = "minikube-preloaded-volume-tarballs" +) - // Enumeration for preload existence cache. - preloadExistsUNKNOWN = 0 // Value when preload status has not been checked. - preloadExistsMISSING = 1 // Value when preload has been checked and is missing. - preloadExistsEXISTS = 2 // Value when preload has been checked and is present. +// Enumeration for preload existence cache. +const ( + preloadUnknown = iota // Value when preload status has not been checked. + preloadMissing = iota // Value when preload has been checked and is missing. + preloadPresent = iota // Value when preload has been checked and is present. ) var ( - preloadExistsState int = preloadExistsUNKNOWN + preloadState int = preloadUnknown ) // TarballName returns name of the tarball @@ -110,15 +112,15 @@ func PreloadExists(k8sVersion, containerRuntime string, forcePreload ...bool) bo } // If the preload existence is cached, just return that value. - if preloadExistsState != preloadExistsUNKNOWN { - return preloadExistsState == preloadExistsEXISTS + 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) - preloadExistsState = preloadExistsEXISTS + preloadState = preloadPresent return true } @@ -126,19 +128,19 @@ func PreloadExists(k8sVersion, containerRuntime string, forcePreload ...bool) bo resp, err := http.Head(url) if err != nil { klog.Warningf("%s fetch error: %v", url, err) - preloadExistsState = preloadExistsMISSING + 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) - preloadExistsState = preloadExistsMISSING + preloadState = preloadMissing return false } klog.Infof("Found remote preload: %s", url) - preloadExistsState = preloadExistsEXISTS + preloadState = preloadPresent return true } @@ -202,7 +204,7 @@ func Preload(k8sVersion, containerRuntime string) error { } // If the download was successful, mark off that the preload exists in the cache. - preloadExistsState = preloadExistsEXISTS + preloadState = preloadPresent return nil } From 89a6820936b9c23018f5ca5a063826610c6311eb Mon Sep 17 00:00:00 2001 From: Andriy Dzikh Date: Tue, 25 May 2021 17:36:46 -0700 Subject: [PATCH 7/8] Remove iota from all except first preload enum. --- pkg/minikube/download/preload.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/minikube/download/preload.go b/pkg/minikube/download/preload.go index 43a89203cabe..1eeecb971d53 100644 --- a/pkg/minikube/download/preload.go +++ b/pkg/minikube/download/preload.go @@ -50,8 +50,8 @@ const ( // Enumeration for preload existence cache. const ( preloadUnknown = iota // Value when preload status has not been checked. - preloadMissing = iota // Value when preload has been checked and is missing. - preloadPresent = iota // Value when preload has been checked and is present. + preloadMissing // Value when preload has been checked and is missing. + preloadPresent // Value when preload has been checked and is present. ) var ( From a89d404903f3fe92f82005b14cfb4c0b1c57df15 Mon Sep 17 00:00:00 2001 From: Andriy Dzikh Date: Wed, 26 May 2021 15:34:13 -0700 Subject: [PATCH 8/8] Skip binary check if preload exists. --- test/integration/aaa_download_only_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/integration/aaa_download_only_test.go b/test/integration/aaa_download_only_test.go index b48bcdb275a1..726395d15d1f 100644 --- a/test/integration/aaa_download_only_test.go +++ b/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)