fix(instances): Refresh ISO list says which pools it could not read

A pool whose refresh failed, one busy with a volume copy for example,
was skipped without a word, so a hand-copied image stayed missing with
no reason given. The other pools are still refreshed; each failure is
shown with libvirt's error.

The API comment no longer says listing the ISO images refreshes pools.

instances/api/serializers.py

@@ -12,8 +12,7 @@ class InstanceSerializer(serializers.ModelSerializer):
 class InstanceDetailsSerializer(serializers.ModelSerializer):
     def get_fields(self):
         fields = super().get_fields()
-        # the host's ISO images are a superuser's, as on the VM page; reading
-        # them refreshes every pool of the host
+        # the host's ISO images are a superuser's, as on the VM page
         request = self.context.get("request")
         if not (request and request.user.is_superuser):
             del fields["media_iso"]

instances/tests/test_disk_isolation.py

@@ -6,6 +6,7 @@ views must not act on volumes or option values taken blindly from the client.
 from unittest.mock import patch
 
 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
 
@@ -90,10 +91,18 @@ class DiskViewsTenantIsolationTestCase(TestCase):
     def test_refresh_iso_reads_the_pools_again_and_returns_to_the_disks_tab(self):
         self.client.force_login(self.superuser)
         with patch("instances.models.wvmInstance") as mock_wvm:
+            mock_wvm.return_value.refresh_iso_pools.return_value = {}
             res = self._post("refresh_iso", {})
         self.assertRedirects(res, self.referer + "#disks", fetch_redirect_response=False)
         mock_wvm.return_value.refresh_iso_pools.assert_called_once_with()
 
+    def test_refresh_iso_reports_the_pools_that_failed(self):
+        self.client.force_login(self.superuser)
+        with patch("instances.models.wvmInstance") as mock_wvm:
+            mock_wvm.return_value.refresh_iso_pools.return_value = {"busy": "pool is busy"}
+            res = self._post("refresh_iso", {})
+        self.assertEqual([str(m) for m in get_messages(res.wsgi_request)], ["Pool busy was not read again: pool is busy"])
+
     def test_delete_vol_deletes_the_volume_attached_to_dev_not_the_posted_one(self):
         self.client.force_login(self.superuser)
         with patch("instances.models.wvmInstance") as mock_wvm, patch(

instances/views.py

@@ -1689,7 +1689,8 @@ def mount_iso(request, pk):
 @superuser_only
 def refresh_iso(request, pk):
     instance = get_instance(request.user, pk, perm_type="change")
-    instance.proxy.refresh_iso_pools()
+    for pool, err in instance.proxy.refresh_iso_pools().items():
+        messages.error(request, _("Pool %(pool)s was not read again: %(err)s") % {"pool": pool, "err": err})
     return _back(request, pk, "disks")
 
 

vrtManager/instance.py

@@ -1715,10 +1715,15 @@ class wvmInstance(wvmConnect):
 
     def refresh_iso_pools(self):
         """Refresh the active pools, so that an image copied to one by hand
-        is listed by get_iso_media."""
+        is listed by get_iso_media. A pool that fails does not stop the
+        others; returns the errors, by pool name."""
+        failed = {}
         for pool in self.wvm.listAllStoragePools(VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE):
-            with contextlib.suppress(libvirtError):  # stopped or removed meanwhile
+            try:
                 pool.refresh(0)
+            except libvirtError as err:
+                failed[pool.name()] = err
+        return failed
 
     def get_iso_media(self):
         """The .iso volumes libvirt knows in the active pools; one copied in

vrtManager/tests/test_iso_media.py

@@ -91,13 +91,21 @@ class IsoListTestCase(unittest.TestCase):
         self.assertEqual(inst.get_iso_media(), ["b.iso"])
 
     def test_refresh_reads_every_active_pool_again(self):
-        a, b = pool([], refresh_error=True), pool([])
+        a, b = pool([]), pool([])
         inst = self.wvm(a, b)
-        inst.refresh_iso_pools()
+        self.assertEqual(inst.refresh_iso_pools(), {})
         inst.wvm.listAllStoragePools.assert_called_once_with(VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE)
         a.refresh.assert_called_once_with(0)
         b.refresh.assert_called_once_with(0)
 
+    def test_a_failed_refresh_is_returned_and_the_others_still_run(self):
+        a, b = pool([], refresh_error=True), pool([])
+        a.name.return_value = "busy"
+        failed = self.wvm(a, b).refresh_iso_pools()
+        self.assertEqual(list(failed), ["busy"])
+        self.assertIsInstance(failed["busy"], libvirtError)
+        b.refresh.assert_called_once_with(0)
+
 
 if __name__ == "__main__":
     unittest.main()