diff --git a/instances/states.py b/instances/states.py index ea74d7d2..404e92e0 100644 --- a/instances/states.py +++ b/instances/states.py @@ -18,7 +18,7 @@ STATES = { } UNKNOWN = (_("Unknown"), "secondary") # the VM cannot be read -RUNNING = {"suspend", "poweroff", "powercycle", "console"} +RUNNING = {"suspend", "poweroff", "powercycle", "force_off", "console"} STATE_ACTIONS = { 1: RUNNING, 2: RUNNING, @@ -49,15 +49,16 @@ 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. + administrator offers its owner nothing. A template is not started (a + power cycle starts it again) 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()) - if not manages(user, instance): + actions = STATE_ACTIONS.get(state, set()) + elif not manages(user, instance) or state == 3: return set() - if state == 3: - return set() - return STATE_ACTIONS.get(state, set()) - SUPERUSER_ONLY + else: + actions = STATE_ACTIONS.get(state, set()) - SUPERUSER_ONLY + return actions - {"powercycle"} if instance.is_template else actions diff --git a/instances/templates/instances/power_tab.html b/instances/templates/instances/power_tab.html index 5171006a..7dc6854c 100644 --- a/instances/templates/instances/power_tab.html +++ b/instances/templates/instances/power_tab.html @@ -1,155 +1,61 @@ {% load i18n %} +{% load vm_states %}
{% if not can_power %}

{% trans "Only the owners of this instance and administrators can change its power state." %}

{% else %} - + {# the actions the VM's state allows this user (states.allowed_actions): those that keep the guest's data, and those that cut the power #} + {% vm_state instance.status as state %}
-
-
    - {% if instance.status == 1 or instance.status == 2 %} - - - - {% if request.user.is_superuser %} - - {% endif %} - {% endif %} - {% if instance.status == 3 %} - {% if request.user.is_superuser %} - - - {% endif %} - {% endif %} - {% if instance.status == 4 or instance.status == 6 or instance.status == 7 %} - - {% endif %} - {% if instance.status == 5 %} - - {% endif %} -
-
- -
- {% if instance.status == 1 or instance.status == 2 %} -
-

{% trans "This action sends an ACPI shutdown signal to the instance." %}

-
- {% csrf_token %} - -
+
+

{% trans "Power" %} ยท {{ state.0 }}

+ {% if "poweroff" in power_actions or "suspend" in power_actions or "resume" in power_actions or "poweron" in power_actions or "force_off" in power_actions or "powercycle" in power_actions %} +
+ {% if "poweroff" in power_actions or "suspend" in power_actions or "resume" in power_actions or "poweron" in power_actions %} + {% csrf_token %} +
+ {% if "poweron" in power_actions %}{% endif %} + {% if "poweroff" in power_actions %}{% endif %} + {% if "resume" in power_actions %}{% endif %} + {% if "suspend" in power_actions %}{% endif %} +
+

+ {% if "poweron" in power_actions %}{% trans "Power On starts the VM." %}{% endif %} + {% if "poweroff" in power_actions %}{% trans "Power Off asks the guest to shut down; the guest decides." %}{% endif %} + {% if "resume" in power_actions %}{% trans "Resume lets the paused VM run on." %}{% endif %} + {% if "suspend" in power_actions %}{% trans "Suspend pauses its vCPUs; the VM stays in memory." %}{% endif %} +

-
-
-

{% trans "This action forcibly powers off and start the instance and may cause data corruption." %}

-
{% csrf_token %} - -
+ {% endif %} + {% if "force_off" in power_actions or "powercycle" in power_actions %} + {% csrf_token %} +
+ {% if "powercycle" in power_actions %} + + {% endif %} + {% if "force_off" in power_actions %} + + {% endif %} +
+

+ {% trans "These cut the power: what the guest has not written to disk is lost." %} + {% if "powercycle" in power_actions %}{% trans "Power Cycle then starts the VM again, with the settings pending its next start." %}{% endif %} +

+ {% endif %}
-
-

{% trans "This action forcibly powers off the instance and may cause data corruption." %}

-
- {% csrf_token %} - -
-
-
- {% if request.user.is_superuser %} -
-

{% trans "This action suspends the instance." %}

-
{% csrf_token %} - -
-
-
- {% endif %} - {% endif %} - {% if instance.status == 3 %} - {% if request.user.is_superuser %} -
-

{% trans "This action restore the instance after suspend." %}

-
{% csrf_token %} - -
-
-
-
-

{% trans "This action forcibly powers off the instance and may cause data corruption." %}

-
{% csrf_token %} - -
-
-
+ {% elif instance.status == 3 %} +

{% trans "The VM is paused. An administrator can resume it." %}

+ {% elif instance.status == 5 and instance.is_template %} +

{% trans "A template is not started: clone it to make a VM." %}

{% else %} -
-

{% trans "Administrator blocked your instance." %}

-
{% csrf_token %} - -
-
-
+

{% trans "There is no power action for the VM in this state." %}

{% 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 #} -
-

{% trans "This action forcibly powers off the instance and may cause data corruption." %}

-
- {% csrf_token %} - -
-
-
- {% endif %} - {% if instance.status == 5 %} -
-

{% trans "Click on Power On button to start this instance." %}

-
- {% csrf_token %} - {% if instance.is_template %} -

{% trans "Template instance cannot be started." %}

- - {% else %} - - {% endif %} -
-
-
- {% endif %} -
+
{% endif %}
diff --git a/instances/tests/test_page_controls.py b/instances/tests/test_page_controls.py index 3f3d2509..5a2bb72b 100644 --- a/instances/tests/test_page_controls.py +++ b/instances/tests/test_page_controls.py @@ -88,7 +88,7 @@ class PageControls: "console": bool(doc.xpath("//button[@id='console_button']")), "vv file": bool(doc.xpath("//a[@href=$a]", a=reverse("instances:getvvfile", args=[pk]))), "vdi": bool(doc.xpath("//*[@id='vdiconsole']")), - "power": form("poweroff") or form("poweron"), + "power": form("poweron") or bool(doc.xpath("//button[@formaction=$a]", a=reverse("instances:poweroff", args=[pk]))), "root password": form("rootpasswd"), "ssh key": form("add_public_key"), # a running VM with hotplug shows per-vCPU buttons instead of the CPU form @@ -197,6 +197,22 @@ class PageControlsTestCase(PageControls, TestCase): ) self.assertEqual(bool(buttons), enabled) + def test_the_power_tab_offers_the_state_s_actions_and_confirms_a_power_cut(self): + admin = get_user_model().objects.create_superuser("power_admin", "pa@example.com", "x") + owner = self.user("power_owner", owner={}) + for user, status, names in ((admin, 1, ["poweroff", "suspend", "powercycle", "powerforce"]), + (admin, 3, ["resume", "powerforce"]), (owner, 3, []), (admin, 5, ["poweron"])): + with self.subTest(user=user.username, status=status): + self.controls(user, status=status) + (tab,) = lxml_html.fromstring(self.last_page).xpath("//div[@id='power']") + self.assertEqual(tab.xpath(".//button/@name"), names) + # the power-cut actions ask first + self.assertTrue(all(b.get("data-confirm") for b in tab.xpath(".//button[contains(@class, 'btn-danger')]"))) + if user == owner: + self.assertIn("The VM is paused. An administrator can resume it.", tab.text_content()) + if status == 5: + self.assertEqual(tab.xpath(".//form/@onsubmit"), ["showPleaseWaitDialog()"]) + def test_a_running_paused_or_shut_off_vm_can_be_cloned(self): changer = self.user("vm_cloner", perms=("clone_instances",), owner={"is_change": True}) for status, enabled in ((1, True), (3, True), (5, True), (4, False)): diff --git a/instances/tests/test_states.py b/instances/tests/test_states.py index ab0aff38..1a977895 100644 --- a/instances/tests/test_states.py +++ b/instances/tests/test_states.py @@ -19,8 +19,8 @@ LABELS = {0: "No state", 1: "Active", 2: "Blocked", 3: "Suspended", 4: "Shutting # actions offered per state, for a superuser ACTIONS = { 0: set(), - 1: {"poweroff", "powercycle", "suspend"}, - 2: {"poweroff", "powercycle", "suspend"}, + 1: {"poweroff", "powercycle", "force_off", "suspend"}, + 2: {"poweroff", "powercycle", "force_off", "suspend"}, 3: {"resume", "force_off"}, 4: {"force_off"}, 5: {"poweron"}, @@ -248,6 +248,16 @@ class AllowedActionsTests(TestCase): self.assertEqual(allowed_actions(other, vm, state), set()) +class TemplatePowerTests(TestCase): + """A running template is not power cycled: that would start it again.""" + + def test_a_running_template_has_no_power_cycle(self): + admin = get_user_model().objects.create_superuser("tp-admin", "tp@example.com", "pw") + compute = Compute.objects.create(name="tp", hostname="10.0.0.5", login="u", password="p", type=1) + vm = Instance.objects.create(compute=compute, name="tp-vm", uuid="dddddddd-0000-0000-0000-000000000000", is_template=True) + self.assertEqual(allowed_actions(admin, vm, 1) - {"console"}, ACTIONS[1] - {"powercycle"}) + + class TemplateCloneTests(TestCase): """A shut-off template offers Clone to whoever may clone it, owner or not.""" diff --git a/instances/views.py b/instances/views.py index f4b9fd52..5b203e51 100755 --- a/instances/views.py +++ b/instances/views.py @@ -61,7 +61,7 @@ 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 +from .states import STATES, UNKNOWN, allowed_actions, state_label # the VM page waits this long (seconds) for the compute's drbdadm status DRBD_STATUS_TIMEOUT = 10 @@ -264,6 +264,7 @@ def _instance_page(request, pk, instance, compute): # view_instances permission are read-only. can_open_console = utils.can_open_console(request.user, instance) can_power = request.user.is_superuser or userinstance is not None + power_actions = allowed_actions(request.user, instance, instance.status) # get_instance(perm_type="delete"): a template only for superusers and staff owners can_delete = request.user.is_superuser or bool( userinstance and userinstance.is_delete and (request.user.is_staff or not instance.is_template)