Only derive the template name from VM names dynamicweb itself produced

On production 143 of 233 active VMs reference a deleted template, and
many of those were created or renamed outside dynamicweb ("gitea",
"ipv6only-Alpine 3.15-27397-Jonas"). Blindly cutting at the last "-"
mangled such names, so only strip a trailing "-<vm id>" suffix, which is
exactly what OpenNebulaManager.create_vm appends.

hosting/mixins.py

@@ -1,7 +1,10 @@
+from django.contrib import messages
 from django.shortcuts import redirect
 from django.conf import settings
 from django.core.urlresolvers import reverse
+from django.utils.translation import ugettext_lazy as _
 
+from opennebula_api.exceptions import TemplateNotFoundError
 from opennebula_api.serializers import VirtualMachineTemplateSerializer
 from opennebula_api.models import OpenNebulaManager
 
@@ -14,7 +17,15 @@ class ProcessVMSelectionMixin(object):
 
         template_id = int(request.POST.get('vm_template_id'))
         configuration_id = int(request.POST.get('configuration'))
-        template = OpenNebulaManager().get_template(template_id)
+        try:
+            template = OpenNebulaManager().get_template(template_id)
+        except TemplateNotFoundError:
+            messages.error(
+                request,
+                _('The selected configuration is no longer available. '
+                  'Please choose another one.')
+            )
+            return redirect(reverse('hosting:index'))
         data = VirtualMachineTemplateSerializer(template).data
         configuration = HostingPlan.objects.get(id=configuration_id)
 

hosting/views.py

@@ -1557,6 +1557,11 @@ class VirtualMachinesPlanListView(LoginRequiredMixin, ListView):
             serializer = VirtualMachineSerializer(queryset, many=True)
             return serializer.data
         except ConnectionRefusedError:
+            logger.exception(
+                "Could not load VMs of user %s (%s) from OpenNebula" % (
+                    owner.username, owner.email
+                )
+            )
             messages.error(self.request,
                            'We could not load your VMs due to a backend connection \
                 error. Please try again in a few minutes'

opennebula_api/exceptions.py

@@ -8,3 +8,13 @@ class UserExistsError(Exception):
 
 class UserCredentialError(Exception):
     pass
+
+
+class TemplateNotFoundError(Exception):
+    """Raised when an OpenNebula VM template id does not exist (any more)."""
+
+    def __init__(self, template_id):
+        super(TemplateNotFoundError, self).__init__(
+            "OpenNebula template %s does not exist" % template_id
+        )
+        self.template_id = template_id

opennebula_api/models.py

@@ -9,7 +9,10 @@ from oca.pool import WrongNameError, WrongIdError
 from hosting.models import HostingOrder
 from utils.models import CustomUser
 from utils.tasks import save_ssh_key, save_ssh_key_error_handler
-from .exceptions import KeyExistsError, UserExistsError, UserCredentialError
+from .exceptions import (
+    KeyExistsError, UserExistsError, UserCredentialError,
+    TemplateNotFoundError
+)
 
 logger = logging.getLogger(__name__)
 
@@ -425,16 +428,60 @@ class OpenNebulaManager():
         except:
             return []
 
-    def get_template(self, template_id):
+    @staticmethod
+    def resolve_template_id(template_id):
+        """
+        Map a (possibly retired) template id to its replacement, as
+        configured in settings.UPDATED_TEMPLATES_DICT. Ids without a
+        mapping are returned unchanged.
+        """
         template_id = int(template_id)
+        return settings.UPDATED_TEMPLATES_DICT.get(template_id, template_id)
+
+    def get_template_names(self):
+        """
+        Return a dict {template_id: template_name} of every template
+        visible to oneadmin, in a single RPC call.
+
+        :raises ConnectionRefusedError: if OpenNebula is unreachable
+        """
+        try:
+            template_pool = self._get_template_pool()
+            return dict(
+                (template.id, template.name) for template in template_pool
+            )
+        except ConnectionRefusedError:
+            raise
+        except Exception:
+            # Any other failure (socket timeout, XML-RPC fault, ...) is a
+            # backend problem as far as the callers are concerned
+            logger.exception("Could not fetch the OpenNebula template pool")
+            raise ConnectionRefusedError
+
+    def get_template(self, template_id):
+        """
+        Return the oca template object for the given id.
+
+        :raises TemplateNotFoundError: if the template (after applying
+            settings.UPDATED_TEMPLATES_DICT) does not exist in OpenNebula
+        :raises ConnectionRefusedError: if OpenNebula is unreachable or
+            fails in any other way
+        """
+        template_id = self.resolve_template_id(template_id)
         try:
             template_pool = self._get_template_pool()
-            if template_id in settings.UPDATED_TEMPLATES_DICT.keys():
-                template_id = settings.UPDATED_TEMPLATES_DICT[template_id]
             return template_pool.get_by_id(template_id)
-        except Exception as ex:
+        except WrongIdError:
-            logger.debug("Template Id we are looking for : %s" % template_id)
+            logger.error(
-            logger.error(str(ex))
+                "OpenNebula template %s does not exist" % template_id
+            )
+            raise TemplateNotFoundError(template_id)
+        except ConnectionRefusedError:
+            raise
+        except Exception:
+            logger.exception(
+                "Could not fetch OpenNebula template %s" % template_id
+            )
             raise ConnectionRefusedError
 
     def create_template(self, name, cores, memory, disk_size, core_price,

opennebula_api/serializers.py

@@ -1,12 +1,44 @@
 import ipaddress
+import logging
 
 from builtins import hasattr
 from rest_framework import serializers
 
 from oca import OpenNebulaException
 
+from .exceptions import TemplateNotFoundError
 from .models import OpenNebulaManager
 
+logger = logging.getLogger(__name__)
+
+TEMPLATE_NAME_PREFIXES = ('public-', 'ipv6only-')
+
+
+def strip_template_prefix(name):
+    """
+    Remove the DCL naming prefix ('public-', 'ipv6only-') from a template
+    or VM name. str.lstrip() must not be used here: it strips a character
+    set, so 'public-ubuntu'.lstrip('public-') gives 'ntu'.
+    """
+    for prefix in TEMPLATE_NAME_PREFIXES:
+        if name.startswith(prefix):
+            return name[len(prefix):]
+    return name
+
+
+def template_name_from_vm_name(vm_name, vm_id):
+    """
+    VMs created by dynamicweb are named '<template name>-<vm id>' (see
+    OpenNebulaManager.create_vm), so the template name can be recovered
+    from the VM name when the template itself has been deleted. VMs that
+    were created or renamed outside dynamicweb do not follow that pattern
+    and are returned unchanged.
+    """
+    suffix = '-%s' % vm_id
+    if vm_name.endswith(suffix):
+        return vm_name[:-len(suffix)]
+    return vm_name
+
 
 class VirtualMachineTemplateSerializer(serializers.Serializer):
     """Serializer to map the virtual machine template instance into JSON format."""
@@ -36,10 +68,7 @@ class VirtualMachineTemplateSerializer(serializers.Serializer):
         return int(obj.template.memory) / 1024
 
     def get_name(self, obj):
-        if obj.name.startswith('public-'):
+        return strip_template_prefix(obj.name)
-            return obj.name.lstrip('public-')
-        else:
-            return obj.name
 
 
 class VirtualMachineSerializer(serializers.Serializer):
@@ -133,13 +162,45 @@ class VirtualMachineSerializer(serializers.Serializer):
             price += int(disk.size) / 1024 * 0.6
         return price
 
+    def _template_name(self, template_id):
+        """
+        Resolve a template id to its name using one template pool fetch per
+        serializer instance. With many=True the child serializer is shared
+        across all VMs, so a whole VM list costs a single RPC call instead
+        of one OpenNebulaManager (and template pool fetch) per VM.
+
+        :raises TemplateNotFoundError: if the template no longer exists
+        :raises ConnectionRefusedError: if OpenNebula is unreachable
+        """
+        if not hasattr(self, '_template_names'):
+            self._template_names = OpenNebulaManager().get_template_names()
+        template_id = OpenNebulaManager.resolve_template_id(template_id)
+        if template_id not in self._template_names:
+            raise TemplateNotFoundError(template_id)
+        return self._template_names[template_id]
+
     def get_configuration(self, obj):
+        """
+        The name of the template the VM was created from, without the DCL
+        prefix. A VM whose template has since been deleted from OpenNebula
+        must not break serialization of the whole VM list, so fall back to
+        the template name embedded in the VM name.
+        """
         template_id = obj.template.template_id
-        template = OpenNebulaManager().get_template(template_id)
+        try:
-        if template.name.startswith('public-'):
+            name = self._template_name(template_id)
-            return template.name.lstrip('public-')
+        except TemplateNotFoundError as err:
-        else:
+            logger.warning(
-            return template.name
+                "VM %s: %s, falling back to VM name" % (obj.id, err)
+            )
+            name = template_name_from_vm_name(obj.name, obj.id)
+        except ConnectionRefusedError:
+            logger.error(
+                "VM %s: could not fetch template pool, falling back to "
+                "VM name" % obj.id
+            )
+            name = template_name_from_vm_name(obj.name, obj.id)
+        return strip_template_prefix(name)
 
     def get_ipv4(self, obj):
         """
@@ -168,10 +229,7 @@ class VirtualMachineSerializer(serializers.Serializer):
             return '-'
 
     def get_name(self, obj):
-        if obj.name.startswith('public-'):
+        return strip_template_prefix(obj.name)
-            return obj.name.lstrip('public-')
-        else:
-            return obj.name
 
 
 class VMTemplateSerializer(serializers.Serializer):

opennebula_api/tests.py

@@ -2,11 +2,18 @@ import random
 import string
 
 from django.conf import settings
-from django.test import TestCase
+from django.test import TestCase, override_settings
 from unittest import skipIf
+from unittest.mock import MagicMock, patch
 
+from oca.pool import WrongIdError
+
+from .exceptions import TemplateNotFoundError
 from .models import OpenNebulaManager
-from .serializers import VirtualMachineSerializer
+from .serializers import (
+    VirtualMachineSerializer, strip_template_prefix,
+    template_name_from_vm_name
+)
 from utils.models import CustomUser
 
 
@@ -149,3 +156,124 @@ class VirtualMachineSerializerTestCase(TestCase):
                 serialized.data.get('name'), vm.name.lstrip('public-')
             )
             break
+
+
+class TemplateNameHelpersTestCase(TestCase):
+    def test_strip_template_prefix(self):
+        self.assertEqual(strip_template_prefix('public-Ubuntu 20.10'),
+                         'Ubuntu 20.10')
+        self.assertEqual(strip_template_prefix('ipv6only-Alpine 3.21'),
+                         'Alpine 3.21')
+        self.assertEqual(strip_template_prefix('Devuan Beowulf'),
+                         'Devuan Beowulf')
+
+    def test_strip_template_prefix_is_not_lstrip(self):
+        # str.lstrip('public-') strips a character set and would give 'ntu'
+        self.assertEqual(strip_template_prefix('public-ubuntu'), 'ubuntu')
+
+    def test_template_name_from_vm_name(self):
+        self.assertEqual(
+            template_name_from_vm_name('public-Ubuntu 20.10-26623', 26623),
+            'public-Ubuntu 20.10'
+        )
+
+    def test_template_name_from_vm_name_keeps_custom_names(self):
+        # VMs created or renamed outside dynamicweb
+        self.assertEqual(template_name_from_vm_name('gitea', 27001), 'gitea')
+        self.assertEqual(
+            template_name_from_vm_name('ipv6only-Alpine 3.15-27397-Jonas',
+                                       27397),
+            'ipv6only-Alpine 3.15-27397-Jonas'
+        )
+
+
+def _fake_vm(vm_id, name, template_id):
+    vm = MagicMock()
+    vm.id = vm_id
+    vm.name = name
+    vm.template.template_id = template_id
+    return vm
+
+
+class VirtualMachineSerializerMissingTemplateTestCase(TestCase):
+    """
+    A VM whose OpenNebula template has been deleted must not break the
+    serialization of the whole VM list (the symptom was the generic
+    "backend connection error" banner on /hosting/my-virtual-machines).
+    """
+
+    def _configuration(self, vms, template_names):
+        serializer = VirtualMachineSerializer(vms, many=True)
+        with patch.object(OpenNebulaManager, '__init__', return_value=None), \
+                patch.object(OpenNebulaManager, 'get_template_names',
+                             return_value=template_names) as get_names:
+            configurations = [
+                serializer.child.get_configuration(vm) for vm in vms
+            ]
+        return configurations, get_names
+
+    @override_settings(UPDATED_TEMPLATES_DICT={})
+    def test_missing_template_falls_back_to_vm_name(self):
+        vms = [
+            _fake_vm(28344, 'public-Alpine 3.21-28344', 310),
+            _fake_vm(26623, 'public-Ubuntu 20.10-26623', 164),
+        ]
+        configurations, _ = self._configuration(
+            vms, {310: 'public-Alpine 3.21'}
+        )
+        self.assertEqual(configurations, ['Alpine 3.21', 'Ubuntu 20.10'])
+
+    @override_settings(UPDATED_TEMPLATES_DICT={164: 192})
+    def test_updated_templates_mapping_is_applied(self):
+        vms = [_fake_vm(26623, 'public-Ubuntu 20.10-26623', 164)]
+        configurations, _ = self._configuration(
+            vms, {192: 'public-Ubuntu 20.10 (new)'}
+        )
+        self.assertEqual(configurations, ['Ubuntu 20.10 (new)'])
+
+    @override_settings(UPDATED_TEMPLATES_DICT={})
+    def test_template_pool_is_fetched_once_per_list(self):
+        vms = [
+            _fake_vm(1, 'public-Alpine 3.21-1', 310),
+            _fake_vm(2, 'public-Alpine 3.21-2', 310),
+            _fake_vm(3, 'public-Ubuntu 20.10-3', 164),
+        ]
+        _, get_names = self._configuration(vms, {310: 'public-Alpine 3.21'})
+        self.assertEqual(get_names.call_count, 1)
+
+    @override_settings(UPDATED_TEMPLATES_DICT={})
+    def test_unreachable_template_pool_falls_back_to_vm_name(self):
+        vms = [_fake_vm(26623, 'public-Ubuntu 20.10-26623', 164)]
+        serializer = VirtualMachineSerializer(vms, many=True)
+        with patch.object(OpenNebulaManager, '__init__', return_value=None), \
+                patch.object(OpenNebulaManager, 'get_template_names',
+                             side_effect=ConnectionRefusedError):
+            configuration = serializer.child.get_configuration(vms[0])
+        self.assertEqual(configuration, 'Ubuntu 20.10')
+
+
+class OpenNebulaManagerGetTemplateTestCase(TestCase):
+    @override_settings(UPDATED_TEMPLATES_DICT={164: 192})
+    def test_resolve_template_id(self):
+        self.assertEqual(OpenNebulaManager.resolve_template_id(164), 192)
+        self.assertEqual(OpenNebulaManager.resolve_template_id('164'), 192)
+        self.assertEqual(OpenNebulaManager.resolve_template_id(310), 310)
+
+    @override_settings(UPDATED_TEMPLATES_DICT={})
+    def test_missing_template_raises_template_not_found(self):
+        pool = MagicMock()
+        pool.get_by_id.side_effect = WrongIdError
+        with patch.object(OpenNebulaManager, '__init__', return_value=None), \
+                patch.object(OpenNebulaManager, '_get_template_pool',
+                             return_value=pool):
+            with self.assertRaises(TemplateNotFoundError) as ctx:
+                OpenNebulaManager().get_template(164)
+        self.assertEqual(ctx.exception.template_id, 164)
+
+    @override_settings(UPDATED_TEMPLATES_DICT={})
+    def test_unreachable_pool_still_raises_connection_error(self):
+        with patch.object(OpenNebulaManager, '__init__', return_value=None), \
+                patch.object(OpenNebulaManager, '_get_template_pool',
+                             side_effect=ConnectionRefusedError):
+            with self.assertRaises(ConnectionRefusedError):
+                OpenNebulaManager().get_template(164)