From 3fed1d04b476d8c549565153d9e1f03d67695182 Mon Sep 17 00:00:00 2001 From: catborise Date: Thu, 5 Nov 2026 16:02:50 +0300 Subject: [PATCH] fix(api): a VM action answers with what the page would have shown after it The API's VM actions call the web views and only looked at the status code; the views report a partial result (a disk that was not deleted or was kept, a template that was not started) as a message for the page they redirect to. The API reported plain success and left the messages in the session. Its answer now carries them in "messages". --- instances/api/viewsets.py | 21 ++++++++++++--------- instances/tests/test_destroy.py | 14 ++++++++++++++ webvirtcloud/serializers.py | 7 +++++++ 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/instances/api/viewsets.py b/instances/api/viewsets.py index 6d1cb8ca..2b5a6cd5 100644 --- a/instances/api/viewsets.py +++ b/instances/api/viewsets.py @@ -1,6 +1,7 @@ from admin.permissions import IsSuperUser from computes import utils from computes.models import Compute +from django.contrib.messages import get_messages from django.http import Http404 from django.shortcuts import get_object_or_404 from django.urls import reverse @@ -23,7 +24,7 @@ from operations.api.serializers import OperationSerializer from rest_framework import permissions, status, viewsets from rest_framework.decorators import action from rest_framework.response import Response -from webvirtcloud.serializers import StatusSerializer +from webvirtcloud.serializers import ActionStatusSerializer from .serializers import ( CreateInstanceSerializer, @@ -39,7 +40,9 @@ def instance_action_response(action, request, pk, message): response = action(request, pk) if response.status_code >= 400: return response - return Response({"status": message}) + # the page's view reports a partial result (a disk that was kept) as a + # message for the page it redirects to: the API answers with them + return Response({"status": message, "messages": [str(m) for m in get_messages(request)]}) class InstancesViewSet(viewsets.ViewSet): @@ -121,14 +124,14 @@ class InstanceViewSet(viewsets.ViewSet): return Response(serializer.data) - @extend_schema(request=DestroyOptionsSerializer, responses=StatusSerializer) + @extend_schema(request=DestroyOptionsSerializer, responses=ActionStatusSerializer) def destroy(self, request, pk=None, compute_pk=None): queryset = get_instance(request.user, pk, perm_type="delete") if compute_pk is not None and str(queryset.compute_id) != str(compute_pk): raise Http404() return instance_action_response(instance_destroy, request, pk, "Instance is destroyed") - @extend_schema(request=None, responses=StatusSerializer) + @extend_schema(request=None, responses=ActionStatusSerializer) @action(detail=True, methods=["post"]) def poweron(self, request, pk=None, compute_pk=None): queryset = get_instance(request.user, pk, perm_type="power") @@ -138,7 +141,7 @@ class InstanceViewSet(viewsets.ViewSet): return Response({"status": "Templates cannot be started"}, status=status.HTTP_409_CONFLICT) return instance_action_response(poweron, request, pk, "poweron command send") - @extend_schema(request=None, responses=StatusSerializer) + @extend_schema(request=None, responses=ActionStatusSerializer) @action(detail=True, methods=["post"]) def poweroff(self, request, pk=None, compute_pk=None): queryset = get_instance(request.user, pk, perm_type="power") @@ -146,7 +149,7 @@ class InstanceViewSet(viewsets.ViewSet): raise Http404() return instance_action_response(poweroff, request, pk, "poweroff command send") - @extend_schema(request=None, responses=StatusSerializer) + @extend_schema(request=None, responses=ActionStatusSerializer) @action(detail=True, methods=["post"]) def powercycle(self, request, pk=None, compute_pk=None): queryset = get_instance(request.user, pk, perm_type="power") @@ -156,7 +159,7 @@ class InstanceViewSet(viewsets.ViewSet): return Response({"status": "Templates cannot be started"}, status=status.HTTP_409_CONFLICT) return instance_action_response(powercycle, request, pk, "powercycle command send") - @extend_schema(request=None, responses=StatusSerializer) + @extend_schema(request=None, responses=ActionStatusSerializer) @action(detail=True, methods=["post"]) def forceoff(self, request, pk=None, compute_pk=None): queryset = get_instance(request.user, pk, perm_type="power") @@ -164,7 +167,7 @@ class InstanceViewSet(viewsets.ViewSet): raise Http404() return instance_action_response(force_off, request, pk, "force off command send") - @extend_schema(request=None, responses=StatusSerializer) + @extend_schema(request=None, responses=ActionStatusSerializer) @action(detail=True, methods=["post"]) def suspend(self, request, pk=None, compute_pk=None): queryset = get_instance(request.user, pk) @@ -172,7 +175,7 @@ class InstanceViewSet(viewsets.ViewSet): raise Http404() return instance_action_response(suspend, request, pk, "suspend command send") - @extend_schema(request=None, responses=StatusSerializer) + @extend_schema(request=None, responses=ActionStatusSerializer) @action(detail=True, methods=["post"]) def resume(self, request, pk=None, compute_pk=None): queryset = get_instance(request.user, pk) diff --git a/instances/tests/test_destroy.py b/instances/tests/test_destroy.py index a10430ab..0115cad8 100644 --- a/instances/tests/test_destroy.py +++ b/instances/tests/test_destroy.py @@ -65,6 +65,20 @@ class DestroyOrderTestCase(TestCase): self.assertEqual(response.status_code, 200) self.assertEqual(proxy.get_volume_by_path.called, deleted) + def test_the_api_tells_which_disks_were_not_deleted(self): + with patch("instances.models.wvmInstance") as wvm: + proxy = wvm.return_value + proxy.instance.isActive.return_value = False + proxy.split_disk_paths_by_use.return_value = (["/pool/own.qcow2"], ["/pool/shared.qcow2"]) + proxy.backing_path.return_value = None + proxy.get_volume_by_path.return_value.delete.side_effect = libvirtError("Permission denied") + url = f"/api/v1/computes/{self.instance.compute_id}/instances/{self.instance.id}/" + response = self.client.delete(url, {"delete_disk": True}, content_type="application/json") + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json(), {"status": "Instance is destroyed", "messages": [ + "The VM is removed, but disk /pool/own.qcow2 was not deleted: Permission denied", + "Disk /pool/shared.qcow2 is used by another VM and was kept"]}) + def test_a_disk_file_that_stays_keeps_the_files_it_needs(self): # the overlay is not deleted: the file below it would leave it broken volumes = {} diff --git a/webvirtcloud/serializers.py b/webvirtcloud/serializers.py index 027a2690..33319f80 100644 --- a/webvirtcloud/serializers.py +++ b/webvirtcloud/serializers.py @@ -5,3 +5,10 @@ class StatusSerializer(serializers.Serializer): """Response body of API actions that only report a status message.""" status = serializers.CharField() + + +class ActionStatusSerializer(StatusSerializer): + """A VM action's status and what the web page would have shown after it, + such as a disk that was kept or a template that was not started.""" + + messages = serializers.ListField(child=serializers.CharField())