diff --git a/controlpanel/api/cluster.py b/controlpanel/api/cluster.py index 90b6ec86a..8e6a9cbd8 100644 --- a/controlpanel/api/cluster.py +++ b/controlpanel/api/cluster.py @@ -991,6 +991,8 @@ def _set_values(self, **kwargs): values.update(self.tool.values) values.update(kwargs) + # override the tool image tag with the value stored in the DB + values.update({self.tool.image_tag_key: self.tool.image_tag}) set_values = [] for key, val in values.items(): if val: @@ -1067,6 +1069,22 @@ def get_deployments(cls, user, id_token, search_name=None, search_version=None): deployments.append(deployment) return deployments + @classmethod + def get_chart_details(cls, chart: str) -> tuple[str, str]: + """ + This is a bit of a hack to safely extract the chart version when it includes an 'rc' tag. + This wont be necessary anymore when we track deployed tools in the database. + See https://github.com/ministryofjustice/analytical-platform/issues/6266 + """ + chart_name, chart_version = chart.rsplit("-", 1) + if "rc" not in chart_version: + return chart_name, chart_version + + rc_tag = chart_version + chart_name, chart_version = chart_name.rsplit("-", 1) + chart_version = f"{chart_version}-{rc_tag}" + return chart_name, chart_version + def get_deployment(self, id_token): deployments = self.__class__.get_deployments( self.user, diff --git a/controlpanel/api/helm.py b/controlpanel/api/helm.py index e6b704961..e91512a0f 100644 --- a/controlpanel/api/helm.py +++ b/controlpanel/api/helm.py @@ -158,6 +158,7 @@ def update_helm_repository(force=False): _execute("repo", "update", timeout=None) # timeout = infinity. +# TODO no longer used, remove def get_default_image_tag_from_helm_chart(chart_url, chart_name): proc = _execute("show", "values", chart_url) if proc: @@ -171,6 +172,8 @@ def get_default_image_tag_from_helm_chart(chart_url, chart_name): return None +# TODO this is no longer called from the Your Tools page +# consider removing as part of further refactoring def get_helm_entries(): # Update repository metadata. update_helm_repository() diff --git a/controlpanel/api/migrations/0050_tool_is_deprecated.py b/controlpanel/api/migrations/0050_tool_is_deprecated.py new file mode 100644 index 000000000..d87c3bf1f --- /dev/null +++ b/controlpanel/api/migrations/0050_tool_is_deprecated.py @@ -0,0 +1,31 @@ +# Generated by Django 5.1.2 on 2024-11-29 16:25 + +# Third-party +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("api", "0049_alter_feedback_suggestions"), + ] + + operations = [ + migrations.AddField( + model_name="tool", + name="is_deprecated", + field=models.BooleanField(default=False), + ), + migrations.AddField( + model_name="tool", + name="deprecated_message", + field=models.TextField( + blank=True, help_text="If no message is provided, a default message will be used." + ), + ), + migrations.AddField( + model_name="tool", + name="is_retired", + field=models.BooleanField(default=False), + ), + ] diff --git a/controlpanel/api/migrations/0051_tool_image_tag.py b/controlpanel/api/migrations/0051_tool_image_tag.py new file mode 100644 index 000000000..6be08f309 --- /dev/null +++ b/controlpanel/api/migrations/0051_tool_image_tag.py @@ -0,0 +1,18 @@ +# Generated by Django 5.1.2 on 2024-12-10 09:01 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("api", "0050_tool_is_deprecated"), + ] + + operations = [ + migrations.AddField( + model_name="tool", + name="image_tag", + field=models.CharField(blank=True, max_length=100, null=True), + ), + ] diff --git a/controlpanel/api/migrations/0052_add_image_tag_value.py b/controlpanel/api/migrations/0052_add_image_tag_value.py new file mode 100644 index 000000000..a058e6f83 --- /dev/null +++ b/controlpanel/api/migrations/0052_add_image_tag_value.py @@ -0,0 +1,26 @@ +# Generated by Django 5.1.2 on 2024-12-10 09:01 + +from django.db import migrations + + +def add_image_tag(apps, schema_editor): + Tool = apps.get_model("api", "Tool") + for tool in Tool.objects.all(): + chart_image_key_name = tool.chart_name.split("-")[0] + values = tool.values or {} + image_tag = values.get("{}.tag".format(chart_image_key_name)) or values.get( + "{}.image.tag".format(chart_image_key_name) + ) + tool.image_tag = image_tag + tool.save() + + +class Migration(migrations.Migration): + + dependencies = [ + ("api", "0051_tool_image_tag"), + ] + + operations = [ + migrations.RunPython(code=add_image_tag, reverse_code=migrations.RunPython.noop), + ] diff --git a/controlpanel/api/migrations/0053_alter_tool_image_tag.py b/controlpanel/api/migrations/0053_alter_tool_image_tag.py new file mode 100644 index 000000000..a0903be6d --- /dev/null +++ b/controlpanel/api/migrations/0053_alter_tool_image_tag.py @@ -0,0 +1,19 @@ +# Generated by Django 5.1.2 on 2024-12-10 09:05 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("api", "0052_add_image_tag_value"), + ] + + operations = [ + migrations.AlterField( + model_name="tool", + name="image_tag", + field=models.CharField(default="", max_length=100), + preserve_default=False, + ), + ] diff --git a/controlpanel/api/migrations/0054_alter_tool_description.py b/controlpanel/api/migrations/0054_alter_tool_description.py new file mode 100644 index 000000000..235c92833 --- /dev/null +++ b/controlpanel/api/migrations/0054_alter_tool_description.py @@ -0,0 +1,18 @@ +# Generated by Django 5.1.2 on 2024-12-11 14:44 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("api", "0053_alter_tool_image_tag"), + ] + + operations = [ + migrations.AlterField( + model_name="tool", + name="description", + field=models.TextField(), + ), + ] diff --git a/controlpanel/api/models/tool.py b/controlpanel/api/models/tool.py index d090967ea..6eddfe883 100644 --- a/controlpanel/api/models/tool.py +++ b/controlpanel/api/models/tool.py @@ -27,8 +27,18 @@ class Tool(TimeStampedModel): "rstudio": "rstudio", "vscode": "vscode", } - - description = models.TextField(blank=True) + DEFAULT_DEPRECATED_MESSAGE = "The selected release has been deprecated and will be retired soon. Please update to a more recent version." # noqa + JUPYTER_DATASCIENCE_CHART_NAME = "jupyter-lab-datascience-notebook" + JUPYTER_ALL_SPARK_CHART_NAME = "jupyter-lab-all-spark" + JUPYTER_LAB_CHART_NAME = "jupyter-lab" + RSTUDIO_CHART_NAME = "rstudio" + VSCODE_CHART_NAME = "vscode" + STATUS_RETIRED = "retired" + STATUS_DEPRECATED = "deprecated" + STATUS_ACTIVE = "active" + STATUS_RESTRICTED = "restricted" + + description = models.TextField(blank=False) chart_name = models.CharField(max_length=100, blank=False) name = models.CharField(max_length=100, blank=False) values = JSONField(default=dict) @@ -51,6 +61,13 @@ class Tool(TimeStampedModel): default=None, ) + is_deprecated = models.BooleanField(default=False) + deprecated_message = models.TextField( + blank=True, help_text="If no message is provided, a default message will be used." + ) + is_retired = models.BooleanField(default=False) + image_tag = models.CharField(max_length=100) + class Meta(TimeStampedModel.Meta): db_table = "control_panel_api_tool" ordering = ("name",) @@ -65,6 +82,8 @@ def url(self, user): def save(self, *args, **kwargs): helm.update_helm_repository(force=True) + # TODO description is now required when creating a release, so this is unlikely to be called + # Consider removing if not self.description: self.description = helm.get_chart_app_version(self.chart_name, self.version) or "" @@ -72,12 +91,45 @@ def save(self, *args, **kwargs): return self @property - def image_tag(self): - chart_image_key_name = self.chart_name.split("-")[0] - values = self.values or {} - return values.get("{}.tag".format(chart_image_key_name)) or values.get( - "{}.image.tag".format(chart_image_key_name) - ) + def get_deprecated_message(self): + if not self.is_deprecated: + return "" + + if self.is_retired: + return "" + + return self.deprecated_message or self.DEFAULT_DEPRECATED_MESSAGE + + @property + def image_tag_key(self): + mapping = { + self.JUPYTER_DATASCIENCE_CHART_NAME: "jupyter.tag", + self.JUPYTER_ALL_SPARK_CHART_NAME: "jupyter.tag", + self.JUPYTER_LAB_CHART_NAME: "jupyterlab.image.tag", + self.RSTUDIO_CHART_NAME: "rstudio.image.tag", + self.VSCODE_CHART_NAME: "vscode.image.tag", + } + return mapping[self.chart_name] + + @property + def status(self): + if self.is_retired: + return self.STATUS_RETIRED.capitalize() + if self.is_deprecated: + return self.STATUS_DEPRECATED.capitalize() + if self.is_restricted: + return self.STATUS_RESTRICTED.capitalize() + return self.STATUS_ACTIVE.capitalize() + + @property + def status_colour(self): + mapping = { + self.STATUS_RETIRED: "red", + self.STATUS_DEPRECATED: "grey", + self.STATUS_RESTRICTED: "yellow", + self.STATUS_ACTIVE: "green", + } + return mapping[self.status.lower()] class ToolDeploymentManager: diff --git a/controlpanel/frontend/consumers.py b/controlpanel/frontend/consumers.py index f34c7f256..b51fdbdb8 100644 --- a/controlpanel/frontend/consumers.py +++ b/controlpanel/frontend/consumers.py @@ -196,7 +196,7 @@ def tool_restart(self, message): log.debug(f"Restarted {tool.name} for {user}") def get_tool_and_user(self, message): - tool = Tool.objects.get(pk=message["tool_id"]) + tool = Tool.objects.get(is_retired=False, pk=message["tool_id"]) if not tool: raise Exception(f"no Tool record found for query {message['tool_id']}") user = User.objects.get(auth0_id=message["user_id"]) diff --git a/controlpanel/frontend/filters.py b/controlpanel/frontend/filters.py index 5c2fd0b0a..ed3430256 100644 --- a/controlpanel/frontend/filters.py +++ b/controlpanel/frontend/filters.py @@ -5,23 +5,67 @@ from controlpanel.api.models.tool import Tool -class ReleaseFilter(django_filters.FilterSet): +class InitialFilterSetMixin(django_filters.FilterSet): + + def __init__(self, data=None, queryset=None, *, request=None, prefix=None): + # if filterset is bound, use initial values as defaults + if data is not None: + # get a mutable copy of the QueryDict + data = data.copy() + + for name, f in self.base_filters.items(): + initial = f.extra.get("initial") + + # filter param is either missing or empty, use initial as default + if not data.get(name) and initial: + data[name] = initial + + super().__init__(data, queryset, request=request, prefix=prefix) + + +class ReleaseFilter(InitialFilterSetMixin): + YES_NO_CHOICES = [("all", "---------"), ("true", "Yes"), ("false", "No")] chart_name = django_filters.ChoiceFilter() - is_restricted = django_filters.BooleanFilter(label="Restricted release?") + # is_restricted = django_filters.BooleanFilter(label="Restricted release?") + status = django_filters.ChoiceFilter( + choices=[ + (Tool.STATUS_ACTIVE, Tool.STATUS_ACTIVE.capitalize()), + (Tool.STATUS_RESTRICTED, Tool.STATUS_RESTRICTED.capitalize()), + (Tool.STATUS_DEPRECATED, Tool.STATUS_DEPRECATED.capitalize()), + (Tool.STATUS_RETIRED, Tool.STATUS_RETIRED.capitalize()), + ("all", "All"), + ], + method="filter_status", + label="Status", + empty_label=None, + initial="active", + ) class Meta: model = Tool - fields = ["chart_name", "is_restricted"] + fields = [ + "chart_name", + ] - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) + def __init__(self, data=None, *args, **kwargs): + super().__init__(data, *args, **kwargs) self.filters["chart_name"].extra["choices"] = ( Tool.objects.values_list("chart_name", "chart_name").order_by().distinct() ) self.filters["chart_name"].field.widget.attrs = {"class": "govuk-select"} - self.filters["is_restricted"].field.widget.choices = [ - ("all", "---------"), - ("true", "Yes"), - ("false", "No"), - ] - self.filters["is_restricted"].field.widget.attrs = {"class": "govuk-select"} + self.filters["status"].field.widget.attrs = {"class": "govuk-select"} + + def filter_status(self, queryset, name, value): + if value == "all": + return queryset + if value == "retired": + return queryset.filter(is_retired=True) + # remove retired tools from the list + queryset = queryset.filter(is_retired=False) + if value == "active": + return queryset.filter(is_restricted=False, is_deprecated=False) + return queryset.filter( + **{ + f"is_{value}": True, + } + ) diff --git a/controlpanel/frontend/forms.py b/controlpanel/frontend/forms.py index 65d2e93b2..301c903f4 100644 --- a/controlpanel/frontend/forms.py +++ b/controlpanel/frontend/forms.py @@ -540,10 +540,14 @@ class Meta: "name", "chart_name", "version", + "image_tag", "values", "is_restricted", "tool_domain", "description", + "is_deprecated", + "deprecated_message", + "is_retired", ] diff --git a/controlpanel/frontend/jinja2/release-create.html b/controlpanel/frontend/jinja2/release-create.html index b63ed43e3..88ff38fc9 100644 --- a/controlpanel/frontend/jinja2/release-create.html +++ b/controlpanel/frontend/jinja2/release-create.html @@ -29,7 +29,7 @@

