Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent downloading duplicate binaries already present in preload #11461

Merged
merged 8 commits into from
May 27, 2021
18 changes: 17 additions & 1 deletion pkg/minikube/machine/cache_binaries.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
36 changes: 35 additions & 1 deletion pkg/minikube/machine/cache_binaries_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"fmt"
"io/ioutil"
"os"
"strings"
"testing"

"k8s.io/minikube/pkg/minikube/assets"
Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
}
}
16 changes: 10 additions & 6 deletions pkg/minikube/node/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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) {
andriyDev marked this conversation as resolved.
Show resolved Hide resolved
existingBinaries = nil
}
return machine.CacheBinariesForBootstrapper(k8sVersion, viper.GetString(cmdcfg.Bootstrapper), existingBinaries)
}

// beginDownloadKicBaseImage downloads the kic image
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/minikube/node/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down