1
0
Fork 0
mirror of https://github.com/retspen/webvirtcloud synced 2026-11-09 03:36:48 +00:00

fix(instances): every VM state has a label and the actions it allows

The instance lists, the VM page header and the power tab knew three
libvirt states: running, paused and shut off. A VM that was blocked,
shutting down, crashed, suspended by its guest or without a state got
an empty status cell and no actions, so a crashed VM could not even be
forced off from the list. The lists also offered actions users could
not use: Suspend and Resume (views for superusers) to everyone, and
power actions to users who see every VM through the read-only
view_instances permission.

instances/states.py holds the rules in one place: the label and color
of each state, the actions each state allows, and which of them a user
may use (superusers all; a VM's owners all but suspend and resume, and
nothing on a VM an administrator suspended, as the VM page already
showed; nobody else any). Blocked is a running VM; shutting down,
crashed and suspended by the guest get Force Off, after which the VM
can be started. A shut-off template offers Clone to whoever has the
clone permission, as the clone view allows.

The templates read them through two tags (vm_states): the lists and the
VM page header show the state, instance_actions.html has one slot per
button, each enabled or disabled from the allowed actions. The power
tab offers Force Off in the states that need it.

The new tests check the rules directly and render every state in the
lists and on the VM page for a superuser, an owner and a
view_instances user; they fail on the previous code.
This commit is contained in:
catborise 2026-10-05 19:11:17 +03:00
parent 405e6f1445
commit 5bc8b1915e
10 changed files with 322 additions and 86 deletions

View file

@ -94,16 +94,7 @@
</em>
</td>
<td>
{% if instance.info.0 == 1 %}
<span class="text-success">{% trans "Active" %}</span>
{% endif %}
{% if instance.info.0 == 5 %}
<span class="text-danger">{% trans "Off" %}</span>
{% endif %}
{% if instance.info.0 == 3 %}
<span class="text-warning">{% trans "Suspended" %}</span>
{% endif %}
{% if instance.info is None %}<span class="text-secondary">{% trans "Unknown" %}</span>{% endif %}
{% include 'instance_state.html' %}
</td>
<td>{{ instance.info.3 }}</td>
<td><p class="text-end">{% if instance.info %}{% widthratio instance.info.1 1024 1 %} MiB{% endif %}</p></td>

58
instances/states.py Normal file
View file

@ -0,0 +1,58 @@
"""
libvirt VM states: their labels, and the actions each one allows a user.
The lists and the VM page use these through templatetags/vm_states.py.
"""
from django.utils.translation import gettext_lazy as _
# state (virDomainState): label, Bootstrap color
STATES = {
0: (_("No state"), "secondary"),
1: (_("Active"), "success"),
2: (_("Blocked"), "success"), # running, waiting for a resource
3: (_("Suspended"), "warning"),
4: (_("Shutting down"), "warning"),
5: (_("Off"), "danger"),
6: (_("Crashed"), "danger"),
7: (_("PM suspended"), "warning"), # suspended by the guest
}
UNKNOWN = (_("Unknown"), "secondary") # the VM cannot be read
RUNNING = {"suspend", "poweroff", "powercycle", "console"}
STATE_ACTIONS = {
1: RUNNING,
2: RUNNING,
3: {"resume", "force_off"},
5: {"poweron"},
# shutting down, crashed, suspended by the guest: a forced power off ends
# it, and the VM can be started again
4: {"force_off"},
6: {"force_off"},
7: {"force_off"},
}
SUPERUSER_ONLY = {"suspend", "resume"} # their views are for superusers
def state_label(state):
"""(label, color) of a state; None for a VM that cannot be read."""
return UNKNOWN if state is None else STATES.get(state, STATES[0])
def allowed_actions(user, instance, state):
"""
The power actions the state allows and the user may use: superusers all,
a VM's owners all but suspend and resume, nobody else any (the
view_instances permission is read-only). A VM suspended by an
administrator offers its owner nothing. A shut-off template is not
started but cloned, by anyone with the clone permission who sees it.
"""
if state == 5 and instance.is_template:
return {"clone"} if user.has_perm("instances.clone_instances") else set()
if user.is_superuser:
return STATE_ACTIONS.get(state, set())
# the lists prefetch userinstance_set
if not any(ui.user_id == user.id for ui in instance.userinstance_set.all()):
return set()
if state == 3:
return set()
return STATE_ACTIONS.get(state, set()) - SUPERUSER_ONLY

