Skip to content

Commit

Permalink
feat: can force delete pods (#205)
Browse files Browse the repository at this point in the history
  • Loading branch information
m-alisafaee authored and rokroskar committed Aug 23, 2019
1 parent 04f9ff6 commit 4d7f2b5
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 0 deletions.
6 changes: 6 additions & 0 deletions helm-chart/renku-notebooks/templates/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,10 @@ rules:
- get
- list
- watch
- apiGroups:
- ""
resources:
- pods
verbs:
- delete
{{- end -}}
12 changes: 12 additions & 0 deletions renku_notebooks/api/notebooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
read_namespaced_pod_log,
get_user_server,
get_user_servers,
delete_user_pod,
)
from .auth import authenticated

Expand Down Expand Up @@ -164,6 +165,17 @@ def launch_notebook(user):
@authenticated
def stop_server(user, server_name):
"""Stop user server with name."""
forced = request.args.get("force", "").lower() == "true"
if forced:
server = get_user_server(user, server_name)
if server:
pod_name = server.get("state", {}).get("pod_name", "")
if delete_user_pod(user, pod_name):
return make_response("", 204)
else:
return make_response("Cannot force delete server", 400)
return make_response("", 404)

r = delete_named_server(user, server_name)
return current_app.response_class(r.content, status=r.status_code)

Expand Down
14 changes: 14 additions & 0 deletions renku_notebooks/util/kubernetes_.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import escapism
from flask import current_app
from kubernetes import client
from kubernetes.client.rest import ApiException
from kubernetes.config.config_exception import ConfigException
from kubernetes.config.incluster_config import (
SERVICE_CERT_FILENAME,
Expand Down Expand Up @@ -93,6 +94,19 @@ def get_user_server(user, server_name):
return servers.get(server_name, {})


def delete_user_pod(user, pod_name):
"""Delete user's server with specific name"""
try:
v1.delete_namespaced_pod(
pod_name, kubernetes_namespace, grace_period_seconds=30
)
return True
except ApiException as e:
msg = f"Cannot delete server: {pod_name} for user: {user}, error: {e}"
current_app.logger.error(msg)
return False


def _get_all_user_servers(user):
def get_user_server_pods(user):
safe_username = escapism.escape(user["name"], escape_char="-").lower()
Expand Down
6 changes: 6 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -272,3 +272,9 @@ def kubernetes_client(mocker):
kubernetes_client_mock.list_namespaced_pod.return_value = namespaced_pods

kubernetes_client_mock.read_namespaced_pod_log.return_value = "some logs"

def force_delete_pod(*args, **kwargs):
empty = _AttributeDictionary({"items": []})
kubernetes_client_mock.list_namespaced_pod.return_value = empty

kubernetes_client_mock.delete_namespaced_pod.side_effect = force_delete_pod
15 changes: 15 additions & 0 deletions tests/test_notebook.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,21 @@ def test_can_delete_created_notebooks(client, kubernetes_client):
assert response.status_code == 204


def test_can_force_delete_created_notebooks(client, kubernetes_client):
create_notebook_with_default_parameters(client)

response = client.delete(
f"/service/servers/{SERVER_NAME}",
query_string={"force": "true"},
headers=AUTHORIZED_HEADERS,
)
assert response.status_code == 204

response = client.get("/service/servers", headers=AUTHORIZED_HEADERS)
assert response.status_code == 200
assert response.json.get("servers") == {}


def test_recreating_notebooks_return_current_server(client, kubernetes_client):
create_notebook_with_default_parameters(client)

Expand Down

0 comments on commit 4d7f2b5

Please sign in to comment.