Skip to content

Commit

Permalink
feat: ensure notebook endpoints do their job (#388)
Browse files Browse the repository at this point in the history
This work brings the notebook related endpoints to a working state
to serve as replacement for the renku-notebooks external service.

Short summary:
- API tests have been added
- Code has been fixed to answer / handle exception
- Automated test cluster creation has been added using k3d

---------

Co-authored-by: Tasko Olevski <[email protected]>
  • Loading branch information
sgaist and olevski authored Sep 25, 2024
1 parent 8878712 commit 30923b5
Show file tree
Hide file tree
Showing 30 changed files with 666 additions and 101 deletions.
1 change: 0 additions & 1 deletion .devcontainer/.poetry_cache/.keep
Original file line number Diff line number Diff line change
@@ -1 +0,0 @@

4 changes: 2 additions & 2 deletions .devcontainer/devcontainer.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
"command": "poetry self add poetry-polylith-plugin"
},
"ghcr.io/devcontainers/features/docker-in-docker:2": {},
"ghcr.io/mpriscella/features/kind:1": {},
"ghcr.io/devcontainers-contrib/features/gh-release:1": {
"repo": "authzed/zed",
"binaryNames": "zed"
Expand All @@ -28,7 +27,8 @@
"ghcr.io/EliiseS/devcontainer-features/bash-profile:1": {
"command": "alias k=kubectl"
},
"ghcr.io/devcontainers-contrib/features/rclone:1": {}
"ghcr.io/devcontainers-contrib/features/rclone:1": {},
"./k3d": {}
},
"overrideFeatureInstallOrder": [
"ghcr.io/devcontainers-contrib/features/poetry",
Expand Down
17 changes: 17 additions & 0 deletions .devcontainer/k3d/devcontainer-feature.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"id": "k3d",
"version": "1.0.0",
"name": "k3s based kubernetes cluster in docker",
"postCreateCommand": "k3d --version",
"installsAfter": [
"ghcr.io/devcontainers-contrib/features/bash-command"
],
"options": {
"k3d_version": {
"type": "string",
"description": "k3d version to install",
"proposals": ["latest", "5.7.4"],
"default": "latest"
}
}
}
14 changes: 14 additions & 0 deletions .devcontainer/k3d/install.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
if [ "${K3D_VERSION}" != "none" ]; then
echo "Downloading k3d..."
if [ "${K3D_VERSION}" = "latest" ]; then
# Install and check the hash
curl -sSL https://raw.githubusercontent.com/k3d-io/k3d/main/install.sh | bash
else
find_version_from_git_tags K3D_VERSION https://github.com/kubernetes/K3D
if [ "${K3D_VERSION::1}" != "v" ]; then
K3D_VERSION="v${K3D_VERSION}"
fi
# Install and check the hash
curl -sSL https://raw.githubusercontent.com/k3d-io/k3d/main/install.sh | TAG="${K3D_VERSION}" bash
fi
fi
5 changes: 5 additions & 0 deletions .github/workflows/test_publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@ jobs:
- uses: actions/checkout@v4
with:
fetch-depth: 0
- uses: actions/cache/restore@v3
name: Restore cache
with:
path: ${{ env.CACHE_PATH }}
key: ${{ env.CACHE_KEY }}
- name: Set Git config
shell: bash
run: |
Expand Down
2 changes: 2 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@ repos:
- id: check-toml
- id: debug-statements
- id: end-of-file-fixer
exclude: 'components/renku_data_services/message_queue/(avro_models|schemas)'
- id: mixed-line-ending
- id: trailing-whitespace
exclude: 'components/renku_data_services/message_queue/(avro_models|schemas)'
- repo: https://github.com/asottile/yesqa
rev: v1.5.0
hooks:
Expand Down
18 changes: 6 additions & 12 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
.PHONY: schemas tests test_setup main_tests schemathesis_tests collect_coverage style_checks pre_commit_checks run download_avro check_avro avro_models update_avro kind_cluster install_amaltheas all
.PHONY: schemas tests test_setup main_tests schemathesis_tests collect_coverage style_checks pre_commit_checks run download_avro check_avro avro_models update_avro k3d_cluster install_amaltheas all

AMALTHEA_JS_VERSION ?= 0.12.2
AMALTHEA_SESSIONS_VERSION ?= 0.0.9-new-operator-chart
AMALTHEA_SESSIONS_VERSION ?= 0.0.10-new-operator-chart
codegen_params = --input-file-type openapi --output-model-type pydantic_v2.BaseModel --use-double-quotes --target-python-version 3.12 --collapse-root-models --field-constraints --strict-nullable --set-default-enum-member --openapi-scopes schemas paths parameters --set-default-enum-member --use-one-literal-as-default --use-default

define test_apispec_up_to_date
Expand Down Expand Up @@ -143,21 +143,15 @@ help: ## Display this help.

##@ Helm/k8s

kind_cluster: ## Creates a kind cluster for testing
kind delete cluster
docker network rm -f kind
docker network create -d=bridge -o com.docker.network.bridge.enable_ip_masquerade=true -o com.docker.network.driver.mtu=1500 --ipv6=false kind
kind create cluster --config kind_config.yaml
kubectl apply -f https://raw.githubusercontent.com/kubernetes/ingress-nginx/main/deploy/static/provider/kind/deploy.yaml
echo "Waiting for ingress controller to initialize"
sleep 15
kubectl wait --namespace ingress-nginx --for=condition=ready pod --selector=app.kubernetes.io/component=controller --timeout=90s
k3d_cluster: ## Creates a k3d cluster for testing
k3d cluster delete
k3d cluster create --agents 1 --k3s-arg --disable=metrics-server@server:0

install_amaltheas: ## Installs both version of amalthea in the. NOTE: It uses the currently active k8s context.
helm repo add renku https://swissdatasciencecenter.github.io/helm-charts
helm repo update
helm upgrade --install amalthea-js renku/amalthea --version $(AMALTHEA_JS_VERSION)
helm upgrade --install amalthea-sessions amalthea-sessions-0.0.9-new-operator-chart.tgz --version $(AMALTHEA_SESSIONS_VERSION)
helm upgrade --install amalthea-se renku/amalthea-sessions --version ${AMALTHEA_SESSIONS_VERSION}

# TODO: Add the version variables from the top of the file here when the charts are fully published
amalthea_schema: ## Updates generates pydantic classes from CRDs
Expand Down
27 changes: 27 additions & 0 deletions components/renku_data_services/base_api/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,3 +180,30 @@ async def decorated_function(*args: _P.args, **kwargs: _P.kwargs) -> _T:
return response

return decorated_function


def internal_gitlab_authenticate(
authenticator: Authenticator,
) -> Callable[
[Callable[Concatenate[Request, APIUser, APIUser, _P], Coroutine[Any, Any, _T]]],
Callable[Concatenate[Request, APIUser, _P], Coroutine[Any, Any, _T]],
]:
"""Decorator for a Sanic handler that that adds a user for the internal gitlab user."""

def decorator(
f: Callable[Concatenate[Request, APIUser, APIUser, _P], Coroutine[Any, Any, _T]],
) -> Callable[Concatenate[Request, APIUser, _P], Coroutine[Any, Any, _T]]:
@wraps(f)
async def decorated_function(
request: Request,
user: APIUser,
*args: _P.args,
**kwargs: _P.kwargs,
) -> _T:
access_token = str(request.headers.get("Gitlab-Access-Token"))
internal_gitlab_user = await authenticator.authenticate(access_token, request)
return await f(request, user, internal_gitlab_user, *args, **kwargs)

return decorated_function

return decorator
25 changes: 11 additions & 14 deletions components/renku_data_services/notebooks/api.spec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -455,11 +455,8 @@ components:
message:
type: string
example: "Something went wrong - please try again later"
required:
- "code"
- "message"
required:
- "error"
required: ["code", "message"]
required: ["error"]
Generated:
properties:
enabled:
Expand All @@ -478,7 +475,7 @@ components:
repositories:
type: array
default: []
items:
items:
"$ref": "#/components/schemas/LaunchNotebookRequestRepository"
cloudstorage:
default: []
Expand All @@ -489,7 +486,7 @@ components:
default: 1
type: integer
resource_class_id:
default:
default:
nullable: true
type: integer
environment_variables:
Expand Down Expand Up @@ -539,7 +536,7 @@ components:
default: {}
type: object
image:
default:
default:
nullable: true
type: string
lfs_auto_fetch:
Expand All @@ -548,13 +545,13 @@ components:
namespace:
type: string
notebook:
default:
default:
nullable: true
type: string
project:
type: string
resource_class_id:
default:
default:
nullable: true
type: integer
serverOptions:
Expand All @@ -565,7 +562,7 @@ components:
user_secrets:
allOf:
- "$ref": "#/components/schemas/UserSecrets"
default:
default:
nullable: true
required:
- commit_sha
Expand Down Expand Up @@ -660,7 +657,7 @@ components:
properties:
configuration:
additionalProperties: {}
default:
default:
nullable: true
type: object
readonly:
Expand All @@ -669,7 +666,7 @@ components:
source_path:
type: string
storage_id:
default:
default:
nullable: true
type: string
target_path:
Expand Down Expand Up @@ -882,7 +879,7 @@ components:
type: integer
description: The size of disk storage for the session, in gigabytes
resource_class_id:
default:
default:
nullable: true
type: integer
cloudstorage:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def session_tolerations(server: "UserServer") -> list[dict[str, Any]]:
"op": "add",
"path": "/statefulset/spec/template/spec/tolerations",
"value": default_tolerations
+ [i.json_match_expression() for i in server.server_options.tolerations],
+ [toleration.json_match_expression() for toleration in server.server_options.tolerations],
}
],
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -312,9 +312,9 @@ def certificates_container(config: _NotebooksConfig) -> tuple[client.V1Container
projected=client.V1ProjectedVolumeSource(
default_mode=440,
sources=[
{"secret": {"name": i.get("secret")}}
for i in config.sessions.ca_certs.secrets
if isinstance(i, dict) and i.get("secret") is not None
{"secret": {"name": secret.get("secret")}}
for secret in config.sessions.ca_certs.secrets
if isinstance(secret, dict) and secret.get("secret") is not None
],
),
)
Expand Down
25 changes: 16 additions & 9 deletions components/renku_data_services/notebooks/api/classes/k8s_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,11 @@ async def get_pod_logs(self, name: str, max_log_lines: Optional[int] = None) ->
"""Get the logs of all containers in the session."""
pod = cast(Pod, await Pod.get(name=name, namespace=self.namespace))
logs: dict[str, str] = {}
containers = [i.name for i in pod.spec.containers] + [i.name for i in pod.spec.initContainers]
containers = [container.name for container in pod.spec.containers + pod.spec.get("initContainers", [])]
for container in containers:
try:
# NOTE: calling pod.logs without a container name set crashes the library
clogs: list[str] = [i async for i in pod.logs(container=container, tail_lines=max_log_lines)]
clogs: list[str] = [clog async for clog in pod.logs(container=container, tail_lines=max_log_lines)]
except NotFoundError:
raise errors.MissingResourceError(message=f"The session pod {name} does not exist.")
except ServerError as err:
Expand Down Expand Up @@ -243,8 +243,10 @@ async def patch_statefulset_tokens(self, name: str, renku_tokens: RenkuTokens) -
except NotFoundError:
return None

containers: list[V1Container] = [V1Container(**i) for i in sts.spec.template.spec.containers]
init_containers: list[V1Container] = [V1Container(**i) for i in sts.spec.template.spec.init_containers]
containers: list[V1Container] = [V1Container(**container) for container in sts.spec.template.spec.containers]
init_containers: list[V1Container] = [
V1Container(**container) for container in sts.spec.template.spec.init_containers
]

git_proxy_container_index, git_proxy_container = next(
((i, c) for i, c in enumerate(containers) if c.name == "git-proxy"),
Expand Down Expand Up @@ -368,7 +370,7 @@ async def list_servers(self, safe_username: str) -> list[_SessionType]:
)
raise JSCacheError(f"The JSCache produced an unexpected status code: {res.status_code}")

return [self.server_type.model_validate(i) for i in res.json()]
return [self.server_type.model_validate(server) for server in res.json()]

async def get_server(self, name: str) -> _SessionType | None:
"""Get a specific jupyter server."""
Expand Down Expand Up @@ -441,7 +443,11 @@ async def get_server_logs(
) -> dict[str, str]:
"""Get the logs from the server."""
# NOTE: this get_server ensures the user has access to the server without it you could read someone elses logs
_ = await self.get_server(server_name, safe_username)
server = await self.get_server(server_name, safe_username)
if not server:
raise MissingResourceError(
f"Cannot find server {server_name} for user " f"{safe_username} to retrieve logs."
)
pod_name = f"{server_name}-0"
return await self.renku_ns_client.get_pod_logs(pod_name, max_log_lines)

Expand Down Expand Up @@ -481,9 +487,10 @@ async def delete_server(self, server_name: str, safe_username: str) -> None:
"""Delete the server."""
server = await self.get_server(server_name, safe_username)
if not server:
return None
await self.renku_ns_client.delete_server(server_name)
return None
raise MissingResourceError(
f"Cannot find server {server_name} for user " f"{safe_username} in order to delete it."
)
return await self.renku_ns_client.delete_server(server_name)

async def patch_tokens(self, server_name: str, renku_tokens: RenkuTokens, gitlab_token: GitlabToken) -> None:
"""Patch the Renku and Gitlab access tokens used in a session."""
Expand Down
Loading

0 comments on commit 30923b5

Please sign in to comment.