-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
KicDriver -- less docker-centric approach for kicBase cache #15491
Closed
Closed
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
ed53a3c
Hints about distinction between minikube cache and KicDriver cache
x7upLime 02b6b58
Adds description to beginDownloadKicBaseImage()
x7upLime ee3b4bc
Moves tag-check logic inside ImageToCache()
x7upLime 132c8e1
Adds docker-agnostic version of kicdriver cache functions
x7upLime 0ed0966
Adds docker-agnostic image pull logic
x7upLime 2c68b45
Moves interactions with kicDriver's bin inside oci driver package
x7upLime 5e0a5b2
Makes beginDownloadkicbaseimage docker-agnostic
x7upLime 5314b71
Clean docker-specific functions from image.go in donwload package
x7upLime f4a5113
gofmts files
x7upLime fc7ef87
Merge branch 'master' into docker-agnostic
x7upLime e7b3a54
prevents makte test from complaining
x7upLime 5886e74
Refactors for more noticeable distinction minikube/driver cache
x7upLime cbfae92
Adds command output for KicDriver pull/load
x7upLime 6245b90
Adds two more output steps: cacheToKicDriver & digestPull
x7upLime 1ae1ace
Apply suggestions from code review
x7upLime File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
/* | ||
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 ( | ||
"os" | ||
"os/exec" | ||
"strings" | ||
|
||
"k8s.io/minikube/pkg/minikube/image" | ||
) | ||
|
||
// 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 | ||
return cmd.Run() | ||
} | ||
|
||
// 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}}")) | ||
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 | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
/* | ||
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 ( | ||
"os/exec" | ||
"strings" | ||
) | ||
|
||
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 fmt.Errorf("Image wasn't loaded: %s", rr.Stdout.String()) | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,7 +19,6 @@ package download | |
import ( | ||
"fmt" | ||
"os" | ||
"os/exec" | ||
"path" | ||
"path/filepath" | ||
"runtime" | ||
|
@@ -28,13 +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" | ||
) | ||
|
||
|
@@ -45,51 +43,56 @@ var ( | |
} | ||
) | ||
|
||
// imagePathInCache returns path in local cache directory | ||
func imagePathInCache(img string) string { | ||
// 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 cache directory | ||
func ImageExistsInCache(img string) bool { | ||
f := imagePathInCache(img) | ||
// ImageExistsInMinikubeCache 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 | ||
} | ||
} | ||
// Else, pull it | ||
return false | ||
} | ||
|
||
var checkImageExistsInCache = ImageExistsInCache | ||
var checkImageExistsInMinikubeCache = ImageExistsInMinikubeCache | ||
|
||
// 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 | ||
} | ||
// 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, pull it | ||
return false | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we keep logging here, or just blindly return inCache? |
||
} | ||
|
||
var checkImageExistsInDaemon = ImageExistsInDaemon | ||
// 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 { | ||
tag, ref, err := parseImage(img) | ||
// do not use cache if image is set in format <name>:latest | ||
if _, ok := ref.(name.Tag); ok { | ||
if tag.Name() == "latest" { | ||
return fmt.Errorf("can't cache 'latest' tag") | ||
} | ||
} | ||
|
||
// ImageToCache downloads img (if not present in cache) and writes it to the local cache directory | ||
func ImageToCache(img string) error { | ||
f := imagePathInCache(img) | ||
f := imagePathInMinikubeCache(img) | ||
fileLock := f + ".lock" | ||
|
||
releaser, err := lockDownload(fileLock) | ||
|
@@ -100,13 +103,13 @@ func ImageToCache(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 { | ||
|
@@ -117,15 +120,7 @@ func ImageToCache(img string) error { | |
// buffered channel | ||
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.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 { | ||
|
@@ -196,36 +191,25 @@ func parseImage(img string) (*name.Tag, name.Reference, error) { | |
return &tag, ref, nil | ||
} | ||
|
||
// CacheToDaemon loads image from tarball in the local cache directory to the local docker daemon | ||
func CacheToDaemon(img string) error { | ||
p := imagePathInCache(img) | ||
// CacheToKicDriver | ||
// loads a locally minikube-cached container image, to the KIC-driver's cache | ||
func CacheToKicDriver(ociBin string, img string) error { | ||
p := imagePathInMinikubeCache(img) | ||
err := oci.ArchiveToDriverCache(ociBin, p) | ||
return err | ||
} | ||
|
||
tag, ref, err := parseImage(img) | ||
// 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 | ||
} | ||
// do not use cache if image is set in format <name>: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 | ||
} | ||
|
||
// 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() | ||
|
@@ -234,80 +218,20 @@ func ImageToDaemon(img string) error { | |
return err | ||
} | ||
|
||
if checkImageExistsInDaemon(img) { | ||
klog.Infof("%s exists in daemon, skipping pull", img) | ||
if ImageExistsInKicDriver(ociBin, img) { | ||
klog.Infof("%s exists in KicDriver, 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 { | ||
// 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 | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this command succeeds this always returns
false
I'm assuming we should also be doing somestrings.Contains
logic.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo.. it should've been "if err == nil"
we only care to do the strings.Contains check if the command returns no error.
if err is returned or string is not found, it's a false