wbrawner/toolbox
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Unbreak sorting and clearly identify copied images in 'list'

Browse source 
Currently, if an image was copied with:
  $ skopeo copy \
      containers-storage:registry.fedoraproject.org/fedora-toolbox:36 \
      containers-storage:localhost/fedora-toolbox:36

... or:
  $ podman tag \
      registry.fedoraproject.org/fedora-toolbox:36 \
      localhost/fedora-toolbox:36

... then it would show up twice in 'list' with the same name, and in the
wrong order.

Either as:
  $ toolbox list --images
  IMAGE ID      IMAGE NAME                                      CREATED
  2110dbbc33d2  localhost/fedora-toolbox:36                     1 day...
  e085805ade4a  registry.access.redhat.com/ubi8/toolbox:latest  1 day...
  2110dbbc33d2  localhost/fedora-toolbox:36                     1 day...
  70cbe2ce60ca  registry.fedoraproject.org/fedora-toolbox:34    1 day...

... or as:
  $ toolbox list --images
  IMAGE ID      IMAGE NAME                                      CREATED
  2110dbbc33d2  registry.fedoraproject.org/fedora-toolbox:36    1 day...
  e085805ade4a  registry.access.redhat.com/ubi8/toolbox:latest  1 day...
  2110dbbc33d2  registry.fedoraproject.org/fedora-toolbox:36    1 day...
  70cbe2ce60ca  registry.fedoraproject.org/fedora-toolbox:34    1 day...

The correct output should be similar to 'podman images', and be sorted
in ascending order of the names:
  $ toolbox list --images
  IMAGE ID      IMAGE NAME                                      CREATED
  2110dbbc33d2  localhost/fedora-toolbox:36                     1 day...
  e085805ade4a  registry.access.redhat.com/ubi8/toolbox:latest  1 day...
  70cbe2ce60ca  registry.fedoraproject.org/fedora-toolbox:34    1 day...
  2110dbbc33d2  registry.fedoraproject.org/fedora-toolbox:36    1 day...

The problem is that, in these situations, 'podman images --format json'
returns separate identical JSON collections for each copy of the image,
and all of those copies have multiple names:
  [
    {
      "Id": "2110dbbc33d2",
      ...
      "Names": [
        "localhost/fedora-toolbox:36",
        "registry.fedoraproject.org/fedora-toolbox:36"
      ],
      ...
    },
    {
      "Id": "e085805ade4a",
      ...
      "Names": [
        "registry.access.redhat.com/ubi8/toolbox:latest"
      ],
      ...
    },
    {
      "Id": "2110dbbc33d2",
      ...
      "Names": [
        "localhost/fedora-toolbox:36",
        "registry.fedoraproject.org/fedora-toolbox:36"
      ],
      ...
    }
    {
      "Id": "70cbe2ce60ca",
      ...
      "Names": [
        "registry.fedoraproject.org/fedora-toolbox:34"
      ],
      ...
    },
  ]

The image objects need to be flattened to have only one unique name per
copy, but with the same ID, and then sorted to ensure the right order.

Note that the ordering was already broken since commit 2369da5d31,
which started using 'podman images --sort repository'.  Podman can sort
by either the image's repository or tag, but not by the unified name,
which is what Toolbx needs.  Therefore, even without copied images,
Toolbx really does need to sort the images itself.

Prior to commit 2369da5d31, the ordering was correct, but copied
images would only show up once.

Fallout from 2369da5d31

This reverts parts of commit 67e210378e.

https://github.com/containers/toolbox/issues/1043
This commit is contained in:
Debarshi Ray 2022-12-06 18:15:15 +01:00
parent 51eccd3da5
commit 6aab0a6175
5 changed files with 90 additions and 28 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

23
src/cmd/completion.go
View file

