diff --git a/computes/tests/test_unreachable_host_pages.py b/computes/tests/test_unreachable_host_pages.py new file mode 100644 index 00000000..b4706605 --- /dev/null +++ b/computes/tests/test_unreachable_host_pages.py @@ -0,0 +1,32 @@ +"""A host page whose list could not be read says why, and not that the host has none.""" + +from unittest.mock import patch + +from django.contrib.auth.models import User +from django.test import TestCase +from django.urls import reverse +from vrtManager.util import ConnectionFailed + +from computes.models import Compute + +EMPTY = { + "storages": "This host has no storage pools.", + "networks": "This host has no networks.", + "interfaces": "libvirt lists no interfaces of this host.", + "nwfilters": "This host has no network filters.", + "virtsecrets": "This host has no secrets.", +} + + +class UnreachableHostPagesTestCase(TestCase): + def setUp(self): + self.compute = Compute.objects.create(name="c", hostname="192.0.2.10", login="u", password="p", type=1) + self.client.force_login(User.objects.create_superuser("hostadmin", "hostadmin@example.com", "pw")) + + def test_the_connection_error_shows_without_an_empty_list(self): + failed = ConnectionFailed("Connection Failed: %(host)s did not answer within 5 s", host="192.0.2.10") + for page, empty in EMPTY.items(): + with self.subTest(page), patch("vrtManager.connection.connection_manager.get_connection", side_effect=failed): + content = self.client.get(reverse(page, args=[self.compute.id])).content.decode() + self.assertIn("192.0.2.10 did not answer within 5 s", content) + self.assertNotIn(empty, content) diff --git a/console/tests/test_views.py b/console/tests/test_views.py index e9139b2c..c9c9c294 100644 --- a/console/tests/test_views.py +++ b/console/tests/test_views.py @@ -14,7 +14,7 @@ from django.conf import settings from django.test import TestCase, override_settings from django.urls import reverse from instances.models import Instance -from libvirt import libvirtError +from vrtManager.util import ConnectionFailed from lxml import html @@ -323,16 +323,26 @@ class ConsoleViewsTestCase(TestCase): self.assertEqual("This console listens only on its host" in response.content.decode(), refused) @patch("console.views.wvmInstance") - def test_console_libvirt_error_fallback(self, mock_wvm): - mock_wvm.side_effect = libvirtError("Connection to libvirtd failed") + def test_a_host_that_cannot_be_reached_is_named_as_the_cause_and_logged(self, mock_wvm): + # not "check the console configuration of your VM": the VM was never read + mock_wvm.side_effect = ConnectionFailed("Connection Failed: %(host)s did not answer within 5 s", host="192.0.2.10") + for user, shown in ((self.admin_user, "Connection Failed: 192.0.2.10 did not answer within 5 s"), + (self.alice, "The host of this virtual machine cannot be reached right now.")): + with self.subTest(user.username): + self.client.force_login(user) + with self.assertLogs("console.views", "WARNING") as logs: + response = self.client.get(self.url) + self.assertTemplateUsed(response, "console-error.html") + self.assertEqual(str(response.context["console_error"]), shown) + self.assertIn("Console of vm-alpha on", logs.output[0]) + self.assertIn("192.0.2.10 did not answer", logs.output[0]) + @patch("console.views.wvmInstance") + def test_a_vm_without_a_readable_console_points_to_its_console_settings(self, mock_wvm): + mock_wvm.return_value.get_console_type.side_effect = IndexError self.client.force_login(self.admin_user) - url = self.url - response = self.client.get(url) - - self.assertEqual(response.status_code, 200) - self.assertTemplateUsed(response, "console-error.html") - self.assertIsNotNone(response.context["console_error"]) + response = self.client.get(self.url) + self.assertIn("check the console configuration", str(response.context["console_error"])) def test_an_unknown_vm_gives_the_superuser_an_error_page(self): self.client.force_login(self.admin_user) diff --git a/console/views.py b/console/views.py index c584550d..3e10bc1a 100644 --- a/console/views.py +++ b/console/views.py @@ -2,6 +2,7 @@ import functools import json +import logging from pathlib import Path from appsettings.settings import app_settings @@ -12,9 +13,13 @@ from django.utils.translation import gettext_lazy as _ from instances.models import Instance from instances.utils import can_open_console from libvirt import libvirtError +from operations.utils import safe_text from vrtManager.connection import CONN_TCP, CONN_TLS from vrtManager.instance import wvmInstance from vrtManager.util import is_loopback +from webvirtcloud.middleware import error_text + +logger = logging.getLogger(__name__) # one file a layout, written by dev/console_layout.py LAYOUTS_DIR = Path(__file__).resolve().parent / "layouts" @@ -60,7 +65,11 @@ def console(request, compute_id, uuid): unreachable = instance.compute.type in (CONN_TCP, CONN_TLS) and bool( conn.get_console_socket() or is_loopback(conn.get_console_listener_addr()) ) - except (libvirtError, ValueError, IndexError): + except libvirtError as err: + # the host or the VM could not be read: the page says that, not to check the console settings + logger.warning("Console of %s on %s: %s", instance.name, instance.compute.name, safe_text(err)) + return render(request, "console-error.html", {"instance": instance, "console_error": error_text(request, err)}) + except (ValueError, IndexError): running = unreachable = False console_type = None console_passwd = None diff --git a/interfaces/templates/interfaces.html b/interfaces/templates/interfaces.html index 2f110010..de16d7b0 100644 --- a/interfaces/templates/interfaces.html +++ b/interfaces/templates/interfaces.html @@ -15,7 +15,7 @@ {% include "computes/host_nav.html" with section="interfaces" %} {% if changeable is False %}{% include 'interfaces_unchangeable.html' %}{% endif %} - {% if not ifaces_all %} + {% if ifaces_all is None %}{# not read: the error above says why #}{% elif not ifaces_all %}
{% bs_icon 'exclamation-triangle' %} {% trans "libvirt lists no interfaces of this host." %}
diff --git a/interfaces/views.py b/interfaces/views.py index db3baad0..40a8f5f0 100644 --- a/interfaces/views.py +++ b/interfaces/views.py @@ -16,6 +16,7 @@ from admin.decorators import superuser_only def interfaces(request, compute_id): """The host's interfaces, and a new one where its libvirt can define them.""" compute = get_object_or_404(Compute, pk=compute_id) + ifaces_all = None # not read: the page shows no list try: conn = wvmInterfaces(compute.hostname, compute.login, compute.password, compute.type) diff --git a/networks/templates/networks.html b/networks/templates/networks.html index b43b559d..ef115643 100644 --- a/networks/templates/networks.html +++ b/networks/templates/networks.html @@ -11,7 +11,7 @@ {% block content %} {% include "computes/host_nav.html" with section="networks" %} - {% if not networks %} + {% if networks is None %}{# not read: the error above says why #}{% elif not networks %}
{% bs_icon 'exclamation-triangle' %} {% trans "This host has no networks." %}
diff --git a/networks/views.py b/networks/views.py index 43f88c9f..b88dc340 100644 --- a/networks/views.py +++ b/networks/views.py @@ -42,6 +42,7 @@ def networks(request, compute_id): form = AddNetPool() # the new network dialog's mode: the one sent back after a refusal net_mode = request.POST.get("forward", "nat") + networks = None # not read: the page shows no list try: conn = wvmNetworks( diff --git a/nwfilters/templates/nwfilters.html b/nwfilters/templates/nwfilters.html index df10c525..f00a7be7 100644 --- a/nwfilters/templates/nwfilters.html +++ b/nwfilters/templates/nwfilters.html @@ -12,7 +12,7 @@ {% block content %} {% include "computes/host_nav.html" with section="nwfilters" %} - {% if not nwfilters %} + {% if nwfilters is None %}{# not read: the error above says why #}{% elif not nwfilters %}
{% bs_icon 'exclamation-triangle' %} {% trans "This host has no network filters." %}
diff --git a/nwfilters/views.py b/nwfilters/views.py index 6aac34ef..f8ab04ac 100644 --- a/nwfilters/views.py +++ b/nwfilters/views.py @@ -16,7 +16,7 @@ def nwfilters(request, compute_id): """The host's network filters; one created, cloned or deleted, after which the list is read again.""" compute = get_object_or_404(Compute, pk=compute_id) - nwfilters_all = [] + nwfilters_all = None # not read: the page shows no list try: conn = wvmNWFilters(compute.hostname, compute.login, compute.password, compute.type) diff --git a/storages/templates/storages.html b/storages/templates/storages.html index 83e40aee..b69fd6f0 100644 --- a/storages/templates/storages.html +++ b/storages/templates/storages.html @@ -15,7 +15,7 @@ - {% if not storages %} + {% if storages is None %}{# not read: the error above says why #}{% elif not storages %}
{% bs_icon 'exclamation-triangle' %} {% trans "This host has no storage pools." %}
diff --git a/storages/views.py b/storages/views.py index b4e5f9f1..b3f92b55 100644 --- a/storages/views.py +++ b/storages/views.py @@ -71,6 +71,7 @@ def storages(request, compute_id): form = AddStgPool() # the new pool dialog's type: its radio cards tell the ISO pool from another directory pool_kind = request.POST.get("pool_kind", "dir") + storages = None # not read: the page shows no list try: conn = wvmStorages( diff --git a/virtsecrets/templates/secrets.html b/virtsecrets/templates/secrets.html index b25c4809..69f68dbd 100644 --- a/virtsecrets/templates/secrets.html +++ b/virtsecrets/templates/secrets.html @@ -12,7 +12,7 @@ {% block content %} {% include "computes/host_nav.html" with section="virtsecrets" %} - {% if not secrets_all %} + {% if secrets_all is None %}{# not read: the error above says why #}{% elif not secrets_all %}
{% bs_icon 'exclamation-triangle' %} {% trans "This host has no secrets." %}
diff --git a/virtsecrets/tests/test_secret_in_use.py b/virtsecrets/tests/test_secret_in_use.py index 310df4c0..539d8398 100644 --- a/virtsecrets/tests/test_secret_in_use.py +++ b/virtsecrets/tests/test_secret_in_use.py @@ -41,4 +41,4 @@ class SecretInUsePageTests(TestCase): "ephemeral": False, "private": False}] conn.return_value.get_users.side_effect = libvirt.libvirtError("connection reset") response = self.client.get(self.url) - self.assertEqual(response.context["secrets_all"], []) + self.assertIsNone(response.context["secrets_all"]) diff --git a/virtsecrets/views.py b/virtsecrets/views.py index 576dfd80..d2d70313 100644 --- a/virtsecrets/views.py +++ b/virtsecrets/views.py @@ -19,7 +19,7 @@ def secrets(request, compute_id): given a value or deleted, after which the list is read again. A value is never shown or logged.""" compute = get_object_or_404(Compute, pk=compute_id) - secrets_all = [] + secrets_all = None # not read: the page shows no list try: conn = wvmSecrets(compute.hostname, compute.login, compute.password, compute.type)