From 09ce5406eecdd9326fa7be7d6d1efdfdd25c941b Mon Sep 17 00:00:00 2001 From: Tobias Kunze Date: Sun, 25 May 2025 22:40:01 +0200 Subject: [PATCH 1/6] Implement service instance delete basics --- src/servala/core/models/service.py | 54 ++++++++++++++++- src/servala/frontend/forms/service.py | 32 ++++++++++ .../service_instance_delete_form.html | 0 src/servala/frontend/urls.py | 5 ++ src/servala/frontend/views/__init__.py | 2 + src/servala/frontend/views/service.py | 59 ++++++++++++++++--- 6 files changed, 142 insertions(+), 10 deletions(-) create mode 100644 src/servala/frontend/templates/frontend/organizations/service_instance_delete_form.html diff --git a/src/servala/core/models/service.py b/src/servala/core/models/service.py index 901a822..20611b2 100644 --- a/src/servala/core/models/service.py +++ b/src/servala/core/models/service.py @@ -7,6 +7,7 @@ from django.conf import settings from django.core.cache import cache from django.core.exceptions import ValidationError from django.db import IntegrityError, models +from django.utils import timezone from django.utils.functional import cached_property from django.utils.translation import gettext_lazy as _ from encrypted_fields.fields import EncryptedJSONField @@ -373,7 +374,13 @@ class ControlPlaneCRD(ServalaModelMixin, models.Model): @cached_property def kind_plural(self): - return self.resource_definition.status.accepted_names.plural + if hasattr(self.resource_definition, 'status') and \ + hasattr(self.resource_definition.status, 'accepted_names') and \ + self.resource_definition.status.accepted_names: + return self.resource_definition.status.accepted_names.plural + if self.kind.endswith("s"): + return self.kind.lower() + return f"{self.kind.lower()}s" @cached_property def resource_definition(self): @@ -399,8 +406,13 @@ class ControlPlaneCRD(ServalaModelMixin, models.Model): if result := cache.get(cache_key): return result + if not self.resource_definition: + return + for v in self.resource_definition.spec.versions: if v.name == self.version: + if not v.schema or not v.schema.open_apiv3_schema: + return result = v.schema.open_apiv3_schema.to_dict() timeout_seconds = 60 * 60 * 24 cache.set(cache_key, result, timeout=timeout_seconds) @@ -410,6 +422,9 @@ class ControlPlaneCRD(ServalaModelMixin, models.Model): def django_model(self): from servala.core.crd import generate_django_model + if not self.resource_schema: + return + kwargs = { "group": self.group, "version": self.version, @@ -421,6 +436,8 @@ class ControlPlaneCRD(ServalaModelMixin, models.Model): def model_form_class(self): from servala.core.crd import generate_model_form_class + if not self.django_model: + return return generate_model_form_class(self.django_model) @@ -514,6 +531,7 @@ class ServiceInstance(ServalaModelMixin, models.Model): class urls(urlman.Urls): base = "{self.organization.urls.instances}{self.name}/" update = "{base}update/" + delete = "{base}delete/" def _clear_kubernetes_caches(self): """Clears cached properties that depend on Kubernetes state.""" @@ -618,9 +636,41 @@ class ServiceInstance(ServalaModelMixin, models.Model): _("Error updating instance: {error}").format(error=str(e)) ) + def delete_instance(self, user): + """ + Soft deletes the instance in Django and initiates deletion of the + corresponding Kubernetes custom resource. + """ + if self.is_deleted: + return + + self.is_deleted = True + self.deleted_at = timezone.now() + self.deleted_by = user + self.save(update_fields=["is_deleted", "deleted_at", "deleted_by", "updated_at"]) + + self._clear_kubernetes_caches() + + try: + api_instance = self.context.control_plane.custom_objects_api + api_instance.delete_namespaced_custom_object( + group=self.context.group, + version=self.context.version, + namespace=self.organization.namespace, + plural=self.context.kind_plural, + name=self.name, + body=client.V1DeleteOptions(), + ) + except ApiException as e: + if e.status != 404: + # 404 is fine, the object was deleted already. + raise + @cached_property def kubernetes_object(self): """Fetch the Kubernetes custom resource object""" + if self.is_deleted: + return try: api_instance = client.CustomObjectsApi( self.context.control_plane.get_kubernetes_client() @@ -654,6 +704,8 @@ class ServiceInstance(ServalaModelMixin, models.Model): @cached_property def spec_object(self): """Dynamically generated CRD object.""" + if not self.context.django_model: + return return self.context.django_model( name=self.name, organization=self.organization, diff --git a/src/servala/frontend/forms/service.py b/src/servala/frontend/forms/service.py index 1b134a2..decb1a0 100644 --- a/src/servala/frontend/forms/service.py +++ b/src/servala/frontend/forms/service.py @@ -86,3 +86,35 @@ class ServiceInstanceFilterForm(forms.Form): else: queryset = queryset.filter(is_deleted=True) return queryset + + +class ServiceInstanceDeleteForm(forms.Form): + name_confirm = forms.CharField( + label=_("Instance Name"), + max_length=63, + widget=forms.TextInput(attrs={"class": "form-control"}), + ) + + def __init__(self, *args, **kwargs): + self.instance_name = kwargs.pop("instance_name", None) + super().__init__(*args, **kwargs) + if self.instance_name: + self.fields["name_confirm"].help_text = _( + "To confirm deletion, please type the instance name: {instance_name}" + ).format(instance_name=self.instance_name) + else: + self.fields["name_confirm"].help_text = _( + "Please type the instance name to confirm deletion." + ) + + def clean_name_confirm(self): + entered_name = self.cleaned_data.get("name_confirm") + if not self.instance_name: + raise ValidationError( + _("Cannot confirm deletion: instance name not provided to form.") + ) + if entered_name != self.instance_name: + raise ValidationError( + _("The entered name does not match the instance name. Deletion not confirmed.") + ) + return entered_name diff --git a/src/servala/frontend/templates/frontend/organizations/service_instance_delete_form.html b/src/servala/frontend/templates/frontend/organizations/service_instance_delete_form.html new file mode 100644 index 0000000..e69de29 diff --git a/src/servala/frontend/urls.py b/src/servala/frontend/urls.py index 64c84bf..6ab6949 100644 --- a/src/servala/frontend/urls.py +++ b/src/servala/frontend/urls.py @@ -55,6 +55,11 @@ urlpatterns = [ views.ServiceInstanceUpdateView.as_view(), name="organization.instance.update", ), + path( + "instances//delete/", + views.ServiceInstanceDeleteView.as_view(), + name="organization.instance.delete", + ), ] ), ), diff --git a/src/servala/frontend/views/__init__.py b/src/servala/frontend/views/__init__.py index d13d4d5..cc43552 100644 --- a/src/servala/frontend/views/__init__.py +++ b/src/servala/frontend/views/__init__.py @@ -7,6 +7,7 @@ from .organization import ( ) from .service import ( ServiceDetailView, + ServiceInstanceDeleteView, ServiceInstanceDetailView, ServiceInstanceListView, ServiceInstanceUpdateView, @@ -21,6 +22,7 @@ __all__ = [ "OrganizationDashboardView", "OrganizationUpdateView", "ServiceDetailView", + "ServiceInstanceDeleteView", "ServiceInstanceDetailView", "ServiceInstanceListView", "ServiceInstanceUpdateView", diff --git a/src/servala/frontend/views/service.py b/src/servala/frontend/views/service.py index 298d5bc..2d0612b 100644 --- a/src/servala/frontend/views/service.py +++ b/src/servala/frontend/views/service.py @@ -3,7 +3,7 @@ from django.core.exceptions import ValidationError from django.shortcuts import redirect from django.utils.functional import cached_property from django.utils.translation import gettext_lazy as _ -from django.views.generic import DetailView, ListView +from django.views.generic import DetailView, UpdateView, ListView from servala.core.crd import deslugify from servala.core.models import ( @@ -15,6 +15,7 @@ from servala.core.models import ( from servala.frontend.forms.service import ( ControlPlaneSelectForm, ServiceFilterForm, + ServiceInstanceDeleteForm, ServiceInstanceFilterForm, ) from servala.frontend.views.mixins import ( @@ -144,7 +145,7 @@ class ServiceOfferingDetailView(OrganizationViewMixin, HtmxViewMixin, DetailView if form.is_valid(): try: service_instance = ServiceInstance.create_instance( - organization=self.organization, + organization=self.request.organization, name=form.cleaned_data["name"], context=self.context_object, created_by=request.user, @@ -185,8 +186,8 @@ class ServiceInstanceMixin: def get_object(self, **kwargs): instance = super().get_object(**kwargs) if ( - not instance.kubernetes_object - and not instance.is_deleted + not instance.is_deleted + and not instance.kubernetes_object and not self._has_warned ): messages.warning( @@ -207,11 +208,13 @@ class ServiceInstanceDetailView( def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) - if self.object.kubernetes_object and self.object.spec: + if not self.object.is_deleted and self.object.kubernetes_object and self.object.spec: context["spec_fieldsets"] = self.get_nested_spec() - permission_required = ServiceInstance.get_perm("change") context["has_change_permission"] = self.request.user.has_perm( - permission_required, self.object + ServiceInstance.get_perm("change"), self.object + ) + context["has_delete_permission"] = self.request.user.has_perm( + ServiceInstance.get_perm("delete"), self.object ) return context @@ -220,8 +223,9 @@ class ServiceInstanceDetailView( Organize spec data into fieldsets similar to how the form does it. """ spec = self.object.spec or {} + if not spec: + return [] - # Process spec fields others = [] nested_fieldsets = {} @@ -379,10 +383,47 @@ class ServiceInstanceListView(OrganizationViewMixin, ListView): ) if self.filter_form.is_valid(): queryset = self.filter_form.filter_queryset(queryset) - return queryset + status_filter = self.filter_form.cleaned_data.get("status") if self.filter_form.is_valid() else "active" + if status_filter == "active": + queryset = queryset.filter(is_deleted=False) + elif status_filter == "deleted": + queryset = queryset.filter(is_deleted=True) + return queryset.order_by("-created_at") def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) context["organization"] = self.request.organization context["filter_form"] = self.filter_form return context + + +class ServiceInstanceDeleteView( + ServiceInstanceMixin, OrganizationViewMixin, HtmxViewMixin, UpdateView +): + template_name = "frontend/organizations/service_instance_delete_form.html" + form_class = ServiceInstanceDeleteForm + permission_type = "delete" + + def form_valid(self, form): + try: + self.object.delete_instance(user=self.request.user) + messages.success( + self.request, + _("Service instance '{name}' has been scheduled for deletion.").format( + name=self.object.name + ), + ) + response = HttpResponse() + response["HX-Redirect"] = self.get_success_url() + return response + except Exception as e: + messages.error( + self.request, + _("An error occurred while trying to delete instance '{name}': {error}").format( + name=self.object.name, error=str(e) + ), + ) + return self.form_invalid(form) + + def get_success_url(self): + return self.request.organization.urls.instances From 3a8b9987b2d4919236b666324aeed915a43537a0 Mon Sep 17 00:00:00 2001 From: Tobias Kunze Date: Sun, 25 May 2025 22:53:22 +0200 Subject: [PATCH 2/6] Improve instance delete --- src/servala/core/models/service.py | 3 +- src/servala/frontend/forms/service.py | 31 ++++++++----------- .../templates/frontend/forms/form.html | 2 +- .../service_instance_detail.html | 30 ++++++++++++++---- src/servala/frontend/views/service.py | 4 ++- src/servala/static/css/servala.css | 3 ++ 6 files changed, 46 insertions(+), 27 deletions(-) diff --git a/src/servala/core/models/service.py b/src/servala/core/models/service.py index 20611b2..4824c47 100644 --- a/src/servala/core/models/service.py +++ b/src/servala/core/models/service.py @@ -6,7 +6,7 @@ import urlman from django.conf import settings from django.core.cache import cache from django.core.exceptions import ValidationError -from django.db import IntegrityError, models +from django.db import IntegrityError, models, transaction from django.utils import timezone from django.utils.functional import cached_property from django.utils.translation import gettext_lazy as _ @@ -636,6 +636,7 @@ class ServiceInstance(ServalaModelMixin, models.Model): _("Error updating instance: {error}").format(error=str(e)) ) + @transaction.atomic def delete_instance(self, user): """ Soft deletes the instance in Django and initiates deletion of the diff --git a/src/servala/frontend/forms/service.py b/src/servala/frontend/forms/service.py index decb1a0..09833d9 100644 --- a/src/servala/frontend/forms/service.py +++ b/src/servala/frontend/forms/service.py @@ -88,33 +88,28 @@ class ServiceInstanceFilterForm(forms.Form): return queryset -class ServiceInstanceDeleteForm(forms.Form): - name_confirm = forms.CharField( +class ServiceInstanceDeleteForm(forms.ModelForm): + name = forms.CharField( label=_("Instance Name"), max_length=63, widget=forms.TextInput(attrs={"class": "form-control"}), ) def __init__(self, *args, **kwargs): - self.instance_name = kwargs.pop("instance_name", None) + kwargs["initial"] = {"name": ""} super().__init__(*args, **kwargs) - if self.instance_name: - self.fields["name_confirm"].help_text = _( - "To confirm deletion, please type the instance name: {instance_name}" - ).format(instance_name=self.instance_name) - else: - self.fields["name_confirm"].help_text = _( - "Please type the instance name to confirm deletion." - ) + self.fields["name"].help_text = _( + "To confirm deletion, please type the instance name: {instance_name}" + ).format(instance_name=self.instance.name) - def clean_name_confirm(self): - entered_name = self.cleaned_data.get("name_confirm") - if not self.instance_name: - raise ValidationError( - _("Cannot confirm deletion: instance name not provided to form.") - ) - if entered_name != self.instance_name: + def clean_name(self): + entered_name = self.cleaned_data.get("name") + if entered_name != self.instance.name: raise ValidationError( _("The entered name does not match the instance name. Deletion not confirmed.") ) return entered_name + + class Meta: + model = ServiceInstance + fields = ("name", ) diff --git a/src/servala/frontend/templates/frontend/forms/form.html b/src/servala/frontend/templates/frontend/forms/form.html index b45c41f..8ab90d1 100644 --- a/src/servala/frontend/templates/frontend/forms/form.html +++ b/src/servala/frontend/templates/frontend/forms/form.html @@ -1,6 +1,6 @@ {% load i18n %} {% if form.non_field_errors or form.errors %} - - {% if instance.status_conditions %} + {% if not instance.is_deleted and instance.status_conditions %}
@@ -101,7 +119,7 @@
{% endif %} - {% if instance.spec %} + {% if not instance.is_deleted and instance.spec and spec_fieldsets %}
diff --git a/src/servala/frontend/views/service.py b/src/servala/frontend/views/service.py index 2d0612b..89b3f3c 100644 --- a/src/servala/frontend/views/service.py +++ b/src/servala/frontend/views/service.py @@ -423,7 +423,9 @@ class ServiceInstanceDeleteView( name=self.object.name, error=str(e) ), ) - return self.form_invalid(form) + response = HttpResponse() + response["HX-Redirect"] = str(self.object.urls.base) + return response def get_success_url(self): return self.request.organization.urls.instances diff --git a/src/servala/static/css/servala.css b/src/servala/static/css/servala.css index e607205..1e418fb 100644 --- a/src/servala/static/css/servala.css +++ b/src/servala/static/css/servala.css @@ -90,3 +90,6 @@ a.btn-keycloak { flex-wrap: wrap; justify-content: space-between; } +.hide-form-errors .alert.form-errors { + display: none; +} From 52aa6acfb63aa706b476201e19cc3fa2a4298f9b Mon Sep 17 00:00:00 2001 From: Tobias Kunze Date: Sun, 25 May 2025 22:54:36 +0200 Subject: [PATCH 3/6] Show instance delete modal --- .../service_instance_delete_form.html | 18 +++++++++++++++ .../service_instance_detail.html | 23 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/src/servala/frontend/templates/frontend/organizations/service_instance_delete_form.html b/src/servala/frontend/templates/frontend/organizations/service_instance_delete_form.html index e69de29..08d4167 100644 --- a/src/servala/frontend/templates/frontend/organizations/service_instance_delete_form.html +++ b/src/servala/frontend/templates/frontend/organizations/service_instance_delete_form.html @@ -0,0 +1,18 @@ +{% load i18n %} +
+ {% csrf_token %} + + + +
diff --git a/src/servala/frontend/templates/frontend/organizations/service_instance_detail.html b/src/servala/frontend/templates/frontend/organizations/service_instance_detail.html index f0867c5..5d6779c 100644 --- a/src/servala/frontend/templates/frontend/organizations/service_instance_detail.html +++ b/src/servala/frontend/templates/frontend/organizations/service_instance_detail.html @@ -218,4 +218,27 @@ {% endif %}
+ + + {% endblock content %} From 3e466fb011a94d4348fa7cfd4921b5768c67afed Mon Sep 17 00:00:00 2001 From: Tobias Kunze Date: Sun, 25 May 2025 22:55:37 +0200 Subject: [PATCH 4/6] Instance form and template improvements --- src/servala/core/models/service.py | 12 ++++-- src/servala/frontend/forms/service.py | 29 +++++++++------ .../service_instance_delete_form.html | 11 +++++- .../service_instance_detail.html | 37 ++++++++++++------- src/servala/frontend/views/service.py | 37 +++++++++++++------ 5 files changed, 82 insertions(+), 44 deletions(-) diff --git a/src/servala/core/models/service.py b/src/servala/core/models/service.py index 4824c47..5540d22 100644 --- a/src/servala/core/models/service.py +++ b/src/servala/core/models/service.py @@ -374,9 +374,11 @@ class ControlPlaneCRD(ServalaModelMixin, models.Model): @cached_property def kind_plural(self): - if hasattr(self.resource_definition, 'status') and \ - hasattr(self.resource_definition.status, 'accepted_names') and \ - self.resource_definition.status.accepted_names: + if ( + hasattr(self.resource_definition, "status") + and hasattr(self.resource_definition.status, "accepted_names") + and self.resource_definition.status.accepted_names + ): return self.resource_definition.status.accepted_names.plural if self.kind.endswith("s"): return self.kind.lower() @@ -648,7 +650,9 @@ class ServiceInstance(ServalaModelMixin, models.Model): self.is_deleted = True self.deleted_at = timezone.now() self.deleted_by = user - self.save(update_fields=["is_deleted", "deleted_at", "deleted_by", "updated_at"]) + self.save( + update_fields=["is_deleted", "deleted_at", "deleted_by", "updated_at"] + ) self._clear_kubernetes_caches() diff --git a/src/servala/frontend/forms/service.py b/src/servala/frontend/forms/service.py index 09833d9..04fe2df 100644 --- a/src/servala/frontend/forms/service.py +++ b/src/servala/frontend/forms/service.py @@ -6,6 +6,7 @@ from servala.core.models import ( ControlPlane, Service, ServiceCategory, + ServiceInstance, ServiceOffering, ) @@ -37,6 +38,8 @@ class ControlPlaneSelectForm(forms.Form): def __init__(self, *args, planes=None, **kwargs): super().__init__(*args, **kwargs) self.fields["control_plane"].queryset = planes + if planes and planes.count() == 1: + self.fields["control_plane"].initial = planes.first() class ServiceInstanceFilterForm(forms.Form): @@ -68,23 +71,23 @@ class ServiceInstanceFilterForm(forms.Form): def filter_queryset(self, queryset): if self.is_valid(): data = self.cleaned_data - if data["name"]: + if data.get("name"): queryset = queryset.filter(name__icontains=data["name"]) - if data["service"]: + if data.get("service"): queryset = queryset.filter( context__service_definition__service=data["service"] ) - if data["provider"]: + if data.get("provider"): queryset = queryset.filter( context__service_offering__provider=data["provider"] ) - if data["control_plane"]: + if data.get("control_plane"): queryset = queryset.filter(context__control_plane=data["control_plane"]) - if data["status"]: - if data["status"] == "active": - queryset = queryset.filter(is_deleted=False) - else: - queryset = queryset.filter(is_deleted=True) + status = data.get("status") + if status == "active": + queryset = queryset.filter(is_deleted=False) + elif status == "deleted": + queryset = queryset.filter(is_deleted=True) return queryset @@ -105,11 +108,13 @@ class ServiceInstanceDeleteForm(forms.ModelForm): def clean_name(self): entered_name = self.cleaned_data.get("name") if entered_name != self.instance.name: - raise ValidationError( - _("The entered name does not match the instance name. Deletion not confirmed.") + raise forms.ValidationError( + _( + "The entered name does not match the instance name. Deletion not confirmed." + ) ) return entered_name class Meta: model = ServiceInstance - fields = ("name", ) + fields = ("name",) diff --git a/src/servala/frontend/templates/frontend/organizations/service_instance_delete_form.html b/src/servala/frontend/templates/frontend/organizations/service_instance_delete_form.html index 08d4167..5296c56 100644 --- a/src/servala/frontend/templates/frontend/organizations/service_instance_delete_form.html +++ b/src/servala/frontend/templates/frontend/organizations/service_instance_delete_form.html @@ -1,11 +1,18 @@ {% load i18n %} -
+ {% csrf_token %} {% endblock page_title_extra %} @@ -45,7 +44,7 @@
{% translate "Created By" %}
- {{ instance.created_by }} + {{ instance.created_by|default:"-" }}
{% translate "Created At" %}
@@ -106,9 +105,9 @@ {{ condition.status }} {% endif %} - {{ condition.lastTransitionTime }} - {{ condition.reason }} - {{ condition.message }} + {{ condition.lastTransitionTime|date:"SHORT_DATETIME_FORMAT" }} + {{ condition.reason|default:"-" }} + {{ condition.message|truncatewords:20|default:"-" }} {% endfor %} @@ -137,6 +136,8 @@ data-bs-toggle="tab" role="tab">{{ fieldset.title }} + {% empty %} + {% endfor %} @@ -153,7 +154,7 @@ {% if field.value|default:""|stringformat:"s"|slice:":1" == "{" or field.value|default:""|stringformat:"s"|slice:":1" == "[" %}
{{ field.value|pprint }}
{% else %} - {{ field.value }} + {{ field.value|default:"-" }} {% endif %}
{% endfor %} @@ -168,13 +169,15 @@ {% if field.value|default:""|stringformat:"s"|slice:":1" == "{" or field.value|default:""|stringformat:"s"|slice:":1" == "[" %}
{{ field.value|pprint }}
{% else %} - {{ field.value }} + {{ field.value|default:"-" }} {% endif %} {% endfor %} {% endfor %}
+ {% empty %} +

{% translate "No specification details to display." %}

{% endfor %}
@@ -218,15 +221,21 @@ {% endif %} - -