Skip to content

Commit

Permalink
Large refactoring of app status. The model AppStatus is being replace…
Browse files Browse the repository at this point in the history
…d by two new fields and a model annotation. This is still in progress.
  • Loading branch information
alfredeen committed Dec 20, 2024
1 parent d8e737b commit 294a961
Show file tree
Hide file tree
Showing 14 changed files with 325 additions and 54 deletions.
3 changes: 2 additions & 1 deletion apps/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,9 @@ class K8sUserAppStatusAdmin(admin.ModelAdmin):


class BaseAppAdmin(admin.ModelAdmin):
# TODO: Change status use to new status
list_display = ("name", "display_owner", "display_project", "display_status", "display_subdomain", "chart")
readonly_fields = ("id", "created_on")
readonly_fields = ("app_status", "id", "created_on")
list_filter = ["owner", "project", "app_status__status", "chart"]
actions = ["redeploy_apps", "deploy_resources", "delete_resources"]

Expand Down
34 changes: 27 additions & 7 deletions apps/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from apps.types_.subdomain import SubdomainCandidateName, SubdomainTuple
from studio.utils import get_logger

from .models import Apps, AppStatus, BaseAppInstance, K8sUserAppStatus, Subdomain
from .models import Apps, BaseAppInstance, K8sUserAppStatus, Subdomain

logger = get_logger(__name__)

