Skip to content

Conversation

@ndeloof
Copy link
Collaborator

@ndeloof ndeloof commented Aug 11, 2022

we can't use referenceBackend when snapshotter is enabled
=> let GetImage lookup named reference for image when details are requested

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
@ndeloof ndeloof requested review from rumpl, thaJeztah and vvoland August 11, 2022 12:58
}

func (s *imageRouter) toImageInspect(img *image.Image) (*types.ImageInspect, error) {
refs := s.referenceBackend.References(img.ID().Digest())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it also work if we'd check if s.referenceBackend is empty, and if so, fill it with img.Name() (or whatever is needed)? Something like;

var refs []reference.Named
if s.referenceBackend != nil {
	refs = s.referenceBackend.References(img.ID().Digest())
} else {
	// FIXME refs should contain all references of the image
	refs = append(refs, img.Name())
}

Or would we need the image name passed for that?

(Mostly seeing if we can keep the change to where it's a problem, and to have a clear TODO that needs fixing)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure we could just drop this line, but my goal here is to prepare the required refactoring so we get the codebase ready while I investigate how to retrieve all tags from containerd snaphsotter

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Comment on lines +176 to +178

references := i.referenceStore.References(img.ID().Digest())

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

options.Details also does the size calculation etc; perhaps we can always set the references, but continue to make the rest of details optional ;

references := i.referenceStore.References(img.ID().Digest())
if options.Details {
	 // get size etc
	img.Details = &image.Details{
		References:  references,
		Size:        size,
		Metadata:    layerMetadata,
		Driver:      i.layerStore.DriverName(),
		LastUpdated: lastUpdated,
	}
} else {
	img.Details = &image.Details{
		References:  references,
	}
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^^ discussed this with @ndeloof, and for this use-case (docker image inspect) we already have options.Details enabled so there's currently no need to set Details.references if it's not set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants