Skip to content
This repository has been archived by the owner on Jun 13, 2021. It is now read-only.

Add -q flag for image ls command #695

Merged
merged 2 commits into from
Oct 18, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
29 changes: 29 additions & 0 deletions e2e/images_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package e2e

import (
"bufio"
"fmt"
"path/filepath"
"strings"
"testing"

"gotest.tools/assert"
Expand All @@ -26,6 +28,23 @@ func expectImageListOutput(t *testing.T, cmd icmd.Cmd, output string) {
assert.Equal(t, result.Stdout(), output)
}

func verifyImageIDListOutput(t *testing.T, cmd icmd.Cmd, count int, distinct int) {
cmd.Command = dockerCli.Command("app", "image", "ls", "-q")
result := icmd.RunCmd(cmd).Assert(t, icmd.Success)
scanner := bufio.NewScanner(strings.NewReader(result.Stdout()))
lines := []string{}
counter := make(map[string]int)
for scanner.Scan() {
lines = append(lines, scanner.Text())
counter[scanner.Text()]++
}
if err := scanner.Err(); err != nil {
assert.Error(t, err, "Verification failed")
}
assert.Equal(t, len(lines), count)
assert.Equal(t, len(counter), distinct)
}

func TestImageList(t *testing.T) {
runWithDindSwarmAndRegistry(t, func(info dindSwarmAndRegistryInfo) {
cmd := info.configuredCmd
Expand All @@ -44,6 +63,16 @@ b-simple-app:latest simple
})
}

func TestImageListQuiet(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think a unit test would be easier for this feature, mocking the bundle store to configure it the way you want.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A unit test has been added

runWithDindSwarmAndRegistry(t, func(info dindSwarmAndRegistryInfo) {
cmd := info.configuredCmd
dir := fs.NewDir(t, "")
defer dir.Remove()
insertBundles(t, cmd, info)
verifyImageIDListOutput(t, cmd, 3, 2)
})
}

func TestImageRm(t *testing.T) {
runWithDindSwarmAndRegistry(t, func(info dindSwarmAndRegistryInfo) {
cmd := info.configuredCmd
Expand Down
34 changes: 32 additions & 2 deletions internal/commands/image/list.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package image

import (
"bytes"
"fmt"
"io"
"strings"
Expand All @@ -11,10 +12,16 @@ import (
"github.com/docker/cli/cli/command"
"github.com/docker/cli/cli/config"
"github.com/docker/distribution/reference"
"github.com/docker/docker/pkg/stringid"
"github.com/spf13/cobra"
)

type imageListOption struct {
quiet bool
}

func listCmd(dockerCli command.Cli) *cobra.Command {
options := imageListOption{}
cmd := &cobra.Command{
Short: "List application images",
Use: "ls",
Expand All @@ -30,14 +37,16 @@ func listCmd(dockerCli command.Cli) *cobra.Command {
return err
}

return runList(dockerCli, bundleStore)
return runList(dockerCli, options, bundleStore)
},
}
flags := cmd.Flags()
flags.BoolVarP(&options.quiet, "quiet", "q", false, "Only show numeric IDs")

return cmd
}

func runList(dockerCli command.Cli, bundleStore store.BundleStore) error {
func runList(dockerCli command.Cli, options imageListOption, bundleStore store.BundleStore) error {
bundles, err := bundleStore.List()
if err != nil {
return err
Expand All @@ -48,6 +57,9 @@ func runList(dockerCli command.Cli, bundleStore store.BundleStore) error {
return err
}

if options.quiet {
return printImageIDs(dockerCli, pkgs)
}
return printImages(dockerCli, pkgs)
}

Expand Down Expand Up @@ -81,6 +93,24 @@ func printImages(dockerCli command.Cli, refs []pkg) error {
return w.Flush()
}

func printImageIDs(dockerCli command.Cli, refs []pkg) error {
var buf bytes.Buffer

for _, ref := range refs {
id, ok := ref.ref.(store.ID)
if !ok {
var err error
id, err = store.FromBundle(ref.bundle)
if err != nil {
return err
}
}
fmt.Fprintln(&buf, stringid.TruncateID(id.String()))
}
fmt.Fprintf(dockerCli.Out(), buf.String())
return nil
}

func printHeaders(w io.Writer) {
var headers []string
for _, column := range listColumns {
Expand Down
6 changes: 6 additions & 0 deletions internal/store/digest.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"io"
"regexp"

"github.com/deislabs/cnab-go/bundle"
"github.com/docker/distribution/reference"
"github.com/opencontainers/go-digest"
)
Expand All @@ -27,6 +28,11 @@ func FromString(s string) (ID, error) {
return ID{s}, nil
}

func FromBundle(bndle *bundle.Bundle) (ID, error) {
Copy link
Member

Choose a reason for hiding this comment

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

It’s an odd name

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed. What about attaching a getID() function to Bundle ?
(sorry, my brain is still object-oriented)

Copy link
Member

@eunomie eunomie Oct 17, 2019

Choose a reason for hiding this comment

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

wasn't the remark about bndle?
Everywhere in the code it's bndl, bndle is used only inside internal/store/bundle.go

Copy link
Member

Choose a reason for hiding this comment

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

Nah, FromBundle is an odd name. When called it's: store.FromBundle(..) and you get ... an ID. Confusing to say the least

Copy link
Member

Choose a reason for hiding this comment

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

Maybe the package is not the right one.
Something like id.FromString and id.FromBundle (or digest)
Not store.FromBundle as we don't return a store.

Copy link
Contributor Author

@jcsirot jcsirot Oct 17, 2019

Choose a reason for hiding this comment

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

store.FromString returns an ID as well... Maybe we should rename them into IDFromString and IDFromBundle?

digest, err := ComputeDigest(bndle)
return ID{digest.Encoded()}, err
}

// ID is an unique identifier for docker app image bundle, implementing reference.Reference
type ID struct {
digest string
Expand Down