Skip to content

Commit

Permalink
feat: 🎸 move the admin endpoints under /admin/ (#467)
Browse files Browse the repository at this point in the history
* feat: 🎸 move the admin endpoints under /admin/

see
#459 (comment)

* test: 💍 fix test

* feat: 🎸 move the technical endpoints under /admin
  • Loading branch information
severo authored Jul 25, 2022
1 parent cc47ea2 commit fcd3c1f
Show file tree
Hide file tree
Showing 8 changed files with 25 additions and 31 deletions.
2 changes: 1 addition & 1 deletion infra/charts/datasets-server/docker-images.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"dockerImage": {
"admin": "707930574880.dkr.ecr.us-east-1.amazonaws.com/hub-datasets-server-admin:sha-640cc19",
"admin": "707930574880.dkr.ecr.us-east-1.amazonaws.com/hub-datasets-server-admin:sha-e996a30",
"api": "707930574880.dkr.ecr.us-east-1.amazonaws.com/hub-datasets-server-api:sha-640cc19",
"reverseProxy": "docker.io/nginx:1.20",
"worker": {
Expand Down
13 changes: 6 additions & 7 deletions infra/charts/datasets-server/env/dev.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,12 @@ secrets:
monitoring:
enabled: false

adminDomain: "admin-datasets-server-dev.us.dev.moon.huggingface.tech"
apiDomain: "datasets-server-dev.us.dev.moon.huggingface.tech"
apiDomain: "datasets-server.us.dev.moon.huggingface.tech"

ingress:
annotations:
# Link to Route53 - we could set any subdomain to us.dev.moon.huggingface.tech (common zone to the k8s cluster)
external-dns.alpha.kubernetes.io/hostname: "datasets-server.us.dev.moon.huggingface.tech,admin-datasets-server-dev.us.dev.moon.huggingface.tech"
external-dns.alpha.kubernetes.io/hostname: "datasets-server.us.dev.moon.huggingface.tech"
alb.ingress.kubernetes.io/healthcheck-path: "/healthcheck"
alb.ingress.kubernetes.io/listen-ports: '[{"HTTP": 80, "HTTPS": 443}]'
alb.ingress.kubernetes.io/load-balancer-name: "hub-datasets-server-dev"
Expand Down Expand Up @@ -47,7 +46,7 @@ api:

worker:
datasets:
replicas: 2
replicas: 1

resources:
requests:
Expand All @@ -56,7 +55,7 @@ worker:
cpu: 1

firstRows:
replicas: 5
replicas: 1

resources:
requests:
Expand All @@ -65,7 +64,7 @@ worker:
cpu: 1

splits:
replicas: 5
replicas: 1

resources:
requests:
Expand All @@ -74,7 +73,7 @@ worker:
cpu: 1

splitsNext:
replicas: 2
replicas: 1

resources:
requests:
Expand Down
3 changes: 0 additions & 3 deletions infra/charts/datasets-server/env/prod.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,10 @@ secrets:
monitoring:
enabled: true

adminDomain: "admin-datasets-server.us.dev.moon.huggingface.tech"
apiDomain: "datasets-server.huggingface.co"

ingress:
annotations:
# Link to Route53 - we could set any subdomain to us.dev.moon.huggingface.tech (common zone to the k8s cluster)
external-dns.alpha.kubernetes.io/hostname: "admin-datasets-server.us.dev.moon.huggingface.tech"
alb.ingress.kubernetes.io/certificate-arn: arn:aws:acm:us-east-1:707930574880:certificate/777e3ae5-0c54-47ee-9b8c-d85eeb6ec4ae
alb.ingress.kubernetes.io/healthcheck-path: "/healthcheck"
alb.ingress.kubernetes.io/listen-ports: '[{"HTTP": 80, "HTTPS": 443}]'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ metadata:
namespace: {{ .Release.Namespace }}
spec:
endpoints:
- path: /metrics
- path: /admin/metrics
port: http
namespaceSelector:
matchNames:
Expand Down
8 changes: 3 additions & 5 deletions infra/charts/datasets-server/templates/ingress.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,16 @@ metadata:
namespace: {{ .Release.Namespace }}
spec:
rules:
- host: {{ .Values.adminDomain }}
- host: {{ .Values.apiDomain }}
http:
paths:
- backend:
service:
name: "{{ include "release" . }}-admin"
port:
name: http
pathType: ImplementationSpecific
- host: {{ .Values.apiDomain }}
http:
paths:
pathType: Prefix
path: "/admin/"
- backend:
service:
name: "{{ include "release" . }}-reverse-proxy"
Expand Down
10 changes: 5 additions & 5 deletions services/admin/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@ The scripts:

## Run the API

The admin service provides technical endpoints:
The admin service provides technical endpoints, all under the `/admin/` path:

- `/healthcheck`
- `/metrics`: gives info about the cache and the queue
- `/cache-reports`: give detailed reports on the content of the cache
- `/pending-jobs`: give the pending jobs, classed by queue and status (waiting or started)
- `/admin/healthcheck`
- `/admin/metrics`: gives info about the cache and the queue
- `/admin/cache-reports`: give detailed reports on the content of the cache
- `/admin/pending-jobs`: give the pending jobs, classed by queue and status (waiting or started)

### /cache-reports

Expand Down
8 changes: 4 additions & 4 deletions services/admin/src/admin/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ def create_app() -> Starlette:

middleware = [Middleware(GZipMiddleware), Middleware(PrometheusMiddleware, filter_unhandled_paths=True)]
routes = [
Route("/healthcheck", endpoint=healthcheck_endpoint),
Route("/metrics", endpoint=prometheus.endpoint),
Route("/admin/healthcheck", endpoint=healthcheck_endpoint),
Route("/admin/metrics", endpoint=prometheus.endpoint),
# used by https://observablehq.com/@huggingface/quality-assessment-of-datasets-loading
Route("/cache-reports", endpoint=cache_reports_endpoint),
Route("/admin/cache-reports", endpoint=cache_reports_endpoint),
# used in a browser tab to monitor the queue
Route("/pending-jobs", endpoint=pending_jobs_endpoint),
Route("/admin/pending-jobs", endpoint=pending_jobs_endpoint),
]
return Starlette(routes=routes, middleware=middleware)

Expand Down
10 changes: 5 additions & 5 deletions services/admin/tests/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ def clean_mongo_databases() -> None:


def test_get_healthcheck(client: TestClient) -> None:
response = client.get("/healthcheck")
response = client.get("/admin/healthcheck")
assert response.status_code == 200
assert response.text == "ok"


def test_metrics(client: TestClient) -> None:
response = client.get("/metrics")
response = client.get("/admin/metrics")
assert response.status_code == 200
text = response.text
lines = text.split("\n")
Expand All @@ -50,11 +50,11 @@ def test_metrics(client: TestClient) -> None:
assert 'cache_entries_total{cache="datasets",status="valid"}' in metrics
assert 'cache_entries_total{cache="splits/",status="BAD_REQUEST"}' in metrics
assert 'cache_entries_total{cache="first-rows/",status="INTERNAL_SERVER_ERROR"}' in metrics
assert 'starlette_requests_total{method="GET",path_template="/metrics"}' in metrics
assert 'starlette_requests_total{method="GET",path_template="/admin/metrics"}' in metrics


def test_pending_jobs(client: TestClient) -> None:
response = client.get("/pending-jobs")
response = client.get("/admin/pending-jobs")
assert response.status_code == 200
json = response.json()
for e in ["/splits", "/rows", "/splits-next", "/first-rows"]:
Expand All @@ -63,7 +63,7 @@ def test_pending_jobs(client: TestClient) -> None:


def test_cache_reports(client: TestClient) -> None:
response = client.get("/cache-reports")
response = client.get("/admin/cache-reports")
assert response.status_code == 200
json = response.json()
assert json["/splits-next"] == []
Expand Down

0 comments on commit fcd3c1f

Please sign in to comment.