Refactor App Status (#269)

Signed-off-by: Nikita Churikov <[email protected]>
Co-authored-by: Jana Awada <[email protected]>
Co-authored-by: Mahbub Ul Alam <[email protected]>
Co-authored-by: Nikita Churikov <[email protected]>
Co-authored-by: Hamza <[email protected]>

Author: 5 people

.github/workflows/ci.yaml

@@ -32,7 +32,7 @@ on:
 jobs:
   CI:
     if: github.repository == 'scilifelabdatacentre/serve'
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-22.04
     env:
       working-directory: .
 

.github/workflows/e2e-tests.yaml

@@ -27,7 +27,7 @@ on:
 jobs:
   e2e:
     if: github.repository == 'scilifelabdatacentre/serve'
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-22.04
     env:
       working-directory: .
 

.github/workflows/pre-commit.yaml

@@ -12,7 +12,7 @@ on:
 
 jobs:
   pre_commit:
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-22.04
     steps:
     - name: Checkout
       uses: actions/checkout@v4

.github/workflows/publish.yaml

@@ -34,7 +34,7 @@ jobs:
         ||
         (github.event_name == 'workflow_dispatch')
       )
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-22.04
     strategy:
       fail-fast: false
       matrix:
@@ -100,7 +100,7 @@ jobs:
           file: ${{ matrix.dockerfile }}
 
   on-failure:
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-22.04
     if: ${{ github.event.workflow_run.conclusion == 'failure' }}
     steps:
       - run: echo 'Not publishing. Triggering workflow failed' && exit 1

Dockerfile

@@ -1,4 +1,4 @@
-FROM python:3.12.3-alpine3.19 as builder
+FROM python:3.12.3-alpine3.19 AS builder
 
 LABEL maintainer="[email protected]"
 WORKDIR /app

api/openapi/public_apps_api.py

@@ -6,7 +6,7 @@
 from rest_framework.exceptions import NotFound
 
 from apps.app_registry import APP_REGISTRY
-from apps.models import Apps, AppStatus
+from apps.models import Apps, BaseAppInstance
 from studio.utils import get_logger
 
 logger = get_logger(__name__)
@@ -30,20 +30,38 @@ def list_apps(self, request):
         # TODO: MAKE SURE THAT THIS IS FILTERED BASED ON ACCESS
         for model_class in APP_REGISTRY.iter_orm_models():
             # Loop over all models, and check if they have the access and description field
+            # Note: It is not possible to use BaseAppInstance.objects.get_app_instances_not_deleted here
+            # This is the reason for looping over model_class instead
             if hasattr(model_class, "description") and hasattr(model_class, "access"):