{{ page_title }}

}, "classes": "govuk-!-width-two-thirds", "hint": { - "text": 'Human readable name. 60 chars max' + "text": 'Human readable name. Only used in the Tool Releases admin page, not to standard users.' }, "name": "name", "attributes": { @@ -62,7 +62,7 @@

{{ page_title }}

}, "classes": "govuk-!-width-two-thirds", "hint": { - "text": 'The description for this version, if you leave it blank, the info from helm repo will be used instead' + "text": 'A brief description of this version. This is displayed to users in the dropdown selections.' }, "name": "description", "attributes": { @@ -88,6 +88,23 @@

{{ page_title }}

"value": form.version.value(), "errorMessage": { "html": form.version.errors|join(". ") } if form.version.errors else {} }) }} + {{ govukInput({ + "label": { + "text": "Image Tag", + "classes": "govuk-label--m", + }, + "classes": "govuk-!-width-two-thirds", + "hint": { + "text": 'Docker image tag for this release. It will be passed to helm as a value when the tool is deployed.' + }, + "name": "image_tag", + "attributes": { + "pattern": "[a-z0-9.-]{1,60}", + "maxlength": "60", + }, + "value": form.image_tag.value(), + "errorMessage": { "html": form.image_tag.errors|join(". ") } if form.image_tag.errors else {} + }) }} {{ govukInput({ "label": { "text": "Custom Domain Name", diff --git a/controlpanel/frontend/jinja2/release-detail.html b/controlpanel/frontend/jinja2/release-detail.html index 6bd054634..62b960369 100644 --- a/controlpanel/frontend/jinja2/release-detail.html +++ b/controlpanel/frontend/jinja2/release-detail.html @@ -65,7 +65,7 @@

{{ page_title }}

}, "classes": "govuk-!-width-two-thirds", "hint": { - "text": 'The description about this version, if leave blank, the info from helm repo will be pulled out' + "text": 'A brief description of this version. This is displayed to users in the dropdown selections.' }, "name": "description", "attributes": { @@ -91,6 +91,23 @@

{{ page_title }}

"value": form.version.value(), "errorMessage": { "html": form.version.errors|join(". ") } if form.version.errors else {} }) }} + {{ govukInput({ + "label": { + "text": "Image Tag", + "classes": "govuk-label--m", + }, + "classes": "govuk-!-width-two-thirds", + "hint": { + "text": 'Image tag for this release.' + }, + "name": "image_tag", + "attributes": { + "pattern": "[a-z0-9.-]{1,60}", + "maxlength": "60", + }, + "value": form.image_tag.value(), + "errorMessage": { "html": form.image_tag.errors|join(". ") } if form.image_tag.errors else {} + }) }} {{ govukInput({ "label": { "text": "Custom Domain Name", @@ -143,6 +160,64 @@

{{ page_title }}

"value": target_users if target_users else "", "errorMessage": { "html": form.target_users_list.errors|join(". ") } if form.target_users_list.errors else {} }) }} + + {{ govukCheckboxes({ + "fieldset": { + "legend": { + "text": "Deprecate release", + "classes": "govuk-fieldset__legend--m", + }, + }, + "classes": "govuk-!-width-two-thirds", + "hint": { + "text": 'Checking this will display a deprecation message to users when they select this release' + }, + "name": "is_deprecated", + "items": [ + { + "value": "True", + "text": "Release is deprecated", + "checked": form.is_deprecated.value(), + }, + ], + "errorMessage": { "html": form.is_deprecated.errors|join(". ") } if form.is_deprecated.errors else {} + }) }} + {{ govukInput({ + "label": { + "text": "Deprecation message", + "classes": "govuk-label--s", + }, + "classes": "govuk-!-width-two-thirds", + "hint": { + "text": form.deprecated_message.help_text + }, + "name": "deprecated_message", + "value": form.deprecated_message.value(), + "errorMessage": { "html": form.deprecated_message.errors|join(". ") } if form.deprecated_message.errors else {} + }) }} + + {{ govukCheckboxes({ + "fieldset": { + "legend": { + "text": "Retire release", + "classes": "govuk-fieldset__legend--m", + }, + }, + "classes": "govuk-!-width-two-thirds", + "hint": { + "text": 'Checking this will remove this release from all users dropdown options on the Your Tools page.' + }, + "name": "is_retired", + "items": [ + { + "value": "True", + "text": "Release is retired", + "checked": form.is_retired.value(), + }, + ], + "errorMessage": { "html": form.is_retired.errors|join(". ") } if form.is_retired.errors else {} + }) }} + {% endcall %} diff --git a/controlpanel/frontend/jinja2/release-list.html b/controlpanel/frontend/jinja2/release-list.html index 11ebb9b47..ef5742cef 100644 --- a/controlpanel/frontend/jinja2/release-list.html +++ b/controlpanel/frontend/jinja2/release-list.html @@ -45,7 +45,7 @@

Filter

Image tag Description Created - Restricted Release? + Status Actions @@ -72,7 +72,9 @@

Filter

{{ release.created.strftime('%d-%m-%Y') }} - {{ release.is_restricted }} + + {{ release.status }} + {{ tool_info.name }} id="tools-{{ chart_name }}" name="version"> {% set installed_chart_version = None %} {% set installed_release_version = None %} - {% if deployment and deployment.tool_id > -1 %} + {% if deployment and not deployment.is_retired %} {% set installed_chart_version = deployment.description %} {% set installed_release_version = deployment.tool_id %} - {% else %} {% endif %} {% for release_version, release_detail in tool_info["releases"].items(): %} {% if release_version != installed_release_version: %} - {% endif %} @@ -61,7 +67,7 @@

{{ tool_info.name }}

Status: - {% if deployment %} + {% if deployment and not deployment.is_retired %} {{ deployment.status | default("") }} {% else %} Not deployed @@ -86,7 +92,8 @@

{{ tool_info.name }}

onclick="window.open('{{ tool_info['url'] }}', '_blank');" rel="noopener" target="_blank" - {% if not deployment %} disabled {% endif %}> + id="open-{{ chart_name }}" + {% if not deployment or deployment.is_retired %} disabled {% endif %}> Open @@ -96,24 +103,29 @@

{{ tool_info.name }}

{% endif %} data-action-name="restart" method="post" - style="display: inline;"> + style="display: inline;" + > {{ csrf_input }}
-{% if deployment and deployment.tool_id == -1 %} -
-

- Your current deployment ({{ deployment.chart_name}}-{{ deployment.chart_version }}: {{ deployment.image_tag }}) - is not recognised as a current maintained tool release. You can still use it, - but it is recommended to switch to a new version from the dropdown list. -

+
Warning{{ deployment.deprecated_message }}
+ +{% if deployment and deployment.is_retired %} + +
+ + Warning + Your previous deployment ({{ deployment.chart_name}}-{{ deployment.chart_version }}: {{ deployment.image_tag }}) + has been retired. You will need to deploy a new version from the dropdown list. +
{% endif %}
diff --git a/controlpanel/frontend/static/javascripts/modules/tool-status.js b/controlpanel/frontend/static/javascripts/modules/tool-status.js index 2121c4b35..3f159db24 100644 --- a/controlpanel/frontend/static/javascripts/modules/tool-status.js +++ b/controlpanel/frontend/static/javascripts/modules/tool-status.js @@ -152,9 +152,41 @@ moj.Modules.toolStatus = { const targetTool = target.attributes["data-action-target"]; const deployButton = document.getElementById("deploy-" + targetTool.value); + const openButton = document.getElementById("open-" + targetTool.value); + const restartButton = document.getElementById("restart-" + targetTool.value); // If "(not installed)" or "(installed)" version selected // the "Deploy" button needs to be disabled deployButton.disabled = notInstalledSelected || installedSelected; + openButton.disabled = !installedSelected; + restartButton.disabled = !installedSelected; + + this.toggleDeprecationMessage(selected, targetTool); }, + + toggleDeprecationMessage(selected, targetTool) { + const isDeprecated = selected.attributes["data-is-deprecated"]; + const deprecationMessageElement = document.getElementById(targetTool.value + "-deprecation-message"); + if (isDeprecated === undefined) { + this.hideDeprecationMessage(deprecationMessageElement); + return; + } + const deprecationMessage = selected.attributes["data-deprecated-message"].value; + + if (isDeprecated.value === "True") { + this.showDeprecationMessage(deprecationMessageElement, deprecationMessage); + } else { + this.hideDeprecationMessage(deprecationMessageElement); + } + }, + + showDeprecationMessage(element, message) { + element.classList.remove(this.hidden); + element.lastChild.innerText = message; + }, + + hideDeprecationMessage(element) { + element.classList.add(this.hidden); + element.lastChild.innerText = ""; + } }; diff --git a/controlpanel/frontend/views/tool.py b/controlpanel/frontend/views/tool.py index 18f1dd85e..d0add9673 100644 --- a/controlpanel/frontend/views/tool.py +++ b/controlpanel/frontend/views/tool.py @@ -13,11 +13,6 @@ # First-party/Local from controlpanel.api import cluster -from controlpanel.api.helm import ( - get_chart_version_info, - get_default_image_tag_from_helm_chart, - get_helm_entries, -) from controlpanel.api.models import Tool, ToolDeployment from controlpanel.oidc import OIDCLoginRequiredMixin from controlpanel.utils import start_background_task @@ -45,7 +40,9 @@ def get_queryset(self): * The current user is in the beta tester group for the tool. """ - return Tool.objects.filter(Q(is_restricted=False) | Q(target_users=self.request.user.id)) + return Tool.objects.filter( + Q(is_restricted=False) | Q(target_users=self.request.user.id) + ).exclude(is_retired=True) def _locate_tool_box_by_chart_name(self, chart_name): tool_box = None @@ -67,13 +64,15 @@ def _find_related_tool_record(self, chart_name, chart_version, image_tag): memory, CPU etc, then the linkage will be confused although it won't affect people usage. """ - tool_set = Tool.objects.filter(chart_name=chart_name, version=chart_version) - for item in tool_set: - if item.image_tag == image_tag: - return item - return tool_set.first() + tools = self.get_queryset().filter(chart_name=chart_name, version=chart_version) + for tool in tools: + if tool.image_tag == image_tag: + return tool + # If we cant find a tool with the same image tag, this must mean that it was retired or + # deleted. So return none, and let the calling function handle it + return None - def _add_new_item_to_tool_box(self, user, tool_box, tool, tools_info, charts_info): + def _add_new_item_to_tool_box(self, user, tool_box, tool, tools_info): if tool_box not in tools_info: tools_info[tool_box] = { "name": tool.name, @@ -81,16 +80,15 @@ def _add_new_item_to_tool_box(self, user, tool_box, tool, tools_info, charts_inf "deployment": None, "releases": {}, } - image_tag = tool.image_tag - if not image_tag: - image_tag = charts_info.get(tool.version, {}) or "unknown" if tool.id not in tools_info[tool_box]["releases"]: tools_info[tool_box]["releases"][tool.id] = { "tool_id": tool.id, "chart_name": tool.chart_name, "description": tool.description, "chart_version": tool.version, - "image_tag": image_tag, + "image_tag": tool.image_tag, + "is_deprecated": tool.is_deprecated, + "deprecated_message": tool.get_deprecated_message, } def _get_tool_deployed_image_tag(self, containers): @@ -99,11 +97,16 @@ def _get_tool_deployed_image_tag(self, containers): return container.image.split(":")[1] return None - def _add_deployed_charts_info(self, tools_info, user, id_token, charts_info): + def _add_deployed_charts_info(self, tools_info, user, id_token): # Get list of deployed tools + # TODO this sets what tool the user currently has deployed. If we were to refactor to store + # deployed tools in the database, we could remove a lot of this logic + # See https://github.com/ministryofjustice/analytical-platform/issues/6266 deployments = cluster.ToolDeployment.get_deployments(user, id_token) for deployment in deployments: - chart_name, chart_version = deployment.metadata.labels["chart"].rsplit("-", 1) + chart_name, chart_version = cluster.ToolDeployment.get_chart_details( + deployment.metadata.labels["chart"] + ) image_tag = self._get_tool_deployed_image_tag(deployment.spec.template.spec.containers) tool_box = self._locate_tool_box_by_chart_name(chart_name) tool_box = tool_box or "Unknown" @@ -115,7 +118,7 @@ def _add_deployed_charts_info(self, tools_info, user, id_token, charts_info): ) ) else: - self._add_new_item_to_tool_box(user, tool_box, tool, tools_info, charts_info) + self._add_new_item_to_tool_box(user, tool_box, tool, tools_info) if tool_box not in tools_info: # up to this stage, if the tool_box is still empty, it means # there is no tool release available in db @@ -127,42 +130,23 @@ def _add_deployed_charts_info(self, tools_info, user, id_token, charts_info): "image_tag": image_tag, "description": tool.description if tool else "Not available", "status": ToolDeployment(tool, user).get_status(id_token, deployment=deployment), + "is_deprecated": tool.is_deprecated if tool else False, + "deprecated_message": tool.get_deprecated_message if tool else "", + "is_retired": tool is None, } - def _retrieve_detail_tool_info(self, user, tools, charts_info): + def _retrieve_detail_tool_info(self, user, tools): + # TODO when deployed tools are tracked in the DB this will not be needed + # see https://github.com/ministryofjustice/analytical-platform/issues/6266 # noqa: E501 tools_info = {} for tool in tools: # Work out which bucket the chart should be in tool_box = self._locate_tool_box_by_chart_name(tool.chart_name) # No matching tool bucket for the given chart. So ignore. if tool_box: - self._add_new_item_to_tool_box(user, tool_box, tool, tools_info, charts_info) + self._add_new_item_to_tool_box(user, tool_box, tool, tools_info) return tools_info - def _get_charts_info(self, tool_list): - # We may need the default image_tag from helm chart - # unless we configure it specifically in parameters of tool release - charts_info = {} - chart_entries = None - for tool in tool_list: - if tool.version in charts_info: - continue - - image_tag = tool.image_tag - if image_tag: - continue - - if chart_entries is None: - # load the potential massive helm chart yaml only it is necessary - chart_entries = get_helm_entries() - chart_app_version = get_chart_version_info(chart_entries, tool.chart_name, tool.version) - if chart_app_version: - image_tag = get_default_image_tag_from_helm_chart( - chart_app_version.chart_url, tool.chart_name - ) - charts_info[tool.version] = image_tag - return charts_info - def get_context_data(self, *args, **kwargs): """ Retrieve information about tools and arrange them for the @@ -224,15 +208,13 @@ def get_context_data(self, *args, **kwargs): context["managed_airflow_dev_url"] = f"{settings.AWS_SERVICE_URL}/?{args_airflow_dev_url}" context["managed_airflow_prod_url"] = f"{settings.AWS_SERVICE_URL}/?{args_airflow_prod_url}" - # Arrange tools information - charts_info = self._get_charts_info(context["tools"]) - tools_info = self._retrieve_detail_tool_info(user, context["tools"], charts_info) + tools_info = self._retrieve_detail_tool_info(user, context["tools"]) if "vscode" in tools_info: url = tools_info["vscode"]["url"] tools_info["vscode"]["url"] = f"{url}?folder=/home/analyticalplatform/workspace" - self._add_deployed_charts_info(tools_info, user, id_token, charts_info) + self._add_deployed_charts_info(tools_info, user, id_token) context["tools_info"] = tools_info return context diff --git a/pyproject.toml b/pyproject.toml index 42ef8e96f..e6029b63f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -20,5 +20,6 @@ import_heading_thirdparty = 'Third-party' line_length = 100 multi_line_output = 3 no_lines_before = 'LOCALFOLDER' -skip = "migrations,venv" +skip_glob = "**/migrations**,venv" +filter_files = true virtual_env = "venv" diff --git a/tests/api/cluster/test_tool_deployment.py b/tests/api/cluster/test_tool_deployment.py index fcf4963a0..46854d6da 100644 --- a/tests/api/cluster/test_tool_deployment.py +++ b/tests/api/cluster/test_tool_deployment.py @@ -1,7 +1,9 @@ # Third-party +import pytest from django.conf import settings # First-party/Local +from controlpanel.api import cluster from controlpanel.api.models import Tool, User @@ -24,3 +26,65 @@ def test_url(): # Now the chart_name is custom, the tool_domain (rstudio) is used, ensuring # the url remains "valid". assert tool.url(user) == expected + + +@pytest.mark.parametrize( + "chart_name", + [ + Tool.RSTUDIO_CHART_NAME, + Tool.VSCODE_CHART_NAME, + Tool.JUPYTER_ALL_SPARK_CHART_NAME, + Tool.JUPYTER_DATASCIENCE_CHART_NAME, + Tool.JUPYTER_LAB_CHART_NAME, + ], +) +def test_image_tag_in_set_values(chart_name): + """ + Test to check the image tag set on the tool instance is used in the helm values, not what is + passed in the values dictionary. + """ + tool = Tool( + name="Test Tool", + chart_name=chart_name, + version="1.0.0", + image_tag="0.2", + values={"rstudio.image.tag": "0.1"}, + ) + user = User(username="test-user") + tool_deployment = cluster.ToolDeployment(tool=tool, user=user) + assert f"{tool.image_tag_key}=0.2" in tool_deployment._set_values() + assert f"{tool.image_tag_key}=0.1" not in tool_deployment._set_values() + + +@pytest.mark.parametrize( + "chart, expected", + [ + (f"{Tool.RSTUDIO_CHART_NAME}-1.0.0", (Tool.RSTUDIO_CHART_NAME, "1.0.0")), + (f"{Tool.RSTUDIO_CHART_NAME}-1.0.0-rc1", (Tool.RSTUDIO_CHART_NAME, "1.0.0-rc1")), + ( + f"{Tool.JUPYTER_DATASCIENCE_CHART_NAME}-1.0.0", + (Tool.JUPYTER_DATASCIENCE_CHART_NAME, "1.0.0"), + ), + ( + f"{Tool.JUPYTER_DATASCIENCE_CHART_NAME}-1.0.0-rc1", + (Tool.JUPYTER_DATASCIENCE_CHART_NAME, "1.0.0-rc1"), + ), + (f"{Tool.JUPYTER_LAB_CHART_NAME}-1.0.0", (Tool.JUPYTER_LAB_CHART_NAME, "1.0.0")), + (f"{Tool.JUPYTER_LAB_CHART_NAME}-1.0.0-rc1", (Tool.JUPYTER_LAB_CHART_NAME, "1.0.0-rc1")), + ( + f"{Tool.JUPYTER_ALL_SPARK_CHART_NAME}-1.0.0", + (Tool.JUPYTER_ALL_SPARK_CHART_NAME, "1.0.0"), + ), + ( + f"{Tool.JUPYTER_ALL_SPARK_CHART_NAME}-1.0.0-rc1", + (Tool.JUPYTER_ALL_SPARK_CHART_NAME, "1.0.0-rc1"), + ), + (f"{Tool.VSCODE_CHART_NAME}-1.0.0", (Tool.VSCODE_CHART_NAME, "1.0.0")), + (f"{Tool.VSCODE_CHART_NAME}-1.0.0-rc1", (Tool.VSCODE_CHART_NAME, "1.0.0-rc1")), + ], +) +def test_get_chart_details(chart, expected): + """ + Ensures the chart details are correctly extracted from the chart name. + """ + assert cluster.ToolDeployment.get_chart_details(chart) == expected diff --git a/tests/api/models/test_tool.py b/tests/api/models/test_tool.py index 7e7466f02..ecd38ad9a 100644 --- a/tests/api/models/test_tool.py +++ b/tests/api/models/test_tool.py @@ -12,7 +12,7 @@ @pytest.fixture def tool(db): - return baker.make("api.Tool") + return baker.make("api.Tool", chart_name="rstudio", version="1.0.0", image_tag="0.0.1") def test_deploy_for_generic(helm, tool, users): @@ -46,6 +46,8 @@ def test_deploy_for_generic(helm, tool, users): f"aws.iamRole={user.iam_role_name}", "--set", f"toolsDomain={settings.TOOLS_DOMAIN}", + "--set", + f"rstudio.image.tag={tool.image_tag}", ) @@ -91,3 +93,57 @@ def test_home_directory_get_status(): hd._poll = MagicMock(return_value=cluster.HOME_RESETTING) hd._subprocess = True assert hd.get_status() == cluster.HOME_RESETTING + + +@pytest.mark.parametrize( + "chart_name, expected", + [ + ("jupyter-lab-datascience-notebook", "jupyter.tag"), + ("jupyter-lab-all-spark", "jupyter.tag"), + ("jupyter-lab", "jupyterlab.image.tag"), + ("rstudio", "rstudio.image.tag"), + ("vscode", "vscode.image.tag"), + ], + ids=[ + "jupyter-lab-datascience-notebook", + "jupyter-lab-all-spark", + "jupyter-lab", + "rstudio", + "vscode", + ], +) +def test_image_tag_key(tool, chart_name, expected): + tool.chart_name = chart_name + assert tool.image_tag_key == expected + + +def test_get_deprecated_message(tool): + assert tool.get_deprecated_message == "" + tool.is_deprecated = True + assert tool.get_deprecated_message == tool.DEFAULT_DEPRECATED_MESSAGE + tool.deprecated_message = "This tool is deprecated" + assert tool.get_deprecated_message == "This tool is deprecated" + tool.is_retired = True + assert tool.get_deprecated_message == "" + + +def test_tool_status(): + tool = Tool(is_restricted=False, is_deprecated=False, is_retired=False) + assert tool.status == "Active" + tool.is_restricted = True + assert tool.status == "Restricted" + tool.is_deprecated = True + assert tool.status == "Deprecated" + tool.is_retired = True + assert tool.status == "Retired" + + +def test_status_colour(): + tool = Tool(is_restricted=False, is_deprecated=False, is_retired=False) + assert tool.status_colour == "green" + tool.is_restricted = True + assert tool.status_colour == "yellow" + tool.is_deprecated = True + assert tool.status_colour == "grey" + tool.is_retired = True + assert tool.status_colour == "red" diff --git a/tests/frontend/test_forms.py b/tests/frontend/test_forms.py index b0fbc3d9d..d04c6f591 100644 --- a/tests/frontend/test_forms.py +++ b/tests/frontend/test_forms.py @@ -13,112 +13,53 @@ from controlpanel.frontend import forms -def test_tool_release_form_check_release_name(): - """ - Ensure valid chart names work, while invalid ones cause a helpful - exception. - """ - data = { +@pytest.fixture +def release_data(): + return { "name": "Test Release", "chart_name": "jupyter-lab", "version": "1.2.3", "values": {"foo": "bar"}, "is_restricted": False, + "image_tag": "1.0.0", + "description": "Test release description", } - f = forms.ToolReleaseForm(data) - assert f.is_valid() - data = { - "name": "Test Release", - "chart_name": "jupyter-lab-all-spark", - "version": "1.2.3", - "values": {"foo": "bar"}, - "is_restricted": False, - } - f = forms.ToolReleaseForm(data) - assert f.is_valid() - data = { - "name": "Test Release", - "chart_name": "rstudio", - "version": "1.2.3", - "values": {"foo": "bar"}, - "is_restricted": False, - } - f = forms.ToolReleaseForm(data) - assert f.is_valid() - data = { - "name": "Test Release", - "chart_name": "airflow-sqlite", - "version": "1.2.3", - "values": {"foo": "bar"}, - "is_restricted": False, - } - f = forms.ToolReleaseForm(data) - assert f.is_valid() - data = { - "name": "Test Release", - "chart_name": "vscode", - "version": "1.2.3", - "values": {"foo": "bar"}, - "is_restricted": False, - } - f = forms.ToolReleaseForm(data) - assert f.is_valid() - data = { - "name": "Test Release", - "chart_name": "invalid-chartname", - "version": "1.2.3", - "values": {"foo": "bar"}, - "is_restricted": False, - } - f = forms.ToolReleaseForm(data) - assert f.is_valid() is False -def test_tool_release_form_check_tool_domain(): +@pytest.mark.parametrize( + "chart_name, expected", + [ + ("jupyter-lab", True), + ("rstudio", True), + ("jupyter-lab-all-spark", True), + ("vscode", True), + ("jupyter-lab-datascience-notebook", True), + ("invalid-chartname", False), + ], +) +def test_tool_release_form_check_chart_name(release_data, chart_name, expected): + """ + Ensure valid chart names work, while invalid ones cause a helpful + exception. + """ + release_data["chart_name"] = chart_name + f = forms.ToolReleaseForm(release_data) + assert f.is_valid() is expected + + +@pytest.mark.parametrize( + "tool_domain, expected", + [("jupyter-lab", True), ("rstudio", True), ("vscode", True), ("invalid-tool-domain", False)], +) +def test_tool_release_form_check_tool_domain(release_data, tool_domain, expected): """ Ensure ONLY valid chart names work, while invalid ones cause a helpful exception. """ - data = { - "name": "Test Release", - "chart_name": "jupyter-lab", - "version": "1.2.3", - "values": {"foo": "bar"}, - "is_restricted": False, - "tool_domain": "jupyter-lab", - } - f = forms.ToolReleaseForm(data) - assert f.is_valid() - data = { - "name": "Test Release", - "chart_name": "jupyter-lab-all-spark", - "version": "1.2.3", - "values": {"foo": "bar"}, - "is_restricted": False, - "tool_domain": "jupyter-lab", - } - f = forms.ToolReleaseForm(data) - assert f.is_valid() - data = { - "name": "Test Release", - "chart_name": "vscode", - "version": "1.2.3", - "values": {"foo": "bar"}, - "is_restricted": False, - "tool_domain": "vscode", - } - f = forms.ToolReleaseForm(data) - assert f.is_valid() - data = { - "name": "Test Release", - "chart_name": "jupyter-lab-all-spark", - "version": "1.2.3", - "values": {"foo": "bar"}, - "is_restricted": False, - "tool_domain": "invalid-tool-domain", - } - f = forms.ToolReleaseForm(data) - assert f.is_valid() is False + release_data["tool_domain"] = tool_domain + + f = forms.ToolReleaseForm(release_data) + assert f.is_valid() is expected def test_tool_release_form_get_target_users(): diff --git a/tests/frontend/views/test_release.py b/tests/frontend/views/test_release.py index 4da5ffd00..29f9ff015 100644 --- a/tests/frontend/views/test_release.py +++ b/tests/frontend/views/test_release.py @@ -35,8 +35,8 @@ def test_release_detail_context_data(): ({}, 2), ({"chart_name": "rstudio"}, 1), ({"chart_name": "jupyter-lab"}, 1), - ({"is_restricted": "true"}, 0), - ({"is_restricted": "false"}, 2), + ({"status": "restricted"}, 0), + ({"status": "active"}, 2), ], ) def test_release_list_get_context_data(rf, data, count):