Do not break the VM list when a VM's OpenNebula template was deleted
The "my virtual machines" page showed "We could not load your VMs due to a
backend connection error" for any user owning a VM whose template had been
removed from OpenNebula. OpenNebulaManager.get_template() converted every
failure, including WrongIdError for a missing template, into
ConnectionRefusedError, and VirtualMachineSerializer.get_configuration()
let that propagate, so one VM with a stale template id took down the whole
list. The only log output was an empty ERROR line, because WrongIdError
has no message.
* Add opennebula_api.exceptions.TemplateNotFoundError and raise it from
get_template() for a missing template; keep ConnectionRefusedError for
genuine backend failures and log them with a traceback.
* Add OpenNebulaManager.get_template_names() and resolve_template_id() so
the serializer fetches the template pool once per list instead of
constructing an OpenNebulaManager and fetching the pool for every VM.
* get_configuration() now falls back to the template name embedded in the
VM name ("<template>-<vm id>") when the template is missing or the pool
cannot be fetched, and logs why.
* Replace str.lstrip('public-') with a real prefix strip (lstrip removes a
character set, so "public-ubuntu" became "ntu") and also strip
"ipv6only-".
* ProcessVMSelectionMixin shows "configuration is no longer available"
instead of an unhandled error when ordering a retired template.
* VirtualMachinesPlanListView logs the exception behind the banner.
* Add mock-based unit tests for the fallback, the UPDATED_TEMPLATES
mapping, the single pool fetch and the new exception.
The UPDATED_TEMPLATES setting keeps working as an operational override for
mapping retired template ids to their replacements.
This commit is contained in:
PCoder
parent
9cdf4593cf
commit
cb16114aba
6 changed files with 268 additions and 23 deletions
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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'
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
logger.debug("Template Id we are looking for : %s" % template_id)
|
||||
logger.error(str(ex))
|
||||
except WrongIdError:
|
||||
logger.error(
|
||||
"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,
|
||||
|
|
|
|||
|
|
@ -1,12 +1,39 @@
|
|||
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):
|
||||
"""
|
||||
VMs are named '<template name>-<vm id>' at creation time (see
|
||||
OpenNebulaManager.create_vm), so the template name can be recovered
|
||||
from the VM name when the template itself has been deleted.
|
||||
"""
|
||||
return vm_name.rsplit('-', 1)[0]
|
||||
|
||||
|
||||
class VirtualMachineTemplateSerializer(serializers.Serializer):
|
||||
"""Serializer to map the virtual machine template instance into JSON format."""
|
||||
|
|
@ -36,10 +63,7 @@ class VirtualMachineTemplateSerializer(serializers.Serializer):
|
|||
return int(obj.template.memory) / 1024
|
||||
|
||||
def get_name(self, obj):
|
||||
if obj.name.startswith('public-'):
|
||||
return obj.name.lstrip('public-')
|
||||
else:
|
||||
return obj.name
|
||||
return strip_template_prefix(obj.name)
|
||||
|
||||
|
||||
class VirtualMachineSerializer(serializers.Serializer):
|
||||
|
|
@ -133,13 +157,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)
|
||||
if template.name.startswith('public-'):
|
||||
return template.name.lstrip('public-')
|
||||
else:
|
||||
return template.name
|
||||
try:
|
||||
name = self._template_name(template_id)
|
||||
except TemplateNotFoundError as err:
|
||||
logger.warning(
|
||||
"VM %s: %s, falling back to VM name" % (obj.id, err)
|
||||
)
|
||||
name = template_name_from_vm_name(obj.name)
|
||||
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)
|
||||
return strip_template_prefix(name)
|
||||
|
||||
def get_ipv4(self, obj):
|
||||
"""
|
||||
|
|
@ -168,10 +224,7 @@ class VirtualMachineSerializer(serializers.Serializer):
|
|||
return '-'
|
||||
|
||||
def get_name(self, obj):
|
||||
if obj.name.startswith('public-'):
|
||||
return obj.name.lstrip('public-')
|
||||
else:
|
||||
return obj.name
|
||||
return strip_template_prefix(obj.name)
|
||||
|
||||
|
||||
class VMTemplateSerializer(serializers.Serializer):
|
||||
|
|
|
|||
|
|
@ -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,115 @@ 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'),
|
||||
'public-Ubuntu 20.10'
|
||||
)
|
||||
|
||||
|
||||
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)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue