Merge pull request #8912 from jwhonce/issues/8891

Restore compatible API for prune endpoints

pkg/api/handlers/compat/containers_prune.go

@@ -1,9 +1,11 @@
 package compat
 
 import (
+	"bytes"
 	"net/http"
 
 	"github.com/containers/podman/v2/libpod"
+	"github.com/containers/podman/v2/pkg/api/handlers"
 	"github.com/containers/podman/v2/pkg/api/handlers/utils"
 	"github.com/containers/podman/v2/pkg/domain/entities/reports"
 	"github.com/containers/podman/v2/pkg/domain/filters"
@@ -32,33 +34,45 @@ func PruneContainers(w http.ResponseWriter, r *http.Request) {
 		filterFuncs = append(filterFuncs, generatedFunc)
 	}
 
-	// Libpod response differs
-	if utils.IsLibpodRequest(r) {
-		report, err := PruneContainersHelper(w, r, filterFuncs)
+	report, err := PruneContainersHelper(r, filterFuncs)
 	if err != nil {
 		utils.InternalServerError(w, err)
 		return
 	}
 
+	// Libpod response differs
+	if utils.IsLibpodRequest(r) {
 		utils.WriteResponse(w, http.StatusOK, report)
 		return
 	}
 
+	var payload handlers.ContainersPruneReport
+	var errorMsg bytes.Buffer
+	for _, pr := range report {
+		if pr.Err != nil {
+			// Docker stops on first error vs. libpod which keeps going. Given API constraints, concatenate all errors
+			// and return that string.
+			errorMsg.WriteString(pr.Err.Error())
+			errorMsg.WriteString("; ")
+			continue
+		}
+		payload.ContainersDeleted = append(payload.ContainersDeleted, pr.Id)
+		payload.SpaceReclaimed += pr.Size
+	}
+	if errorMsg.Len() > 0 {
+		utils.InternalServerError(w, errors.New(errorMsg.String()))
+		return
+	}
+
+	utils.WriteResponse(w, http.StatusOK, payload)
+}
+
+func PruneContainersHelper(r *http.Request, filterFuncs []libpod.ContainerFilter) ([]*reports.PruneReport, error) {
+	runtime := r.Context().Value("runtime").(*libpod.Runtime)
+
 	report, err := runtime.PruneContainers(filterFuncs)
 	if err != nil {
-		utils.InternalServerError(w, err)
-		return
-	}
-	utils.WriteResponse(w, http.StatusOK, report)
-}
-
-func PruneContainersHelper(w http.ResponseWriter, r *http.Request, filterFuncs []libpod.ContainerFilter) (
-	[]*reports.PruneReport, error) {
-	runtime := r.Context().Value("runtime").(*libpod.Runtime)
-	reports, err := runtime.PruneContainers(filterFuncs)
-	if err != nil {
-		utils.InternalServerError(w, err)
 		return nil, err
 	}
-	return reports, nil
+	return report, nil
 }

pkg/api/handlers/compat/images.go

@@ -18,7 +18,6 @@ import (
 	"github.com/containers/podman/v2/pkg/api/handlers/utils"
 	"github.com/containers/podman/v2/pkg/auth"
 	"github.com/containers/podman/v2/pkg/domain/entities"
-	"github.com/docker/docker/api/types"
 	"github.com/gorilla/schema"
 	"github.com/opencontainers/go-digest"
 	"github.com/pkg/errors"
@@ -74,52 +73,6 @@ func ExportImage(w http.ResponseWriter, r *http.Request) {
 	utils.WriteResponse(w, http.StatusOK, rdr)
 }
 
-func PruneImages(w http.ResponseWriter, r *http.Request) {
-	var (
-		filters []string
-	)
-	decoder := r.Context().Value("decoder").(*schema.Decoder)
-	runtime := r.Context().Value("runtime").(*libpod.Runtime)
-
-	query := struct {
-		All     bool
-		Filters map[string][]string `schema:"filters"`
-	}{
-		// This is where you can override the golang default value for one of fields
-	}
-
-	if err := decoder.Decode(&query, r.URL.Query()); err != nil {
-		utils.Error(w, "Something went wrong.", http.StatusBadRequest, errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String()))
-		return
-	}
-
-	idr := []types.ImageDeleteResponseItem{}
-	for k, v := range query.Filters {
-		for _, val := range v {
-			filters = append(filters, fmt.Sprintf("%s=%s", k, val))
-		}
-	}
-	imagePruneReports, err := runtime.ImageRuntime().PruneImages(r.Context(), query.All, filters)
-	if err != nil {
-		utils.InternalServerError(w, err)
-		return
-	}
-	reclaimedSpace := uint64(0)
-	for _, p := range imagePruneReports {
-		idr = append(idr, types.ImageDeleteResponseItem{
-			Deleted: p.Id,
-		})
-		reclaimedSpace = reclaimedSpace + p.Size
-	}
-
-	// FIXME/TODO to do this exactly correct, pruneimages needs to return idrs and space-reclaimed, then we are golden
-	ipr := types.ImagesPruneReport{
-		ImagesDeleted:  idr,
-		SpaceReclaimed: reclaimedSpace,
-	}
-	utils.WriteResponse(w, http.StatusOK, handlers.ImagesPruneReport{ImagesPruneReport: ipr})
-}
-
 func CommitContainer(w http.ResponseWriter, r *http.Request) {
 	var (
 		destImage string

pkg/api/handlers/compat/images_prune.go

@@ -0,0 +1,75 @@
+package compat
+
+import (
+	"bytes"
+	"fmt"
+	"net/http"
+
+	"github.com/containers/podman/v2/libpod"
+	"github.com/containers/podman/v2/pkg/api/handlers"
+	"github.com/containers/podman/v2/pkg/api/handlers/utils"
+	"github.com/docker/docker/api/types"
+	"github.com/gorilla/schema"
+	"github.com/pkg/errors"
+)
+
+func PruneImages(w http.ResponseWriter, r *http.Request) {
+	var (
+		filters []string
+	)
+	decoder := r.Context().Value("decoder").(*schema.Decoder)
+	runtime := r.Context().Value("runtime").(*libpod.Runtime)
+
+	query := struct {
+		All     bool
+		Filters map[string][]string `schema:"filters"`
+	}{
+		// This is where you can override the golang default value for one of fields
+	}
+
+	if err := decoder.Decode(&query, r.URL.Query()); err != nil {
+		utils.Error(w, "Something went wrong.", http.StatusBadRequest, errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String()))
+		return
+	}
+
+	for k, v := range query.Filters {
+		for _, val := range v {
+			filters = append(filters, fmt.Sprintf("%s=%s", k, val))
+		}
+	}
+	imagePruneReports, err := runtime.ImageRuntime().PruneImages(r.Context(), query.All, filters)
+	if err != nil {
+		utils.InternalServerError(w, err)
+		return
+	}
+
+	idr := make([]types.ImageDeleteResponseItem, len(imagePruneReports))
+	var reclaimedSpace uint64
+	var errorMsg bytes.Buffer
+	for _, p := range imagePruneReports {
+		if p.Err != nil {
+			// Docker stops on first error vs. libpod which keeps going. Given API constraints, concatenate all errors
+			// and return that string.
+			errorMsg.WriteString(p.Err.Error())
+			errorMsg.WriteString("; ")
+			continue
+		}
+
+		idr = append(idr, types.ImageDeleteResponseItem{
+			Deleted: p.Id,
+		})
+		reclaimedSpace = reclaimedSpace + p.Size
+	}
+	if errorMsg.Len() > 0 {
+		utils.InternalServerError(w, errors.New(errorMsg.String()))
+		return
+	}
+
+	payload := handlers.ImagesPruneReport{
+		ImagesPruneReport: types.ImagesPruneReport{
+			ImagesDeleted:  idr,
+			SpaceReclaimed: reclaimedSpace,
+		},
+	}
+	utils.WriteResponse(w, http.StatusOK, payload)
+}

pkg/api/handlers/compat/volumes.go

@@ -1,6 +1,7 @@
 package compat
 
 import (
+	"bytes"
 	"encoding/json"
 	"net/http"
 	"net/url"
@@ -8,6 +9,7 @@ import (
 
 	"github.com/containers/podman/v2/libpod"
 	"github.com/containers/podman/v2/libpod/define"
+	"github.com/containers/podman/v2/pkg/api/handlers"
 	"github.com/containers/podman/v2/pkg/api/handlers/utils"
 	"github.com/containers/podman/v2/pkg/domain/filters"
 	"github.com/containers/podman/v2/pkg/domain/infra/abi/parse"
@@ -268,17 +270,29 @@ func PruneVolumes(w http.ResponseWriter, r *http.Request) {
 		utils.InternalServerError(w, err)
 		return
 	}
+
+	var errorMsg bytes.Buffer
+	var reclaimedSpace uint64
 	prunedIds := make([]string, 0, len(pruned))
 	for _, v := range pruned {
-		// XXX: This drops any pruning per-volume error messages on the floor
-		prunedIds = append(prunedIds, v.Id)
+		if v.Err != nil {
+			errorMsg.WriteString(v.Err.Error())
+			errorMsg.WriteString("; ")
+			continue
 		}
-	pruneResponse := docker_api_types.VolumesPruneReport{
-		VolumesDeleted: prunedIds,
-		// TODO: We don't have any insight into how much space was reclaimed
-		// from `PruneVolumes()` but it's not nullable
-		SpaceReclaimed: 0,
+		prunedIds = append(prunedIds, v.Id)
+		reclaimedSpace += v.Size
+	}
+	if errorMsg.Len() > 0 {
+		utils.InternalServerError(w, errors.New(errorMsg.String()))
+		return
 	}
 
-	utils.WriteResponse(w, http.StatusOK, pruneResponse)
+	payload := handlers.VolumesPruneReport{
+		VolumesPruneReport: docker_api_types.VolumesPruneReport{
+			VolumesDeleted: prunedIds,
+			SpaceReclaimed: reclaimedSpace,
+		},
+	}
+	utils.WriteResponse(w, http.StatusOK, payload)
 }

pkg/api/handlers/libpod/pods.go

@@ -235,7 +235,7 @@ func PodRestart(w http.ResponseWriter, r *http.Request) {
 }
 
 func PodPrune(w http.ResponseWriter, r *http.Request) {
-	reports, err := PodPruneHelper(w, r)
+	reports, err := PodPruneHelper(r)
 	if err != nil {
 		utils.InternalServerError(w, err)
 		return
@@ -243,7 +243,7 @@ func PodPrune(w http.ResponseWriter, r *http.Request) {
 	utils.WriteResponse(w, http.StatusOK, reports)
 }
 
-func PodPruneHelper(w http.ResponseWriter, r *http.Request) ([]*entities.PodPruneReport, error) {
+func PodPruneHelper(r *http.Request) ([]*entities.PodPruneReport, error) {
 	var (
 		runtime = r.Context().Value("runtime").(*libpod.Runtime)
 	)

pkg/api/handlers/libpod/system.go

@@ -30,7 +30,7 @@ func SystemPrune(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	podPruneReport, err := PodPruneHelper(w, r)
+	podPruneReport, err := PodPruneHelper(r)
 	if err != nil {
 		utils.InternalServerError(w, err)
 		return
@@ -38,7 +38,7 @@ func SystemPrune(w http.ResponseWriter, r *http.Request) {
 	systemPruneReport.PodPruneReport = podPruneReport
 
 	// We could parallelize this, should we?
-	containerPruneReports, err := compat.PruneContainersHelper(w, r, nil)
+	containerPruneReports, err := compat.PruneContainersHelper(r, nil)
 	if err != nil {
 		utils.InternalServerError(w, err)
 		return

pkg/api/server/register_networks.go

@@ -9,6 +9,19 @@ import (
 )
 
 func (s *APIServer) registerNetworkHandlers(r *mux.Router) error {
+	// swagger:operation POST /networks/prune compat compatPruneNetwork
+	// ---
+	// tags:
+	// - networks (compat)
+	// Summary: Delete unused networks
+	// description: Not supported
+	// produces:
+	// - application/json
+	// responses:
+	//   404:
+	//     $ref: "#/responses/NoSuchNetwork"
+	r.HandleFunc(VersionedPath("/networks/prune"), compat.UnsupportedHandler).Methods(http.MethodPost)
+	r.HandleFunc("/networks/prune", compat.UnsupportedHandler).Methods(http.MethodPost)
 	// swagger:operation DELETE /networks/{name} compat compatRemoveNetwork
 	// ---
 	// tags:

test/apiv2/rest_api/test_rest_v2_0_0.py

@@ -1,13 +1,15 @@
 import json
+import os
 import random
+import shutil
 import string
 import subprocess
+import sys
+import time
 import unittest
 from multiprocessing import Process
 
 import requests
-import sys
-import time
 from dateutil.parser import parse
 
 from test.apiv2.rest_api import Podman
@@ -449,7 +451,7 @@ class TestApi(unittest.TestCase):
         self.assertEqual(inspect.status_code, 404, inspect.content)
 
         prune = requests.post(PODMAN_URL + "/v1.40/networks/prune")
-        self.assertEqual(prune.status_code, 405, prune.content)
+        self.assertEqual(prune.status_code, 404, prune.content)
 
     def test_volumes_compat(self):
         name = "Volume_" + "".join(random.choice(string.ascii_letters) for i in range(10))
@@ -499,8 +501,18 @@ class TestApi(unittest.TestCase):
         rm = requests.delete(PODMAN_URL + f"/v1.40/volumes/{name}")
         self.assertEqual(rm.status_code, 204, rm.content)
 
+        # recreate volume with data and then prune it
+        r = requests.post(PODMAN_URL + "/v1.40/volumes/create", json={"Name": name})
+        self.assertEqual(create.status_code, 201, create.content)
+        create = json.loads(r.content)
+        with open(os.path.join(create["Mountpoint"], "test_prune"), "w") as file:
+            file.writelines(["This is a test\n", "This is a good test\n"])
+
         prune = requests.post(PODMAN_URL + "/v1.40/volumes/prune")
         self.assertEqual(prune.status_code, 200, prune.content)
+        payload = json.loads(prune.content)
+        self.assertIn(name, payload["VolumesDeleted"])
+        self.assertGreater(payload["SpaceReclaimed"], 0)
 
     def test_auth_compat(self):
         r = requests.post(
@@ -530,6 +542,50 @@ class TestApi(unittest.TestCase):
         self.assertIn("Volumes", obj)
         self.assertIn("BuildCache", obj)
 
+    def test_prune_compat(self):
+        name = "Ctnr_" + "".join(random.choice(string.ascii_letters) for i in range(10))
+
+        r = requests.post(
+            PODMAN_URL + f"/v1.40/containers/create?name={name}",
+            json={
+                "Cmd": ["cp", "/etc/motd", "/motd.size_test"],
+                "Image": "alpine:latest",
+                "NetworkDisabled": True,
+            },
+        )
+        self.assertEqual(r.status_code, 201, r.text)
+        create = json.loads(r.text)
+
+        r = requests.post(PODMAN_URL + f"/v1.40/containers/{create['Id']}/start")
+        self.assertEqual(r.status_code, 204, r.text)
+
+        r = requests.post(PODMAN_URL + f"/v1.40/containers/{create['Id']}/wait")
+        self.assertEqual(r.status_code, 200, r.text)
+        wait = json.loads(r.text)
+        self.assertEqual(wait["StatusCode"], 0, wait["Error"]["Message"])
+
+        prune = requests.post(PODMAN_URL + "/v1.40/containers/prune")
+        self.assertEqual(prune.status_code, 200, prune.status_code)
+        prune_payload = json.loads(prune.text)
+        self.assertGreater(prune_payload["SpaceReclaimed"], 0)
+        self.assertIn(create["Id"], prune_payload["ContainersDeleted"])
+
+        # Delete any orphaned containers
+        r = requests.get(PODMAN_URL + "/v1.40/containers/json?all=true")
+        self.assertEqual(r.status_code, 200, r.text)
+        for ctnr in json.loads(r.text):
+            requests.delete(PODMAN_URL + f"/v1.40/containers/{ctnr['Id']}?force=true")
+
+        prune = requests.post(PODMAN_URL + "/v1.40/images/prune")
+        self.assertEqual(prune.status_code, 200, prune.text)
+        prune_payload = json.loads(prune.text)
+        self.assertGreater(prune_payload["SpaceReclaimed"], 0)
+
+        # FIXME need method to determine which image is going to be "pruned" to fix test
+        # TODO should handler be recursive when deleting images?
+        # self.assertIn(img["Id"], prune_payload["ImagesDeleted"][1]["Deleted"])
+        self.assertIsNotNone(prune_payload["ImagesDeleted"][1]["Deleted"])
+
 
 if __name__ == "__main__":
     unittest.main()