mirror of
https://github.com/retspen/webvirtcloud
synced 2026-11-08 11:16:47 +00:00
fix(api): VM API answers as the VM pages do
- A VM delete reads delete_disk and delete_nvram as booleans from a JSON
or form body. The view read them as HTML checkboxes from the form body
only: JSON {"delete_disk": true} kept the disks while reporting success,
and a form "delete_disk=false" deleted them. Refused options stop
nothing; the API schema names both options.
- Power on and power cycle of a template answer 409 instead of reporting a
start that did not happen.
- The VM details leave out the host's ISO images for non-superusers, as
the VM page does; reading them also refreshed every pool of the host.
- Flavors take values of at least 1, as the web form does, and show their
id for later changes; a migration needs neither live nor offline; a VM
create on an unknown compute answers 404 instead of 500.
This commit is contained in:
parent
726f330dd0
commit
8d7f28482f
6 changed files with 119 additions and 6 deletions
|
|
@ -10,6 +10,15 @@ class InstanceSerializer(serializers.ModelSerializer):
|
|||
|
||||
|
||||
class InstanceDetailsSerializer(serializers.ModelSerializer):
|
||||
def get_fields(self):
|
||||
fields = super().get_fields()
|
||||
# the host's ISO images are a superuser's, as on the VM page; reading
|
||||
# them refreshes every pool of the host
|
||||
request = self.context.get("request")
|
||||
if not (request and request.user.is_superuser):
|
||||
del fields["media_iso"]
|
||||
return fields
|
||||
|
||||
class Meta:
|
||||
model = Instance
|
||||
fields = [
|
||||
|
|
@ -49,7 +58,17 @@ class InstanceDetailsSerializer(serializers.ModelSerializer):
|
|||
class FlavorSerializer(serializers.ModelSerializer):
|
||||
class Meta:
|
||||
model = Flavor
|
||||
fields = ["label", "memory", "vcpu", "disk"]
|
||||
fields = ["id", "label", "memory", "vcpu", "disk"]
|
||||
# as the web form (instances.forms.FlavorForm): MiB, vCPUs and GiB, each at least 1
|
||||
extra_kwargs = {"memory": {"min_value": 1}, "vcpu": {"min_value": 1}, "disk": {"min_value": 1}}
|
||||
|
||||
|
||||
class DestroyOptionsSerializer(serializers.Serializer):
|
||||
"""What a VM's delete takes besides the VM: from the web form's checkboxes
|
||||
or an API body, JSON or form-encoded ("false" is false)."""
|
||||
|
||||
delete_disk = serializers.BooleanField(default=False)
|
||||
delete_nvram = serializers.BooleanField(default=False)
|
||||
|
||||
|
||||
class CreateInstanceSerializer(serializers.ModelSerializer):
|
||||
|
|
@ -92,7 +111,8 @@ class CreateInstanceSerializer(serializers.ModelSerializer):
|
|||
|
||||
class MigrateSerializer(serializers.ModelSerializer):
|
||||
instance = Instance.objects.all().prefetch_related("userinstance_set")
|
||||
live = serializers.BooleanField(initial=True)
|
||||
live = serializers.BooleanField(default=True)
|
||||
offline = serializers.BooleanField(default=False)
|
||||
# left out, also from a form-encoded request: auto converge on
|
||||
autoconverge = serializers.BooleanField(default=True)
|
||||
compress = serializers.BooleanField(default=False)
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ from webvirtcloud.serializers import StatusSerializer
|
|||
|
||||
from .serializers import (
|
||||
CreateInstanceSerializer,
|
||||
DestroyOptionsSerializer,
|
||||
FlavorSerializer,
|
||||
InstanceDetailsSerializer,
|
||||
InstanceSerializer,
|
||||
|
|
@ -119,7 +120,7 @@ class InstanceViewSet(viewsets.ViewSet):
|
|||
|
||||
return Response(serializer.data)
|
||||
|
||||
@extend_schema(responses=StatusSerializer)
|
||||
@extend_schema(request=DestroyOptionsSerializer, responses=StatusSerializer)
|
||||
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):
|
||||
|
|
@ -132,6 +133,8 @@ class InstanceViewSet(viewsets.ViewSet):
|
|||
queryset = get_instance(request.user, pk, perm_type="power")
|
||||
if compute_pk is not None and str(queryset.compute_id) != str(compute_pk):
|
||||
raise Http404()
|
||||
if queryset.is_template:
|
||||
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)
|
||||
|
|
@ -148,6 +151,8 @@ class InstanceViewSet(viewsets.ViewSet):
|
|||
queryset = get_instance(request.user, pk, perm_type="power")
|
||||
if compute_pk is not None and str(queryset.compute_id) != str(compute_pk):
|
||||
raise Http404()
|
||||
if queryset.is_template:
|
||||
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)
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import re
|
|||
import time
|
||||
|
||||
from django.core.exceptions import PermissionDenied
|
||||
from django.shortcuts import get_object_or_404
|
||||
from django.template.loader import render_to_string
|
||||
from django.utils.translation import gettext_noop
|
||||
from django.utils.translation import gettext_noop as _
|
||||
|
|
@ -648,7 +649,7 @@ class CreateInstance(Kind):
|
|||
if not user.is_superuser:
|
||||
raise PermissionDenied
|
||||
data = params
|
||||
c = Compute.objects.get(pk=data["compute"])
|
||||
c = get_object_or_404(Compute, pk=data["compute"])
|
||||
conn = wvmCreate(c.hostname, c.login, c.password, c.type)
|
||||
cdrom_buses = [bus for bus in conn.get_disk_bus_types(data["arch"], data["machine"]) if bus in CDROM_BUSES]
|
||||
if data["name"] in conn.get_instances():
|
||||
|
|
|
|||
62
instances/tests/test_api_parity.py
Normal file
62
instances/tests/test_api_parity.py
Normal file
|
|
@ -0,0 +1,62 @@
|
|||
"""The VM API answers as its web pages do: a template is not started, the
|
||||
host's ISO images are a superuser's, flavors and migrations take what the
|
||||
web forms take."""
|
||||
|
||||
from unittest.mock import patch
|
||||
|
||||
from accounts.models import UserInstance
|
||||
from computes.models import Compute
|
||||
from django.contrib.auth import get_user_model
|
||||
from django.test import TestCase
|
||||
from instances.api.serializers import InstanceDetailsSerializer, MigrateSerializer
|
||||
from instances.models import Flavor, Instance
|
||||
|
||||
|
||||
class ApiParityTests(TestCase):
|
||||
def setUp(self):
|
||||
self.user = get_user_model().objects.create_user("api-owner", password="pw")
|
||||
self.admin = get_user_model().objects.create_superuser("api-admin", "api@example.com", "pw")
|
||||
self.compute = Compute.objects.create(name="api-host", hostname="192.0.2.30", login="u", password="p", type=1)
|
||||
self.vm = Instance.objects.create(compute=self.compute, name="api-vm", uuid="31313131-0000-0000-0000-000000000000",
|
||||
is_template=True)
|
||||
UserInstance.objects.create(user=self.user, instance=self.vm)
|
||||
self.url = f"/api/v1/computes/{self.compute.id}/instances/{self.vm.id}/"
|
||||
|
||||
def test_a_template_is_not_started(self):
|
||||
self.client.force_login(self.user)
|
||||
with patch("instances.models.wvmInstance") as wvm:
|
||||
for action in ("poweron", "powercycle"):
|
||||
with self.subTest(action=action):
|
||||
self.assertEqual(self.client.post(self.url + f"{action}/").status_code, 409)
|
||||
wvm.return_value.start.assert_not_called()
|
||||
|
||||
def test_the_hosts_iso_images_are_a_superusers(self):
|
||||
class Request:
|
||||
def __init__(self, user):
|
||||
self.user = user
|
||||
|
||||
for user, shown in ((self.user, False), (self.admin, True)):
|
||||
fields = InstanceDetailsSerializer(self.vm, context={"request": Request(user)}).fields
|
||||
self.assertEqual("media_iso" in fields, shown, user)
|
||||
|
||||
def test_a_flavor_is_at_least_one_of_each_and_has_its_id(self):
|
||||
self.client.force_login(self.admin)
|
||||
response = self.client.post("/api/v1/flavor/", {"label": "zero", "memory": 0, "vcpu": -1, "disk": 0},
|
||||
content_type="application/json")
|
||||
self.assertEqual(set(response.json()), {"memory", "vcpu", "disk"})
|
||||
flavor = Flavor.objects.create(label="one", memory=1024, vcpu=1, disk=10)
|
||||
self.assertEqual(self.client.get(f"/api/v1/flavor/{flavor.id}/").json()["id"], flavor.id)
|
||||
|
||||
def test_a_migration_needs_neither_live_nor_offline(self):
|
||||
other = Compute.objects.create(name="api-host-2", hostname="192.0.2.31", login="u", password="p", type=1)
|
||||
data = {"instance": self.vm.id, "target_compute": other.id, "submission_key": "k" * 12}
|
||||
serializer = MigrateSerializer(data=data)
|
||||
self.assertTrue(serializer.is_valid(), serializer.errors)
|
||||
self.assertEqual((serializer.validated_data["live"], serializer.validated_data["offline"]), (True, False))
|
||||
|
||||
def test_a_vm_on_an_unknown_compute_is_not_found(self):
|
||||
from django.http import Http404
|
||||
from instances.jobs import CreateInstance
|
||||
|
||||
with self.assertRaises(Http404):
|
||||
CreateInstance().validate(self.admin, {"compute": 999999})
|
||||
|
|
@ -44,6 +44,26 @@ class DestroyOrderTestCase(TestCase):
|
|||
self.assertTrue(flags & wanted)
|
||||
self.assertFalse(flags & unwanted)
|
||||
|
||||
def test_the_api_takes_its_options_as_booleans(self):
|
||||
url = f"/api/v1/computes/{self.instance.compute_id}/instances/{self.instance.id}/"
|
||||
for body, content_type, deleted in (
|
||||
({"delete_disk": True}, "application/json", True),
|
||||
("delete_disk=false", "application/x-www-form-urlencoded", False),
|
||||
):
|
||||
with self.subTest(body=body):
|
||||
Instance.objects.get_or_create(
|
||||
compute=self.instance.compute, name="destroy-vm", uuid="11111111-2222-3333-4444-555555555555"
|
||||
)
|
||||
self.instance = Instance.objects.get(uuid="11111111-2222-3333-4444-555555555555")
|
||||
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"], [])
|
||||
url = f"/api/v1/computes/{self.instance.compute_id}/instances/{self.instance.id}/"
|
||||
response = self.client.delete(url, body, content_type=content_type)
|
||||
self.assertEqual(response.status_code, 200)
|
||||
self.assertEqual(proxy.get_volume_by_path.called, deleted)
|
||||
|
||||
def test_disks_are_deleted_after_the_undefine(self):
|
||||
proxy = self.destroy()
|
||||
proxy.get_volume_by_path.assert_called_once_with("/pool/own.qcow2")
|
||||
|
|
|
|||
|
|
@ -55,6 +55,7 @@ from vrtManager.util import randomPasswd
|
|||
from instances.models import Instance
|
||||
|
||||
from . import addresses, report, utils
|
||||
from .api.serializers import DestroyOptionsSerializer
|
||||
from .forms import ConsoleForm, FlavorForm, NewVMForm
|
||||
from .models import Flavor
|
||||
from .states import STATES, UNKNOWN, state_label
|
||||
|
|
@ -764,6 +765,10 @@ def force_off(request, pk):
|
|||
def destroy(request, pk):
|
||||
if request.method in ["POST", "DELETE"]:
|
||||
instance = get_instance(request.user, pk, perm_type="delete")
|
||||
# an API request is a DRF request whose body is in .data; refused
|
||||
# options stop nothing
|
||||
options = DestroyOptionsSerializer(data=getattr(request, "data", request.POST))
|
||||
options.is_valid(raise_exception=True)
|
||||
with libvirt_instance_lock(instance):
|
||||
with audit_vm(request, instance, "delete"):
|
||||
proxy = instance.proxy
|
||||
|
|
@ -773,14 +778,14 @@ def destroy(request, pk):
|
|||
proxy.force_shutdown()
|
||||
|
||||
to_delete, shared = [], []
|
||||
if request.POST.get("delete_disk", ""):
|
||||
if options.validated_data["delete_disk"]:
|
||||
to_delete, shared = proxy.split_disk_paths_by_use()
|
||||
|
||||
# Undefine before deleting any disk: if it fails, nothing is lost.
|
||||
# Snapshot metadata goes with the domain; internal snapshot data
|
||||
# stays in the disk images that are kept.
|
||||
flags = VIR_DOMAIN_UNDEFINE_MANAGED_SAVE | VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA
|
||||
if request.POST.get("delete_nvram", ""):
|
||||
if options.validated_data["delete_nvram"]:
|
||||
flags |= VIR_DOMAIN_UNDEFINE_NVRAM
|
||||
else:
|
||||
flags |= VIR_DOMAIN_UNDEFINE_KEEP_NVRAM
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue