Skip to content

Commit

Permalink
Merge pull request #1090 from ministryofjustice/anpl-1258-status-mess…
Browse files Browse the repository at this point in the history
…age-not-refresh

ANPL-1258  The bug related to status and messages on tooling env and also performance of this page
  • Loading branch information
ymao2 authored Dec 5, 2022
2 parents 801ec2f + b4884c6 commit ab8d751
Show file tree
Hide file tree
Showing 24 changed files with 287 additions and 317 deletions.
15 changes: 0 additions & 15 deletions controlpanel/api/cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -744,21 +744,6 @@ def get_deployment(self, id_token):

return deployments[0]

def get_installed_chart_version(self, id_token):
"""
Returns the installed helm chart version of the tool
This is extracted from the `chart` label in the corresponding
`Deployment`.
"""

try:
deployment = self.get_deployment(id_token)
_, chart_version = deployment.metadata.labels["chart"].rsplit("-", 1)
return chart_version
except ObjectDoesNotExist:
return None

def get_status(self, id_token, deployment=None):
try:
if deployment is None:
Expand Down
81 changes: 24 additions & 57 deletions controlpanel/api/helm.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,29 +113,20 @@ def _execute(*args, **kwargs):
return proc


def update_helm_repository():
def update_helm_repository(force=False):
"""
Updates the helm repository and returns a dictionary representation of
all the available charts. Raises a HelmError if there's a problem reading
the helm repository cache.
all the available charts.
"""
repo_path = get_repo_path()
# If there's no helm repository cache, call helm repo update to fill it.
if not os.path.exists(repo_path):
_execute("repo", "update", timeout=None) # timeout = infinity.
# Execute the helm repo update command if the helm repository cache is
# stale (older than CACHE_FOR_MINUTES value).
if os.path.getmtime(repo_path) + CACHE_FOR_MINUTES < time.time():
_execute("repo", "update", timeout=None) # timeout = infinity.
try:
with open(repo_path) as f:
return yaml.load(f, Loader=yaml.FullLoader)
except Exception as ex:
error = HelmError(ex)
error.detail = (
f"Error while opening/parsing helm repository cache: '{repo_path}'"
)
raise HelmError(error)
_execute("repo", "update", timeout=None)
else:
# Execute the helm repo update command if the helm repository cache is
# stale (older than CACHE_FOR_MINUTES value).
if force or os.path.getmtime(repo_path) + CACHE_FOR_MINUTES < time.time():
_execute("repo", "update", timeout=None) # timeout = infinity.


def get_default_image_tag_from_helm_chart(chart_url, chart_name):
Expand All @@ -151,8 +142,20 @@ def get_default_image_tag_from_helm_chart(chart_url, chart_name):


def get_helm_entries():
# Update and grab repository metadata.
repository = update_helm_repository()
# Update repository metadata.
update_helm_repository()
# Grab repository metadata.
repo_path = get_repo_path()
try:
with open(repo_path) as f:
repository = yaml.load(f, Loader=yaml.FullLoader)
except Exception as ex:
error = HelmError(ex)
error.detail = (
f"Error while opening/parsing helm repository cache: '{repo_path}'"
)
raise HelmError(error)

if not repository:
# Metadata not available, so bail with empty dictionary.
return {}
Expand Down Expand Up @@ -225,42 +228,6 @@ def delete(namespace, *args, dry_run=False):
log.info(stdout)


def get_chart_info(chart_name):
"""
Get information about the given chart.
Returns a dictionary with the chart versions as keys and instances of the
HelmChart class as associated values.
Returns an empty dictionary when the chart is not in the helm repository
index.
"""
chart_info = {} # The end result.
# Update and grab repository metadata.
repository = update_helm_repository()
if not repository:
# Metadata not available, so bail with empty dictionary.
return chart_info
entries = repository.get("entries")
if entries:
versions = entries.get(chart_name)
if versions:
# There are versions for the referenced chart. Add them to the
# result as HelmChart instances.
for version in versions:
chart = HelmChart(
version["name"],
version["description"],
version["version"],
# appVersion is relatively new so some old charts don't
# have it.
version.get("appVersion"),
version["urls"][0],
)
chart_info[chart.version] = chart
return chart_info


def get_chart_app_version(chart_name, chart_version):
"""
Returns the "appVersion" metadata for the helm chart with the referenced
Expand All @@ -270,8 +237,8 @@ def get_chart_app_version(chart_name, chart_version):
"appVersion" metadata.
"""

chart = get_chart_info(chart_name)
chart_at_version = chart.get(chart_version)
entries = get_helm_entries()
chart_at_version = get_chart_version_info(entries, chart_name, chart_version)
if chart_at_version:
return chart_at_version.app_version
else:
Expand Down
67 changes: 9 additions & 58 deletions controlpanel/api/models/tool.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import structlog
import secrets

from django.conf import settings
from django.db.models import JSONField
import django.core.exceptions
from django.db import models
from django.db.models import Q
from django_extensions.db.models import TimeStampedModel

from controlpanel.api import cluster
Expand Down Expand Up @@ -85,24 +83,17 @@ def url(self, user):
f"https://{user.slug}-{tool}.{settings.TOOLS_DOMAIN}/"
)

@property
def app_version(self):
"""
Returns the "appVersion" for this tool.
def save(self, *args, **kwargs):
is_create = not self.pk

This is metadata in the helm chart which we use to maintain details
of the actual tool version, e.g.:
if is_create:
helm.update_helm_repository(force=True)

"RStudio: 1.2.1335+conda, R: 3.5.1, ..."
if not self.description:
self.description = helm.get_chart_app_version(self.chart_name, self.version) or ''

as opposed to the chart version.
Returns None if this information is not available for this tool and
chart version (e.g. the chart was released before the `appVersion`
was introduced) or because the chart doesn't exist in the helm
reporitory.
"""
return helm.get_chart_app_version(self.chart_name, self.version)
super().save(*args, **kwargs)
return self

@property
def image_tag(self):
Expand All @@ -111,9 +102,6 @@ def image_tag(self):
return values.get("{}.tag".format(chart_image_key_name)) or \
values.get("{}.image.tag".format(chart_image_key_name))

def tool_release_tag(self, image_tag=None):
return "{}-{}-{}".format(self.chart_name, self.version, image_tag or self.image_tag)


class ToolDeploymentManager:
"""
Expand Down Expand Up @@ -146,42 +134,6 @@ def __init__(self, tool, user, old_chart_name=None):
def __repr__(self):
return f"<ToolDeployment: {self.tool!r} {self.user!r}>"

def get_installed_chart_version(self, id_token):
"""
Returns the installed chart version for this tool
Returns None if the chart is not installed for the user
"""
td = cluster.ToolDeployment(self.user, self.tool)
return td.get_installed_chart_version(id_token)

def get_installed_app_version(self, id_token):
"""
Returns the version of the deployed tool
NOTE: This is the version coming from the helm
chart `appVersion` field, **not** the version
of the chart released in the user namespace.
e.g. if user has `rstudio-2.2.5` (chart version)
installed in his namespace, this would return
"RStudio: 1.2.1335+conda, R: 3.5.1, Python: 3.7.1, patch: 10"
**not** "2.2.5".
Also bear in mind that Helm added this `appVersion`
field only "recently" so if a user has an old
version of a tool chart installed this would return
`None` as we can't determine the tool version
as this information is simply not available
in the helm repository index.
"""
chart_version = self.get_installed_chart_version(id_token)
if chart_version:
return helm.get_chart_app_version(
self.tool.chart_name, chart_version
)
return None

def delete(self, id_token):
"""
Remove the release from the cluster
Expand Down Expand Up @@ -215,8 +167,7 @@ def get_status(self, id_token, deployment=None):
log.info(status)
return status
return cluster.ToolDeployment(self.user, self.tool).get_status(
id_token,
deployment=deployment
id_token, deployment=deployment
)

def _poll(self):
Expand Down
12 changes: 12 additions & 0 deletions controlpanel/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,18 @@ class Meta:
)


class ToolDeploymentSerializer(serializers.Serializer):
old_chart_name = serializers.CharField(max_length=64, required=False)
version = serializers.CharField(max_length=64, required=True)

def validate_version(self, value):
try:
_, _, _ = value.split("__")
except ValueError:
raise serializers.ValidationError('This field include chart name, version and tool.id,'
' they are joined by "__".')


class ESBucketHitsSerializer(serializers.BaseSerializer):
def to_representation(self, bucket_hits):
access_count = defaultdict(int)
Expand Down
5 changes: 5 additions & 0 deletions controlpanel/api/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,9 @@
views.AppCustomersDetailAPIView.as_view(),
name="appcustomers-detail",
),
path(
"tool-deployments/<str:tool_name>/<str:action>",
views.ToolDeploymentAPIView.as_view(),
name="tool-deployments",
),
]
3 changes: 3 additions & 0 deletions controlpanel/api/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,6 @@
from controlpanel.api.views.health_check import (
health_check,
)
from controlpanel.api.views.tool_deployments import (
ToolDeploymentAPIView
)
59 changes: 59 additions & 0 deletions controlpanel/api/views/tool_deployments.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
from rest_framework.response import Response
from rest_framework.generics import GenericAPIView
from rest_framework import status

from controlpanel.frontend.consumers import start_background_task
from controlpanel.api import serializers


class ToolDeploymentAPIView(GenericAPIView):

http_method_names = ['post']
serializer_class = serializers.ToolDeploymentSerializer

def _deploy(self, chart_name, data):
"""
This is the most backwards thing you'll see for a while. The helm
task to deploy the tool apparently must happen when the view class
attempts to redirect to the target url. I'm sure there's a good
reason why.
"""
# If there's already a tool deployed, we need to get this from a
# hidden field posted back in the form. This is used by helm to delete
# the currently installed chart for the tool before installing the
# new chart.
old_chart_name = data.get("deployed_chart_name", None)
# The selected option from the "version" select control contains the
# data we need.
chart_info = data.get("version")
# The tool name and version are stored in the selected option's value
# attribute and then split on "__" to extract them. Why? Because we
# need both pieces of information to kick off the background helm
# deploy.
tool_name, tool_version, tool_id = chart_info.split("__")

# Kick off the helm chart as a background task.
start_background_task(
"tool.deploy",
{
"tool_name": chart_name,
"version": tool_version,
"tool_id": tool_id,
"user_id": self.request.user.id,
"id_token": self.request.user.get_id_token(),
"old_chart_name": old_chart_name,
},
)

def post(self, request, *args, **kwargs):
serializer = self.get_serializer(data=request.data)
serializer.is_valid(raise_exception=True)

chart_name = self.kwargs["tool_name"]
tool_action = self.kwargs["action"]
tool_action_function = getattr(self, f"_{tool_action}", None)
if tool_action_function and callable(tool_action_function):
tool_action_function(chart_name, request.data)
return Response(status=status.HTTP_200_OK)
else:
return Response(status=status.HTTP_400_BAD_REQUEST)
5 changes: 2 additions & 3 deletions controlpanel/frontend/consumers.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,11 @@
import structlog
from pathlib import Path
from time import sleep
import uuid

from asgiref.sync import async_to_sync
from channels.consumer import SyncConsumer
from channels.layers import get_channel_layer
from django.conf import settings
from django.urls import reverse


from controlpanel.api.cluster import (
TOOL_DEPLOYING,
Expand Down Expand Up @@ -199,6 +197,7 @@ def update_tool_status(tool_deployment, id_token, status):
payload = {
"toolName": tool.chart_name,
"version": tool.version,
"tool_id": tool.id,
"status": status,
}
send_sse(user.auth0_id, {"event": "toolStatus", "data": json.dumps(payload),})
Expand Down
1 change: 1 addition & 0 deletions controlpanel/frontend/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,7 @@ class Meta:
"values",
"is_restricted",
"tool_domain",
"description"
]


Expand Down
2 changes: 1 addition & 1 deletion controlpanel/frontend/jinja2/base.html
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@
<script src="{{ static('jquery/jquery.min.js') }}"></script>
<script src="{{ static('jquery-ui/jquery-ui.min.js') }}"></script>

<script src="{{ static('app.js') }}"></script>
<script src="{{ static('app.js') }}?version=v0.29.27"></script>
<script>window.moj.init();</script>
<!-- Global site tag (gtag.js) - Google Analytics -->
<script async src="https://www.googletagmanager.com/gtag/js?id={{ google_analytics_id }}"></script>
Expand Down
Loading

0 comments on commit ab8d751

Please sign in to comment.