View file

@ -72,12 +72,7 @@
</em>
</td>
<td>
{% if instance.info.0 == 1 %}<span class="text-success">{% trans "Active" %}</span>{% endif %}
{% if instance.info.0 == 5 %}<span class="text-danger">{% trans "Off" %}</span>{% endif %}
{% if instance.info.0 == 3 %}
<span class="text-warning">{% trans "Suspended" %}</span>
{% endif %}
{% if instance.info is None %}<span class="text-secondary">{% trans "Unknown" %}</span>{% endif %}
{% include 'instance_state.html' %}
</td>
{% if app_settings.VM_DRBD_STATUS == 'True' %}
<td>

View file

@ -39,13 +39,7 @@
</td>
{% endif %}
<td>
{% if instance.info.0 == 1 %}<span
class="text-success">{% trans "Active" %}</span>{% endif %}
{% if instance.info.0 == 5 %}<span
class="text-danger">{% trans "Off" %}</span>{% endif %}
{% if instance.info.0 == 3 %}<span
class="text-warning">{% trans "Suspended" %}</span>{% endif %}
{% if instance.info is None %}<span class="text-secondary">{% trans "Unknown" %}</span>{% endif %}
{% include 'instance_state.html' %}
</td>
{% if app_settings.VM_DRBD_STATUS == 'True' %}
<td>

View file

@ -1,5 +1,6 @@
{% extends "base.html" %}
{% load static %}
{% load vm_states %}
{% load i18n %}
{% load bootstrap_icons %}
@ -13,15 +14,8 @@
{% include 'pleasewaitdialog.html' %}
<div>
<div>
{% if instance.status == 5 %}
<span class="badge bg-danger">{% trans "Off" %}</span>
{% endif %}
{% if instance.status == 1 %}
<span class="badge bg-success">{% trans "Active" %}</span>
{% endif %}
{% if instance.status == 3 %}
<span class="badge bg-warning">{% trans "Suspend" %}</span>
{% endif %}
{% vm_state instance.status as state %}
<span class="badge bg-{{ state.1 }}">{{ state.0 }}</span>
|
<span
{%if instance.guest_agent %}

View file