-                queryset = model_class.objects.filter(~Q(app_status__status="Deleted"), access="public").values(
-                    "id", "name", "app_id", "url", "description", "created_on", "updated_on", "app_status"
+                queryset = model_class.objects.filter(access="public").values(
+                    "id",
+                    "name",
+                    "app_id",
+                    "url",
+                    "description",
+                    "created_on",
+                    "updated_on",
+                    "latest_user_action",
+                    "k8s_user_app_status",
                 )
-                # using a dictionary to avoid duplicates for shiny apps
+                # Using a dictionary to avoid duplicates for shiny apps
                 for item in queryset:
-                    list_apps_dict[item["id"]] = item
+                    # Do not include deleted apps
+                    app_status = BaseAppInstance.convert_to_app_status(
+                        item.get("latest_user_action"),
+                        item.get("k8s_user_app_status"),
+                    )
+                    if app_status != "Deleted":
+                        item["app_status"] = app_status
+                        list_apps_dict[item["id"]] = item
 
             # Order the combined list by "created_on"
             list_apps = sorted(list_apps_dict.values(), key=lambda x: x["created_on"], reverse=True)
 
         for app in list_apps:
+            assert app["app_status"] != "Deleted"
+
             app["app_type"] = Apps.objects.get(id=app["app_id"]).name
-            app["app_status"] = AppStatus.objects.get(pk=app["app_status"]).status
 
             # Add the previous url key located at app.table_field.url to support clients using the previous schema
             app["table_field"] = {"url": app["url"]}
@@ -71,8 +89,18 @@ def retrieve(self, request, app_slug=None, pk=None):
 
         try:
             queryset = model_class.objects.all().values(
-                "id", "name", "app_id", "url", "description", "created_on", "updated_on", "access", "app_status"
+                "id",
+                "name",
+                "app_id",
+                "url",
+                "description",
+                "created_on",
+                "updated_on",
+                "access",
+                "latest_user_action",
+                "k8s_user_app_status",
             )
+
             logger.info("Queryset: %s", queryset)
         except FieldError as e:
             message = f"Error in field: {e}"
@@ -84,24 +112,26 @@ def retrieve(self, request, app_slug=None, pk=None):
             logger.error("App instance is not available")
             raise NotFound("App instance is not available")
 
-        app_status_pk = app_instance.get("app_status", None)
-        logger.info("DID WE GET HERE?!")
-        if app_status_pk is None:
-            raise NotFound("App status is not available")
+        app_status = BaseAppInstance.convert_to_app_status(
+            app_instance.get("latest_user_action"),
+            app_instance.get("k8s_user_app_status"),
+        )
 
-        app_status = AppStatus.objects.get(pk=app_status_pk)
-        if app_status.status == "Deleted":
-            logger.error("This app has been deleted")
+        if app_status == "Deleted":
+            logger.info("This app has been deleted")
             raise NotFound("This app has been deleted")
 
+        app_instance["app_status"] = app_status
+
         if app_instance.get("access", False) != "public":
-            logger.error("This app is non-existent or not public")
             raise NotFound("This app is non-existent or not public")
 
-        app_instance["app_status"] = app_status.status
+        app_type_info = Apps.objects.get(id=app_instance["app_id"])
+        app_instance["app_type"] = app_type_info.name
+
+        # Remove misleading app_id from the final output because it only refers to the app type
+        del app_instance["app_id"]
 
-        add_data = Apps.objects.get(id=app_instance["app_id"])
-        app_instance["app_type"] = add_data.name
         data = {"app": app_instance}
         logger.info("API call successful")
         return JsonResponse(data)

api/tests/test_openapi_public_apps.py

@@ -6,7 +6,7 @@
 from rest_framework import status
 from rest_framework.test import APITestCase
 
-from apps.models import Apps, AppStatus, CustomAppInstance, Subdomain
+from apps.models import Apps, CustomAppInstance, K8sUserAppStatus, Subdomain
 from projects.models import Project
 from studio.utils import get_logger
 
@@ -25,12 +25,13 @@ class PublicAppsApiTests(APITestCase):
 
     @classmethod
     def setUpTestData(cls):
+        # Create a public app with status Running
         cls.user = User.objects.create_user(test_user["username"], test_user["email"], test_user["password"])
         cls.project = Project.objects.create_project(name="test-perm", owner=cls.user, description="")
         cls.app = Apps.objects.create(name="Some App", slug="customapp")
 
         subdomain = Subdomain.objects.create(subdomain="test_internal")
-        app_status = AppStatus.objects.create(status="Running")
+        k8s_user_app_status = K8sUserAppStatus.objects.create(status="Running")
         cls.app_instance = CustomAppInstance.objects.create(
             access="public",
             owner=cls.user,
@@ -42,7 +43,7 @@ def setUpTestData(cls):
                 "environment": {"pk": ""},
             },
             subdomain=subdomain,
-            app_status=app_status,
+            k8s_user_app_status=k8s_user_app_status,
         )
 
     def test_public_apps_list(self):
@@ -80,7 +81,6 @@ def test_public_apps_single_app(self):
         self.assertEqual(actual["id"], self.app_instance.id)
         self.assertIsNotNone(actual["name"])
         self.assertEqual(actual["name"], self.app_instance.name)
-        self.assertTrue(actual["app_id"] > 0)
         self.assertEqual(actual["description"], self.app_instance.description)
         updated_on = datetime.fromisoformat(actual["updated_on"][:-1])
         self.assertEqual(datetime.date(updated_on), datetime.date(self.app_instance.updated_on))

api/urls.py

@@ -54,6 +54,7 @@
     # Internal API endpoints
     path("token-auth/", CustomAuthToken.as_view(), name="api_token_auth"),
     path("settings/", get_studio_settings),
+    # TODO: Consider renaming this endpoint to update-k8s-app-status
     path("app-status/", update_app_status),
     path("app-subdomain/validate/", get_subdomain_is_valid),
     path("app-subdomain/is-available/", get_subdomain_is_available),

api/views.py

@@ -519,6 +519,9 @@ def create(self, request, *args, **kwargs):
         return HttpResponse("App created.", status=200)
 
     def destroy(self, request, *args, **kwargs):
+        # TODO: Revisit
+        raise RuntimeError("api/AppInstanceList destroy. Is this function used?")
+
         appinstance = self.get_object()
         # Check that user is allowed to delete app:
         # Either user owns the app, or is a member of the project
@@ -892,9 +895,9 @@ def update_app_status(request: HttpRequest) -> HttpResponse:
 
             new_status = request.data["new-status"]
 
-            if len(new_status) > 15:
-                logger.debug(f"Status code is longer than 15 chars so shortening: {new_status}")
-                new_status = new_status[:15]
+            if len(new_status) > 20:
+                logger.debug(f"Status code is longer than 20 chars so shortening: {new_status}")
+                new_status = new_status[:20]
 
             event_ts = datetime.strptime(request.data["event-ts"], "%Y-%m-%dT%H:%M:%S.%fZ")
             event_ts = utc.localize(event_ts)
@@ -947,8 +950,8 @@ def update_app_status(request: HttpRequest) -> HttpResponse:
                 return Response(f"Unknown return code from handle_update_status_request() = {result}", 500)
 
         except ObjectDoesNotExist:
-            # This is often not a problem. It typically happens during app re-deployemnts.
-            logger.warning(f"The specified app instance was not found release={release}")
+            # This is often not a problem. It typically happens during app re-deployments.
+            logger.info(f"The specified app instance was not found release={release}")
             return Response(f"The specified app instance was not found {release=}", 404)
 
         except Exception as err:

apps/admin.py

@@ -16,6 +16,7 @@
     FilemanagerInstance,
     GradioInstance,
     JupyterInstance,
+    K8sUserAppStatus,
     MLFlowInstance,
     NetpolicyInstance,
     RStudioInstance,
@@ -54,18 +55,34 @@ class AppsAdmin(admin.ModelAdmin):
 admin.site.register(Apps, AppsAdmin)
 
 
+class K8sUserAppStatusAdmin(admin.ModelAdmin):
+    list_display = (
+        "status",
+        "time",
+    )
+    list_filter = ["status", "time"]
+
+
 class BaseAppAdmin(admin.ModelAdmin):
-    list_display = ("name", "display_owner", "display_project", "display_status", "display_subdomain", "chart")
+    list_display = (
+        "name",
+        "display_owner",
+        "display_project",
+        "display_status",
+        "display_subdomain",
+        "chart",
+        "upload_size",
+    )
     readonly_fields = ("id", "created_on")
-    list_filter = ["owner", "project", "app_status__status", "chart"]
+    list_filter = ["owner", "project", "k8s_user_app_status__status", "chart"]
     actions = ["redeploy_apps", "deploy_resources", "delete_resources"]
 
     def display_status(self, obj):
-        status_object = obj.app_status
-        if status_object:
-            return status_object.status
-        else:
-            "No status"
+        try:
+            return obj.get_app_status()
+        except Exception as err:
+            logger.warn("Error getting app status: %s", err)
+            return "No status"
 
     display_status.short_description = "Status"
 
@@ -263,3 +280,4 @@ class StreamlitInstanceAdmin(BaseAppAdmin):
 admin.site.register(Subdomain)
 admin.site.register(AppCategories)
 admin.site.register(AppStatus, AppStatusAdmin)
+admin.site.register(K8sUserAppStatus, K8sUserAppStatusAdmin)