j3d1/webvirtcloud
1
0
Fork 0
mirror of https://github.com/retspen/webvirtcloud synced 2026-11-08 11:16:47 +00:00
Code Issues Projects Releases Wiki Activity

fix(instances): the quota counts every VM and refuses what it cannot verify

Browse source 
check_user_quota added up the vCPUs, memory and disks of the user's
VMs, but skipped a VM whose host failed a TCP probe. A host that was
down therefore let users exceed their quota, and the probe always failed
for ssh computes named by an ssh_config alias or reached through
ProxyJump, so their VMs were never counted. A VM that could not be read
turned the page into the error page instead.

- Hosts are checked with Compute.status, the connection WebVirtCloud
  uses, once per host. A VM that cannot be read (host down, VM gone)
  makes the usage unknown: clone and resize up are refused with their
  own message (fail closed).
- A request that adds nothing is not checked, so a user over quota can
  still shrink a VM. Users without limits, and an exceeded instance
  count (from the database), need no host either.
- Memory resize of a running VM changes only the current memory, but
  the quota was charged with the submitted maximum. It is now charged
  with no change, and resize_mem gets the current maximum, so a VM that
  shuts down meanwhile cannot take an unchecked one.
- The VM page no longer computes the quota on every load (it read every
  VM of the user on every host for a "quota reached" banner). The limit
  is reported when a change is refused.

The test that expected a down host to pass the quota now expects it to
be refused. Checked against the test hosts: with a VM behind an
ssh_config alias the previous code counted 29 vCPUs, the new one 31;
with a VM on an unreachable host the previous code allowed +1 vCPU,
the new one refuses it after 5.1 s and allows -1 at once. The new
tests fail on the previous code, except the ones guarding unchanged
behaviour.
This commit is contained in:
catborise 2026-10-05 12:26:57 +03:00
parent a3d3cca477
commit fb3f686532
5 changed files with 245 additions and 35 deletions
Download patch file Download diff file Expand all files Collapse all files

7
computes/test_quota_down_host.py
View file

@ -6,11 +6,12 @@ from computes.models import Compute
from django.contrib.auth import get_user_model
from django.test import TestCase
from instances.models import Instance
from instances.utils import check_user_quota
from instances.utils import QUOTA_UNVERIFIED, check_user_quota
class QuotaWithDownHostTestCase(TestCase):
def test_vm_on_an_unreachable_host_does_not_break_the_quota_check(self):
def test_vm_on_an_unreachable_host_makes_the_quota_unverified(self):
# its usage is unknown, so an increase is refused (fail closed), without an error page
sock = socket.socket()
sock.bind(("127.0.0.1", 0))
port = sock.getsockname()[1]
@ -22,4 +23,4 @@ class QuotaWithDownHostTestCase(TestCase):
UserInstance.objects.create(user=user, instance=vm)
get_settings() # app settings are normally loaded by middleware
self.assertEqual(check_user_quota(user, 1, 1, 128, 1), "")
self.assertEqual(check_user_quota(user, 1, 1, 128, 1), QUOTA_UNVERIFIED)

5
instances/templates/instance.html
View file

@ -67,11 +67,6 @@
<span>{% bs_icon 'repeat'%} </span>
</a>
</div>
{% if user_quota_msg %}
<div class="alert alert-warning fade show">
{{ user_quota_msg|capfirst }} {% trans "quota reached" %}.
</div>
{% endif %}
<hr>
</div>

179
instances/test_quota.py Normal file
View file