@ -1,78 +1,70 @@
{% load i18n %}
{% load bootstrap_icons %}
{% if instance.info %}
{% load vm_states %}
{% vm_actions instance as actions %}
{% if actions %}
<div class="btn-group" role="group" aria-label="Instance actions">
{% if instance.info.0 == 5 %}
{% if instance.is_template %}
<a href="{% url 'instances:instance' instance.id %}#clone" class="btn btn-sm btn-secondary" title="{% trans "Clone" %}">
{% bs_icon 'copy' %}
</a>
{% else %}
<form action="{% url 'instances:poweron' instance.id %}" method="post" style="display:inline;" onsubmit="showPleaseWaitDialog()">
{% csrf_token %}
<button type="submit" class="btn btn-sm btn-secondary" title="{% trans "Power On" %}">
{% bs_icon 'play-fill' %}
</button>
</form>
{% endif %}
<button class="btn btn-sm btn-secondary disabled" title="{% trans "Suspend" %}" disabled>{% bs_icon 'pause-fill' %}</button>
<button class="btn btn-sm btn-secondary disabled" title="{% trans "Power Off" %}" disabled>{% bs_icon 'power' %}</button>
<button class="btn btn-sm btn-secondary disabled" title="{% trans "Power Cycle" %}" disabled>{% bs_icon 'repeat' %}</button>
<button class="btn btn-sm btn-secondary disabled" title="{% trans "VNC Console" %}" disabled>
<span>{% bs_icon 'eye-fill' %}</span>
</button>
{% endif %}
{% if instance.info.0 == 3 %}
{% if "clone" in actions %}
<a href="{% url 'instances:instance' instance.id %}#clone" class="btn btn-sm btn-secondary" title="{% trans "Clone" %}">
{% bs_icon 'copy' %}
</a>
{% elif "poweron" in actions %}
<form action="{% url 'instances:poweron' instance.id %}" method="post" style="display:inline;" onsubmit="showPleaseWaitDialog()">
{% csrf_token %}
<button type="submit" class="btn btn-sm btn-secondary" title="{% trans "Power On" %}">{% bs_icon 'play-fill' %}</button>
</form>
{% elif "resume" in actions %}
<form action="{% url 'instances:resume' instance.id %}" method="post" style="display:inline;">
{% csrf_token %}
<button type="submit" class="btn btn-sm btn-secondary" title="{% trans "Resume" %}">
{% bs_icon 'play-fill' %}
</button>
<button type="submit" class="btn btn-sm btn-secondary" title="{% trans "Resume" %}">{% bs_icon 'play-fill' %}</button>
</form>
<button class="btn btn-sm btn-secondary disabled" title="{% trans "Suspend" %}" disabled>{% bs_icon 'pause-fill' %}</button>
<form action="{% url 'instances:force_off' instance.id %}" method="post" style="display:inline;">
{% csrf_token %}
<button type="submit" class="btn btn-sm btn-secondary" title="{% trans "Force Off" %}">
{% bs_icon 'power' %}
</button>
</form>
<button class="btn btn-sm btn-secondary disabled" title="{% trans "Power Cycle" %}" disabled>{% bs_icon 'repeat' %}</button>
<button class="btn btn-sm btn-secondary disabled" title="{% trans "VNC Console" %}" disabled>
<span>{% bs_icon 'eye-fill' %}</span>
</button>
{% endif %}
{% if instance.info.0 == 1 %}
{% else %}
<button class="btn btn-sm btn-secondary disabled" title="{% trans "Power On" %}" disabled>{% bs_icon 'play-fill' %}</button>
{% endif %}
{% if "suspend" in actions %}
<form action="{% url 'instances:suspend' instance.id %}" method="post" style="display:inline;">
{% csrf_token %}
<button type="submit" class="btn btn-sm btn-secondary" title="{% trans "Suspend" %}">
{% bs_icon 'pause-fill' %}
</button>
<button type="submit" class="btn btn-sm btn-secondary" title="{% trans "Suspend" %}">{% bs_icon 'pause-fill' %}</button>
</form>
{% else %}
<button class="btn btn-sm btn-secondary disabled" title="{% trans "Suspend" %}" disabled>{% bs_icon 'pause-fill' %}</button>
{% endif %}
{% if "poweroff" in actions %}
<form action="{% url 'instances:poweroff' instance.id %}" method="post" style="display:inline;">
{% csrf_token %}
<button type="submit" class="btn btn-sm btn-secondary" title="{% trans "Power Off" %}">
{% bs_icon 'power' %}
</button>
<button type="submit" class="btn btn-sm btn-secondary" title="{% trans "Power Off" %}">{% bs_icon 'power' %}</button>
</form>
{% elif "force_off" in actions %}
<form action="{% url 'instances:force_off' instance.id %}" method="post" style="display:inline;">
{% csrf_token %}
<button type="submit" class="btn btn-sm btn-secondary" title="{% trans "Force Off" %}">{% bs_icon 'power' %}</button>
</form>
{% else %}
<button class="btn btn-sm btn-secondary disabled" title="{% trans "Power Off" %}" disabled>{% bs_icon 'power' %}</button>
{% endif %}
{% if "powercycle" in actions %}
<form action="{% url 'instances:powercycle' instance.id %}" method="post" style="display:inline;">
{% csrf_token %}
<button type="submit" class="btn btn-sm btn-secondary" title="{% trans "Power Cycle" %}">
{% bs_icon 'repeat' %}
</button>
<button type="submit" class="btn btn-sm btn-secondary" title="{% trans "Power Cycle" %}">{% bs_icon 'repeat' %}</button>
</form>
{% else %}
<button class="btn btn-sm btn-secondary disabled" title="{% trans "Power Cycle" %}" disabled>{% bs_icon 'repeat' %}</button>
{% endif %}
{% if "console" in actions %}
<button class="btn btn-sm btn-secondary" type="button" onclick='open_console("{{ instance.compute.id }}-{{ instance.get_uuid }}")'
title="{% trans "Console" %}">
<span>{% bs_icon 'eye-fill' %}</span>
</button>
title="{% trans "Console" %}">{% bs_icon 'eye-fill' %}</button>
{% else %}
<button class="btn btn-sm btn-secondary disabled" title="{% trans "VNC Console" %}" disabled>{% bs_icon 'eye-fill' %}</button>
{% endif %}
</div>
{% block script %}
<script>
function open_console(uuid) {
url = '{% url 'console' %}?token=' + uuid + '';
window.open(url, '', 'width=850,height=650')
}
</script>
{% endblock script %}
{% endif %}

