Skip to content

Commit

Permalink
Allow workstation editors to launch debug sessions (#2772)
Browse files Browse the repository at this point in the history
This PR allows workstation editors to set the region and add extra
environment variables for a new session. Limitation is that the editor
cannot have more than 1 session running to prevent flooding of the
server and confusion about env vars being passed, so they may have to
wait for a workstation session to finish.

Workstation detail view:

![image](https://user-images.githubusercontent.com/12661555/218788675-ed5c4ad2-f8d5-4791-a7dd-c810b93df830.png)

The form (with validation):

![image](https://user-images.githubusercontent.com/12661555/218788497-212c1576-71eb-4389-b6a6-9fbd7a7b7ec4.png)

The env vars being passed:

![image](https://user-images.githubusercontent.com/12661555/218788938-ed628d46-0287-493e-b3ae-5781b1928b6d.png)

Note that the env vars editors set here are used as the default
environment variables, we will overwrite some keys if editors try to set
them (e.g. `GRAND_CHALLENGE_UNSAFE`, `GRAND_CHALLENGE_AUTHORIZATION`,
`GRAND_CHALLENGE_API_ROOT`). Editors can only create debug sessions for
themselves so should be safe.

Also removes `WORKSTATIONS_DNS_RESOLVER` which is no longer used.

Old style forms and tests are used for consistency with the rest of the
workstations application.
  • Loading branch information
jmsmkn authored Feb 20, 2023
1 parent 6639453 commit 533a0c5
Show file tree
Hide file tree
Showing 13 changed files with 343 additions and 14 deletions.
3 changes: 0 additions & 3 deletions app/config/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -1062,9 +1062,6 @@
DEFAULT_WORKSTATION_SLUG = os.environ.get(
"DEFAULT_WORKSTATION_SLUG", "cirrus-core"
)
WORKSTATIONS_DNS_RESOLVER = os.environ.get(
"WORKSTATIONS_DNS_RESOLVER", "1.1.1.1"
)
WORKSTATIONS_BASE_IMAGE_QUERY_PARAM = "image"
WORKSTATIONS_OVERLAY_QUERY_PARAM = "overlay"
WORKSTATIONS_READY_STUDY_QUERY_PARAM = "readerStudy"
Expand Down
2 changes: 2 additions & 0 deletions app/grandchallenge/workstations/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class SessionHistoryAdmin(SimpleHistoryAdmin):
"creator",
"region",
"ping_times",
"extra_env_vars",
]
list_filter = ["status", "region", "workstation_image__workstation__slug"]
readonly_fields = [
Expand All @@ -47,6 +48,7 @@ class SessionHistoryAdmin(SimpleHistoryAdmin):
"region",
"ping_times",
"auth_token",
"extra_env_vars",
]
search_fields = [
"logs",
Expand Down
46 changes: 46 additions & 0 deletions app/grandchallenge/workstations/forms.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
from crispy_forms.helper import FormHelper
from django.conf import settings
from django.core.exceptions import ValidationError
from django.forms import ChoiceField, HiddenInput, ModelChoiceField, ModelForm

from grandchallenge.components.forms import ContainerImageForm
from grandchallenge.core.forms import SaveFormInitMixin
from grandchallenge.core.widgets import JSONEditorWidget
from grandchallenge.workstations.models import (
ENV_VARS_SCHEMA,
Session,
Workstation,
WorkstationImage,
Expand Down Expand Up @@ -60,3 +63,46 @@ def __init__(self, *args, **kwargs):
class Meta:
model = Session
fields = ("region", "ping_times")


class DebugSessionForm(SaveFormInitMixin, ModelForm):
region = ChoiceField(
required=True,
choices=[
c
for c in Session.Region.choices
if c[0] in settings.WORKSTATIONS_ACTIVE_REGIONS
],
)

def __init__(self, *args, user, workstation, **kwargs):
super().__init__(*args, **kwargs)
self.__user = user
self.__workstation = workstation
self.fields["extra_env_vars"].initial = [
{"name": "LOG_LEVEL", "value": "DEBUG"},
{"name": "CIRRUS_PROFILING_ENABLED", "value": "True"},
]

def clean(self):
cleaned_data = super().clean()

if Session.objects.filter(
creator=self.__user,
workstation_image__workstation=self.__workstation,
status__in=[Session.QUEUED, Session.STARTED, Session.RUNNING],
region=cleaned_data["region"],
).exists():
raise ValidationError(
"You already have a running workstation in the selected "
"region, please wait for that session to finish"
)

return cleaned_data

class Meta:
model = Session
fields = ("region", "extra_env_vars")
widgets = {
"extra_env_vars": JSONEditorWidget(schema=ENV_VARS_SCHEMA),
}
110 changes: 110 additions & 0 deletions app/grandchallenge/workstations/migrations/0016_auto_20230214_1453.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
# Generated by Django 3.2.18 on 2023-02-14 14:53

from django.db import migrations, models

import grandchallenge.core.validators


class Migration(migrations.Migration):

dependencies = [
(
"workstations",
"0015_feedbackgroupobjectpermission_feedbackuserobjectpermission_sessiongroupobjectpermission_sessionusero",
),
]

operations = [
migrations.AddField(
model_name="historicalsession",
name="extra_env_vars",
field=models.JSONField(
blank=True,
default=list,
help_text="Extra environment variables to include in this session",
validators=[
grandchallenge.core.validators.JSONValidator(
schema={
"$schema": "http://json-schema.org/draft-06/schema",
"description": "Defines environment variable names and values",
"items": {
"$id": "#/items",
"additionalProperties": False,
"description": "Defines an environment variable",
"properties": {
"name": {
"$id": "#/items/properties/name",
"default": "ENV_VAR",
"description": "The name of this environment variable",
"examples": ["ENV_VAR"],
"pattern": "^[A-Z0-9\\_]+$",
"title": "The Name Schema",
"type": "string",
},
"value": {
"$id": "#/items/properties/value",
"default": "env_var_value",
"description": "The value of this environment variable",
"examples": ["env_var_value"],
"title": "The Value Schema",
"type": "string",
},
},
"required": ["name", "value"],
"title": "The Environment Variable Schema",
"type": "object",
},
"title": "The Environment Variables Schema",
"type": "array",
}
)
],
),
),
migrations.AddField(
model_name="session",
name="extra_env_vars",
field=models.JSONField(
blank=True,
default=list,
help_text="Extra environment variables to include in this session",
validators=[
grandchallenge.core.validators.JSONValidator(
schema={
"$schema": "http://json-schema.org/draft-06/schema",
"description": "Defines environment variable names and values",
"items": {
"$id": "#/items",
"additionalProperties": False,
"description": "Defines an environment variable",
"properties": {
"name": {
"$id": "#/items/properties/name",
"default": "ENV_VAR",
"description": "The name of this environment variable",
"examples": ["ENV_VAR"],
"pattern": "^[A-Z0-9\\_]+$",
"title": "The Name Schema",
"type": "string",
},
"value": {
"$id": "#/items/properties/value",
"default": "env_var_value",
"description": "The value of this environment variable",
"examples": ["env_var_value"],
"title": "The Value Schema",
"type": "string",
},
},
"required": ["name", "value"],
"title": "The Environment Variable Schema",
"type": "object",
},
"title": "The Environment Variables Schema",
"type": "array",
}
)
],
),
),
]
59 changes: 52 additions & 7 deletions app/grandchallenge/workstations/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
protected_s3_storage,
public_s3_storage,
)
from grandchallenge.core.validators import JSONValidator
from grandchallenge.subdomains.utils import reverse
from grandchallenge.workstations.emails import send_new_feedback_email_to_staff

Expand Down Expand Up @@ -271,6 +272,41 @@ class WorkstationImageGroupObjectPermission(GroupObjectPermissionBase):
)


ENV_VARS_SCHEMA = {
"$schema": "http://json-schema.org/draft-06/schema",
"type": "array",
"title": "The Environment Variables Schema",
"description": "Defines environment variable names and values",
"items": {
"$id": "#/items",
"type": "object",
"title": "The Environment Variable Schema",
"description": "Defines an environment variable",
"required": ["name", "value"],
"additionalProperties": False,
"properties": {
"name": {
"$id": "#/items/properties/name",
"type": "string",
"title": "The Name Schema",
"description": "The name of this environment variable",
"default": "ENV_VAR",
"pattern": r"^[A-Z0-9\_]+$",
"examples": ["ENV_VAR"],
},
"value": {
"$id": "#/items/properties/value",
"type": "string",
"title": "The Value Schema",
"description": "The value of this environment variable",
"default": "env_var_value",
"examples": ["env_var_value"],
},
},
},
}


class Session(UUIDModel):
"""
Tracks who has launched workstation images. The ``WorkstationImage`` will
Expand Down Expand Up @@ -365,6 +401,12 @@ class Region(models.TextChoices):
history = HistoricalRecords(
excluded_fields=["logs", "ping_times", "auth_token"]
)
extra_env_vars = models.JSONField(
default=list,
blank=True,
help_text="Extra environment variables to include in this session",
validators=[JSONValidator(schema=ENV_VARS_SCHEMA)],
)

class Meta(UUIDModel.Meta):
ordering = ("created", "creator")
Expand Down Expand Up @@ -412,13 +454,16 @@ def environment(self) -> dict:
-------
The environment variables that should be set on the container.
"""
env = {
"NGINX_RESOLVER": settings.WORKSTATIONS_DNS_RESOLVER,
"GRAND_CHALLENGE_API_ROOT": unquote(reverse("api:api-root")),
"WORKSTATION_SENTRY_DSN": settings.WORKSTATION_SENTRY_DSN,
"WORKSTATION_SESSION_ID": str(self.pk),
"CIRRUS_KEEP_ALIVE_METHOD": "old",
}
env = {var["name"]: var["value"] for var in self.extra_env_vars}

env.update(
{
"GRAND_CHALLENGE_API_ROOT": unquote(reverse("api:api-root")),
"WORKSTATION_SENTRY_DSN": settings.WORKSTATION_SENTRY_DSN,
"WORKSTATION_SESSION_ID": str(self.pk),
"CIRRUS_KEEP_ALIVE_METHOD": "old",
}
)

if self.creator:
if self.auth_token:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
{% extends "base.html" %}
{% load crispy_forms_tags %}
{% load static %}

{% block title %}Create Debug Session - {{ block.super }}{% endblock %}

{% block breadcrumbs %}
<ol class="breadcrumb">
<li class="breadcrumb-item"><a href="{% url 'workstations:list' %}">Viewers</a>
</li>
<li class="breadcrumb-item"><a
href="{{ object.workstation.get_absolute_url }}">{{ object.workstation.title }}</a>
</li>
<li class="breadcrumb-item"><a
href="{{ object.get_absolute_url }}">{{ object }}</a>
</li>
<li class="breadcrumb-item active"
aria-current="page">Create Debug Session
</li>
</ol>
{% endblock %}

{% block content %}
<h2>Create Debug Session</h2>

{% if unsupported_browser_message %}
<div class="alert alert-danger">{{ unsupported_browser_message }}</div>
{% endif %}

{% crispy form %}
{% endblock %}
Original file line number Diff line number Diff line change
Expand Up @@ -154,5 +154,9 @@ <h3>Sessions</h3>
<i class="fas fa-flask"></i> Start a new session
</a>

<a class="btn btn-warning" href="{% url 'workstations:workstation-debug-session-create' slug=object.slug %}">
<i class="fas fa-flask"></i> Start a new debug session
</a>

{% endif %}
{% endblock %}
7 changes: 6 additions & 1 deletion app/grandchallenge/workstations/urls.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from django.urls import path

from grandchallenge.workstations.views import (
DebugSessionCreate,
SessionCreate,
WorkstationCreate,
WorkstationDetail,
Expand All @@ -18,7 +19,6 @@
urlpatterns = [
path("", WorkstationList.as_view(), name="list"),
path("create/", WorkstationCreate.as_view(), name="create"),
# TODO - add region
path(
"sessions/create/",
SessionCreate.as_view(),
Expand All @@ -29,6 +29,11 @@
SessionCreate.as_view(),
name="workstation-session-create",
),
path(
"<slug>/sessions/debug/create/",
DebugSessionCreate.as_view(),
name="workstation-debug-session-create",
),
path(
"<slug>/editors/update/",
WorkstationEditorsUpdate.as_view(),
Expand Down
7 changes: 5 additions & 2 deletions app/grandchallenge/workstations/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ def get_or_create_active_session(
user,
workstation_image: WorkstationImage,
region: str,
ping_times: str = None,
ping_times=None,
extra_env_vars=None,
) -> Session:
"""
Queries the database to see if there is an active session for this user and
Expand Down Expand Up @@ -40,7 +41,9 @@ def get_or_create_active_session(
creator=user,
workstation_image=workstation_image,
region=region,
ping_times=ping_times,
# Ping times and env vars are only set for new Sessions
ping_times=ping_times or None,
extra_env_vars=extra_env_vars or [],
)

return session
Loading

0 comments on commit 533a0c5

Please sign in to comment.