@ -128,13 +128,13 @@ func completionImageNames(cmd *cobra.Command, _ []string, _ string) ([]string, c
}
var imageNames []string
if images, err := getImages(); err == nil {
if images, err := getImages(true); err == nil {
for _, image := range images {
if len(image.Names) > 0 {
imageNames = append(imageNames, image.Names[0])
} else {
imageNames = append(imageNames, image.ID)
if len(image.Names) != 1 {
panic("cannot complete unflattened Image")
}
imageNames = append(imageNames, image.Names[0])
}
}
@ -143,19 +143,16 @@ func completionImageNames(cmd *cobra.Command, _ []string, _ string) ([]string, c
func completionImageNamesFiltered(_ *cobra.Command, args []string, _ string) ([]string, cobra.ShellCompDirective) {
var imageNames []string
if images, err := getImages(); err == nil {
if images, err := getImages(true); err == nil {
for _, image := range images {
skip := false
var imageName string
if len(image.Names) > 0 {
imageName = image.Names[0]
} else {
imageName = image.ID
if len(image.Names) != 1 {
panic("cannot complete unflattened Image")
}
for _, arg := range args {
if arg == imageName {
if arg == image.Names[0] {
skip = true
break
}
@ -165,7 +162,7 @@ func completionImageNamesFiltered(_ *cobra.Command, args []string, _ string) ([]
continue
}
imageNames = append(imageNames, imageName)
imageNames = append(imageNames, image.Names[0])
}
}

31
src/cmd/list.go
View file

@ -21,6 +21,7 @@ import (
"errors"
"fmt"
"os"
"sort"
"text/tabwriter"
"github.com/containers/toolbox/pkg/podman"
@ -105,7 +106,7 @@ func list(cmd *cobra.Command, args []string) error {
var err error
if lsImages {
images, err = getImages()
images, err = getImages(false)
if err != nil {
return err
}
@ -180,26 +181,41 @@ func listHelp(cmd *cobra.Command, args []string) {
}
}
func getImages() ([]podman.Image, error) {
func getImages(fillNameWithID bool) ([]podman.Image, error) {
logrus.Debug("Fetching all images")
args := []string{"--sort", "repository"}
var args []string
images, err := podman.GetImages(args...)
if err != nil {
logrus.Debugf("Fetching all images failed: %s", err)
return nil, errors.New("failed to get images")
}
processed := make(map[string]struct{})
var toolboxImages []podman.Image
for _, image := range images {
if _, ok := processed[image.ID]; ok {
continue
}
processed[image.ID] = struct{}{}
var isToolboxImage bool
for label := range toolboxLabels {
if _, ok := image.Labels[label]; ok {
toolboxImages = append(toolboxImages, image)
isToolboxImage = true
break
}
}
if isToolboxImage {
flattenedImages := image.FlattenNames(fillNameWithID)
toolboxImages = append(toolboxImages, flattenedImages...)
}
}
sort.Sort(podman.ImageSlice(toolboxImages))
return toolboxImages, nil
}
@ -209,14 +225,13 @@ func listOutput(images []podman.Image, containers []toolboxContainer) {
fmt.Fprintf(writer, "%s\t%s\t%s\n", "IMAGE ID", "IMAGE NAME", "CREATED")
for _, image := range images {
imageName := "<none>"
if len(image.Names) != 0 {
imageName = image.Names[0]
if len(image.Names) != 1 {
panic("cannot list unflattened Image")
}
fmt.Fprintf(writer, "%s\t%s\t%s\n",
utils.ShortID(image.ID),
imageName,
image.Names[0],
image.Created)
}

2
src/cmd/rmi.go
View file

@ -70,7 +70,7 @@ func rmi(cmd *cobra.Command, args []string) error {
}
if rmiFlags.deleteAll {
toolboxImages, err := getImages()
toolboxImages, err := getImages(false)
if err != nil {
return err
}

50
src/pkg/podman/podman.go
View file

@ -35,6 +35,8 @@ type Image struct {
Labels map[string]string
}
type ImageSlice []Image
var (
podmanVersion string
)
@ -43,6 +45,34 @@ var (
LogLevel = logrus.ErrorLevel
)
func (image *Image) FlattenNames(fillNameWithID bool) []Image {
var ret []Image
if len(image.Names) == 0 {
flattenedImage := *image
if fillNameWithID {
shortID := utils.ShortID(image.ID)
flattenedImage.Names = []string{shortID}
} else {
flattenedImage.Names = []string{"<none>"}
}
ret = []Image{flattenedImage}
return ret
}
ret = make([]Image, 0, len(image.Names))
for _, name := range image.Names {
flattenedImage := *image
flattenedImage.Names = []string{name}
ret = append(ret, flattenedImage)
}
return ret
}
func (image *Image) UnmarshalJSON(data []byte) error {
var raw struct {
ID string
@ -72,6 +102,26 @@ func (image *Image) UnmarshalJSON(data []byte) error {
return nil
}
func (images ImageSlice) Len() int {
return len(images)
}
func (images ImageSlice) Less(i, j int) bool {
if len(images[i].Names) != 1 {
panic("cannot sort unflattened ImageSlice")
}
if len(images[j].Names) != 1 {
panic("cannot sort unflattened ImageSlice")
}
return images[i].Names[0] < images[j].Names[0]
}
func (images ImageSlice) Swap(i, j int) {
images[i], images[j] = images[j], images[i]
}
// CheckVersion compares provided version with the version of Podman.
//
// Takes in one string parameter that should be in the format that is used for versioning (eg. 1.0.0, 2.5.1-dev).

12
test/system/102-list.bats
View file

@ -90,8 +90,8 @@ teardown() {
run --keep-empty-lines --separate-stderr $TOOLBOX list --images
assert_success
assert_line --index 1 --partial "$(get_system_id)-toolbox:$(get_system_version)"
assert_line --index 2 --partial "fedora-toolbox:34"
assert_line --index 1 --partial "fedora-toolbox:34"
assert_line --index 2 --partial "$(get_system_id)-toolbox:$(get_system_version)"
assert [ ${#lines[@]} -eq 4 ]
if check_bats_version 1.7.0; then
assert [ ${#stderr_lines[@]} -eq 0 ]
@ -113,8 +113,8 @@ teardown() {
run --keep-empty-lines --separate-stderr $TOOLBOX list
assert_success
assert_line --index 1 --partial "$(get_system_id)-toolbox:$(get_system_version)"
assert_line --index 2 --partial "fedora-toolbox:34"
assert_line --index 1 --partial "fedora-toolbox:34"
assert_line --index 2 --partial "$(get_system_id)-toolbox:$(get_system_version)"
assert_line --index 5 --partial "$(get_system_id)-toolbox-$(get_system_version)"
assert_line --index 6 --partial "non-default-one"
assert_line --index 7 --partial "non-default-two"
@ -136,8 +136,8 @@ teardown() {
assert_success
assert_line --index 1 --partial "<none>"
assert_line --index 2 --partial "$default_image"
assert_line --index 3 --partial "fedora-toolbox:34"
assert_line --index 2 --partial "fedora-toolbox:34"
assert_line --index 3 --partial "$default_image"
assert [ ${#lines[@]} -eq 5 ]
if check_bats_version 1.7.0; then
assert [ ${#stderr_lines[@]} -eq 0 ]