Skip to content
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.

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

bugfix: removing image by ID should conflict with running container. #2927

Merged
merged 1 commit into from
Jul 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
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
13 changes: 11 additions & 2 deletions apis/server/image_bridge.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,17 @@ func (s *Server) removeImage(ctx context.Context, rw http.ResponseWriter, req *h
}(time.Now())

isForce := httputils.BoolValue(req, "force")
// We only should check the image whether used by container when there is only one primary reference.
if len(refs) == 1 {

isImageIDPrefix := func(imageID string, name string) bool {
if strings.HasPrefix(imageID, name) || strings.HasPrefix(digest.Digest(imageID).Hex(), name) {
return true
}
return false
}

// We should check the image whether used by container when there is only one primary reference
// or the image is removed by image ID.
if len(refs) == 1 || isImageIDPrefix(image.ID, name) {
containers, err := s.ContainerMgr.List(ctx, &mgr.ContainerListOption{
All: true,
FilterFunc: func(c *mgr.Container) bool {
Expand Down
51 changes: 50 additions & 1 deletion test/cli_rmi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,31 @@ func (suite *PouchRmiSuite) TestRmiWorks(c *check.C) {
}
}

// TestRmiWithRunningContainer tests "pouch rmi" won't work without force
func (suite *PouchRmiSuite) TestRmiWithRunningContainer(c *check.C) {
name := "TestRmiWithRunningContainer"
command.PouchRun("pull", busyboxImage).Assert(c, icmd.Success)
command.PouchRun("run", "-d", "--name", name, busyboxImage, "top").Assert(c, icmd.Success)
defer DelContainerForceMultyTime(c, name)

// Can't remove without "--force"
command.PouchRun("rmi", busyboxImage)
res := command.PouchRun("images").Assert(c, icmd.Success)
if out := res.Combined(); !strings.Contains(out, busyboxImage) {
c.Fatalf("unexpected output %s: shouldn't rm image %s without force\n", out, busyboxImage)
}

command.PouchRun("rmi", "-f", busyboxImage).Assert(c, icmd.Success)
res = command.PouchRun("images").Assert(c, icmd.Success)
if out := res.Combined(); strings.Contains(out, busyboxImage) {
c.Fatalf("unexpected output %s: should rm image %s\n", out, busyboxImage)
}
}

// TestRmiForce tests "pouch rmi -f" work
func (suite *PouchRmiSuite) TestRmiForce(c *check.C) {
command.PouchRun("pull", helloworldImage).Assert(c, icmd.Success)

// TODO: rmi -f after create/start containers.
command.PouchRun("rmi", "-f", helloworldImage).Assert(c, icmd.Success)

res := command.PouchRun("images").Assert(c, icmd.Success)
Expand Down Expand Up @@ -102,6 +122,35 @@ func (suite *PouchRmiSuite) TestRmiByImageIDWithTwoPrimaryReferences(c *check.C)
}
}

// TestRmiByImageIDWithPrimaryReferencesAndRunningContainers tests "pouch rmi {ID}" won't work without force.
func (suite *PouchRmiSuite) TestRmiByImageIDWithPrimaryReferencesAndRunningContainers(c *check.C) {
busyboxImageAlias := busyboxImage + "Alias"
name := "TestRmiByImageIDWithRunningContainer"

command.PouchRun("pull", busyboxImage).Assert(c, icmd.Success)
command.PouchRun("tag", busyboxImage, busyboxImageAlias).Assert(c, icmd.Success)
res := command.PouchRun("images")
res.Assert(c, icmd.Success)
imageID := imagesListToKV(res.Combined())[busyboxImage][0]

command.PouchRun("run", "-d", "--name", name, busyboxImage, "top").Assert(c, icmd.Success)
defer DelContainerForceMultyTime(c, name)

// Can't remove without "--force"
res = command.PouchRun("rmi", imageID)
c.Assert(res.Stderr(), check.NotNil, check.Commentf("Unable to remove the image"))
res = command.PouchRun("images").Assert(c, icmd.Success)
if out := res.Combined(); !strings.Contains(out, busyboxImage) {
c.Fatalf("unexpected output %s: shouldn't rm image %s without force\n", out, busyboxImage)
}

command.PouchRun("rmi", "-f", imageID).Assert(c, icmd.Success)
res = command.PouchRun("images").Assert(c, icmd.Success)
if out := res.Combined(); strings.Contains(out, busyboxImage) {
c.Fatalf("unexpected output %s: should rm image %s\n", out, busyboxImage)
}
}

// TestRmiInWrongWay run rmi in wrong ways.
func (suite *PouchRmiSuite) TestRmiInWrongWay(c *check.C) {
for _, tc := range []struct {
Expand Down
2 changes: 1 addition & 1 deletion test/environment/cleanup.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func PruneAllContainers(apiClient client.ContainerAPIClient) error {
ctx := context.Background()
containers, err := apiClient.ContainerList(ctx, types.ContainerListOptions{All: true})
if err != nil {
return errors.Wrap(err, "fail to list images")
return errors.Wrap(err, "fail to list containers")
}

for _, ctr := range containers {
Expand Down