Expand Down Expand Up @@ -202,6 +202,7 @@ def _handle_update_status_request_old(

raise Exception("This method has been deprecated. To be removed.")

"""
if len(new_status) > 15:
new_status = new_status[:15]
Expand Down Expand Up @@ -266,6 +267,7 @@ def _handle_update_status_request_old(
except Exception as err:
logger.error(f"Unable to fetch or update the specified app instance with release={release}. {err}, {type(err)}")
raise
"""


@transaction.atomic
Expand Down Expand Up @@ -351,6 +353,7 @@ def get_URI(instance):
return URI


# TODO: Status. Unit test this. mock deploy_resource()
@transaction.atomic
def create_instance_from_form(form, project, app_slug, app_id=None):
"""
Expand Down Expand Up @@ -380,7 +383,11 @@ def create_instance_from_form(form, project, app_slug, app_id=None):

if new_app:
do_deploy = True
user_action = "Creating"
else:
# Update an existing app
user_action = "Changing"

# Only re-deploy existing apps if one of the following fields was changed:
redeployment_fields = [
"subdomain",
Expand Down Expand Up @@ -409,24 +416,32 @@ def create_instance_from_form(form, project, app_slug, app_id=None):
instance = form.save(commit=False)

# Handle status creation or retrieval
status = get_or_create_status(instance, app_id)
# TODO: Status. Check this use.
# We no longer update the app status based on user actions.
# status = get_or_create_status(instance, app_id)

# Retrieve or create the subdomain
subdomain, created = Subdomain.objects.get_or_create(
subdomain=subdomain_name, project=project, is_created_by_user=is_created_by_user
)

if app_id:
if not new_app:
# Previously: if app_id:
handle_subdomain_change(instance, subdomain, subdomain_name)

app_slug = handle_shiny_proxy_case(instance, app_slug, app_id)

app = get_app(app_slug)

setup_instance(instance, subdomain, app, project, status)
# TODO: Status. Check this use:
setup_instance(instance, subdomain, app, project, user_action)
save_instance_and_related_data(instance, form)

if do_deploy:
logger.debug(f"Now deploying resource app with app_id = {app_id}")

# TODO: Status. Also set the last user action to Updating

deploy_resource.delay(instance.serialize())
else:
logger.debug(f"Not re-deploying this app with app_id = {app_id}")
Expand All @@ -440,7 +455,8 @@ def get_subdomain_name(form):


def get_or_create_status(instance, app_id):
return instance.app_status if app_id else AppStatus.objects.create()
raise Exception("Deprecated function.")
# return instance.app_status if app_id else AppStatus.objects.create()


def handle_subdomain_change(instance, subdomain, subdomain_name):
Expand Down Expand Up @@ -474,13 +490,17 @@ def get_app(app_slug):
raise ValueError(f"App with slug {app_slug} not found")


def setup_instance(instance, subdomain, app, project, status, is_created_by_user=False):
# TODO: Status.
def setup_instance(instance, subdomain, app, project, user_action=None, is_created_by_user=False):
instance.subdomain = subdomain
instance.app = app
instance.chart = instance.app.chart
instance.project = project
instance.owner = project.owner
instance.app_status = status
# TODO: Status. Is this unit tested?
instance.latest_user_action = user_action
# TODO: Status. We do not set app status any longer.
# instance.app_status = status


def save_instance_and_related_data(instance, form):
Expand Down
2 changes: 2 additions & 0 deletions apps/models/base/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
from .app_categories import AppCategories

# TODO: Status.
from .app_status import AppStatus
from .app_template import Apps
from .base import AppInstanceManager, BaseAppInstance
Expand Down
100 changes: 95 additions & 5 deletions apps/models/base/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from django.contrib.auth import get_user_model
from django.core import serializers
from django.db import models
from django.db.models import Q
from django.db.models import Case, CharField, F, Q, Value, When

from apps.models.base.app_status import AppStatus
from apps.models.base.app_template import Apps
Expand All @@ -24,15 +24,70 @@
class AppInstanceManager(models.Manager):
model_type = "appinstance"

def with_app_status(self):
"""
Define and add a reusable, chainable annotation app_status.
The atn prefix is used to clarify that this value is computed on the fly.
"""
return self.get_queryset().annotate(
atn_app_status=Case(
When(latest_user_action="Deleting", then=Value("Deleted")),
When(
k8s_user_app_status__status__in=["CrashLoopBackoff", "ErrImagePull", "PostStartHookError"],
then=Value("Error"),
),
When(
latest_user_action__in=["Creating", "Changing"],
k8s_user_app_status__status="NotFound",
then=Value("Error (NotFound)"),
),
When(
latest_user_action__in=["Creating", "Changing"],
k8s_user_app_status__status="Running",
then=Value("Running"),
),
When(
latest_user_action="Creating",
k8s_user_app_status__status__in=["ContainerCreating", "PodInitializing"],
then=Value("Creating"),
),
When(
latest_user_action="Changing",
k8s_user_app_status__status__in=["ContainerCreating", "PodInitializing"],
then=Value("Changing"),
),
default=Value("Unknown"),
output_field=CharField(),
)
)

def _hide_with_app_status(self):
"""Define and add a reusable, chainable annotation app_status."""
# TODO: Call a function from above
# The atn prefix is used to clarify that this value is computed on the fly
return self.get_queryset().annotate(
atn_app_status=BaseAppInstance.convert_to_app_status(F("latest_user_action"), F("k8s_user_app_status"))
)

def get_apps_not_deleted(self):
"""A queryset returning all user apps excluding Deleted apps."""
# TODO: Define a reusable queryset that returns all apps not set to Deleted.
queryset = self.with_app_status().exclude(atn_app_status="Deleted")
print(str(queryset.query))
return self.with_app_status().exclude(atn_app_status="Deleted")

def get_app_instances_of_project_filter(self, user, project, include_deleted=False, deleted_time_delta=None):
q = Q()

if not include_deleted:
if deleted_time_delta is None:
q &= ~Q(app_status__status="Deleted")
# TODO: Status. Replace this
q &= self.get_apps_not_deleted()
# q &= ~Q(app_status__status="Deleted")
else:
time_threshold = datetime.now() - timedelta(minutes=deleted_time_delta)
q &= ~Q(app_status__status="Deleted") | Q(deleted_on__gte=time_threshold)
# q &= get_app_status()="Deleted" | Q(deleted_on__gte=time_threshold)
q &= self.get_apps_not_deleted() | Q(deleted_on__gte=time_threshold)

if hasattr(self.model, "access"):
q &= Q(owner=user) | Q(
Expand Down Expand Up @@ -83,7 +138,8 @@ def user_can_create(self, user, project, app_slug):
return False

num_of_app_instances = self.filter(
~Q(app_status__status="Deleted"),
self.get_apps_not_deleted(),
# ~Q(app_status__status="Deleted"),
app__slug=app_slug,
project=project,
).count()
Expand Down Expand Up @@ -129,9 +185,22 @@ class BaseAppInstance(models.Model):
k8s_user_app_status = models.OneToOneField(
K8sUserAppStatus, on_delete=models.RESTRICT, related_name="%(class)s", null=True
)
# TODO: Break connection to model AppStatus and deprecate AppStatus

# 20241216: Refactoring of app status.
# TODO: Status. Break connection to model AppStatus and deprecate AppStatus
# Also need to migrate the model?

# Old: The model AppStatus and related FK app_status is retained for historical reasons
# Remove or make read-only?
# Check the db, is the field still there?
app_status = models.OneToOneField(AppStatus, on_delete=models.RESTRICT, related_name="%(class)s", null=True)

# TODO: Status. Create function to get app_status
def get_app_status(self):
# This model function replaces the old app_status field and related AppStatus model.
# TODO: Implement
raise Exception("Not yet implemented")

url = models.URLField(blank=True, null=True)
updated_on = models.DateTimeField(auto_now=True)

Expand Down Expand Up @@ -171,3 +240,24 @@ def set_k8s_values(self):

def serialize(self):
return json.loads(serializers.serialize("json", [self]))[0]

@staticmethod
def convert_to_app_status(latest_user_action: str, k8s_user_app_status: str) -> str:
"""Converts latest user action and k8s pod status to app status"""
match latest_user_action, k8s_user_app_status:
case "Deleting", _:
return "Deleted"
case "Creating", "ContainerCreating" | "PodInitializing":
return "Creating"
case "Changing", "ContainerCreating" | "PodInitializing":
return "Changing"
case _, "NotFound":
return "Error (NotFound)"
case _, "CrashLoopBackoff" | "ErrImagePull" | "PostStartHookError":
return "Error"
case _, "Running":
return "Running"
case _, _:
return f"Invalid input. No match for input {latest_user_action}, {k8s_user_app_status}"

raise ValueError("Invalid input")
16 changes: 12 additions & 4 deletions apps/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,10 +248,11 @@ def deploy_resource(serialized_instance):

helm_info = {"success": success, "info": {"stdout": output, "stderr": error}}

# TODO: Status. Here we no longer save an app_status property.
instance.info = dict(helm=helm_info)
instance.app_status.status = "Created" if success else "Failed"
# instance.app_status.status = "Created" if success else "Failed"

instance.app_status.save()
# instance.app_status.save()
instance.save()

# In development, also generate and validate the k8s deployment manifest
Expand All @@ -276,18 +277,25 @@ def delete_resource(serialized_instance):
output, error = helm_delete(values["subdomain"], values["namespace"])
success = not error

# TODO: Status. Save this to K8sUserAppStatus instead of app_status.
if success:
# TODO: Status. User actions are now saved by views:
"""
if instance.app.slug in ("volumeK8s", "netpolicy"):
instance.app_status.status = "Deleted"
else:
instance.app_status.status = "Deleting..."
"""
logger.info(f"Successfully deleted resource type {instance.app.slug}, {values['subdomain']}")
else:
instance.app_status.status = "FailedToDelete"
instance.k8s_user_app_status.status = "FailedToDelete"
instance.k8s_user_app_status.save()
# instance.app_status.status = "FailedToDelete"

helm_info = {"success": success, "info": {"stdout": output, "stderr": error}}

instance.info = dict(helm=helm_info)
instance.app_status.save()
# instance.app_status.save()
instance.save()


Expand Down
56 changes: 56 additions & 0 deletions apps/tests/test_app_helper_utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
"""This module is used to test the helper functions that are used by user app instance functionality."""

from unittest.mock import patch

from django.contrib.auth import get_user_model
from django.test import TestCase

from projects.models import Project

from ..models import Apps, JupyterInstance, K8sUserAppStatus

User = get_user_model()


class CreateAppInstanceTestCase(TestCase):
"""Test case for testing create_instance_from_form"""

def setUp(self):
self.user = User.objects.create_user("foo1", "[email protected]", "bar")

def test_create_instance_from_form_valid_input(self):
self.assertTrue(1 == 0)

def test_create_instance_from_form_invalid_input(self):
self.assertTrue(1 == 0)


class UpdateExistingAppInstanceTestCase(TestCase):
"""Test case for test create_instance_from_form and an existing user app instance."""

def setUp(self) -> None:
self.user = User.objects.create_user("foo1", "[email protected]", "bar")
self.app = Apps.objects.create(
name="Custom App",
slug="customapp",
)

self.project = Project.objects.create_project(name="test-perm", owner=self.user, description="")

k8s_user_app_status = K8sUserAppStatus.objects.create()
self.app_instance = JupyterInstance.objects.create(
access="public",
owner=self.user,
name="test_app_instance_public",
app=self.app,
project=self.project,
k8s_user_app_status=k8s_user_app_status,
)

def test_create_instance_from_form_valid_input(self):
# TODO: Status. Implement
self.assertTrue(1 == 0)

def test_create_instance_from_form_invalid_input(self):
# TODO: Status. Implement
self.assertTrue(1 == 0)
Loading

0 comments on commit 294a961

Please sign in to comment.