fix(storage): return 409 when deleting attached volume

internal/api/handler/common.go

@@ -2,8 +2,11 @@ package handler
 
 import (
 	"encoding/json"
+	"errors"
 	"net/http"
 	"strings"
+
+	iaerrors "github.com/virtfoundry/core/internal/pkg/errors"
 )
 
 func respondJSON(w http.ResponseWriter, status int, data interface{}) {
@@ -15,6 +18,15 @@ func respondJSON(w http.ResponseWriter, status int, data interface{}) {
 }
 
 func respondError(w http.ResponseWriter, err error) {
+	var iaErr *iaerrors.IaaSError
+	if errors.As(err, &iaErr) {
+		msg := iaErr.Message
+		if iaErr.Detail != "" {
+			msg = msg + ": " + iaErr.Detail
+		}
+		respondJSON(w, iaErr.HTTPStatus(), map[string]string{"error": sanitizeClientError(msg)})
+		return
+	}
 	status := http.StatusInternalServerError
 	msg := sanitizeClientError(err.Error())
 	if msg == "tenant not found" || msg == "tenant_id required for root" || msg == "no tenant assigned" {

internal/service/storage/service.go

@@ -7,6 +7,7 @@ import (
 	platformk8s "github.com/virtfoundry/core/internal/platform/k8s"
 	"github.com/virtfoundry/core/internal/platform"
 	"github.com/virtfoundry/core/internal/platform/store"
+	iaerrors "github.com/virtfoundry/core/internal/pkg/errors"
 	"github.com/virtfoundry/core/internal/service/shared"
 )
 
@@ -51,10 +52,10 @@ func (s *Service) ListVolumes(tenantID string) []*platform.Volume {
 func (s *Service) DeleteVolume(ctx context.Context, tenantID, volumeID string) error {
 	vol, ok := s.store.GetVolume(volumeID)
 	if !ok || vol.TenantID != tenantID {
-		return fmt.Errorf("volume not found")
+		return iaerrors.NewNotFoundError("volume", volumeID)
 	}
 	if vol.VMID != "" {
-		return fmt.Errorf("volume is attached to a VM")
+		return iaerrors.NewResourceInUseError("volume", "attached to a VM")
 	}
 	if err := s.k8s.DeletePVC(ctx, vol.Namespace, vol.PVCName); err != nil {
 		return fmt.Errorf("delete pvc: %w", err)

internal/service/storage/service_test.go

@@ -0,0 +1,46 @@
+package storage
+
+import (
+	"context"
+	"errors"
+	"net/http"
+	"testing"
+
+	"github.com/virtfoundry/core/internal/platform"
+	"github.com/virtfoundry/core/internal/platform/store"
+	iaerrors "github.com/virtfoundry/core/internal/pkg/errors"
+)
+
+func TestDeleteVolume_AttachedReturnsConflict(t *testing.T) {
+	st := store.NewMemory()
+	svc := New(st, nil)
+	tenantID := store.NewID()
+	volID := store.NewID()
+	st.SaveVolume(&platform.Volume{
+		ID: volID, TenantID: tenantID, VMID: "vm-1",
+		Namespace: "tenant-ns", PVCName: "vol-pvc", State: "attached",
+	})
+
+	err := svc.DeleteVolume(context.Background(), tenantID, volID)
+	var iaErr *iaerrors.IaaSError
+	if !errors.As(err, &iaErr) {
+		t.Fatalf("expected IaaSError, got %v", err)
+	}
+	if iaErr.HTTPStatus() != http.StatusConflict {
+		t.Fatalf("HTTP status %d, want 409", iaErr.HTTPStatus())
+	}
+}
+
+func TestDeleteVolume_NotFound(t *testing.T) {
+	st := store.NewMemory()
+	svc := New(st, nil)
+
+	err := svc.DeleteVolume(context.Background(), store.NewID(), "missing")
+	var iaErr *iaerrors.IaaSError
+	if !errors.As(err, &iaErr) {
+		t.Fatalf("expected IaaSError, got %v", err)
+	}
+	if iaErr.HTTPStatus() != http.StatusNotFound {
+		t.Fatalf("HTTP status %d, want 404", iaErr.HTTPStatus())
+	}
+}

scripts/e2e/phase-1-volumes.sh

@@ -41,7 +41,7 @@ print(len([v for v in json.load(sys.stdin)['volumes'] if v['id']=='$VOL_ID']))
   [ "$COUNT" = "1" ] || e2e_fail "volume not listed on VM"
 
   DEL_ATT=$(curl -sS -o /dev/null -w "%{http_code}" "$BASE_URL/volumes/$VOL_ID" -X DELETE -H "$AUTH")
-  [[ "$DEL_ATT" == "200" || "$DEL_ATT" == "204" ]] && e2e_fail "delete should fail while attached (got $DEL_ATT)"
+  [ "$DEL_ATT" = "409" ] || e2e_fail "delete should return 409 while attached (got $DEL_ATT)"
   e2e_pass "delete blocked while attached (HTTP $DEL_ATT)"
 
   DET=$(curl -sS -o /dev/null -w "%{http_code}" "$BASE_URL/vms/$VM_NAME/volumes/$VOL_ID" -X DELETE -H "$AUTH")