@ -0,0 +1,179 @@
"""check_user_quota counts every VM of the user and refuses an increase it
cannot verify (fail closed); requests that add nothing are not checked."""
from types import SimpleNamespace
from unittest.mock import patch
from accounts.models import UserAttributes, UserInstance
from appsettings.settings import get_settings
from computes.models import Compute
from django.contrib.auth import get_user_model
from django.contrib.messages import get_messages
from django.test import TestCase
from django.urls import reverse
from instances.models import Instance
from instances.utils import QUOTA_UNVERIFIED, check_user_quota
from libvirt import VIR_DOMAIN_RUNNING
from vrtManager.util import OperationError
def vm_reader(vcpu=2, memory=1024, disk_gib=10):
def make(*args, **kwargs):
return SimpleNamespace(
get_vcpu=lambda: vcpu,
get_memory=lambda: memory,
get_disk_devices=lambda: [{"size": disk_gib << 30}],
)
return make
class QuotaTests(TestCase):
def setUp(self):
get_settings() # app settings are normally loaded by middleware
self.user = get_user_model().objects.create_user("quota-user", password="pw")
UserAttributes.objects.create(user=self.user, max_instances=5, max_cpus=8, max_memory=8192, max_disk_size=100)
self.up = Compute.objects.create(name="up", hostname="wvc-alias", login="root", password="", type=2)
self.down = Compute.objects.create(name="down", hostname="192.0.2.1", login="u", password="p", type=1)
self.status_checks = []
def own(self, compute, n):
vm = Instance.objects.create(compute=compute, name=f"vm-{compute.name}-{n}", uuid=f"{n:08d}-0000-0000-0000-{compute.pk:012d}")
UserInstance.objects.create(user=self.user, instance=vm)
return vm
def check(self, *delta, reader=None):
test = self
def status(compute):
test.status_checks.append(id(compute))
return compute.pk != test.down.pk
reader = reader or vm_reader()
with patch.object(Compute, "status", property(status)), patch(
"instances.utils.wvmInstance", side_effect=reader
) as wvm:
result = check_user_quota(self.user, *delta)
return result.split(" ")[0], wvm.call_count # without the QUOTA_DEBUG detail
def test_vms_on_a_host_without_a_tcp_route_are_counted(self):
# an ssh_config alias: the old TCP probe failed and skipped these VMs
for n in range(4):
self.own(self.up, n)
self.assertEqual(self.check(0, 1, 0, 0), ("cpu", 4))
def test_a_host_that_is_down_refuses_an_increase(self):
self.own(self.up, 1)
self.own(self.down, 2)
self.assertEqual(self.check(0, 1, 0, 0)[0], QUOTA_UNVERIFIED)
def test_a_vm_that_cannot_be_read_refuses_an_increase(self):
self.own(self.up, 1)
def gone(*args, **kwargs):
raise OperationError("Domain not found")
self.assertEqual(self.check(0, 0, 512, 0, reader=gone)[0], QUOTA_UNVERIFIED)
def test_each_host_is_checked_once(self):
for n in range(3):
self.own(self.up, n)
self.check(0, 1, 0, 0)
# one Compute object asked (its status is cached), not one per VM
self.assertEqual(len(set(self.status_checks)), 1)
def test_a_user_over_quota_can_still_shrink(self):
for n in range(5):
self.own(self.up, n) # 10 vCPUs used of 8
# (the TCP probe of the previous code would skip these VMs and hide the case)
with patch("instances.utils.connection_manager.host_is_up", return_value=True):
self.assertEqual(self.check(0, -1, 0, 0), ("", 0))
self.assertEqual(self.check(0, 0, -512, 0), ("", 0))
def test_unlimited_users_are_not_checked(self):
UserAttributes.objects.filter(user=self.user).update(max_instances=-1, max_cpus=-1, max_memory=0, max_disk_size=-1)
self.own(self.down, 1)
self.assertEqual(self.check(1, 4, 4096, 50), ("", 0))
def test_only_an_instance_limit_needs_no_host(self):
UserAttributes.objects.filter(user=self.user).update(max_instances=2, max_cpus=-1, max_memory=-1, max_disk_size=-1)
self.own(self.down, 1)
self.assertEqual(self.check(1, 4, 4096, 50), ("", 0))
self.own(self.up, 2)
self.assertEqual(self.check(1, 4, 4096, 50), ("instance", 0))
def test_instance_limit_is_checked_before_any_host(self):
for n in range(5):
self.own(self.down, n)
self.assertEqual(self.check(1, 0, 0, 0), ("instance", 0))
def test_templates_are_not_counted(self):
for n in range(4):
vm = self.own(self.up, n)
Instance.objects.filter(pk=vm.pk).update(is_template=True)
self.assertEqual(self.check(0, 2, 0, 0), ("", 3))
class QuotaViewTests(TestCase):
"""The resize views turn the result into a message and charge only what changes."""
def setUp(self):
get_settings()
self.user = get_user_model().objects.create_user("quota-view-user", password="pw")
self.client.force_login(self.user)
compute = Compute.objects.create(name="qv", hostname="10.0.0.1", login="u", password="p", type=1)
self.vm = Instance.objects.create(compute=compute, name="qv-vm", uuid="12121212-0000-0000-0000-000000000000")
UserInstance.objects.create(user=self.user, instance=self.vm, is_change=True)
def post(self, view, data, quota="", status=VIR_DOMAIN_RUNNING):
with patch("instances.views.utils.check_user_quota", return_value=quota) as check, patch(
"instances.models.wvmInstance"
) as wvm:
wvm.return_value.get_vcpu.return_value = 1
wvm.return_value.get_memory.return_value = 2048
wvm.return_value.get_cur_memory.return_value = 2048
wvm.return_value.get_status.return_value = status
response = self.client.post(reverse(view, args=[self.vm.id]), data)
messages = [str(m) for m in get_messages(response.wsgi_request)]
return messages, check, wvm.return_value
def test_unverified_quota_has_its_own_message(self):
messages, _, proxy = self.post("instances:resizevm_cpu", {"vcpu": 2, "cur_vcpu": 2}, quota=QUOTA_UNVERIFIED)
self.assertEqual(len(messages), 1)
self.assertIn("a host of your virtual machines cannot be reached", messages[0])
self.assertNotIn("quota reached", messages[0])
proxy.resize_cpu.assert_not_called()
def test_running_vm_memory_charges_no_maximum_change(self):
# a running VM keeps its maximum: resize_mem changes the current memory only
_, check, proxy = self.post("instances:resize_memory", {"memory": 8192, "cur_memory": 1024})
self.assertEqual(check.call_args.args[1:], (0, 0, 0, 0))
# the maximum passed is the current one: if the VM shut down meanwhile,
# resize_mem would write it, and it must not be the unchecked 8192
proxy.resize_mem.assert_called_once_with("1024", 2048)
def test_shut_off_vm_memory_charges_the_maximum_change(self):
_, check, _ = self.post("instances:resize_memory", {"memory": 4096, "cur_memory": 1024}, status=5)
self.assertEqual(check.call_args.args[1:], (0, 0, 2048, 0))
class DetailPageQuotaTests(TestCase):
def test_the_detail_page_does_not_read_every_vm_for_the_quota(self):
get_settings()
user = get_user_model().objects.create_user("quota-detail-user", password="pw")
compute = Compute.objects.create(name="qd", hostname="10.0.0.1", login="u", password="p", type=1)
vm = Instance.objects.create(compute=compute, name="qd-vm", uuid="34343434-0000-0000-0000-000000000000")
UserInstance.objects.create(user=user, instance=vm)
self.client.force_login(user)
with patch("instances.models.wvmInstance") as wvm, patch(
"instances.views.utils.check_user_quota", side_effect=AssertionError("quota checked on GET")
):
proxy = wvm.return_value
proxy.get_memory.return_value = 1024
proxy.get_cur_memory.return_value = 1024
proxy.get_max_memory.return_value = 4096
for name in ("get_networks", "get_ifaces", "get_storages", "get_disk_devices", "get_media_devices", "get_net_devices"):
getattr(proxy, name).return_value = []
proxy.get_status.return_value = 5
response = self.client.get(reverse("instances:instance", args=[vm.id]))
self.assertEqual(response.status_code, 200)

