Skip to content

Commit 5c64880

Browse files
fix: fix empty tarball when generating image save
1 parent 796759d commit 5c64880

File tree

6 files changed

+79
-15
lines changed

6 files changed

+79
-15
lines changed

pkg/minikube/cruntime/containerd.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ func (r *Containerd) Disable() error {
265265
// ImageExists checks if image exists based on image name and optionally image sha
266266
func (r *Containerd) ImageExists(name string, sha string) bool {
267267
klog.Infof("Checking existence of image with name %q and sha %q", name, sha)
268-
c := exec.Command("sudo", "ctr", "-n=k8s.io", "images", "check")
268+
c := exec.Command("sudo", "ctr", "-n=k8s.io", "images", "ls", fmt.Sprintf("name==%s", name))
269269
// note: image name and image id's sha can be on different lines in ctr output
270270
if rr, err := r.Runner.RunCmd(c); err != nil ||
271271
!strings.Contains(rr.Output(), name) ||

pkg/minikube/cruntime/cri.go

+26-3
Original file line numberDiff line numberDiff line change
@@ -220,10 +220,28 @@ func removeCRIImage(cr CommandRunner, name string) error {
220220
crictl := getCrictlPath(cr)
221221
args := append([]string{crictl, "rmi"}, name)
222222
c := exec.Command("sudo", args...)
223-
if _, err := cr.RunCmd(c); err != nil {
224-
return errors.Wrap(err, "crictl")
223+
var err error
224+
if _, err = cr.RunCmd(c); err == nil {
225+
return nil
225226
}
226-
return nil
227+
// the reason why we are doing this is that
228+
// unlike other tool, podman assume the image has a localhost registry (not docker.io)
229+
// if the image is loaded with a tarball without a registry specified in tag
230+
// see https://github.com/containers/podman/issues/15974
231+
232+
// then retry with dockerio prefix
233+
if _, err := cr.RunCmd(exec.Command("sudo", crictl, "rmi", AddDockerIO(name))); err == nil {
234+
return nil
235+
}
236+
237+
// then retry with localhost prefix
238+
if _, err := cr.RunCmd(exec.Command("sudo", crictl, "rmi", AddLocalhostPrefix(name))); err == nil {
239+
240+
return nil
241+
}
242+
243+
// if all of above failed, return original error
244+
return errors.Wrap(err, "crictl")
227245
}
228246

229247
// stopCRIContainers stops containers using crictl
@@ -358,3 +376,8 @@ func checkCNIPlugins(kubernetesVersion semver.Version) error {
358376
_, err := os.Stat("/opt/cni/bin")
359377
return err
360378
}
379+
380+
// Add localhost prefix if the registy part is missing
381+
func AddLocalhostPrefix(name string) string {
382+
return addRegistryPreix(name, "localhost")
383+
}

pkg/minikube/cruntime/docker.go

+10-4
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,7 @@ func (r *Docker) ListImages(ListImagesOptions) ([]ListImage, error) {
292292
result = append(result, ListImage{
293293
ID: strings.TrimPrefix(jsonImage.ID, "sha256:"),
294294
RepoDigests: []string{},
295-
RepoTags: []string{addDockerIO(repoTag)},
295+
RepoTags: []string{AddDockerIO(repoTag)},
296296
Size: fmt.Sprintf("%d", size),
297297
})
298298
}
@@ -696,14 +696,20 @@ func dockerImagesPreloaded(runner command.Runner, images []string) bool {
696696
}
697697

698698
// Add docker.io prefix
699-
func addDockerIO(name string) string {
699+
func AddDockerIO(name string) string {
700+
return addRegistryPreix(name, "docker.io")
701+
}
702+
func addRegistryPreix(name string, prefix string) string {
700703
var reg, usr, img string
701704
p := strings.SplitN(name, "/", 2)
702-
if len(p) > 1 && strings.Contains(p[0], ".") {
705+
// containing . means that it is a valid url for registry(e.g. xxx.io)
706+
// containing : means it contains some port number (e.g. xxx:5432)
707+
// it may also start with localhost, which is also regarded as a valid registry
708+
if len(p) > 1 && (strings.Contains(p[0], ".") || strings.Contains(p[0], ":") || strings.Contains(p[0], "localhost")) {
703709
reg = p[0]
704710
img = p[1]
705711
} else {
706-
reg = "docker.io"
712+
reg = prefix
707713
img = name
708714
p = strings.SplitN(img, "/", 2)
709715
if len(p) > 1 {

pkg/minikube/image/image.go

+19-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package image
1919
import (
2020
"context"
2121
"fmt"
22+
"io"
2223
"os"
2324
"path/filepath"
2425
"runtime"
@@ -207,7 +208,24 @@ func UploadCachedImage(imgName string) error {
207208
klog.Infof("error parsing image name %s tag %v ", imgName, err)
208209
return err
209210
}
210-
return uploadImage(tag, imagePathInCache(imgName))
211+
if err := uploadImage(tag, imagePathInCache(imgName)); err != nil {
212+
// this time try determine image tags from tarball
213+
214+
manifest, err := tarball.LoadManifest(func() (io.ReadCloser, error) {
215+
return os.Open(imagePathInCache(imgName))
216+
})
217+
if len(manifest) == 0 || len(manifest[0].RepoTags) == 0 {
218+
return fmt.Errorf("failed to determine the image tag from tarball")
219+
}
220+
221+
tag, err = name.NewTag(manifest[0].RepoTags[0], name.WeakValidation)
222+
if err != nil {
223+
klog.Infof("error parsing image name: %s ", err.Error())
224+
return err
225+
}
226+
return uploadImage(tag, imagePathInCache(imgName))
227+
}
228+
return nil
211229
}
212230

213231
func uploadImage(tag name.Tag, p string) error {

pkg/minikube/machine/cache_images.go

+16-3
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,7 @@ func removeExistingImage(r cruntime.Manager, src string, imgName string) error {
335335
}
336336

337337
errStr := strings.ToLower(err.Error())
338-
if !strings.Contains(errStr, "no such image") {
338+
if !strings.Contains(errStr, "no such image") && !strings.Contains(errStr, "unable to remove the image") {
339339
return errors.Wrap(err, "removing image")
340340
}
341341

@@ -472,9 +472,22 @@ func transferAndSaveImage(cr command.Runner, k8s config.KubernetesConfig, dst st
472472
if err != nil {
473473
return errors.Wrap(err, "runtime")
474474
}
475+
found := false
476+
// the reason why we are doing this is that
477+
// unlike other tool, podman assume the image has a localhost registry (not docker.io)
478+
// if the image is loaded with a tarball without a registry specified in tag
479+
// see https://github.com/containers/podman/issues/15974
480+
tryImageExist := []string{imgName, cruntime.AddDockerIO(imgName), cruntime.AddLocalhostPrefix(imgName)}
481+
for _, img := range tryImageExist {
482+
if !r.ImageExists(img, "") {
483+
found = true
484+
break
485+
}
486+
}
475487

476-
if !r.ImageExists(imgName, "") {
477-
return errors.Errorf("image %s not found", imgName)
488+
if !found {
489+
// if all of this failed, return the original error
490+
return fmt.Errorf("image not found %s", imgName)
478491
}
479492

480493
klog.Infof("Saving image to: %s", dst)

test/integration/functional_test.go

+7-3
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,10 @@ import (
5555
"github.com/phayes/freeport"
5656
"github.com/pkg/errors"
5757
"golang.org/x/build/kubernetes/api"
58+
"k8s.io/minikube/pkg/minikube/cruntime"
5859
)
5960

60-
const echoServerImg = "docker.io/kicbase/echo-server"
61+
const echoServerImg = "kicbase/echo-server"
6162

6263
// validateFunc are for subtests that share a single setup
6364
type validateFunc func(context.Context, *testing.T, string)
@@ -424,8 +425,11 @@ func validateImageCommands(ctx context.Context, t *testing.T, profile string) {
424425
if err != nil {
425426
t.Fatalf("saving image from minikube to daemon: %v\n%s", err, rr.Output())
426427
}
427-
428-
rr, err = Run(t, exec.CommandContext(ctx, "docker", "image", "inspect", taggedImage))
428+
imageToDelete := taggedImage
429+
if ContainerRuntime() == "crio" {
430+
imageToDelete = cruntime.AddLocalhostPrefix(imageToDelete)
431+
}
432+
rr, err = Run(t, exec.CommandContext(ctx, "docker", "image", "inspect", imageToDelete))
429433
if err != nil {
430434
t.Fatalf("expected image to be loaded into Docker, but image was not found: %v\n%s", err, rr.Output())
431435
}

0 commit comments

Comments
 (0)