From ed53a3cf3b1064120c2fb85e36d7459e7991cadb Mon Sep 17 00:00:00 2001 From: x7upLime Date: Tue, 6 Dec 2022 18:06:51 +0200 Subject: [PATCH 01/14] Hints about distinction between minikube cache and KicDriver cache --- pkg/minikube/download/download_test.go | 4 ++-- pkg/minikube/download/image.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/minikube/download/download_test.go b/pkg/minikube/download/download_test.go index 3ae3731555f3..6386e0e1aaa0 100644 --- a/pkg/minikube/download/download_test.go +++ b/pkg/minikube/download/download_test.go @@ -168,7 +168,7 @@ func testImageToCache(t *testing.T) { var group sync.WaitGroup group.Add(2) dlCall := func() { - if err := ImageToCache("testimg"); err != nil { + if err := ImageToMinikubeCache("testimg"); err != nil { t.Errorf("Failed to download preload: %+v", err) } group.Done() @@ -193,7 +193,7 @@ func testImageToDaemon(t *testing.T) { var group sync.WaitGroup group.Add(2) dlCall := func() { - if err := ImageToCache("testimg"); err != nil { + if err := ImageToMinikubeCache("testimg"); err != nil { t.Errorf("Failed to download preload: %+v", err) } group.Done() diff --git a/pkg/minikube/download/image.go b/pkg/minikube/download/image.go index 254a98ff1658..024ae4a8e0e6 100644 --- a/pkg/minikube/download/image.go +++ b/pkg/minikube/download/image.go @@ -46,7 +46,7 @@ var ( ) // imagePathInCache returns path in local cache directory -func imagePathInCache(img string) string { +func imagePathInMinikubeCache(img string) string { f := filepath.Join(detect.KICCacheDir(), path.Base(img)+".tar") f = localpath.SanitizeCacheDir(f) return f @@ -54,7 +54,7 @@ func imagePathInCache(img string) string { // ImageExistsInCache if img exist in local cache directory func ImageExistsInCache(img string) bool { - f := imagePathInCache(img) + f := imagePathInMinikubeCache(img) // Check if image exists locally klog.Infof("Checking for %s in local cache directory", img) From 02b6b58d41a3da1bb3f22473746e8f835959035c Mon Sep 17 00:00:00 2001 From: x7upLime Date: Tue, 6 Dec 2022 18:09:42 +0200 Subject: [PATCH 02/14] Adds description to beginDownloadKicBaseImage() --- pkg/minikube/node/cache.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/minikube/node/cache.go b/pkg/minikube/node/cache.go index 7acaee0474f6..b2660e04a5f8 100644 --- a/pkg/minikube/node/cache.go +++ b/pkg/minikube/node/cache.go @@ -114,7 +114,11 @@ func doCacheBinaries(k8sVersion, containerRuntime, driverName, binariesURL strin return machine.CacheBinariesForBootstrapper(k8sVersion, viper.GetString(cmdcfg.Bootstrapper), existingBinaries, binariesURL) } -// beginDownloadKicBaseImage downloads the kic image +// beginDownloadKicBaseImage +// Its behaviour changes based on on ClusterConfig and flags.. +// It downloads the tar archive of the specified kicbase image to the local minikube cache (if not already present) +// It updates the KicDriver's cache with the just downloaded image archive for offline usage +// It pulls the kicbase image to the KicDriver at the specified digest func beginDownloadKicBaseImage(g *errgroup.Group, cc *config.ClusterConfig, downloadOnly bool) { klog.Infof("Beginning downloading kic base image for %s with %s", cc.Driver, cc.KubernetesConfig.ContainerRuntime) From ee3b4bcd262eb79ccb89c329fec0afe91ec8c4b6 Mon Sep 17 00:00:00 2001 From: x7upLime Date: Tue, 6 Dec 2022 18:17:10 +0200 Subject: [PATCH 03/14] Moves tag-check logic inside ImageToCache() We cannot cache :latest images, this check is currently done inside download.CacheToDaemon which prevents from moving a :latest from minikube cache to docker. Moving this check here to prevent pulling a :latest in the first place --- pkg/minikube/download/image.go | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/pkg/minikube/download/image.go b/pkg/minikube/download/image.go index 024ae4a8e0e6..60554aeb8d1c 100644 --- a/pkg/minikube/download/image.go +++ b/pkg/minikube/download/image.go @@ -87,9 +87,19 @@ func ImageExistsInDaemon(img string) bool { var checkImageExistsInDaemon = ImageExistsInDaemon -// ImageToCache downloads img (if not present in cache) and writes it to the local cache directory -func ImageToCache(img string) error { - f := imagePathInCache(img) +// ImageToCache +// downloads specified container image in tar format, to local minikube's cache +// does nothing if image is already present. +func ImageToMinikubeCache(img string) error { + tag, ref, err := parseImage(img) + // do not use cache if image is set in format :latest + if _, ok := ref.(name.Tag); ok { + if tag.Name() == "latest" { + return fmt.Errorf("can't cache 'latest' tag") + } + } + + f := imagePathInMinikubeCache(img) fileLock := f + ".lock" releaser, err := lockDownload(fileLock) @@ -118,14 +128,6 @@ func ImageToCache(img string) error { c := make(chan v1.Update, 200) klog.Infof("Writing %s to local cache", img) - ref, err := name.ParseReference(img) - if err != nil { - return errors.Wrap(err, "parsing reference") - } - tag, err := name.NewTag(strings.Split(img, "@")[0]) - if err != nil { - return errors.Wrap(err, "parsing tag") - } klog.V(3).Infof("Getting image %v", ref) i, err := remote.Image(ref, remote.WithPlatform(defaultPlatform)) if err != nil { From 132c8e1ffa7d8175c59450871b9c77f9140d3e53 Mon Sep 17 00:00:00 2001 From: x7upLime Date: Tue, 6 Dec 2022 18:26:48 +0200 Subject: [PATCH 04/14] Adds docker-agnostic version of kicdriver cache functions --- pkg/minikube/download/image.go | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/pkg/minikube/download/image.go b/pkg/minikube/download/image.go index 60554aeb8d1c..3d0e64f647c0 100644 --- a/pkg/minikube/download/image.go +++ b/pkg/minikube/download/image.go @@ -33,6 +33,7 @@ import ( "github.com/google/go-containerregistry/pkg/v1/tarball" "github.com/pkg/errors" "k8s.io/klog/v2" + "k8s.io/minikube/pkg/drivers/kic/oci" "k8s.io/minikube/pkg/minikube/detect" "k8s.io/minikube/pkg/minikube/image" "k8s.io/minikube/pkg/minikube/localpath" @@ -70,6 +71,19 @@ func ImageExistsInCache(img string) bool { var checkImageExistsInCache = ImageExistsInCache +// ImageExistsInKICDriver +// checks for the specified image in the container engine's local cache +func ImageExistsInKicDriver(ociBin, img string) bool { + klog.Infof("Checking for %s in local KICdriver's cache", img) + inCache := oci.IsImageInCache(ociBin, img) + if inCache { + klog.Infof("Found %s in local KICdriver's cache, skipping pull", img) + return true + } else { + return false + } +} + // ImageExistsInDaemon if img exist in local docker daemon func ImageExistsInDaemon(img string) bool { // Check if image exists locally @@ -198,9 +212,18 @@ func parseImage(img string) (*name.Tag, name.Reference, error) { return &tag, ref, nil } +// CacheToKICDriver +// loads a locally minikube-cached container image, to the KIC-driver's cache +func CacheToKicDriver(ociBin string, img string) (error) { + p := imagePathInMinikubeCache(img) + resp, err := oci.ArchiveToDriverCache(ociBin, p) + klog.V(2).Infof("response: %s", resp) + return err +} + // CacheToDaemon loads image from tarball in the local cache directory to the local docker daemon func CacheToDaemon(img string) error { - p := imagePathInCache(img) + p := imagePathInMinikubeCache(img) tag, ref, err := parseImage(img) if err != nil { From 0ed09666156e14b2e4139bdf773160777379ee06 Mon Sep 17 00:00:00 2001 From: x7upLime Date: Tue, 6 Dec 2022 18:34:47 +0200 Subject: [PATCH 05/14] Adds docker-agnostic image pull logic This a simpler version of the ImageToDaemon; here the docker save/load logic is thrown away since its already carried inside the previous calls to ImageToMinikubeCache and CacheToKicdriver. This function's only concern is to pull the specified image from remote thus using the proper digest. --- pkg/minikube/download/image.go | 37 ++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/pkg/minikube/download/image.go b/pkg/minikube/download/image.go index 3d0e64f647c0..91685c900319 100644 --- a/pkg/minikube/download/image.go +++ b/pkg/minikube/download/image.go @@ -246,6 +246,43 @@ func CacheToDaemon(img string) error { return err } +// ImageToKicDriver +// Makes a direct pull of the specified image to the kicdriver's cache +// maintaining reference to the image digest. +func ImageToKicDriver(ociBin, img string) error { + _, ref, err := parseImage(img) + if err != nil { + return err + } + + fileLock := filepath.Join(detect.KICCacheDir(), path.Base(img)+".d.lock") + fileLock = localpath.SanitizeCacheDir(fileLock) + releaser, err := lockDownload(fileLock) + if releaser != nil { + defer releaser.Release() + } + if err != nil { + return err + } + + if ImageExistsInKicDriver(ociBin, img) { + klog.Infof("%s exists in KicDriver, skipping pull", img) + return nil + } + + + if DownloadMock != nil { + klog.Infof("Mock download: %s -> daemon", img) + return DownloadMock(img, "daemon") + } + + klog.V(3).Infof("Pulling image %v", ref) + if _, err := oci.PullImage(ociBin, img); err != nil { + return errors.Wrap(err, "pulling remote image") + } + return nil +} + // ImageToDaemon downloads img (if not present in daemon) and writes it to the local docker daemon func ImageToDaemon(img string) error { fileLock := filepath.Join(detect.KICCacheDir(), path.Base(img)+".d.lock") From 2c68b459c3cdff3f999869ddb132370419ff636b Mon Sep 17 00:00:00 2001 From: x7upLime Date: Tue, 6 Dec 2022 18:43:42 +0200 Subject: [PATCH 06/14] Moves interactions with kicDriver's bin inside oci driver package --- pkg/drivers/kic/oci/cache.go | 35 +++++++++++++++++++++++++++++++++ pkg/drivers/kic/oci/download.go | 11 +++++++++++ 2 files changed, 46 insertions(+) create mode 100644 pkg/drivers/kic/oci/cache.go create mode 100644 pkg/drivers/kic/oci/download.go diff --git a/pkg/drivers/kic/oci/cache.go b/pkg/drivers/kic/oci/cache.go new file mode 100644 index 000000000000..2b03506743b0 --- /dev/null +++ b/pkg/drivers/kic/oci/cache.go @@ -0,0 +1,35 @@ +package oci + +import ( + "os/exec" + "strings" + + "k8s.io/minikube/pkg/minikube/image" +) + +// ToDriverCache +// calls OCIBIN's load command at specified path: +// loads the archived container image at provided PATH. +func ArchiveToDriverCache(ociBin, path string) (string, error) { + _, err := runCmd(exec.Command(ociBin, "load", "-i", path)) + return "", err +} + +// IsInCache +// searches in OCIBIN's cache for the IMG; returns true if found. no error handling +func IsImageInCache(ociBin, img string) (bool) { + res, err := runCmd(exec.Command(ociBin, "images", "--format", "{{.Repository}}:{{.Tag}}@{{.Digest}}")) + if err != nil { + // only the docker binary seems to have this issue.. + // the docker.io/ substring is cut from the output and formatting doesn't help + if ociBin == Docker { + img = image.TrimDockerIO(img) + } + + if strings.Contains(res.Stdout.String(), img){ + return true + } + } + + return false +} diff --git a/pkg/drivers/kic/oci/download.go b/pkg/drivers/kic/oci/download.go new file mode 100644 index 000000000000..534512328889 --- /dev/null +++ b/pkg/drivers/kic/oci/download.go @@ -0,0 +1,11 @@ +package oci + +import ( + "os/exec" + "bytes" +) + +func PullImage(ociBin, img string) (bytes.Buffer, error) { + res, err := runCmd(exec.Command(ociBin, "pull", "--quiet", img)) + return res.Stdout, err +} From 5e0a5b229084dfe88f012b4eca234d1e72ff5fcc Mon Sep 17 00:00:00 2001 From: x7upLime Date: Tue, 6 Dec 2022 18:47:02 +0200 Subject: [PATCH 07/14] Makes beginDownloadkicbaseimage docker-agnostic --- pkg/minikube/node/cache.go | 48 +++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 27 deletions(-) diff --git a/pkg/minikube/node/cache.go b/pkg/minikube/node/cache.go index b2660e04a5f8..dacd93f230da 100644 --- a/pkg/minikube/node/cache.go +++ b/pkg/minikube/node/cache.go @@ -34,7 +34,7 @@ import ( "k8s.io/minikube/pkg/minikube/config" "k8s.io/minikube/pkg/minikube/constants" "k8s.io/minikube/pkg/minikube/download" - "k8s.io/minikube/pkg/minikube/driver" + // "k8s.io/minikube/pkg/minikube/driver" "k8s.io/minikube/pkg/minikube/exit" "k8s.io/minikube/pkg/minikube/image" "k8s.io/minikube/pkg/minikube/localpath" @@ -142,14 +142,14 @@ func beginDownloadKicBaseImage(g *errgroup.Group, cc *config.ClusterConfig, down var err error var isFromCache bool - if driver.IsDocker(cc.Driver) && download.ImageExistsInDaemon(img) && !downloadOnly { - klog.Infof("%s exists in daemon, skipping load", img) + if !downloadOnly && download.ImageExistsInKicDriver(cc.Driver, img) { + klog.Infof("%s exists in KicDriver, skipping load", img) finalImg = img return nil } klog.Infof("Downloading %s to local cache", img) - err = download.ImageToCache(img) + err = download.ImageToMinikubeCache(img) if err == nil { klog.Infof("successfully saved %s as a tarball", img) finalImg = img @@ -158,31 +158,25 @@ func beginDownloadKicBaseImage(g *errgroup.Group, cc *config.ClusterConfig, down return err } - if cc.Driver == driver.Podman { - return fmt.Errorf("not yet implemented, see issue #8426") - } - if driver.IsDocker(cc.Driver) { - klog.Infof("Loading %s from local cache", img) - err = download.CacheToDaemon(img) - if err == nil { - klog.Infof("successfully loaded %s from cached tarball", img) - isFromCache = true - } - } + klog.Infof("Loading %s from local cache", img) + err = download.CacheToKicDriver(cc.Driver, img) + if err == nil { + klog.Infof("successfully loaded %s from cached tarball", img) + isFromCache = true + } - if driver.IsDocker(cc.Driver) { - klog.Infof("Downloading %s to local daemon", img) - err = download.ImageToDaemon(img) - if err == nil { - klog.Infof("successfully downloaded %s", img) - finalImg = img - return nil - } else if isFromCache { - klog.Infof("use image loaded from cache %s", strings.Split(img, "@")[0]) - finalImg = strings.Split(img, "@")[0] - return nil - } + klog.Infof("Downloading %s to local KicDriver", img) + err = download.ImageToKicDriver(cc.Driver, img) + if err == nil { + klog.Infof("successfully downloaded %s", img) + finalImg = img + return nil + } else if isFromCache { + klog.Infof("use image loaded from cache %s", strings.Split(img, "@")[0]) + finalImg = strings.Split(img, "@")[0] + return nil } + klog.Infof("failed to download %s, will try fallback image if available: %v", img, err) } return fmt.Errorf("failed to download kic base image or any fallback image") From 5314b71235a553ce8ed8eec1f65ba8054d8dc26e Mon Sep 17 00:00:00 2001 From: x7upLime Date: Wed, 7 Dec 2022 13:30:27 +0200 Subject: [PATCH 08/14] Clean docker-specific functions from image.go in donwload package --- pkg/minikube/download/image.go | 137 --------------------------------- 1 file changed, 137 deletions(-) diff --git a/pkg/minikube/download/image.go b/pkg/minikube/download/image.go index 91685c900319..992c25ab5367 100644 --- a/pkg/minikube/download/image.go +++ b/pkg/minikube/download/image.go @@ -19,7 +19,6 @@ package download import ( "fmt" "os" - "os/exec" "path" "path/filepath" "runtime" @@ -28,14 +27,12 @@ import ( "github.com/cheggaaa/pb/v3" "github.com/google/go-containerregistry/pkg/name" v1 "github.com/google/go-containerregistry/pkg/v1" - "github.com/google/go-containerregistry/pkg/v1/daemon" "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/google/go-containerregistry/pkg/v1/tarball" "github.com/pkg/errors" "k8s.io/klog/v2" "k8s.io/minikube/pkg/drivers/kic/oci" "k8s.io/minikube/pkg/minikube/detect" - "k8s.io/minikube/pkg/minikube/image" "k8s.io/minikube/pkg/minikube/localpath" ) @@ -84,23 +81,6 @@ func ImageExistsInKicDriver(ociBin, img string) bool { } } -// ImageExistsInDaemon if img exist in local docker daemon -func ImageExistsInDaemon(img string) bool { - // Check if image exists locally - klog.Infof("Checking for %s in local docker daemon", img) - cmd := exec.Command("docker", "images", "--format", "{{.Repository}}:{{.Tag}}@{{.Digest}}") - if output, err := cmd.Output(); err == nil { - if strings.Contains(string(output), image.TrimDockerIO(img)) { - klog.Infof("Found %s in local docker daemon, skipping pull", img) - return true - } - } - // Else, pull it - return false -} - -var checkImageExistsInDaemon = ImageExistsInDaemon - // ImageToCache // downloads specified container image in tar format, to local minikube's cache // does nothing if image is already present. @@ -221,31 +201,6 @@ func CacheToKicDriver(ociBin string, img string) (error) { return err } -// CacheToDaemon loads image from tarball in the local cache directory to the local docker daemon -func CacheToDaemon(img string) error { - p := imagePathInMinikubeCache(img) - - tag, ref, err := parseImage(img) - if err != nil { - return err - } - // do not use cache if image is set in format :latest - if _, ok := ref.(name.Tag); ok { - if tag.Name() == "latest" { - return fmt.Errorf("can't cache 'latest' tag") - } - } - - i, err := tarball.ImageFromPath(p, tag) - if err != nil { - return errors.Wrap(err, "tarball") - } - - resp, err := daemon.Write(*tag, i) - klog.V(2).Infof("response: %s", resp) - return err -} - // ImageToKicDriver // Makes a direct pull of the specified image to the kicdriver's cache // maintaining reference to the image digest. @@ -282,95 +237,3 @@ func ImageToKicDriver(ociBin, img string) error { } return nil } - -// ImageToDaemon downloads img (if not present in daemon) and writes it to the local docker daemon -func ImageToDaemon(img string) error { - fileLock := filepath.Join(detect.KICCacheDir(), path.Base(img)+".d.lock") - fileLock = localpath.SanitizeCacheDir(fileLock) - - releaser, err := lockDownload(fileLock) - if releaser != nil { - defer releaser.Release() - } - if err != nil { - return err - } - - if checkImageExistsInDaemon(img) { - klog.Infof("%s exists in daemon, skipping pull", img) - return nil - } - // buffered channel - c := make(chan v1.Update, 200) - - klog.Infof("Writing %s to local daemon", img) - ref, err := name.ParseReference(img) - if err != nil { - return errors.Wrap(err, "parsing reference") - } - tag, err := name.NewTag(strings.Split(img, "@")[0]) - if err != nil { - return errors.Wrap(err, "parsing tag") - } - - if DownloadMock != nil { - klog.Infof("Mock download: %s -> daemon", img) - return DownloadMock(img, "daemon") - } - - klog.V(3).Infof("Getting image %v", ref) - i, err := remote.Image(ref, remote.WithPlatform(defaultPlatform)) - if err != nil { - if strings.Contains(err.Error(), "GitHub Docker Registry needs login") { - ErrGithubNeedsLogin := errors.New(err.Error()) - return ErrGithubNeedsLogin - } else if strings.Contains(err.Error(), "UNAUTHORIZED") { - ErrNeedsLogin := errors.New(err.Error()) - return ErrNeedsLogin - } - - return errors.Wrap(err, "getting remote image") - } - - klog.V(3).Infof("Writing image %v", tag) - errchan := make(chan error) - p := pb.Full.Start64(0) - fn := strings.Split(ref.Name(), "@")[0] - // abbreviate filename for progress - maxwidth := 30 - len("...") - if len(fn) > maxwidth { - fn = fn[0:maxwidth] + "..." - } - p.Set("prefix", " > "+fn+": ") - p.Set(pb.Bytes, true) - - // Just a hair less than 80 (standard terminal width) for aesthetics & pasting into docs - p.SetWidth(79) - - go func() { - _, err = daemon.Write(tag, i) - errchan <- err - }() - var update v1.Update -loop: - for { - select { - case update = <-c: - p.SetCurrent(update.Complete) - p.SetTotal(update.Total) - case err = <-errchan: - p.Finish() - if err != nil { - return errors.Wrap(err, "writing daemon image") - } - break loop - } - } - klog.V(3).Infof("Pulling image %v", ref) - // Pull digest - cmd := exec.Command("docker", "pull", "--quiet", img) - if _, err := cmd.Output(); err != nil { - return errors.Wrap(err, "pulling remote image") - } - return nil -} From f4a511333eac580307e5c84e779852fad9d9e63e Mon Sep 17 00:00:00 2001 From: x7upLime Date: Wed, 7 Dec 2022 14:17:19 +0200 Subject: [PATCH 09/14] gofmts files --- go.sum | 1 + pkg/drivers/kic/oci/cache.go | 8 ++++---- pkg/drivers/kic/oci/download.go | 2 +- pkg/minikube/download/image.go | 12 +++++------- pkg/minikube/node/cache.go | 3 +-- 5 files changed, 12 insertions(+), 14 deletions(-) diff --git a/go.sum b/go.sum index 5e8586cb5de6..f993a0580a4a 100644 --- a/go.sum +++ b/go.sum @@ -1405,6 +1405,7 @@ golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.3.0 h1:w8ZOecv6NaNa/zC8944JTU3vz4u6Lagfk4RPQxv92NQ= golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= diff --git a/pkg/drivers/kic/oci/cache.go b/pkg/drivers/kic/oci/cache.go index 2b03506743b0..5f2bad07d342 100644 --- a/pkg/drivers/kic/oci/cache.go +++ b/pkg/drivers/kic/oci/cache.go @@ -3,7 +3,7 @@ package oci import ( "os/exec" "strings" - + "k8s.io/minikube/pkg/minikube/image" ) @@ -17,7 +17,7 @@ func ArchiveToDriverCache(ociBin, path string) (string, error) { // IsInCache // searches in OCIBIN's cache for the IMG; returns true if found. no error handling -func IsImageInCache(ociBin, img string) (bool) { +func IsImageInCache(ociBin, img string) bool { res, err := runCmd(exec.Command(ociBin, "images", "--format", "{{.Repository}}:{{.Tag}}@{{.Digest}}")) if err != nil { // only the docker binary seems to have this issue.. @@ -25,8 +25,8 @@ func IsImageInCache(ociBin, img string) (bool) { if ociBin == Docker { img = image.TrimDockerIO(img) } - - if strings.Contains(res.Stdout.String(), img){ + + if strings.Contains(res.Stdout.String(), img) { return true } } diff --git a/pkg/drivers/kic/oci/download.go b/pkg/drivers/kic/oci/download.go index 534512328889..9148114be38f 100644 --- a/pkg/drivers/kic/oci/download.go +++ b/pkg/drivers/kic/oci/download.go @@ -1,8 +1,8 @@ package oci import ( - "os/exec" "bytes" + "os/exec" ) func PullImage(ociBin, img string) (bytes.Buffer, error) { diff --git a/pkg/minikube/download/image.go b/pkg/minikube/download/image.go index 992c25ab5367..f08804461e2a 100644 --- a/pkg/minikube/download/image.go +++ b/pkg/minikube/download/image.go @@ -76,9 +76,8 @@ func ImageExistsInKicDriver(ociBin, img string) bool { if inCache { klog.Infof("Found %s in local KICdriver's cache, skipping pull", img) return true - } else { - return false } + return false } // ImageToCache @@ -194,8 +193,8 @@ func parseImage(img string) (*name.Tag, name.Reference, error) { // CacheToKICDriver // loads a locally minikube-cached container image, to the KIC-driver's cache -func CacheToKicDriver(ociBin string, img string) (error) { - p := imagePathInMinikubeCache(img) +func CacheToKicDriver(ociBin string, img string) error { + p := imagePathInMinikubeCache(img) resp, err := oci.ArchiveToDriverCache(ociBin, p) klog.V(2).Infof("response: %s", resp) return err @@ -209,7 +208,7 @@ func ImageToKicDriver(ociBin, img string) error { if err != nil { return err } - + fileLock := filepath.Join(detect.KICCacheDir(), path.Base(img)+".d.lock") fileLock = localpath.SanitizeCacheDir(fileLock) releaser, err := lockDownload(fileLock) @@ -219,13 +218,12 @@ func ImageToKicDriver(ociBin, img string) error { if err != nil { return err } - + if ImageExistsInKicDriver(ociBin, img) { klog.Infof("%s exists in KicDriver, skipping pull", img) return nil } - if DownloadMock != nil { klog.Infof("Mock download: %s -> daemon", img) return DownloadMock(img, "daemon") diff --git a/pkg/minikube/node/cache.go b/pkg/minikube/node/cache.go index dacd93f230da..8a1a9b39b547 100644 --- a/pkg/minikube/node/cache.go +++ b/pkg/minikube/node/cache.go @@ -34,7 +34,6 @@ import ( "k8s.io/minikube/pkg/minikube/config" "k8s.io/minikube/pkg/minikube/constants" "k8s.io/minikube/pkg/minikube/download" - // "k8s.io/minikube/pkg/minikube/driver" "k8s.io/minikube/pkg/minikube/exit" "k8s.io/minikube/pkg/minikube/image" "k8s.io/minikube/pkg/minikube/localpath" @@ -163,7 +162,7 @@ func beginDownloadKicBaseImage(g *errgroup.Group, cc *config.ClusterConfig, down if err == nil { klog.Infof("successfully loaded %s from cached tarball", img) isFromCache = true - } + } klog.Infof("Downloading %s to local KicDriver", img) err = download.ImageToKicDriver(cc.Driver, img) From e7b3a54b3a1cdd1412e5855c9f66f361b5cda9b4 Mon Sep 17 00:00:00 2001 From: x7upLime Date: Wed, 7 Dec 2022 14:38:28 +0200 Subject: [PATCH 10/14] prevents makte test from complaining --- pkg/drivers/kic/oci/cache.go | 16 ++++++++++++++++ pkg/drivers/kic/oci/download.go | 16 ++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/pkg/drivers/kic/oci/cache.go b/pkg/drivers/kic/oci/cache.go index 5f2bad07d342..558eee2b9997 100644 --- a/pkg/drivers/kic/oci/cache.go +++ b/pkg/drivers/kic/oci/cache.go @@ -1,3 +1,19 @@ +/* +Copyright 2022 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 oci import ( diff --git a/pkg/drivers/kic/oci/download.go b/pkg/drivers/kic/oci/download.go index 9148114be38f..dad58db2187f 100644 --- a/pkg/drivers/kic/oci/download.go +++ b/pkg/drivers/kic/oci/download.go @@ -1,3 +1,19 @@ +/* +Copyright 2022 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 oci import ( From 5886e74bc73954509931849ec4f04759f42168d5 Mon Sep 17 00:00:00 2001 From: x7upLime Date: Fri, 9 Dec 2022 10:10:41 +0200 Subject: [PATCH 11/14] Refactors for more noticeable distinction minikube/driver cache --- pkg/minikube/download/download_test.go | 4 ++-- pkg/minikube/download/image.go | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/pkg/minikube/download/download_test.go b/pkg/minikube/download/download_test.go index 6386e0e1aaa0..5a29e47d45f0 100644 --- a/pkg/minikube/download/download_test.go +++ b/pkg/minikube/download/download_test.go @@ -163,7 +163,7 @@ func testImageToCache(t *testing.T) { downloadNum := 0 DownloadMock = mockSleepDownload(&downloadNum) - checkImageExistsInCache = func(img string) bool { return downloadNum > 0 } + checkImageExistsInMinikubeCache = func(img string) bool { return downloadNum > 0 } var group sync.WaitGroup group.Add(2) @@ -188,7 +188,7 @@ func testImageToDaemon(t *testing.T) { downloadNum := 0 DownloadMock = mockSleepDownload(&downloadNum) - checkImageExistsInCache = func(img string) bool { return downloadNum > 0 } + checkImageExistsInMinikubeCache = func(img string) bool { return downloadNum > 0 } var group sync.WaitGroup group.Add(2) diff --git a/pkg/minikube/download/image.go b/pkg/minikube/download/image.go index f08804461e2a..5cc629d98951 100644 --- a/pkg/minikube/download/image.go +++ b/pkg/minikube/download/image.go @@ -50,15 +50,15 @@ func imagePathInMinikubeCache(img string) string { return f } -// ImageExistsInCache if img exist in local cache directory -func ImageExistsInCache(img string) bool { +// ImageExistsInCache if img exist in local minikube cache directory +func ImageExistsInMinikubeCache(img string) bool { f := imagePathInMinikubeCache(img) // Check if image exists locally - klog.Infof("Checking for %s in local cache directory", img) + klog.Infof("Checking for %s in local minikube cache directory", img) if st, err := os.Stat(f); err == nil { if st.Size() > 0 { - klog.Infof("Found %s in local cache directory, skipping pull", img) + klog.Infof("Found %s in local minikube cache directory, skipping pull", img) return true } } @@ -66,7 +66,7 @@ func ImageExistsInCache(img string) bool { return false } -var checkImageExistsInCache = ImageExistsInCache +var checkImageExistsInMinikubeCache = ImageExistsInMinikubeCache // ImageExistsInKICDriver // checks for the specified image in the container engine's local cache @@ -103,13 +103,13 @@ func ImageToMinikubeCache(img string) error { return err } - if checkImageExistsInCache(img) { - klog.Infof("%s exists in cache, skipping pull", img) + if checkImageExistsInMinikubeCache(img) { + klog.Infof("%s exists in minikube cache, skipping pull", img) return nil } if err := os.MkdirAll(filepath.Dir(f), 0777); err != nil { - return errors.Wrapf(err, "making cache image directory: %s", f) + return errors.Wrapf(err, "making minikube cache image directory: %s", f) } if DownloadMock != nil { @@ -120,7 +120,7 @@ func ImageToMinikubeCache(img string) error { // buffered channel c := make(chan v1.Update, 200) - klog.Infof("Writing %s to local cache", img) + klog.Infof("Writing %s to local minikube cache", img) klog.V(3).Infof("Getting image %v", ref) i, err := remote.Image(ref, remote.WithPlatform(defaultPlatform)) if err != nil { From cbfae92d6cf3b0df796603590693f3b4b67f9c25 Mon Sep 17 00:00:00 2001 From: x7upLime Date: Sat, 10 Dec 2022 20:08:49 +0200 Subject: [PATCH 12/14] Adds command output for KicDriver pull/load we only return err, because messages are already carried by the KicDriver bin itself --- pkg/drivers/kic/oci/cache.go | 10 +++++++--- pkg/drivers/kic/oci/download.go | 12 ++++++++---- pkg/minikube/download/image.go | 7 ++++--- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/pkg/drivers/kic/oci/cache.go b/pkg/drivers/kic/oci/cache.go index 558eee2b9997..e6fcd4de2f1c 100644 --- a/pkg/drivers/kic/oci/cache.go +++ b/pkg/drivers/kic/oci/cache.go @@ -17,6 +17,7 @@ limitations under the License. package oci import ( + "os" "os/exec" "strings" @@ -26,9 +27,12 @@ import ( // ToDriverCache // calls OCIBIN's load command at specified path: // loads the archived container image at provided PATH. -func ArchiveToDriverCache(ociBin, path string) (string, error) { - _, err := runCmd(exec.Command(ociBin, "load", "-i", path)) - return "", err +func ArchiveToDriverCache(ociBin, path string) error { + cmd := exec.Command(ociBin, "load", "-i", path) + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + err := cmd.Run() + return err } // IsInCache diff --git a/pkg/drivers/kic/oci/download.go b/pkg/drivers/kic/oci/download.go index dad58db2187f..cd92e8edc033 100644 --- a/pkg/drivers/kic/oci/download.go +++ b/pkg/drivers/kic/oci/download.go @@ -17,11 +17,15 @@ limitations under the License. package oci import ( - "bytes" "os/exec" + "strings" ) -func PullImage(ociBin, img string) (bytes.Buffer, error) { - res, err := runCmd(exec.Command(ociBin, "pull", "--quiet", img)) - return res.Stdout, err +func PullImage(ociBin, img string) error { + rr, err := runCmd(exec.Command(ociBin, "pull", "--quiet", img)) + if strings.Contains(rr.Stdout.String(), "Loaded image:") { + return nil + } + return err + } diff --git a/pkg/minikube/download/image.go b/pkg/minikube/download/image.go index 5cc629d98951..3ae147900772 100644 --- a/pkg/minikube/download/image.go +++ b/pkg/minikube/download/image.go @@ -195,8 +195,7 @@ func parseImage(img string) (*name.Tag, name.Reference, error) { // loads a locally minikube-cached container image, to the KIC-driver's cache func CacheToKicDriver(ociBin string, img string) error { p := imagePathInMinikubeCache(img) - resp, err := oci.ArchiveToDriverCache(ociBin, p) - klog.V(2).Infof("response: %s", resp) + err := oci.ArchiveToDriverCache(ociBin, p) return err } @@ -230,7 +229,9 @@ func ImageToKicDriver(ociBin, img string) error { } klog.V(3).Infof("Pulling image %v", ref) - if _, err := oci.PullImage(ociBin, img); err != nil { + // an image pull for the digest at this point is not a bad thing.. + // images are pulled by layers and we already have the biggest part + if err := oci.PullImage(ociBin, img); err != nil { return errors.Wrap(err, "pulling remote image") } return nil From 6245b903ba28c72a0dea0dd74383bda1da43a34b Mon Sep 17 00:00:00 2001 From: x7upLime Date: Sat, 10 Dec 2022 20:10:40 +0200 Subject: [PATCH 13/14] Adds two more output steps: cacheToKicDriver & digestPull ..with some handsome emojis --- pkg/minikube/node/cache.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/minikube/node/cache.go b/pkg/minikube/node/cache.go index 8a1a9b39b547..ab40991036e6 100644 --- a/pkg/minikube/node/cache.go +++ b/pkg/minikube/node/cache.go @@ -122,7 +122,7 @@ func beginDownloadKicBaseImage(g *errgroup.Group, cc *config.ClusterConfig, down klog.Infof("Beginning downloading kic base image for %s with %s", cc.Driver, cc.KubernetesConfig.ContainerRuntime) register.Reg.SetStep(register.PullingBaseImage) - out.Step(style.Pulling, "Pulling base image ...") + out.Step(style.Pulling, "Pulling base image to minikube cache ...") g.Go(func() error { baseImg := cc.KicBaseImage if baseImg == kic.BaseImage && len(cc.KubernetesConfig.ImageRepository) != 0 { @@ -157,6 +157,7 @@ func beginDownloadKicBaseImage(g *errgroup.Group, cc *config.ClusterConfig, down return err } + out.Step(style.Waiting, "Loading cached image to KicDriver...") klog.Infof("Loading %s from local cache", img) err = download.CacheToKicDriver(cc.Driver, img) if err == nil { @@ -164,6 +165,7 @@ func beginDownloadKicBaseImage(g *errgroup.Group, cc *config.ClusterConfig, down isFromCache = true } + out.Step(style.FileDownload, "Downloading digest-specific layer to KicDriver...") klog.Infof("Downloading %s to local KicDriver", img) err = download.ImageToKicDriver(cc.Driver, img) if err == nil { From 1ae1acea7cd9af30a93c724811ee666da6c2a924 Mon Sep 17 00:00:00 2001 From: x7upLime <100956049+x7upLime@users.noreply.github.com> Date: Thu, 15 Dec 2022 14:37:18 +0200 Subject: [PATCH 14/14] Apply suggestions from code review Co-authored-by: Steven Powell <44844360+spowelljr@users.noreply.github.com> --- pkg/drivers/kic/oci/cache.go | 7 +++---- pkg/drivers/kic/oci/download.go | 5 ++++- pkg/minikube/download/image.go | 10 +++++----- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/pkg/drivers/kic/oci/cache.go b/pkg/drivers/kic/oci/cache.go index e6fcd4de2f1c..74525fd72472 100644 --- a/pkg/drivers/kic/oci/cache.go +++ b/pkg/drivers/kic/oci/cache.go @@ -24,18 +24,17 @@ import ( "k8s.io/minikube/pkg/minikube/image" ) -// ToDriverCache +// ArchiveToDriverCache // calls OCIBIN's load command at specified path: // loads the archived container image at provided PATH. func ArchiveToDriverCache(ociBin, path string) error { cmd := exec.Command(ociBin, "load", "-i", path) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr - err := cmd.Run() - return err + return cmd.Run() } -// IsInCache +// IsImageInCache // searches in OCIBIN's cache for the IMG; returns true if found. no error handling func IsImageInCache(ociBin, img string) bool { res, err := runCmd(exec.Command(ociBin, "images", "--format", "{{.Repository}}:{{.Tag}}@{{.Digest}}")) diff --git a/pkg/drivers/kic/oci/download.go b/pkg/drivers/kic/oci/download.go index cd92e8edc033..717bbfa7052d 100644 --- a/pkg/drivers/kic/oci/download.go +++ b/pkg/drivers/kic/oci/download.go @@ -23,9 +23,12 @@ import ( func PullImage(ociBin, img string) error { rr, err := runCmd(exec.Command(ociBin, "pull", "--quiet", img)) + if err != nil { + return err + } if strings.Contains(rr.Stdout.String(), "Loaded image:") { return nil } - return err + return fmt.Errorf("Image wasn't loaded: %s", rr.Stdout.String()) } diff --git a/pkg/minikube/download/image.go b/pkg/minikube/download/image.go index 3ae147900772..f905e0315ba2 100644 --- a/pkg/minikube/download/image.go +++ b/pkg/minikube/download/image.go @@ -43,14 +43,14 @@ var ( } ) -// imagePathInCache returns path in local cache directory +// imagePathInMinikubeCache returns path in local cache directory func imagePathInMinikubeCache(img string) string { f := filepath.Join(detect.KICCacheDir(), path.Base(img)+".tar") f = localpath.SanitizeCacheDir(f) return f } -// ImageExistsInCache if img exist in local minikube cache directory +// ImageExistsInMinikubeCache if img exist in local minikube cache directory func ImageExistsInMinikubeCache(img string) bool { f := imagePathInMinikubeCache(img) @@ -68,7 +68,7 @@ func ImageExistsInMinikubeCache(img string) bool { var checkImageExistsInMinikubeCache = ImageExistsInMinikubeCache -// ImageExistsInKICDriver +// ImageExistsInKicDriver // checks for the specified image in the container engine's local cache func ImageExistsInKicDriver(ociBin, img string) bool { klog.Infof("Checking for %s in local KICdriver's cache", img) @@ -80,7 +80,7 @@ func ImageExistsInKicDriver(ociBin, img string) bool { return false } -// ImageToCache +// ImageToMinikubeCache // downloads specified container image in tar format, to local minikube's cache // does nothing if image is already present. func ImageToMinikubeCache(img string) error { @@ -191,7 +191,7 @@ func parseImage(img string) (*name.Tag, name.Reference, error) { return &tag, ref, nil } -// CacheToKICDriver +// CacheToKicDriver // loads a locally minikube-cached container image, to the KIC-driver's cache func CacheToKicDriver(ociBin string, img string) error { p := imagePathInMinikubeCache(img)