54
instances/utils.py
View file

@ -9,6 +9,7 @@ from computes.utils import libvirt_compute_lock
from django.conf import settings
from django.db import transaction
from django.utils.translation import gettext_lazy as _
from libvirt import libvirtError
from vrtManager import util
from vrtManager.connection import connection_manager
from vrtManager.instance import wvmInstance, wvmInstances
@ -43,40 +44,63 @@ def get_clone_free_names(size=10):
return free_names
# check_user_quota could not read every VM of the user: an increase is refused
QUOTA_UNVERIFIED = "unverified"
def check_user_quota(user, instance, cpu, memory, disk_size):
"""
Which quota the user would exceed by adding these amounts (instances,
vCPUs, memory MiB, disk GiB) to their VMs, or "". QUOTA_UNVERIFIED if a
VM that counts cannot be read (its host is down, the VM is gone): the
usage is unknown, so an increase is refused.
"""
ua, attributes_created = UserAttributes.objects.get_or_create(user=user)
msg = ""
if user.is_superuser:
return msg
# adding nothing (a shrink) cannot exceed a quota
if instance <= 0 and cpu <= 0 and memory <= 0 and disk_size <= 0:
return msg
quota_debug = app_settings.QUOTA_DEBUG
user_instances = UserInstance.objects.filter(user=user, instance__is_template=False)
user_instances = UserInstance.objects.filter(user=user, instance__is_template=False).select_related(
"instance__compute"
)
instance += user_instances.count()
if ua.max_instances > 0 and instance > ua.max_instances:
msg = "instance"
if quota_debug:
msg += f" ({instance} > {ua.max_instances})"
return msg
if ua.max_cpus <= 0 and ua.max_memory <= 0 and ua.max_disk_size <= 0:
return msg
computes = {} # status is cached per object: check each host once
for usr_inst in user_instances:
if connection_manager.host_is_up(
usr_inst.instance.compute.type,
usr_inst.instance.compute.hostname,
):
vm = usr_inst.instance
compute = computes.setdefault(vm.compute_id, vm.compute)
if compute.status is not True:
return QUOTA_UNVERIFIED
try:
conn = wvmInstance(
usr_inst.instance.compute.hostname,
usr_inst.instance.compute.login,
usr_inst.instance.compute.password,
usr_inst.instance.compute.type,
usr_inst.instance.name,
uuid=usr_inst.instance.uuid,
compute.hostname,
compute.login,
compute.password,
compute.type,
vm.name,
uuid=vm.uuid,
)
cpu += int(conn.get_vcpu())
memory += int(conn.get_memory())
for disk in conn.get_disk_devices():
if disk["size"]:
disk_size += int(disk["size"]) >> 30
except libvirtError:
return QUOTA_UNVERIFIED
if ua.max_instances > 0 and instance > ua.max_instances:
msg = "instance"
if quota_debug:
msg += f" ({instance} > {ua.max_instances})"
if ua.max_cpus > 0 and cpu > ua.max_cpus:
msg = "cpu"
if quota_debug:

