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>

2 files changed, 36 insertions, 12 deletions

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) {
diff --git a/pkg/api/server/register_volumes.go b/pkg/api/server/register_volumes.go
index 7a64a27ec..0f4f18b0a 100644
--- a/pkg/api/server/register_volumes.go
+++ b/pkg/api/server/register_volumes.go
@@ -186,7 +186,10 @@ func (s *APIServer) registerVolumeHandlers(r *mux.Router) error {
 	//  - in: query
 	//    name: force
 	//    type: boolean
-	//    description: force removal
+	//    description: |
+	//      Force removal of the volume. This actually only causes errors due
+	//      to the names volume not being found to be suppressed, which is the
+	//      behaviour Docker implements.
 	// produces:
 	// - application/json
 	// responses: