From 01a42079be9df55c8b24b3b756b8a743d6352608 Mon Sep 17 00:00:00 2001 From: catborise Date: Mon, 5 Oct 2026 12:32:42 +0300 Subject: [PATCH] fix(instances): the page of a VM on an unreachable host points to the list The VM page reads the VM from libvirt throughout. When its host could not be reached, the connection error replaced the page with the error page and showed libvirt's message, which names the host and how it is reached, to any user who may view the VM. The page now checks Compute.status first. If the host is not connected, it goes to the instance list with a message: administrators (superuser or view_instances) get the host and the error, other users only that the VM's host cannot be reached right now. The list shows the VM as unknown. This covers the page itself; actions on the VM still report libvirt's error if the host drops. The detail page tests now mock a connected host. Checked against the test hosts: a real VM's page opens as before, a VM on an unreachable compute leads to the list with the user's or the administrator's message. The new tests fail on the previous code. --- instances/test_console_settings_access.py | 6 +-- instances/test_detail_unreachable.py | 45 +++++++++++++++++++++++ instances/test_quota.py | 4 +- instances/views.py | 13 +++++++ 4 files changed, 63 insertions(+), 5 deletions(-) create mode 100644 instances/test_detail_unreachable.py diff --git a/instances/test_console_settings_access.py b/instances/test_console_settings_access.py index a8023269..b9195803 100644 --- a/instances/test_console_settings_access.py +++ b/instances/test_console_settings_access.py @@ -5,7 +5,7 @@ change console settings, which is the rule update_console enforces: a superuser, or an owner with both is_change and is_vnc. In particular, is_staff and the global view_instances permission must not reveal the VNC password. """ -from unittest.mock import patch +from unittest.mock import PropertyMock, patch from django.contrib.auth import get_user_model from django.contrib.auth.models import Permission @@ -52,8 +52,8 @@ class ConsoleSettingsAccessTestCase(TestCase): def _detail_page(self, user): self.client.force_login(user) - with patch("instances.models.wvmInstance") as mock_wvm, patch( - "instances.views.utils.check_user_quota", return_value="" + with patch("instances.models.wvmInstance") as mock_wvm, patch.object( + Compute, "status", new_callable=PropertyMock, return_value=True ): proxy = mock_wvm.return_value proxy.get_memory.return_value = 1024 diff --git a/instances/test_detail_unreachable.py b/instances/test_detail_unreachable.py new file mode 100644 index 00000000..cc082f13 --- /dev/null +++ b/instances/test_detail_unreachable.py @@ -0,0 +1,45 @@ +"""The page of a VM whose host cannot be reached sends the user to the list +with a message, instead of the error page with libvirt's connection error.""" + +from unittest.mock import PropertyMock, patch + +from accounts.models import UserInstance +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 + +ERROR = "Connection Failed: ssh: connect to host 10.0.0.9 port 22: No route to host" + + +class DetailOfUnreachableHostTests(TestCase): + def setUp(self): + self.compute = Compute.objects.create(name="far-host", hostname="10.0.0.9", login="root", password="", type=2) + self.vm = Instance.objects.create(compute=self.compute, name="far-vm", uuid="56565656-0000-0000-0000-000000000000") + self.owner = get_user_model().objects.create_user("far-owner", password="pw") + UserInstance.objects.create(user=self.owner, instance=self.vm) + + def get(self, user): + self.client.force_login(user) + with patch.object(Compute, "status", new_callable=PropertyMock, return_value=False), patch.object( + Compute, "connection_error", new_callable=PropertyMock, return_value=ERROR + ), patch("instances.models.wvmInstance", side_effect=AssertionError("VM read on a down host")): + response = self.client.get(reverse("instances:instance", args=[self.vm.id])) + self.assertRedirects(response, reverse("instances:index"), fetch_redirect_response=False) + return [str(m) for m in get_messages(response.wsgi_request)] + + def test_owner_is_told_without_host_details(self): + messages = self.get(self.owner) + self.assertEqual(len(messages), 1) + self.assertIn("far-vm", messages[0]) + self.assertNotIn("far-host", messages[0]) + self.assertNotIn("10.0.0.9", messages[0]) + + def test_superuser_sees_the_host_and_its_error(self): + admin = get_user_model().objects.create_superuser("far-admin", password="pw", email="far@example.com") + messages = self.get(admin) + self.assertEqual(len(messages), 1) + self.assertIn("far-host", messages[0]) + self.assertIn(ERROR, messages[0]) diff --git a/instances/test_quota.py b/instances/test_quota.py index 28d81d6c..e64f8072 100644 --- a/instances/test_quota.py +++ b/instances/test_quota.py @@ -2,7 +2,7 @@ cannot verify (fail closed); requests that add nothing are not checked.""" from types import SimpleNamespace -from unittest.mock import patch +from unittest.mock import PropertyMock, patch from accounts.models import UserAttributes, UserInstance from appsettings.settings import get_settings @@ -167,7 +167,7 @@ class DetailPageQuotaTests(TestCase): 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") - ): + ), 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 diff --git a/instances/views.py b/instances/views.py index cd7dad54..59ae796b 100755 --- a/instances/views.py +++ b/instances/views.py @@ -82,6 +82,19 @@ def index(request): def instance(request, pk): instance: Instance = get_instance(request.user, pk) compute: Compute = instance.compute + # the page reads the VM from libvirt throughout; without the host it would + # be the error page, with a connection error that names the host + if compute.status is not True: + if request.user.is_superuser or request.user.has_perm("instances.view_instances"): + msg = _("The host %(host)s of '%(name)s' cannot be reached: %(error)s") % { + "host": compute.name, + "name": instance.name, + "error": compute.connection_error, + } + else: + msg = _("The host of '%(name)s' cannot be reached right now.") % {"name": instance.name} + messages.error(request, msg) + return redirect(reverse("instances:index")) computes = Compute.objects.all().order_by("name") computes_count = computes.count() users = User.objects.all().order_by("username")