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

Add command to remove application images #648

Merged
merged 1 commit into from Sep 30, 2019
Merged

Conversation

rumpl
Copy link
Member

@rumpl rumpl commented Sep 27, 2019

- What I did

Added a new comand to remove application images, example:

$ docker app image ls
REPOSITORY TAG   APP NAME
my-app     1.0.0 voting-app
my-app     1.0.1 voting-app

$ docker app image rm my-app:1.0.0 my-app:1.0.1 my-app:2.0.0
Deleted: docker.io/library/my-app:1.0.0
Deleted: docker.io/library/my-app:1.0.1
Error: no such image docker.io/library/my-app:2.0.0

$ docker app image ls
REPOSITORY TAG APP NAME

- How I did it

The image rm command only remove the bundle from the bundle store, container images are not touched

- How to verify it

Look at the e2e test to see sample usage.

- Description for the changelog
New subcommand docker app image rm to remove a docker application from the local bundle store.

- A picture of a cute animal (not mandatory but encouraged)
image

@silvin-lubecki silvin-lubecki changed the title Add command to remove applicationh images Add command to remove application images Sep 27, 2019
@@ -97,6 +98,20 @@ func (b *bundleStore) List() ([]reference.Named, error) {
return references, nil
}

// Remove removes a bundle from the bundle store.
func (b *bundleStore) Remove(ref reference.Named) error {
if _, err := b.Read(ref); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

What about using os.Stat(path) instead?

@codecov
Copy link

codecov bot commented Sep 27, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@98cd1c5). Click here to learn what that means.
The diff coverage is 64.1%.

Impacted file tree graph

@@           Coverage Diff            @@
##             master    #648   +/-   ##
========================================
  Coverage          ?   71.8%           
========================================
  Files             ?      52           
  Lines             ?    2713           
  Branches          ?       0           
========================================
  Hits              ?    1948           
  Misses            ?     513           
  Partials          ?     252
Impacted Files Coverage Δ
internal/commands/image/command.go 100% <100%> (ø)
internal/commands/image/rm.go 57.14% <57.14%> (ø)
internal/store/bundle.go 69% <71.42%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 98cd1c5...3385987. Read the comment docs.

Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@eunomie eunomie left a comment

Choose a reason for hiding this comment

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

LGTM

@eunomie eunomie merged commit 5b857bc into docker:master Sep 30, 2019

func rmCmd() *cobra.Command {
return &cobra.Command{
Use: "rm [APP_IMAGE] [APP_IMAGE...]",
Copy link
Member

Choose a reason for hiding this comment

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

Sorry for the late reply, but could you add remove as alias for this?

Suggested change
Use: "rm [APP_IMAGE] [APP_IMAGE...]",
Use: "rm [APP_IMAGE] [APP_IMAGE...]",
Aliases: []string{"remove"},

Copy link
Member

Choose a reason for hiding this comment

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

Actually, let me open a quick PR

return err
}

fmt.Println("Deleted: " + ref.String())
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we should consistently use dockerCli.Out() / dockerCli.Err() for these; ideally we would print the reference on stdout, and Deleted: on stderr, but I don't think we do that currently on the CLI

@rumpl rumpl deleted the feat-image-rm branch October 18, 2019 08:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants