From fb5a6e9a4265e26afde03fa0d75b642ce8123ef9 Mon Sep 17 00:00:00 2001 From: Tobias Kunze Date: Thu, 3 Apr 2025 17:54:31 +0200 Subject: [PATCH 1/2] Improve name uniqueness feedback --- src/servala/core/models/service.py | 23 +++++++++++++++-------- src/servala/frontend/views/service.py | 3 ++- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/servala/core/models/service.py b/src/servala/core/models/service.py index f60d57c..4cd3096 100644 --- a/src/servala/core/models/service.py +++ b/src/servala/core/models/service.py @@ -3,7 +3,7 @@ import urlman from django.conf import settings from django.core.cache import cache from django.core.exceptions import ValidationError -from django.db import models +from django.db import IntegrityError, models from django.utils.functional import cached_property from django.utils.translation import gettext_lazy as _ from encrypted_fields.fields import EncryptedJSONField @@ -492,6 +492,19 @@ class ServiceInstance(ServalaModelMixin, models.Model): def create_instance(cls, name, organization, context, created_by, spec_data): # Ensure the namespace exists context.control_plane.get_or_create_namespace(organization.namespace) + try: + instance = cls.objects.create( + name=name, + organization=organization, + created_by=created_by, + context=context, + ) + except IntegrityError: + raise ValidationError( + _( + "An instance with this name already exists in this organization. Please choose a different name." + ) + ) group = context.service_definition.api_definition["group"] version = context.service_definition.api_definition["version"] @@ -521,10 +534,4 @@ class ServiceInstance(ServalaModelMixin, models.Model): plural=plural, body=create_data, ) - - return cls.objects.create( - name=name, - organization=organization, - created_by=created_by, - context=context, - ) + return instance diff --git a/src/servala/frontend/views/service.py b/src/servala/frontend/views/service.py index c19aff5..8816721 100644 --- a/src/servala/frontend/views/service.py +++ b/src/servala/frontend/views/service.py @@ -135,7 +135,8 @@ class ServiceOfferingDetailView(OrganizationViewMixin, HtmxViewMixin, DetailView ) return redirect(service_instance.urls.base) except Exception as e: - messages.error(self.request, str(e)) + error_message = getattr(e, "message", None) or str(e) + messages.error(self.request, error_message) # If the form is not valid or if the service creation failed, we render it again context["service_form"] = form From 4d13d71953f4c7a2f144094596c8dec1b8486601 Mon Sep 17 00:00:00 2001 From: Tobias Kunze Date: Thu, 3 Apr 2025 18:01:15 +0200 Subject: [PATCH 2/2] Improve display of kubernetes errors --- src/servala/core/models/service.py | 69 ++++++++++++++++----------- src/servala/frontend/views/service.py | 6 ++- 2 files changed, 46 insertions(+), 29 deletions(-) diff --git a/src/servala/core/models/service.py b/src/servala/core/models/service.py index 4cd3096..41cc896 100644 --- a/src/servala/core/models/service.py +++ b/src/servala/core/models/service.py @@ -1,3 +1,5 @@ +import json + import kubernetes import urlman from django.conf import settings @@ -506,32 +508,45 @@ class ServiceInstance(ServalaModelMixin, models.Model): ) ) - group = context.service_definition.api_definition["group"] - version = context.service_definition.api_definition["version"] - kind = context.service_definition.api_definition["kind"] - create_data = { - "apiVersion": f"{group}/{version}", - "kind": kind, - "metadata": { - "name": name, - "namespace": organization.namespace, - }, - "spec": spec_data or {}, - } - if label := context.control_plane.required_label: - create_data["metadata"]["labels"] = {settings.DEFAULT_LABEL_KEY: label} - api_instance = client.CustomObjectsApi( - context.control_plane.get_kubernetes_client() - ) - plural = kind.lower() - if not plural.endswith("s"): - plural = f"{plural}s" + try: + group = context.service_definition.api_definition["group"] + version = context.service_definition.api_definition["version"] + kind = context.service_definition.api_definition["kind"] + create_data = { + "apiVersion": f"{group}/{version}", + "kind": kind, + "metadata": { + "name": name, + "namespace": organization.namespace, + }, + "spec": spec_data or {}, + } + if label := context.control_plane.required_label: + create_data["metadata"]["labels"] = [ + {settings.DEFAULT_LABEL_KEY: label} + ] + api_instance = client.CustomObjectsApi( + context.control_plane.get_kubernetes_client() + ) + plural = kind.lower() + if not plural.endswith("s"): + plural = f"{plural}s" - api_instance.create_namespaced_custom_object( - group=group, - version=version, - namespace=organization.namespace, - plural=plural, - body=create_data, - ) + api_instance.create_namespaced_custom_object( + group=group, + version=version, + namespace=organization.namespace, + plural=plural, + body=create_data, + ) + except Exception as e: + instance.delete() + if isinstance(e, ApiException): + try: + error_body = json.loads(e.body) + reason = error_body.get("message", str(e)) + raise ValidationError(_("Kubernetes API error: {}").format(reason)) + except (ValueError, TypeError): + raise ValidationError(_("Kubernetes API error: {}").format(str(e))) + raise ValidationError(_("Error creating instance: {}").format(str(e))) return instance diff --git a/src/servala/frontend/views/service.py b/src/servala/frontend/views/service.py index 8816721..edf889a 100644 --- a/src/servala/frontend/views/service.py +++ b/src/servala/frontend/views/service.py @@ -1,4 +1,5 @@ from django.contrib import messages +from django.core.exceptions import ValidationError from django.shortcuts import redirect from django.utils.functional import cached_property from django.views.generic import DetailView, ListView @@ -134,9 +135,10 @@ class ServiceOfferingDetailView(OrganizationViewMixin, HtmxViewMixin, DetailView spec_data=form.get_nested_data().get("spec"), ) return redirect(service_instance.urls.base) + except ValidationError as e: + messages.error(self.request, e.message or str(e)) except Exception as e: - error_message = getattr(e, "message", None) or str(e) - messages.error(self.request, error_message) + messages.error(self.request, str(e)) # If the form is not valid or if the service creation failed, we render it again context["service_form"] = form