35
instances/views.py
View file

@ -34,6 +34,7 @@ from libvirt import (VIR_DOMAIN_UNDEFINE_KEEP_NVRAM,
VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA,
VIR_DOMAIN_UNDEFINE_NVRAM,
VIR_DOMAIN_START_PAUSED,
VIR_DOMAIN_RUNNING,
libvirtError)
from logs.views import addlogmsg
from vrtManager import util
@ -118,7 +119,6 @@ def instance(request, pk):
if instance.cur_memory not in memory_range:
insort(memory_range, instance.cur_memory)
clone_free_names = utils.get_clone_free_names()
user_quota_msg = utils.check_user_quota(request.user, 0, 0, 0, 0)
default_bus = app_settings.INSTANCE_VOLUME_DEFAULT_BUS
default_io = app_settings.INSTANCE_VOLUME_DEFAULT_IO
@ -403,6 +403,14 @@ def serialize_instance_mutation(func):
return wrapper
def quota_refused(quota_msg, refused):
"""The message for a refused change: refused is the action's own
'User %(quota_msg)s quota reached, ...' text, already formatted."""
if quota_msg == utils.QUOTA_UNVERIFIED:
return _("The quota cannot be checked because a host of your virtual machines cannot be reached. Nothing was changed.")
return refused
def serialize_user_quota(func):
"""Outermost lock for quota-checked changes: concurrent requests of one
user (also on different computes) must not pass the check together."""
@ -766,12 +774,12 @@ def resizevm_cpu(request, pk):
request.user, 0, int(new_vcpu) - vcpu, 0, 0
)
if not request.user.is_superuser and quota_msg:
msg = _(
msg = quota_refused(quota_msg, _(
"User %(quota_msg)s quota reached, cannot resize CPU of '%(instance_name)s'!"
) % {
"quota_msg": quota_msg,
"instance_name": instance.name,
}
})
messages.error(request, msg)
else:
cur_vcpu = new_cur_vcpu
@ -804,16 +812,19 @@ def resize_memory(request, pk):
new_cur_memory_custom = request.POST.get("cur_memory_custom", "")
if new_cur_memory_custom:
new_cur_memory = new_cur_memory_custom
quota_msg = utils.check_user_quota(
request.user, 0, 0, int(new_memory) - memory, 0
)
# The quota charges the maximum. A running VM keeps it (resize_mem changes
# only the current memory there); pass the current maximum, so a VM that
# shuts down meanwhile does not get an unchecked one.
if instance.proxy.get_status() == VIR_DOMAIN_RUNNING:
new_memory = memory
quota_msg = utils.check_user_quota(request.user, 0, 0, int(new_memory) - memory, 0)
if not request.user.is_superuser and quota_msg:
msg = _(
msg = quota_refused(quota_msg, _(
"User %(quota_msg)s quota reached, cannot resize memory of '%(instance_name)s'!"
) % {
"quota_msg": quota_msg,
"instance_name": instance.name,
}
})
messages.error(request, msg)
else:
instance.proxy.resize_mem(new_cur_memory, new_memory)
@ -859,12 +870,12 @@ def resize_disk(request, pk):
request.user, 0, 0, 0, disk_new_sum - disk_sum
)
if not request.user.is_superuser and quota_msg:
msg = _(
msg = quota_refused(quota_msg, _(
"User %(quota_msg)s quota reached, cannot resize disks of '%(instance_name)s'!"
) % {
"quota_msg": quota_msg,
"instance_name": instance.name,
}
})
messages.error(request, msg)
else:
instance.proxy.resize_disk(disks_new)
@ -1789,12 +1800,12 @@ def clone(request, pk):
]
if not request.user.is_superuser and quota_msg:
msg = _(
msg = quota_refused(quota_msg, _(
"User '%(quota_msg)s' quota reached, cannot create '%(clone_name)s'!"
) % {
"quota_msg": quota_msg,
"clone_name": clone_data["name"],
}
})
messages.error(request, msg)
elif check_instance:
msg = _("Instance '%(clone_name)s' already exists!") % {