View file

@ -0,0 +1,3 @@
{% load vm_states %}
{% if instance.info %}{% vm_state instance.info.0 as state %}{% else %}{% vm_state None as state %}{% endif %}
<span class="text-{{ state.1 }}">{{ state.0 }}</span>

View file

@ -2,7 +2,7 @@
<div class="tab-pane active" id="power" role="tabpanel">
<!-- Nav tabs -->
<ul class="nav nav-tabs" role="tablist" aria-label="Instance power actions">
{% if instance.status == 1 %}
{% if instance.status == 1 or instance.status == 2 %}
<li class="nav-item" role="presentation">
<button class="nav-link active" data-bs-toggle="tab" data-bs-target="#poweroff" type="button" role="tab" aria-controls="poweroff" aria-selected="true">
{% trans "Power Off" %}
@ -40,6 +40,13 @@
</li>
{% endif %}
{% endif %}
{% if instance.status == 4 or instance.status == 6 or instance.status == 7 %}
<li class="nav-item" role="presentation">
<button class="nav-link active" data-bs-toggle="tab" data-bs-target="#powerforce" type="button" role="tab" aria-controls="powerforce" aria-selected="true">
{% trans "Force Off" %}
</button>
</li>
{% endif %}
{% if instance.status == 5 %}
<li class="nav-item" role="presentation">
<button class="nav-link active" data-bs-toggle="tab" data-bs-target="#boot" type="button" role="tab" aria-controls="boot" aria-selected="true">
@ -50,7 +57,7 @@
</ul>
<!-- Tab panes -->
<div class="tab-content">
{% if instance.status == 1 %}
{% if instance.status == 1 or instance.status == 2 %}
<div class="tab-pane tab-pane-bordered active" id="poweroff" role="tabpanel" aria-labelledby="poweroff-tab">
<p>{% trans "This action sends an ACPI shutdown signal to the instance." %}</p>
<form action="{% url 'instances:poweroff' instance.id %}" method="post" role="form" aria-label0="Power off instance form">
@ -110,6 +117,17 @@
</div>
{% endif %}
{% endif %}
{% if instance.status == 4 or instance.status == 6 or instance.status == 7 %}
{# shutting down, crashed or suspended by the guest: a forced power off ends it, then it can be started #}
<div class="tab-pane tab-pane-bordered active" id="powerforce" role="tabpanel" aria-labelledby="powerforce-tab">
<p>{% trans "This action forcibly powers off the instance and may cause data corruption." %}</p>
<form action="{% url 'instances:force_off' instance.id %}" method="post" role="form" aria-label="Force to shutdown instance form">
{% csrf_token %}
<input type="submit" name="powerforce" class="btn btn-lg btn-success float-end" value="{% trans "Force Off" %}">
<div class="clearfix"></div>
</form>
</div>
{% endif %}
{% if instance.status == 5 %}
<div class="tab-pane tab-pane-bordered active" id="boot" role="tabpanel" aria-labelledby="boot-tab">
<p>{% trans "Click on Power On button to start this instance." %}</p>

View file

@ -0,0 +1,19 @@
from django import template
from instances.states import allowed_actions, state_label
register = template.Library()
@register.simple_tag
def vm_state(state):
"""{% vm_state state as label_color %}: label and Bootstrap color."""
return state_label(state)
@register.simple_tag(takes_context=True)
def vm_actions(context, instance):
"""{% vm_actions instance as actions %}: the actions this user may use now."""
if not instance.info:
return set()
return allowed_actions(context["request"].user, instance, instance.info[0])

172
instances/test_states.py Normal file
View file

@ -0,0 +1,172 @@
"""Every libvirt state has a label, and the lists offer the actions the state
and the user's permissions allow."""
from types import SimpleNamespace
from unittest.mock import PropertyMock, patch
from accounts.models import UserInstance
from appsettings.models import AppSettings
from computes.models import Compute
from django.contrib.auth import get_user_model
from django.contrib.auth.models import Permission
from django.test import TestCase
from django.urls import reverse
from instances.models import Instance
from instances.states import allowed_actions
LABELS = {0: "No state", 1: "Active", 2: "Blocked", 3: "Suspended", 4: "Shutting down", 5: "Off", 6: "Crashed", 7: "PM suspended"}
# actions offered per state, for a superuser
ACTIONS = {
0: set(),
1: {"poweroff", "powercycle", "suspend"},
2: {"poweroff", "powercycle", "suspend"},
3: {"resume", "force_off"},
4: {"force_off"},
5: {"poweron"},
6: {"force_off"},
7: {"force_off"},
}
ALL = {"poweron", "poweroff", "powercycle", "force_off", "suspend", "resume"}
class Domain:
def __init__(self, state):
self.state = state
def info(self):
return [self.state, 4194304, 2097152, 2, 0]
class StateTests(TestCase):
def setUp(self):
User = get_user_model()
self.admin = User.objects.create_superuser("st-admin", "st@example.com", "pw")
self.owner = User.objects.create_user("st-owner", password="pw")
self.viewer = User.objects.create_user("st-viewer", password="pw")
self.viewer.user_permissions.add(Permission.objects.get(codename="view_instances"))
self.compute = Compute.objects.create(name="st", hostname="10.0.0.1", login="u", password="p", type=1)
self.vms = {}
for state in LABELS:
vm = Instance.objects.create(compute=self.compute, name=f"st-vm-{state}", uuid=f"{state:08d}-0000-0000-0000-000000000000")
UserInstance.objects.create(user=self.owner, instance=vm)
self.vms[state] = vm
AppSettings.objects.filter(key="VIEW_INSTANCES_LIST_STYLE").update(value="nongrouped")
def get(self, user, url):
by_uuid = {vm.uuid: state for state, vm in self.vms.items()}
def wvm(*args, uuid=None, **kwargs):
return SimpleNamespace(instance=Domain(by_uuid[uuid]), get_title=lambda: "", get_uuid=lambda: uuid)
self.client.force_login(user)
props = {"status": True, "connection_error": None, "cpu_count": 1, "ram_size": 1, "ram_usage": 0, "cpu_usage": 0}
patches = [patch.object(Compute, k, new_callable=PropertyMock, return_value=v) for k, v in props.items()]
patches += [patch("instances.views.utils.refr"), patch("computes.views.utils.refresh_instance_database"),
patch("instances.models.wvmInstance", side_effect=wvm)]
for p in patches:
p.start()
try:
response = self.client.get(url)
finally:
for p in reversed(patches):
p.stop()
self.assertEqual(response.status_code, 200)
return response.content.decode()
def offered(self, page, vm):
return {a for a in ALL if reverse(f"instances:{a}", args=[vm.id]) in page}
def test_every_state_has_a_label(self):
page = self.get(self.admin, reverse("instances:index"))
for state, label in LABELS.items():
with self.subTest(state=state):
self.assertIn(f">{label}<", page)
def test_superuser_actions_follow_the_state(self):
for url in (reverse("instances:index"), reverse("instances", args=[self.compute.id])):
page = self.get(self.admin, url)
for state, vm in self.vms.items():
with self.subTest(url=url, state=state):
self.assertEqual(self.offered(page, vm), ACTIONS[state])
def test_owner_is_not_offered_suspend_or_resume(self):
# both views are for superusers only; a VM an administrator suspended
# stays as it is for its owner, as on the VM page
page = self.get(self.owner, reverse("instances:index"))
for state, vm in self.vms.items():
with self.subTest(state=state):
expected = set() if state == 3 else ACTIONS[state] - {"suspend", "resume"}
self.assertEqual(self.offered(page, vm), expected)
def test_a_viewer_who_owns_nothing_is_offered_no_actions(self):
page = self.get(self.viewer, reverse("instances:index"))
for state, vm in self.vms.items():
with self.subTest(state=state):
self.assertIn(vm.name, page)
self.assertEqual(self.offered(page, vm), set())
class DetailStateTests(TestCase):
"""The VM page names every state and offers a forced power off where only that helps."""
def test_states(self):
owner = get_user_model().objects.create_user("st-detail-owner", password="pw")
compute = Compute.objects.create(name="st-d", hostname="10.0.0.2", login="u", password="p", type=1)
vm = Instance.objects.create(compute=compute, name="st-d-vm", uuid="99999999-0000-0000-0000-00000000d000")
UserInstance.objects.create(user=owner, instance=vm, is_change=True)
self.client.force_login(owner)
badge = LABELS
force_off = reverse("instances:force_off", args=[vm.id])
poweroff = reverse("instances:poweroff", args=[vm.id])
for state, label in badge.items():
with self.subTest(state=state), patch("instances.models.wvmInstance") as wvm, patch.object(
Compute, "status", new_callable=PropertyMock, return_value=True
):
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 = state
page = self.client.get(reverse("instances:instance", args=[vm.id])).content.decode()
self.assertIn(f'">{label}</span>', page.replace("\n", "").replace(" ", ""))
self.assertEqual(poweroff in page, state in (1, 2))
self.assertEqual(force_off in page, state in (1, 2, 4, 6, 7))
class AllowedActionsTests(TestCase):
"""The rules themselves, without templates."""
def test_rules(self):
User = get_user_model()
admin = User.objects.create_superuser("aa-admin", "aa@example.com", "pw")
owner = User.objects.create_user("aa-owner", password="pw")
other = User.objects.create_user("aa-other", password="pw")
compute = Compute.objects.create(name="aa", hostname="10.0.0.3", login="u", password="p", type=1)
vm = Instance.objects.create(compute=compute, name="aa-vm", uuid="aaaaaaaa-0000-0000-0000-000000000000")
UserInstance.objects.create(user=owner, instance=vm)
for state in LABELS:
with self.subTest(state=state):
self.assertEqual(allowed_actions(admin, vm, state) - {"console"}, ACTIONS[state])
expected = set() if state == 3 else ACTIONS[state] - {"suspend", "resume"}
self.assertEqual(allowed_actions(owner, vm, state) - {"console"}, expected)
self.assertEqual(allowed_actions(other, vm, state), set())
class TemplateCloneTests(TestCase):
"""A shut-off template offers Clone to whoever may clone it, owner or not."""
def test_clone_link(self):
User = get_user_model()
compute = Compute.objects.create(name="tc", hostname="10.0.0.4", login="u", password="p", type=1)
vm = Instance.objects.create(compute=compute, name="tc-vm", uuid="cccccccc-0000-0000-0000-000000000000", is_template=True)
cloner = User.objects.create_user("tc-viewer", password="pw")
cloner.user_permissions.add(*Permission.objects.filter(codename__in=["view_instances", "clone_instances"]))
cloner = User.objects.get(pk=cloner.pk) # no cached permissions
owner = User.objects.create_user("tc-owner", password="pw")
UserInstance.objects.create(user=owner, instance=vm)
admin = User.objects.create_superuser("tc-admin", "tc@example.com", "pw")
self.assertEqual(allowed_actions(cloner, vm, 5), {"clone"})
self.assertEqual(allowed_actions(admin, vm, 5), {"clone"})
self.assertEqual(allowed_actions(owner, vm, 5), set()) # no clone permission