From e5101bc62f475da75e2d4041d7c2482f6403b214 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Tue, 16 Jan 2024 11:27:17 -0500 Subject: [PATCH] Minor cleanups Signed-off-by: Natalie Arellano --- cnb_image.go | 4 +-- image.go | 2 ++ layout/layout.go | 2 +- local/local.go | 2 +- locallayout/image.go | 13 ++++++++-- locallayout/{new_test.go => image_test.go} | 0 locallayout/new.go | 2 +- locallayout/store.go | 29 ++++++---------------- locallayout/v1_facade.go | 2 +- remote/remote.go | 2 +- 10 files changed, 26 insertions(+), 32 deletions(-) rename locallayout/{new_test.go => image_test.go} (100%) diff --git a/cnb_image.go b/cnb_image.go index 847a733d..d36b2f53 100644 --- a/cnb_image.go +++ b/cnb_image.go @@ -105,8 +105,6 @@ func (i *CNBImageCore) History() ([]v1.History, error) { return configFile.History, nil } -// Kind exposes the type of image (via the Store) that backs the imgutil.Image implementation. -// It could be `locallayout`, `remote`, or `layout`. func (i *CNBImageCore) Kind() string { storeType := fmt.Sprintf("%T", i.Store) parts := strings.Split(storeType, ".") @@ -351,7 +349,7 @@ func (i *CNBImageCore) Rebase(baseTopLayerDiffID string, withNewBase Image) erro if i.Kind() != withNewBase.Kind() { return fmt.Errorf("expected new base to be a %s image; got %s", i.Kind(), withNewBase.Kind()) } - newBase := withNewBase.UnderlyingImage() + newBase := withNewBase.UnderlyingImage() // FIXME: when all imgutil.Images are v1.Images, we can remove this part var err error i.Image, err = mutate.Rebase(i.Image, i.newV1ImageFacade(baseTopLayerDiffID), newBase) if err != nil { diff --git a/image.go b/image.go index 057b3047..d6feaef0 100644 --- a/image.go +++ b/image.go @@ -26,6 +26,8 @@ type Image interface { GetLayer(diffID string) (io.ReadCloser, error) History() ([]v1.History, error) Identifier() (Identifier, error) + // Kind exposes the type of image that backs the imgutil.Image implementation. + // It could be `local`, `locallayout`, `remote`, or `layout`. Kind() string Label(string) (string, error) Labels() (map[string]string, error) diff --git a/layout/layout.go b/layout/layout.go index 5f421cc2..715fc805 100644 --- a/layout/layout.go +++ b/layout/layout.go @@ -152,7 +152,7 @@ func (i *Image) Identifier() (imgutil.Identifier, error) { } func (i *Image) Kind() string { - return fmt.Sprintf("%T", i) + return `layout` } func (i *Image) Label(key string) (string, error) { diff --git a/local/local.go b/local/local.go index c6dc0a72..1a5edcec 100644 --- a/local/local.go +++ b/local/local.go @@ -116,7 +116,7 @@ func (i *Image) Identifier() (imgutil.Identifier, error) { } func (i *Image) Kind() string { - return fmt.Sprintf("%T", i) + return `local` } func (i *Image) Label(key string) (string, error) { diff --git a/locallayout/image.go b/locallayout/image.go index 19962b28..99a27919 100644 --- a/locallayout/image.go +++ b/locallayout/image.go @@ -41,16 +41,25 @@ func (i idStringer) String() string { } // GetLayer returns an io.ReadCloser with uncompressed layer data. -// The layer will always have data, even if that means downloading all of the image layers from the daemon. +// The layer will always have data, even if that means downloading ALL the image layers from the daemon. func (i *Image) GetLayer(diffID string) (io.ReadCloser, error) { layerHash, err := v1.NewHash(diffID) if err != nil { return nil, err } + layer, err := i.LayerByDiffID(layerHash) + if err == nil { + // this avoids downloading ALL the image layers from the daemon + // if the layer is available locally + // (e.g., it was added using AddLayer). + if size, err := layer.Size(); err != nil && size != -1 { + return layer.Uncompressed() + } + } if err = i.ensureLayers(); err != nil { return nil, err } - layer, err := i.LayerByDiffID(layerHash) + layer, err = i.LayerByDiffID(layerHash) if err != nil { return nil, fmt.Errorf("image %q does not contain layer with diff ID %q", i.Name(), layerHash.String()) } diff --git a/locallayout/new_test.go b/locallayout/image_test.go similarity index 100% rename from locallayout/new_test.go rename to locallayout/image_test.go diff --git a/locallayout/new.go b/locallayout/new.go index ff4d7228..a6eb4057 100644 --- a/locallayout/new.go +++ b/locallayout/new.go @@ -13,7 +13,7 @@ import ( ) // NewImage returns a new image that can be modified and saved to a docker daemon -// via a tarball in OCI layout format. +// via a tarball in legacy format. func NewImage(repoName string, dockerClient DockerClient, ops ...func(*imgutil.ImageOptions)) (imgutil.Image, error) { options := &imgutil.ImageOptions{} for _, op := range ops { diff --git a/locallayout/store.go b/locallayout/store.go index 3757ec57..f001f6ff 100644 --- a/locallayout/store.go +++ b/locallayout/store.go @@ -144,7 +144,7 @@ func (s *Store) doSave(image v1.Image, withName string) (types.ImageInspect, err tw := tar.NewWriter(pw) defer tw.Close() - if err = s.addImageToTar(tw, image, withName, false); err != nil { + if err = s.addImageToTar(tw, image, withName); err != nil { return types.ImageInspect{}, err } tw.Close() @@ -164,7 +164,7 @@ func (s *Store) doSave(image v1.Image, withName string) (types.ImageInspect, err return inspect, nil } -func (s *Store) addImageToTar(tw *tar.Writer, image v1.Image, withName string, populateEmptyLayers bool) error { +func (s *Store) addImageToTar(tw *tar.Writer, image v1.Image, withName string) error { rawConfigFile, err := image.RawConfigFile() if err != nil { return err @@ -187,7 +187,7 @@ func (s *Store) addImageToTar(tw *tar.Writer, image v1.Image, withName string, p if err != nil { return err } - if size == -1 && !populateEmptyLayers { // layer facade fronting empty layer + if size == -1 { // layer facade fronting empty layer layerName = fmt.Sprintf("blank_%d", blankIdx) blankIdx++ hdr := &tar.Header{Name: layerName, Mode: 0644, Size: 0} @@ -221,24 +221,9 @@ func (s *Store) addLayerToTar(tw *tar.Writer, layer v1.Layer) (string, error) { if err != nil { return "", err } - - var layerToUse v1.Layer - compressedSize, err := layer.Size() - if err != nil { - return "", err - } - if compressedSize == -1 { - onDiskLayer := findLayer(layerDiffID, s.Layers()) - if onDiskLayer == nil { - return "", fmt.Errorf("failed to find layer with diff ID %q", layerDiffID.String()) - } - layerToUse = onDiskLayer - } else { - layerToUse = layer - } - withName := fmt.Sprintf("/%s.tar", layerDiffID.String()) - uncompressedSize, err := getLayerSize(layerToUse) + + uncompressedSize, err := getLayerSize(layer) if err != nil { return "", err } @@ -247,7 +232,7 @@ func (s *Store) addLayerToTar(tw *tar.Writer, layer v1.Layer) (string, error) { return "", err } - layerReader, err := layerToUse.Uncompressed() + layerReader, err := layer.Uncompressed() if err != nil { return "", err } @@ -350,7 +335,7 @@ func (s *Store) SaveFile(image imgutil.IdentifiableV1Image, withName string) (st tw := tar.NewWriter(pw) defer tw.Close() - return s.addImageToTar(tw, image, withName, true) + return s.addImageToTar(tw, image, withName) }) err = errs.Wait() diff --git a/locallayout/v1_facade.go b/locallayout/v1_facade.go index 8a57923a..af8fbb61 100644 --- a/locallayout/v1_facade.go +++ b/locallayout/v1_facade.go @@ -31,7 +31,7 @@ type v1ImageFacade struct { // for downloading layers from the daemon as needed store imgutil.ImageStore - downloadLayersOnAccess bool // set to true to download ALL image layers when LayerByDiffID is called + downloadLayersOnAccess bool // set to true to downloading ALL the image layers from the daemon when LayerByDiffID is called downloadOnce *sync.Once identifier string } diff --git a/remote/remote.go b/remote/remote.go index cdd078df..581593f7 100644 --- a/remote/remote.go +++ b/remote/remote.go @@ -185,7 +185,7 @@ func (i *Image) Identifier() (imgutil.Identifier, error) { } func (i *Image) Kind() string { - return fmt.Sprintf("%T", i) + return `remote` } func (i *Image) Label(key string) (string, error) {