Fix rmi by ID untagging image on error

Do not untag image if it would later get a hard conflict because of running containers.

Fixes #18873

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>

daemon/image_delete.go

@@ -102,6 +102,10 @@ func (daemon *Daemon) ImageDelete(imageRef string, force, prune bool) ([]types.I
 		// remove that reference.
 		// FIXME: Is this the behavior we want?
 		if len(repoRefs) == 1 {
+			if conflict := daemon.checkImageDeleteConflict(imgID, force, true); conflict != nil {
+				return nil, conflict
+			}
+
 			parsedRef, err := daemon.removeImageRef(repoRefs[0])
 			if err != nil {
 				return nil, err
@@ -215,7 +219,7 @@ func (idc *imageDeleteConflict) Error() string {
 func (daemon *Daemon) imageDeleteHelper(imgID image.ID, records *[]types.ImageDelete, force, prune, quiet bool) error {
 	// First, determine if this image has any conflicts. Ignore soft conflicts
 	// if force is true.
-	if conflict := daemon.checkImageDeleteConflict(imgID, force); conflict != nil {
+	if conflict := daemon.checkImageDeleteConflict(imgID, force, false); conflict != nil {
 		if quiet && (!daemon.imageIsDangling(imgID) || conflict.used) {
 			// Ignore conflicts UNLESS the image is "dangling" or not being used in
 			// which case we want the user to know.
@@ -267,7 +271,7 @@ func (daemon *Daemon) imageDeleteHelper(imgID image.ID, records *[]types.ImageDe
 // using the image. A soft conflict is any tags/digest referencing the given
 // image or any stopped container using the image. If ignoreSoftConflicts is
 // true, this function will not check for soft conflict conditions.
-func (daemon *Daemon) checkImageDeleteConflict(imgID image.ID, ignoreSoftConflicts bool) *imageDeleteConflict {
+func (daemon *Daemon) checkImageDeleteConflict(imgID image.ID, ignoreSoftConflicts bool, ignoreRefConflict bool) *imageDeleteConflict {
 	// Check for hard conflicts first.
 	if conflict := daemon.checkImageDeleteHardConflict(imgID); conflict != nil {
 		return conflict
@@ -279,7 +283,7 @@ func (daemon *Daemon) checkImageDeleteConflict(imgID image.ID, ignoreSoftConflic
 		return nil
 	}
 
-	return daemon.checkImageDeleteSoftConflict(imgID)
+	return daemon.checkImageDeleteSoftConflict(imgID, ignoreRefConflict)
 }
 
 func (daemon *Daemon) checkImageDeleteHardConflict(imgID image.ID) *imageDeleteConflict {
@@ -312,9 +316,9 @@ func (daemon *Daemon) checkImageDeleteHardConflict(imgID image.ID) *imageDeleteC
 	return nil
 }
 
-func (daemon *Daemon) checkImageDeleteSoftConflict(imgID image.ID) *imageDeleteConflict {
+func (daemon *Daemon) checkImageDeleteSoftConflict(imgID image.ID, ignoreRefConflict bool) *imageDeleteConflict {
 	// Check if any repository tags/digest reference this image.
-	if len(daemon.referenceStore.References(imgID)) > 0 {
+	if !ignoreRefConflict && len(daemon.referenceStore.References(imgID)) > 0 {
 		return &imageDeleteConflict{
 			imgID:   imgID,
 			message: "image is referenced in one or more repositories",

integration-cli/docker_cli_rmi_test.go

@@ -337,3 +337,20 @@ func (s *DockerSuite) TestRmiWithParentInUse(c *check.C) {
 
 	dockerCmd(c, "rmi", imageID)
 }
+
+// #18873
+func (s *DockerSuite) TestRmiByIDHardConflict(c *check.C) {
+	testRequires(c, DaemonIsLinux)
+	dockerCmd(c, "create", "busybox")
+
+	imgID, err := inspectField("busybox:latest", "Id")
+	c.Assert(err, checker.IsNil)
+
+	_, _, err = dockerCmdWithError("rmi", imgID[:12])
+	c.Assert(err, checker.NotNil)
+
+	// check that tag was not removed
+	imgID2, err := inspectField("busybox:latest", "Id")
+	c.Assert(err, checker.IsNil)
+	c.Assert(imgID, checker.Equals, imgID2)
+}