From 3aa22cb1be83dbe322c3fabec29b231bdc1be141 Mon Sep 17 00:00:00 2001
From: maybe-sybr <58414429+maybe-sybr@users.noreply.github.com>
Date: Wed, 1 Jul 2020 09:49:44 +1000
Subject: APIv2:fix: Handle docker volume force as expected

In response to input regarding the semantic difference for the `force`
parameter for volume removal between Docker and us, this change ensures
that we emulate the Dockr behaviour correctly when this parameter is
specified.

Signed-off-by: Matt Brindley <58414429+maybe-sybr@users.noreply.github.com>
---
 pkg/api/handlers/compat/volumes.go | 43 ++++++++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 11 deletions(-)

(limited to 'pkg/api/handlers/compat')

diff --git a/pkg/api/handlers/compat/volumes.go b/pkg/api/handlers/compat/volumes.go
index 3f4e6eac1..1c9366409 100644
--- a/pkg/api/handlers/compat/volumes.go
+++ b/pkg/api/handlers/compat/volumes.go
@@ -176,21 +176,42 @@ func RemoveVolume(w http.ResponseWriter, r *http.Request) {
 			errors.Wrapf(err, "Failed to parse parameters for %s", r.URL.String()))
 		return
 	}
+
+	/* The implications for `force` differ between Docker and us, so we can't
+	 * simply pass the `force` parameter to `runeimt.RemoveVolume()`.
+	 * Specifically, Docker's behavior seems to be that `force` means "do not
+	 * error on missing volume"; ours means "remove any not-running containers
+	 * using the volume at the same time".
+	 *
+	 * With this in mind, we only consider the `force` query parameter when we
+	 * hunt for specified volume by name, using it to seletively return a 204
+	 * or blow up depending on `force` being truthy or falsey/unset
+	 * respectively.
+	 */
 	name := utils.GetName(r)
 	vol, err := runtime.LookupVolume(name)
-	if err != nil {
-		utils.VolumeNotFound(w, name, err)
-		return
-	}
-	if err := runtime.RemoveVolume(r.Context(), vol, query.Force); err != nil {
-		if errors.Cause(err) == define.ErrVolumeBeingUsed {
-			utils.Error(w, "volumes being used", http.StatusConflict, err)
-			return
+	if err == nil {
+		// As above, we do not pass `force` from the query parameters here
+		if err := runtime.RemoveVolume(r.Context(), vol, false); err != nil {
+			if errors.Cause(err) == define.ErrVolumeBeingUsed {
+				utils.Error(w, "volumes being used", http.StatusConflict, err)
+			} else {
+				utils.InternalServerError(w, err)
+			}
+		} else {
+			// Success
+			utils.WriteResponse(w, http.StatusNoContent, "")
+		}
+	} else {
+		if !query.Force {
+			utils.VolumeNotFound(w, name, err)
+		} else {
+			// Volume does not exist and `force` is truthy - this emulates what
+			// Docker would do when told to `force` removal of a nonextant
+			// volume
+			utils.WriteResponse(w, http.StatusNoContent, "")
 		}
-		utils.InternalServerError(w, err)
-		return
 	}
-	utils.WriteResponse(w, http.StatusNoContent, "")
 }
 
 func PruneVolumes(w http.ResponseWriter, r *http.Request) {
-- 
cgit v1.2.3-54-g00ecf