Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: ensure notebook endpoints do their job #388

Merged
merged 30 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
ac57289
feat: update and expand apispec for environments
olevski Jul 10, 2024
8e0cfb1
feat: add command and args to environments
olevski Aug 25, 2024
5bef056
squashme: notebooks changes
olevski Aug 27, 2024
ed3b7c5
chore: resolve changes from conflict resolution
olevski Aug 27, 2024
4790016
chore: do not use the complicated notebooks gitlab header
olevski Sep 4, 2024
60dae48
fix(pre-commit): fix applied by pre-commit
sgaist Sep 11, 2024
5c2b105
fix(notebooks): fix url args handling for /notebook/images
sgaist Aug 30, 2024
08242e3
fix(notebooks): correct logs retrieval
sgaist Aug 30, 2024
87a60b1
feat(notebook): add test for server option endpoint
sgaist Sep 4, 2024
74b3a31
fix(notebook): fix log retrieval
sgaist Sep 6, 2024
013bade
fix(notebooks): add fake renku token header to fixture for user authe…
sgaist Sep 7, 2024
aea155e
fix(notebook): make server stop endpoint work as expected
sgaist Sep 7, 2024
7de5cd3
fix(notebooks): make patch endpoint work
sgaist Sep 10, 2024
0b07d54
refactor(notebooks): rename i in loops to something more meaningful
sgaist Sep 11, 2024
b05b25d
fix(notebooks): correct logic for old start project endpoint
sgaist Sep 11, 2024
79ed03b
fix(notebooks): correct logic for start project endpoint
sgaist Sep 11, 2024
0d1336c
fix(notebook): cleanup tests
sgaist Sep 13, 2024
7570e1e
fix(pyproject): moved pytest plugins to dev dependencies
sgaist Sep 17, 2024
4fff79b
refactor(test_notebooks): move AttributeDictionary to tests
sgaist Sep 17, 2024
a504d2b
refactor(test_notebooks): clean server creation and handling
sgaist Sep 17, 2024
6007f13
refactor(test_notebook): remove httpx mocking
sgaist Sep 17, 2024
849759a
feat(tests): implement cluster creation for notebook and sessions
sgaist Sep 20, 2024
5c6a562
squashme: fix style checks and mypy checks
olevski Sep 23, 2024
ff88481
fix(test_notebooks): use specific kind configuration for test cluster
sgaist Sep 24, 2024
32cb3e6
fix(notebooks): use ErrorResponse and remove ErrorResponseNested
sgaist Sep 24, 2024
b99befe
refactor(utils): explicitly print subprocess output on error
sgaist Sep 24, 2024
e6a20b8
refactor(test): use k3d in place of kind for cluster creation
sgaist Sep 24, 2024
9dfb613
fix(test_notebooks): delete cluster if image upload fails
sgaist Sep 24, 2024
f9c4f05
fix(tests): use different cluster names for notebooks and sessions
sgaist Sep 24, 2024
7a0f8e3
refactor: move from kind to k3d
sgaist Sep 24, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading