From fcd3c1f3a8d90987e67c746b8095ad9bbd05946b Mon Sep 17 00:00:00 2001 From: Sylvain Lesage Date: Mon, 25 Jul 2022 21:01:23 +0200 Subject: [PATCH] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20move=20the=20admin=20end?= =?UTF-8?q?points=20under=20/admin/=20(#467)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: 🎸 move the admin endpoints under /admin/ see https://github.com/huggingface/datasets-server/issues/459#issuecomment-1194150221 * test: 💍 fix test * feat: 🎸 move the technical endpoints under /admin --- infra/charts/datasets-server/docker-images.yaml | 2 +- infra/charts/datasets-server/env/dev.yaml | 13 ++++++------- infra/charts/datasets-server/env/prod.yaml | 3 --- .../templates/admin/servicemonitor.yaml | 2 +- infra/charts/datasets-server/templates/ingress.yaml | 8 +++----- services/admin/README.md | 10 +++++----- services/admin/src/admin/app.py | 8 ++++---- services/admin/tests/test_app.py | 10 +++++----- 8 files changed, 25 insertions(+), 31 deletions(-) diff --git a/infra/charts/datasets-server/docker-images.yaml b/infra/charts/datasets-server/docker-images.yaml index e0968abecd..050733c4d7 100644 --- a/infra/charts/datasets-server/docker-images.yaml +++ b/infra/charts/datasets-server/docker-images.yaml @@ -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": { diff --git a/infra/charts/datasets-server/env/dev.yaml b/infra/charts/datasets-server/env/dev.yaml index d0d9401e49..733cb17eac 100644 --- a/infra/charts/datasets-server/env/dev.yaml +++ b/infra/charts/datasets-server/env/dev.yaml @@ -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" @@ -47,7 +46,7 @@ api: worker: datasets: - replicas: 2 + replicas: 1 resources: requests: @@ -56,7 +55,7 @@ worker: cpu: 1 firstRows: - replicas: 5 + replicas: 1 resources: requests: @@ -65,7 +64,7 @@ worker: cpu: 1 splits: - replicas: 5 + replicas: 1 resources: requests: @@ -74,7 +73,7 @@ worker: cpu: 1 splitsNext: - replicas: 2 + replicas: 1 resources: requests: diff --git a/infra/charts/datasets-server/env/prod.yaml b/infra/charts/datasets-server/env/prod.yaml index 63f47fbb7d..564f58af76 100644 --- a/infra/charts/datasets-server/env/prod.yaml +++ b/infra/charts/datasets-server/env/prod.yaml @@ -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}]' diff --git a/infra/charts/datasets-server/templates/admin/servicemonitor.yaml b/infra/charts/datasets-server/templates/admin/servicemonitor.yaml index 234943acae..7f78297a89 100644 --- a/infra/charts/datasets-server/templates/admin/servicemonitor.yaml +++ b/infra/charts/datasets-server/templates/admin/servicemonitor.yaml @@ -8,7 +8,7 @@ metadata: namespace: {{ .Release.Namespace }} spec: endpoints: - - path: /metrics + - path: /admin/metrics port: http namespaceSelector: matchNames: diff --git a/infra/charts/datasets-server/templates/ingress.yaml b/infra/charts/datasets-server/templates/ingress.yaml index a14eb10539..27e1e1e9c5 100644 --- a/infra/charts/datasets-server/templates/ingress.yaml +++ b/infra/charts/datasets-server/templates/ingress.yaml @@ -10,7 +10,7 @@ metadata: namespace: {{ .Release.Namespace }} spec: rules: - - host: {{ .Values.adminDomain }} + - host: {{ .Values.apiDomain }} http: paths: - backend: @@ -18,10 +18,8 @@ spec: 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" diff --git a/services/admin/README.md b/services/admin/README.md index b780fc7f0c..365ff2e5f6 100644 --- a/services/admin/README.md +++ b/services/admin/README.md @@ -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 diff --git a/services/admin/src/admin/app.py b/services/admin/src/admin/app.py index 8e0fd50065..aa98773e93 100644 --- a/services/admin/src/admin/app.py +++ b/services/admin/src/admin/app.py @@ -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) diff --git a/services/admin/tests/test_app.py b/services/admin/tests/test_app.py index 9618efdfd9..097f3cad80 100644 --- a/services/admin/tests/test_app.py +++ b/services/admin/tests/test_app.py @@ -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") @@ -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"]: @@ -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"] == []