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

Introduce --force option on image rm command #762

Merged
merged 2 commits into from
Nov 27, 2019
Merged

Conversation

ndeloof
Copy link
Contributor

@ndeloof ndeloof commented Nov 22, 2019

- What I did

Added --force option on docker app image rm command

- How I did it

Bundlestore do check all reference for ID to remove, and delete them
Also introduced bundle store cleanup by removing empty paths as a tag is deleted

- How to verify it

docker app build -t foo (get built image ID)
docker app tag foo bar
docker app image rm {ID} // will fail
docker app image rm --force {ID} // won't fail
docker app image ls

- Description for the changelog

Added --force option on docker app image rm command

- A picture of a cute animal (not mandatory)

image

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
@ndeloof ndeloof marked this pull request as ready for review November 25, 2019 11:18
e2e/images_test.go Show resolved Hide resolved
internal/store/bundle.go Outdated Show resolved Hide resolved
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
@ndeloof ndeloof requested a review from eunomie November 26, 2019 12:13
}
}
return failures.ErrorOrNil()
}
return fmt.Errorf("unable to delete %q - App is referenced in multiple repositories", reference.FamiliarString(ref))
Copy link
Member

Choose a reason for hiding this comment

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

Should we say here something like ", can be forced (-f)" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

docker image CLI for equivalent scenario:
conflict: unable to delete xyz (must be forced) - image is referenced in multiple repositories

Copy link
Member

Choose a reason for hiding this comment

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

Would be great yeah

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll address this in a subsequent PR with bundlestore refactoring

@rumpl
Copy link
Member

rumpl commented Nov 26, 2019

What does the output of go-multierror look like?

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

@ndeloof ndeloof merged commit 06944d9 into docker:master Nov 27, 2019
@ndeloof ndeloof deleted the force branch November 27, 2019 07:42
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.

5 participants