From 0f0bac4ab31f06403a567d9f28ee4ef69cfe94ca Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Fri, 7 Feb 2020 00:50:09 +0100 Subject: Call NewImageRuntimeFromStore from NewImageRuntimeFromOptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... so that there ultimately is only one constructor. Should not change behavior. Signed-off-by: Miloslav Trmač --- libpod/image/image.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'libpod') diff --git a/libpod/image/image.go b/libpod/image/image.go index ba1080a71..f773ab74a 100644 --- a/libpod/image/image.go +++ b/libpod/image/image.go @@ -99,10 +99,7 @@ func NewImageRuntimeFromOptions(options storage.StoreOptions) (*Runtime, error) if err != nil { return nil, err } - - return &Runtime{ - store: store, - }, nil + return NewImageRuntimeFromStore(store), nil } func setStore(options storage.StoreOptions) (storage.Store, error) { -- cgit v1.2.3-54-g00ecf From d9bfaf51977cdc7088fe168d871ac5ce787c9d6a Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Fri, 7 Feb 2020 02:43:14 +0100 Subject: Create two separate newImage instances in Runtime.New MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is not _trivially_ safe because newImage.getLocalImage() modifies newImage.ImageName, but we overwrite that value anyway. So, this should not change behavior, and it will make future refactoring easier to verify. Signed-off-by: Miloslav Trmač --- libpod/image/image.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'libpod') diff --git a/libpod/image/image.go b/libpod/image/image.go index f773ab74a..28148b7c3 100644 --- a/libpod/image/image.go +++ b/libpod/image/image.go @@ -145,11 +145,11 @@ func (ir *Runtime) New(ctx context.Context, name, signaturePolicyPath, authfile defer span.Finish() // We don't know if the image is local or not ... check local first - newImage := Image{ - InputName: name, - imageruntime: ir, - } if pullType != util.PullImageAlways { + newImage := Image{ + InputName: name, + imageruntime: ir, + } localImage, err := newImage.getLocalImage() if err == nil { newImage.image = localImage @@ -168,6 +168,10 @@ func (ir *Runtime) New(ctx context.Context, name, signaturePolicyPath, authfile return nil, errors.Wrapf(err, "unable to pull %s", name) } + newImage := Image{ + InputName: name, + imageruntime: ir, + } newImage.InputName = imageName[0] img, err := newImage.getLocalImage() if err != nil { -- cgit v1.2.3-54-g00ecf From a433b858d275557e1469fdcd917f1ef2a93011cd Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Fri, 7 Feb 2020 01:52:04 +0100 Subject: Trivial simplification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should not change behavior. Signed-off-by: Miloslav Trmač --- libpod/image/image.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'libpod') diff --git a/libpod/image/image.go b/libpod/image/image.go index 28148b7c3..432b8b848 100644 --- a/libpod/image/image.go +++ b/libpod/image/image.go @@ -169,10 +169,9 @@ func (ir *Runtime) New(ctx context.Context, name, signaturePolicyPath, authfile } newImage := Image{ - InputName: name, + InputName: imageName[0], imageruntime: ir, } - newImage.InputName = imageName[0] img, err := newImage.getLocalImage() if err != nil { return nil, errors.Wrapf(err, "error retrieving local image after pulling %s", name) -- cgit v1.2.3-54-g00ecf From ede412f40f662b506d1d7d44d51cd51ef5922f64 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Fri, 7 Feb 2020 01:55:30 +0100 Subject: Use Runtime.NewFromLocal instead of open-coded copies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All code creating an Image by looking up a name now uses Runtime.NewFromLocal. Should not change behavior. Signed-off-by: Miloslav Trmač --- libpod/image/image.go | 27 ++++++--------------------- 1 file changed, 6 insertions(+), 21 deletions(-) (limited to 'libpod') diff --git a/libpod/image/image.go b/libpod/image/image.go index 432b8b848..db14eab18 100644 --- a/libpod/image/image.go +++ b/libpod/image/image.go @@ -146,14 +146,9 @@ func (ir *Runtime) New(ctx context.Context, name, signaturePolicyPath, authfile // We don't know if the image is local or not ... check local first if pullType != util.PullImageAlways { - newImage := Image{ - InputName: name, - imageruntime: ir, - } - localImage, err := newImage.getLocalImage() + newImage, err := ir.NewFromLocal(name) if err == nil { - newImage.image = localImage - return &newImage, nil + return newImage, nil } else if pullType == util.PullImageNever { return nil, err } @@ -168,16 +163,11 @@ func (ir *Runtime) New(ctx context.Context, name, signaturePolicyPath, authfile return nil, errors.Wrapf(err, "unable to pull %s", name) } - newImage := Image{ - InputName: imageName[0], - imageruntime: ir, - } - img, err := newImage.getLocalImage() + newImage, err := ir.NewFromLocal(imageName[0]) if err != nil { return nil, errors.Wrapf(err, "error retrieving local image after pulling %s", name) } - newImage.image = img - return &newImage, nil + return newImage, nil } // LoadFromArchiveReference creates a new image object for images pulled from a tar archive and the like (podman load) @@ -194,16 +184,11 @@ func (ir *Runtime) LoadFromArchiveReference(ctx context.Context, srcRef types.Im } for _, name := range imageNames { - newImage := Image{ - InputName: name, - imageruntime: ir, - } - img, err := newImage.getLocalImage() + newImage, err := ir.NewFromLocal(name) if err != nil { return nil, errors.Wrapf(err, "error retrieving local image after pulling %s", name) } - newImage.image = img - newImages = append(newImages, &newImage) + newImages = append(newImages, newImage) } ir.newImageEvent(events.LoadFromArchive, "") return newImages, nil -- cgit v1.2.3-54-g00ecf From 1ab4038807eb1586901460119820a4b865ac644f Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Fri, 7 Feb 2020 02:03:03 +0100 Subject: Remove the getLocalImage() call from Image.Size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All ways to create an Image{} have a non-nil .image field, and it is never set to nil, so this is dead code. Should not change behavior. Signed-off-by: Miloslav Trmač --- libpod/image/image.go | 7 ------- 1 file changed, 7 deletions(-) (limited to 'libpod') diff --git a/libpod/image/image.go b/libpod/image/image.go index db14eab18..7efac4214 100644 --- a/libpod/image/image.go +++ b/libpod/image/image.go @@ -687,13 +687,6 @@ func (i *Image) toImageSourceRef(ctx context.Context) (types.ImageSource, error) //Size returns the size of the image func (i *Image) Size(ctx context.Context) (*uint64, error) { - if i.image == nil { - localImage, err := i.getLocalImage() - if err != nil { - return nil, err - } - i.image = localImage - } sum, err := i.imageruntime.store.ImageSize(i.ID()) if err == nil && sum >= 0 { usum := uint64(sum) -- cgit v1.2.3-54-g00ecf From 933edf4a3767a1431257ba9d2948821eba967c23 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Fri, 7 Feb 2020 02:16:34 +0100 Subject: Move Image.getLocalImage to Runtime.getLocalImage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of the function updating image.InputName (the only reason for it to need an image), have it return the updated value separately. This will allow simplifying the constructors of Image further. Should not change behavior. Signed-off-by: Miloslav Trmač --- libpod/image/image.go | 51 +++++++++++++++++++++++++-------------------------- 1 file changed, 25 insertions(+), 26 deletions(-) (limited to 'libpod') diff --git a/libpod/image/image.go b/libpod/image/image.go index 7efac4214..9cfcf3c67 100644 --- a/libpod/image/image.go +++ b/libpod/image/image.go @@ -125,16 +125,15 @@ func (ir *Runtime) newFromStorage(img *storage.Image) *Image { // to only deal with local images already in the store (or // its aliases) func (ir *Runtime) NewFromLocal(name string) (*Image, error) { - image := Image{ - InputName: name, - imageruntime: ir, - } - localImage, err := image.getLocalImage() + updatedInputName, localImage, err := ir.getLocalImage(name) if err != nil { return nil, err } - image.image = localImage - return &image, nil + return &Image{ + InputName: updatedInputName, + imageruntime: ir, + image: localImage, + }, nil } // New creates a new image object where the image could be local @@ -232,60 +231,60 @@ func stripSha256(name string) string { } // getLocalImage resolves an unknown input describing an image and -// returns a storage.Image or an error. It is used by NewFromLocal. -func (i *Image) getLocalImage() (*storage.Image, error) { - imageError := fmt.Sprintf("unable to find '%s' in local storage", i.InputName) - if i.InputName == "" { - return nil, errors.Errorf("input name is blank") +// returns an updated input name, and a storage.Image, or an error. It is used by NewFromLocal. +func (ir *Runtime) getLocalImage(inputName string) (string, *storage.Image, error) { + imageError := fmt.Sprintf("unable to find '%s' in local storage", inputName) + if inputName == "" { + return "", nil, errors.Errorf("input name is blank") } // Check if the input name has a transport and if so strip it - dest, err := alltransports.ParseImageName(i.InputName) + dest, err := alltransports.ParseImageName(inputName) if err == nil && dest.DockerReference() != nil { - i.InputName = dest.DockerReference().String() + inputName = dest.DockerReference().String() } - img, err := i.imageruntime.getImage(stripSha256(i.InputName)) + img, err := ir.getImage(stripSha256(inputName)) if err == nil { - return img.image, err + return inputName, img.image, err } // container-storage wasn't able to find it in its current form // check if the input name has a tag, and if not, run it through // again - decomposedImage, err := decompose(i.InputName) + decomposedImage, err := decompose(inputName) if err != nil { - return nil, err + return "", nil, err } // The image has a registry name in it and we made sure we looked for it locally // with a tag. It cannot be local. if decomposedImage.hasRegistry { - return nil, errors.Wrapf(ErrNoSuchImage, imageError) + return "", nil, errors.Wrapf(ErrNoSuchImage, imageError) } // if the image is saved with the repository localhost, searching with localhost prepended is necessary // We don't need to strip the sha because we have already determined it is not an ID ref, err := decomposedImage.referenceWithRegistry(DefaultLocalRegistry) if err != nil { - return nil, err + return "", nil, err } - img, err = i.imageruntime.getImage(ref.String()) + img, err = ir.getImage(ref.String()) if err == nil { - return img.image, err + return inputName, img.image, err } // grab all the local images - images, err := i.imageruntime.GetImages() + images, err := ir.GetImages() if err != nil { - return nil, err + return "", nil, err } // check the repotags of all images for a match repoImage, err := findImageInRepotags(decomposedImage, images) if err == nil { - return repoImage, nil + return inputName, repoImage, nil } - return nil, errors.Wrapf(ErrNoSuchImage, err.Error()) + return "", nil, errors.Wrapf(ErrNoSuchImage, err.Error()) } // ID returns the image ID as a string -- cgit v1.2.3-54-g00ecf From 972df82632bf0840ac84756306ef607ea5be72d8 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Fri, 7 Feb 2020 02:26:44 +0100 Subject: Introduce a Runtime.newImage constructor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... so that _all_ Image objects are created in a single place that is easy to update. Should not change behavior. Signed-off-by: Miloslav Trmač --- libpod/image/image.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'libpod') diff --git a/libpod/image/image.go b/libpod/image/image.go index 9cfcf3c67..f6792eeac 100644 --- a/libpod/image/image.go +++ b/libpod/image/image.go @@ -111,14 +111,18 @@ func setStore(options storage.StoreOptions) (storage.Store, error) { return store, nil } -// newFromStorage creates a new image object from a storage.Image -func (ir *Runtime) newFromStorage(img *storage.Image) *Image { - image := Image{ - InputName: img.ID, +// newImage creates a new image object given an "input name" and a storage.Image +func (ir *Runtime) newImage(inputName string, img *storage.Image) *Image { + return &Image{ + InputName: inputName, imageruntime: ir, image: img, } - return &image +} + +// newFromStorage creates a new image object from a storage.Image +func (ir *Runtime) newFromStorage(img *storage.Image) *Image { + return ir.newImage(img.ID, img) } // NewFromLocal creates a new image object that is intended @@ -129,11 +133,7 @@ func (ir *Runtime) NewFromLocal(name string) (*Image, error) { if err != nil { return nil, err } - return &Image{ - InputName: updatedInputName, - imageruntime: ir, - image: localImage, - }, nil + return ir.newImage(updatedInputName, localImage), nil } // New creates a new image object where the image could be local -- cgit v1.2.3-54-g00ecf From 5b5bacbd95fb4a87bdb294504621dba766aed626 Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Fri, 7 Feb 2020 02:32:01 +0100 Subject: Document an aspect of newFromStorage behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should not change behavior. Signed-off-by: Miloslav Trmač --- libpod/image/image.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'libpod') diff --git a/libpod/image/image.go b/libpod/image/image.go index f6792eeac..ddde1c8da 100644 --- a/libpod/image/image.go +++ b/libpod/image/image.go @@ -120,7 +120,7 @@ func (ir *Runtime) newImage(inputName string, img *storage.Image) *Image { } } -// newFromStorage creates a new image object from a storage.Image +// newFromStorage creates a new image object from a storage.Image. Its "input name" will be its ID. func (ir *Runtime) newFromStorage(img *storage.Image) *Image { return ir.newImage(img.ID, img) } -- cgit v1.2.3-54-g00ecf From dafd212870585313442de0342c5455ef42099d5e Mon Sep 17 00:00:00 2001 From: Miloslav Trmač Date: Fri, 7 Feb 2020 02:37:36 +0100 Subject: Modify Runtime.getImage to return a storage.Image MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... because both callers only care about that aspect of the return value. Signed-off-by: Miloslav Trmač --- libpod/image/image.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'libpod') diff --git a/libpod/image/image.go b/libpod/image/image.go index ddde1c8da..43fd52a1a 100644 --- a/libpod/image/image.go +++ b/libpod/image/image.go @@ -218,7 +218,7 @@ func (i *Image) reloadImage() error { if err != nil { return errors.Wrapf(err, "unable to reload image") } - i.image = newImage.image + i.image = newImage return nil } @@ -245,7 +245,7 @@ func (ir *Runtime) getLocalImage(inputName string) (string, *storage.Image, erro img, err := ir.getImage(stripSha256(inputName)) if err == nil { - return inputName, img.image, err + return inputName, img, err } // container-storage wasn't able to find it in its current form @@ -269,7 +269,7 @@ func (ir *Runtime) getLocalImage(inputName string) (string, *storage.Image, erro } img, err = ir.getImage(ref.String()) if err == nil { - return inputName, img.image, err + return inputName, img, err } // grab all the local images @@ -444,7 +444,7 @@ func (i *Image) Remove(ctx context.Context, force bool) error { // getImage retrieves an image matching the given name or hash from system // storage // If no matching image can be found, an error is returned -func (ir *Runtime) getImage(image string) (*Image, error) { +func (ir *Runtime) getImage(image string) (*storage.Image, error) { var img *storage.Image ref, err := is.Transport.ParseStoreReference(ir.store, image) if err == nil { @@ -460,8 +460,7 @@ func (ir *Runtime) getImage(image string) (*Image, error) { } img = img2 } - newImage := ir.newFromStorage(img) - return newImage, nil + return img, nil } // GetImages retrieves all images present in storage -- cgit v1.2.3-54-g00ecf