Skip to content

GetImage to return image tags with details #55

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.

Sign up for GitHub

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

Merged
ndeloof merged 2 commits into from
Aug 12, 2022
Merged

GetImage to return image tags with details #55

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c68a3c2
GetImage to return image tags with details
ndeloof Aug 11, 2022
da0ac40
list images matching digest to discover all tags
ndeloof Aug 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions api/server/router/image/image_routes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,9 @@ func (s *imageRouter) getImagesByName(ctx context.Context, w http.ResponseWriter
}

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

@thaJeztah thaJeztah Aug 11, 2022

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
Copy Markdown
Collaborator Author

@ndeloof ndeloof Aug 11, 2022

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

repoTags := []string{}
repoDigests := []string{}
for _, ref := range refs {
for _, ref := range img.Details.References {
switch ref.(type) {
case reference.NamedTagged:
repoTags = append(repoTags, reference.FamiliarString(ref))
Expand Down
12 changes: 12 additions & 0 deletions daemon/containerd/image.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,19 @@ func (i *ImageService) GetImage(ctx context.Context, refOrID string, options ima
if err != nil {
return nil, err
}

tagged, err := i.client.ImageService().List(ctx, fmt.Sprintf("target.digest==%s", ii.Target().Digest.String()))
tags := make([]reference.Named, 0, len(tagged))
for _, i := range tagged {
name, err := reference.ParseNamed(i.Name)
if err != nil {
return nil, err
}
tags = append(tags, name)
}

img.Details = &image.Details{
References: tags,
Size: size,
Metadata: nil,
Driver: i.GraphDriverName(),
Expand Down
4 changes: 4 additions & 0 deletions daemon/images/image.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,14 @@ func (i *ImageService) GetImage(ctx context.Context, refOrID string, options ima
}

lastUpdated, err := i.imageStore.GetLastUpdated(img.ID())

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

Comment on lines +176 to +178
Copy link
Copy Markdown
Collaborator

@thaJeztah thaJeztah Aug 11, 2022

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
Copy Markdown
Collaborator

@thaJeztah thaJeztah Aug 12, 2022

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.

if err != nil {
return nil, err
}
img.Details = &image.Details{
References: references,
Size: size,
Metadata: layerMetadata,
Driver: i.layerStore.DriverName(),
Expand Down
2 changes: 2 additions & 0 deletions image/image.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"strings"
"time"

"github.com/docker/distribution/reference"
"github.com/docker/docker/api/types/container"
"github.com/docker/docker/dockerversion"
"github.com/docker/docker/layer"
Expand Down Expand Up @@ -119,6 +120,7 @@ type Image struct {

// Details provides additional image data
type Details struct {
References []reference.Named
Size int64
Metadata map[string]string
Driver string
Expand Down