diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b4f160..f0e4efd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,12 +8,23 @@ no leading `v`. ## [Unreleased] ### Added +- `JobSLORequirement` routes. The index watches both kinds, and a route + resolves across them; a job route's body has `queue` (`maxDepth`, `promql`) + in place of `ttft` / `otps`. A cluster without the JobSLO CRD serves the LLM + routes as before. The RBAC in `config/rbac/` grants `jobslorequirements`. +- The `pdengine` bound type on `LLMSLORequirement` (helm-charts#95). +- Bound values of 0 (scale-to-zero), which the CRD has allowed since + helm-charts#20; they were refused with `400` before. - CI on every pull request: pytest and the end-to-end smoke run on Python 3.10 and 3.12, a docker build, and a gate for the license text and committed credentials. - Dependabot for the Python dependencies and the GitHub Actions. - `NOTICE`. +### Changed +- A write the CRD's rules reject (API server `422`) is `400 invalid_request` + with the API server's message, not `502`. + ### Removed - The internal GitLab pipeline (`.gitlab-ci.yml`). diff --git a/README.md b/README.md index d539ee6..4fec8a9 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,8 @@ A small HTTP API for reading and changing the SLO of an LLM inference service, without handing the caller access to Kubernetes. -The SLO itself lives in an `LLMSLORequirement` custom resource, which +The SLO itself lives in an `LLMSLORequirement` custom resource (or a +`JobSLORequirement`, for a queue-driven job service), which [slo-scaler-decision-gen](https://github.com/modelsphere/slo-scaler-decision-gen) reads to decide replica counts. slo-api is a front door onto those resources: callers address a service by its route name, send plain JSON, and never see a @@ -21,11 +22,11 @@ namespace, a CR name or a kubeconfig. the service. The RBAC in `config/rbac/` grants exactly `get / list / watch / patch / update` and nothing else. - **It only touches the SLO fields** — `priority`, `minimumDeployment`, - `maximumDeployment`, `ttft`, `otps`. Anything else in the spec, written by - other controllers, survives a write. -- **A route resolves through `spec.serviceId`.** If two namespaces have a CR with - the same `serviceId`, writes are refused with `409` rather than guessing; reads - return the first one seen. + `maximumDeployment`, `ttft`, `otps` (`queue` on a job route). Anything else in + the spec, written by other controllers, survives a write. +- **A route resolves through `spec.serviceId`**, across both kinds. If two CRs + have the same `serviceId` (two namespaces, or one of each kind), reads and + writes are refused with `409` rather than guessing. - Writes merge onto the **live** CR, read fresh from the API server rather than from the watch cache, so a concurrent change is not silently reverted. @@ -56,8 +57,25 @@ boolean: } ``` -`highPriority: true` writes `priority: 10`; `false` writes `0`. Metric types are -`avg`, `p50`, `p80`, `p90`, `p95`, `p99`. Errors come back as +`highPriority: true` writes `priority: 10`; `false` writes `0`. A bound's +`value` may be 0: `{min: 0, max: 0}` turns the service off (scale-to-zero). Metric types are +`avg`, `p50`, `p80`, `p90`, `p95`, `p99`. A bound's `type` is `replica`, +`concurrency` (minimum only) or `pdengine` (a PD service's bounds counted in +prefill + decode engines; both bounds must then be `pdengine`). + +A route backed by a `JobSLORequirement` has `queue` in place of `ttft` and +`otps`, and its bounds are `replica` only (`concurrency` for the minimum): + +```json +{"route": "img-gen", "queue": {"maxDepth": 8, "promql": "sum(jobs_pending{service=\"img-gen\"})"}} +``` + +`queue.promql` may be left out of a `PUT` to keep the CR's own. `DELETE` keeps +`queue` (the CRD requires it) and resets the rest. + +The CRD's own cross-field rules (minimum at most maximum, matching bound types, +...) are checked by the API server against the merged spec; a write that breaks +one is `400` with the API server's message. Errors come back as `{"error": {"code", "message", "field"}}`, and an `X-Request-Id` header on the request is echoed on the response. diff --git a/config/rbac/rbac.yaml b/config/rbac/rbac.yaml index 741ad23..8243ab6 100644 --- a/config/rbac/rbac.yaml +++ b/config/rbac/rbac.yaml @@ -16,7 +16,7 @@ rules: # Both groups: the CRDs moved from inference.x-k8s.io, and slo-api picks # whichever the cluster serves at startup (slo_api.resolve_group). - apiGroups: ["inference.modelsphere.dev", "inference.x-k8s.io"] - resources: ["llmslorequirements"] + resources: ["llmslorequirements", "jobslorequirements"] verbs: ["get", "list", "watch", "patch", "update"] --- apiVersion: rbac.authorization.k8s.io/v1 diff --git a/docs/openapi.yaml b/docs/openapi.yaml index bb8064c..dff46db 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -4,25 +4,27 @@ info: version: 0.2.1 summary: Read and change the SLO of an LLM inference service by its route. description: | - slo-api is a front door onto `LLMSLORequirement` custom resources, which + slo-api is a front door onto `LLMSLORequirement` and `JobSLORequirement` + custom resources, which slo-scaler-decision-gen reads to size each service. Callers address a service by its route and send plain JSON; they never see a namespace, a CR name or a kubeconfig. ``` - client -- Bearer token --> slo-api --> LLMSLORequirement CRs --> decision-gen + client -- Bearer token --> slo-api --> {LLM,Job}SLORequirement CRs --> decision-gen (patch only; never creates or deletes a CR) ``` ## Routes - A route is a CR's `spec.serviceId`. The CR's lifecycle belongs to whatever + A route is a CR's `spec.serviceId`, of either kind. The CR's lifecycle belongs to whatever deployed the service (for swiss, the chart at install, with the release name as the serviceId). Here a route is only read and patched: - An unknown route is `404`, for writes too. Nothing here creates a CR. - - A serviceId found in more than one namespace is `409` for reads and - writes alike, with the candidates in `error.details`. + - A serviceId found on more than one CR (two namespaces, or one of each + kind) is `409` for reads and writes alike, with the candidates in + `error.details`. ## Readiness @@ -33,9 +35,11 @@ info: ## Fields slo-api owns five fields of the CR spec: `priority`, `minimumDeployment`, - `maximumDeployment`, `ttft` and `otps`. Every other field, written by other - controllers, survives a write. `priority` (0-10 in the CR) is exposed as - the boolean `highPriority`. + `maximumDeployment`, `ttft` and `otps` -- on a `JobSLORequirement` route, + `queue` in place of `ttft` and `otps`, and bounds of type `replica` (or + `concurrency` for the minimum) only. A section of the other kind is `400`. + Every other field, written by other controllers, survives a write. + `priority` (0-10 in the CR) is exposed as the boolean `highPriority`. ## Errors @@ -68,9 +72,9 @@ paths: summary: List routes description: | Routes in name order, read from the watch index rather than the API - server. Items leave out `ttft` and `otps`; read one route for those. + server. Items leave out `ttft`, `otps` and `queue`; read one route for those. - A route found in more than one namespace counts in `total` but is left + A route found on more than one CR counts in `total` but is left out of `items`, so a page can be shorter than `pageSize`. operationId: listConfigs parameters: @@ -138,9 +142,12 @@ paths: the write, so a concurrent change to another field is not reverted. `spec.serviceId` is kept. + A `queue.promql` left out is kept, the same way. + The CRD's own rules, such as `minimumDeployment.value` at most - `maximumDeployment.value`, are checked by the API server. A body that - breaks one comes back as `502`, not `400`. + `maximumDeployment.value`, or both bounds of the same `type`, are + checked by the API server on the merged spec. A body that breaks one + comes back as `400` with the API server's message. operationId: putConfig requestBody: required: true @@ -176,7 +183,8 @@ paths: description: | Resets the five fields to the CRD defaults; the CR stays. `priority` becomes 0 and `minimumDeployment` `{type: replica, value: 1}`; - `maximumDeployment`, `ttft` and `otps` are removed. + `maximumDeployment`, `ttft` and `otps` are removed. A + `JobSLORequirement` keeps its `queue`, which its CRD requires. decision-gen treats a requirement with no `maximumDeployment` as opted out of autoscaling, so after a reset it stops sizing the service. @@ -297,14 +305,14 @@ components: example: error: { code: not_found, message: "route 'ghost' not registered" } Ambiguous: - description: The route is the serviceId of CRs in more than one namespace. + description: The route is the serviceId of more than one CR. content: application/json: schema: { $ref: "#/components/schemas/Error" } example: error: code: conflict - message: "route 'my-model' maps to multiple LLMSLORequirement CRs; cannot auto-select a target" + message: "route 'my-model' maps to multiple SLO requirement CRs; cannot auto-select a target" details: - { namespace: team-a, name: my-model } - { namespace: team-b, name: my-model } @@ -332,10 +340,11 @@ components: description: Absent means decision-gen does not autoscale the service. ttft: { $ref: "#/components/schemas/TTFT" } otps: { $ref: "#/components/schemas/OTPS" } + queue: { $ref: "#/components/schemas/Queue" } SLOSummary: type: object - description: A list item. `ttft` and `otps` are left out. + description: A list item. `ttft`, `otps` and `queue` are left out. required: [route, highPriority] properties: route: { type: string } @@ -369,24 +378,40 @@ components: type: object required: [value] properties: - type: { type: string, enum: [replica, concurrency], default: replica } - value: { type: integer, minimum: 1 } + type: { type: string, enum: [replica, concurrency, pdengine], default: replica } + value: { type: integer, minimum: 0 } maximumDeployment: type: object required: [value] properties: - type: { type: string, enum: [replica], default: replica } - value: { type: integer, minimum: 1 } + type: { type: string, enum: [replica, pdengine], default: replica } + value: { type: integer, minimum: 0 } ttft: { $ref: "#/components/schemas/TTFT" } otps: { $ref: "#/components/schemas/OTPS" } + queue: + $ref: "#/components/schemas/Queue" + description: JobSLORequirement routes only. `promql` may be left out to keep the stored one. Bound: type: object - description: A replica bound as stored. The CRD allows 0 (scale to zero), which this API does not write. + description: | + A bound as stored. 0 is scale-to-zero: a minimum of 0 needs a maximum of + 0, which turns the service off (a CRD rule, so breaking it is `400`). + `pdengine` counts a PD service's prefill plus decode engines + rather than replicas, and needs both bounds to be `pdengine`. properties: - type: { type: string, enum: [replica, concurrency] } + type: { type: string, enum: [replica, concurrency, pdengine] } value: { type: integer, minimum: 0 } + Queue: + type: object + description: Queue SLO of a `JobSLORequirement`. + additionalProperties: false + required: [maxDepth] + properties: + maxDepth: { type: integer, minimum: 0, description: Target pending jobs per replica. } + promql: { type: string, minLength: 1, description: PromQL that decision-gen evaluates to the queue depth. } + TTFT: description: | Time to first token, a ceiling in seconds: a metric is violated when the diff --git a/src/slo_api/__init__.py b/src/slo_api/__init__.py index fc617ed..053896b 100644 --- a/src/slo_api/__init__.py +++ b/src/slo_api/__init__.py @@ -1,4 +1,4 @@ -"""slo-api: HTTP facade over LLMSLORequirement CRs. +"""slo-api: HTTP facade over LLMSLORequirement and JobSLORequirement CRs. Exposes a route-keyed REST API and translates to/from the underlying custom resource. Never creates new CRs; only get/patch/replace/delete @@ -9,7 +9,10 @@ _log = _logging.getLogger("slo-api") VERSION = "v1alpha1" -PLURAL = "llmslorequirements" +LLM_PLURAL = "llmslorequirements" +JOB_PLURAL = "jobslorequirements" +# Both kinds share one route space, as decision-gen sizes them side by side. +PLURALS = (LLM_PLURAL, JOB_PLURAL) # The CRDs moved from inference.x-k8s.io to inference.modelsphere.dev. Both are # listed so this runs against a cluster on either, in the order to prefer them: diff --git a/src/slo_api/cr_index.py b/src/slo_api/cr_index.py index 2354621..ff58abf 100644 --- a/src/slo_api/cr_index.py +++ b/src/slo_api/cr_index.py @@ -1,15 +1,15 @@ -"""In-memory index of LLMSLORequirement CRs, keyed by serviceId. +"""In-memory index of SLO requirement CRs (every kind in PLURALS), keyed by serviceId. The public API routes by the opaque `route` string, which we resolve against `spec.serviceId`. A CR is an addressable target iff it has a non-empty serviceId and at least one namespace hosting it. -Two CRs in different namespaces may share a serviceId. Writes against -such routes are rejected 409 (we refuse to guess); reads return the -first-seen CR and log a warning. +Two CRs may share a serviceId (different namespaces, or one of each +kind). Such routes are rejected 409 (we refuse to guess). -The index is maintained by a background watch with capped exponential -backoff reconnect, mirroring decision_gen.SLOStore's pattern. +The index is maintained by one background watch per kind with capped +exponential backoff reconnect, mirroring decision_gen.SLOStore's pattern. +A kind whose CRD is not installed is indexed as empty. """ import logging @@ -17,8 +17,9 @@ import time from kubernetes import client, watch +from kubernetes.client import ApiException -from slo_api import VERSION, PLURAL, group +from slo_api import VERSION, PLURALS, group log = logging.getLogger(__name__) @@ -29,16 +30,16 @@ class CRIndex: def __init__(self, api=None, watch_factory=None): self._lock = threading.Lock() - # (ns, name) -> full CR dict (authoritative record) + # (plural, ns, name) -> full CR dict (authoritative record) self._by_key = {} - # service_id -> [(ns, name), ...] in first-seen order + # service_id -> [(plural, ns, name), ...] in first-seen order self._by_service = {} self._api = api self._watch_factory = watch_factory self._stop = threading.Event() - self._synced = threading.Event() - self._thread = None + self._synced = {p: threading.Event() for p in PLURALS} + self._threads = [] self._started = False # ---------- lifecycle ---------- @@ -47,21 +48,26 @@ def start(self): if self._started: return self._started = True - self._thread = threading.Thread( - target=self._run, daemon=True, name="cr-index", - ) - self._thread.start() + for plural in PLURALS: + t = threading.Thread( + target=self._run, args=(plural,), daemon=True, + name=f"cr-index-{plural}", + ) + t.start() + self._threads.append(t) def stop(self): self._stop.set() - if self._thread: - self._thread.join(timeout=5) + for t in self._threads: + t.join(timeout=5) def synced(self): - return self._synced.is_set() + return all(e.is_set() for e in self._synced.values()) def wait_synced(self, timeout): - return self._synced.wait(timeout) + deadline = time.monotonic() + timeout + return all(e.wait(max(0.0, deadline - time.monotonic())) + for e in self._synced.values()) # ---------- lookups ---------- @@ -69,8 +75,8 @@ def resolve(self, route): """Return (status, payload) for `route`. status: - "ok" — payload = (ns, name, cr) - "ambiguous" — payload = [(ns, name), ...] + "ok" — payload = (plural, ns, name, cr) + "ambiguous" — payload = [(plural, ns, name), ...] "missing" — payload = None """ with self._lock: @@ -79,12 +85,11 @@ def resolve(self, route): return "missing", None if len(keys) > 1: return "ambiguous", list(keys) - ns, name = keys[0] - cr = self._by_key.get((ns, name)) + cr = self._by_key.get(keys[0]) if cr is None: # index said it existed but the record is gone — treat as missing return "missing", None - return "ok", (ns, name, cr) + return "ok", (*keys[0], cr) def list_routes(self): """All currently indexed routes (serviceIds), sorted.""" @@ -92,7 +97,7 @@ def list_routes(self): return sorted(self._by_service.keys()) def all_crs(self): - """Snapshot of indexed CRs: {(ns, name): cr}.""" + """Snapshot of indexed CRs: {(plural, ns, name): cr}.""" with self._lock: return dict(self._by_key) @@ -103,28 +108,39 @@ def _api_client(self): self._api = client.CustomObjectsApi() return self._api - def _run(self): + def _run(self, plural): backoff = _BACKOFF_INITIAL while not self._stop.is_set(): try: - self._watch_once() + self._watch_once(plural) backoff = _BACKOFF_INITIAL except Exception: if self._stop.is_set(): break - log.exception("cr-index watch error; retry in %.1fs", backoff) + log.exception("cr-index %s watch error; retry in %.1fs", plural, backoff) self._stop.wait(backoff) backoff = min(backoff * 2, _BACKOFF_MAX) - def _watch_once(self): + def _watch_once(self, plural): api = self._api_client() # List first, then watch from the list's resourceVersion. A bare watch only marks the index # synced on its first event, and with no CRs in the cluster there is none: the API server # sends no ADDED event and its first BOOKMARK can take minutes, so readiness stayed 503 and # the liveness probe killed the pod before it ever served. An empty list is a complete one. - listing = api.list_cluster_custom_object(group(), VERSION, PLURAL) - self._replace((listing or {}).get("items") or []) - self._synced.set() + try: + listing = api.list_cluster_custom_object(group(), VERSION, plural) + except ApiException as e: + if e.status != 404: + raise + # CRD not installed (JobSLORequirement is optional): no CRs of this kind is a complete + # answer. Look again later, in case it gets installed. + log.debug("cr-index: %s not served; retry in %.0fs", plural, _BACKOFF_MAX) + self._replace(plural, []) + self._synced[plural].set() + self._stop.wait(_BACKOFF_MAX) + return + self._replace(plural, (listing or {}).get("items") or []) + self._synced[plural].set() rv = ((listing or {}).get("metadata") or {}).get("resourceVersion") wf = self._watch_factory or watch.Watch @@ -135,7 +151,7 @@ def _watch_once(self): try: for event in w.stream( api.list_cluster_custom_object, - group(), VERSION, PLURAL, + group(), VERSION, plural, **kwargs): if self._stop.is_set(): return @@ -144,35 +160,37 @@ def _watch_once(self): if etype == "ERROR": raise RuntimeError(f"watch ERROR event: {cr!r}") if etype == "BOOKMARK": - self._synced.set() + self._synced[plural].set() continue - self._apply(etype, cr) - self._synced.set() + self._apply(plural, etype, cr) + self._synced[plural].set() finally: stop = getattr(w, "stop", None) if callable(stop): stop() - def _replace(self, items): - """Rebuild the index from a full list: drops CRs deleted while no watch was running. + def _replace(self, plural, items): + """Rebuild one kind's part of the index from a full list: drops CRs deleted while no watch + was running. Other kinds' entries are kept. Built aside and swapped in under the lock, so a reader sees the old index or the new one, never the empty or half-built one in between -- this runs on every watch reconnect.""" - by_key, by_service = {}, {} + fresh = {} for cr in items: meta = cr.get("metadata") or {} ns, name = meta.get("namespace"), meta.get("name") - if not ns or not name: - continue - sid = (cr.get("spec") or {}).get("serviceId") or "" - by_key[(ns, name)] = cr - lst = by_service.setdefault(sid, []) - if (ns, name) not in lst: - lst.append((ns, name)) + if ns and name: + fresh[(plural, ns, name)] = cr with self._lock: + by_key = {k: v for k, v in self._by_key.items() if k[0] != plural} + by_key.update(fresh) + by_service = {} + for key, cr in by_key.items(): + sid = (cr.get("spec") or {}).get("serviceId") or "" + by_service.setdefault(sid, []).append(key) self._by_key, self._by_service = by_key, by_service - def _apply(self, etype, cr): + def _apply(self, plural, etype, cr): meta = cr.get("metadata") or {} spec = cr.get("spec") or {} ns = meta.get("namespace") @@ -180,29 +198,30 @@ def _apply(self, etype, cr): if not ns or not name: return service_id = spec.get("serviceId") or "" + key = (plural, ns, name) with self._lock: if etype == "DELETED": - self._by_key.pop((ns, name), None) + self._by_key.pop(key, None) lst = self._by_service.get(service_id) - if lst and (ns, name) in lst: - lst.remove((ns, name)) + if lst and key in lst: + lst.remove(key) if not lst: self._by_service.pop(service_id, None) return # ADDED / MODIFIED - prev = self._by_key.get((ns, name)) + prev = self._by_key.get(key) prev_sid = (prev or {}).get("spec", {}).get("serviceId") or "" if prev_sid and prev_sid != service_id: # serviceId changed: detach from old key's index lst = self._by_service.get(prev_sid) - if lst and (ns, name) in lst: - lst.remove((ns, name)) + if lst and key in lst: + lst.remove(key) if not lst: self._by_service.pop(prev_sid, None) - self._by_key[(ns, name)] = cr + self._by_key[key] = cr lst = self._by_service.setdefault(service_id, []) - if (ns, name) not in lst: - lst.append((ns, name)) + if key not in lst: + lst.append(key) diff --git a/src/slo_api/server.py b/src/slo_api/server.py index 975b5de..bfec745 100644 --- a/src/slo_api/server.py +++ b/src/slo_api/server.py @@ -20,7 +20,7 @@ from kubernetes.client import ApiException -from slo_api import PLURAL, VERSION, group +from slo_api import VERSION, group from slo_api.translate import ( ValidationError, api_to_spec, merge_spec, spec_to_api, ) @@ -48,6 +48,21 @@ def _error(status, code, message, field=None, details=None): return status, body +def _invalid(e): + """400 for a 422 from the API server. Built from the Status causes, not + its message, which leads with the CR's kind and name; a lone cause's + field (`spec.x.y`) becomes the API's own path (`x.y`).""" + try: + causes = json.loads(e.body)["details"]["causes"] + message = "; ".join(c["message"] for c in causes) + except (TypeError, ValueError, KeyError): + return _error(400, "invalid_request", + f"kubernetes rejected the update: {e.reason}") + field = causes[0].get("field", "") if len(causes) == 1 else "" + return _error(400, "invalid_request", message, + field=field.removeprefix("spec.") if field != "spec" else None) + + def make_handler(index, api, auth_token): """Factory. All three args dependency-injectable for tests.""" @@ -170,10 +185,10 @@ def _resolve(self, route): if status == "ambiguous": return _error( 409, "conflict", - f"route '{route}' maps to multiple LLMSLORequirement CRs; " + f"route '{route}' maps to multiple SLO requirement CRs; " "cannot auto-select a target", details=[{"namespace": ns, "name": n} - for ns, n in payload]), None + for _, ns, n in payload]), None return None, payload # ---------- handlers ---------- @@ -181,19 +196,19 @@ def _resolve(self, route): def _handle_get(self, route): err, target = self._resolve(route) if err: return self._respond(*err) - ns, name, _ = target - cr, gerr = self._fresh_get(ns, name, route) + plural, ns, name, _ = target + cr, gerr = self._fresh_get(plural, ns, name, route) if gerr: return self._respond(*gerr) return self._respond(200, spec_to_api(route, cr.get("spec") or {})) - def _fresh_get(self, ns, name, route): + def _fresh_get(self, plural, ns, name, route): """Read the CR from the apiserver, bypassing the watch cache. Guarantees read-after-write consistency: callers that just modified the resource see their own writes. Returns (cr_dict, None) on success, (None, error_tuple) on failure.""" try: cr = api.get_namespaced_custom_object( - group(), VERSION, ns, PLURAL, name) + group(), VERSION, ns, plural, name) return cr, None except ApiException as e: if e.status == 404: @@ -206,7 +221,7 @@ def _fresh_get(self, ns, name, route): def _handle_put(self, route): err, target = self._resolve(route) if err: return self._respond(*err) - ns, name, _ = target + plural, ns, name, _ = target body, perr = self._read_json() if perr: return self._respond(*perr) @@ -217,14 +232,14 @@ def _handle_put(self, route): 400, "invalid_request", "body route does not match URL", field="route")) try: - update = api_to_spec(body) + update = api_to_spec(body, plural) except ValidationError as e: return self._respond(*_error( 400, "invalid_request", e.message, field=e.field)) # Read the live CR (not the watch cache) so we merge over the # freshest state and never silently revert a concurrent write. - cr, gerr = self._fresh_get(ns, name, route) + cr, gerr = self._fresh_get(plural, ns, name, route) if gerr: return self._respond(*gerr) existing = cr.get("spec") or {} merged = merge_spec(existing, update) @@ -232,7 +247,7 @@ def _handle_put(self, route): try: api.patch_namespaced_custom_object( - group(), VERSION, ns, PLURAL, name, + group(), VERSION, ns, plural, name, {"spec": merged}, ) except ApiException as e: @@ -241,6 +256,10 @@ def _handle_put(self, route): return self._respond(*_error( 404, "not_found", f"CR for route '{route}' disappeared during update")) + if e.status == 422: + # The CRD's own rules (CEL) rejected the merged spec, e.g. + # min/max bound types that differ: the caller's to fix. + return self._respond(*_invalid(e)) return self._respond(*_error( 502, "upstream_error", f"kubernetes patch failed: {e.reason}")) @@ -252,17 +271,19 @@ def _handle_put(self, route): def _handle_delete(self, route): err, target = self._resolve(route) if err: return self._respond(*err) - ns, name, _ = target + plural, ns, name, _ = target # Read the live CR (not the watch cache) so we know exactly # which SLO fields exist and need nulling out. - cr, gerr = self._fresh_get(ns, name, route) + cr, gerr = self._fresh_get(plural, ns, name, route) if gerr: return self._respond(*gerr) # Reset SLO-owned fields to their CRD defaults. priority and # minimumDeployment have explicit CRD defaults; the other # three have none, so "reset" = absent. JSON merge-patch only - # removes keys sent explicitly as null. + # removes keys sent explicitly as null, and nulls a key the CR + # does not have as a no-op: on a JobSLORequirement ttft/otps are + # nothing, and its queue (required, no default) is kept. patch = {"spec": { "priority": 0, "minimumDeployment": {"type": "replica", "value": 1}, @@ -273,7 +294,7 @@ def _handle_delete(self, route): try: api.patch_namespaced_custom_object( - group(), VERSION, ns, PLURAL, name, patch, + group(), VERSION, ns, plural, name, patch, ) except ApiException as e: log.warning("reset %s/%s failed: %s %s", ns, name, e.status, e.reason) @@ -310,10 +331,10 @@ def _handle_list(self, params): # not 500'd — list offers a "what exists" view, not # target resolution continue - _, _, cr = payload + *_, cr = payload summary = spec_to_api(r, cr.get("spec") or {}) - summary.pop("ttft", None) - summary.pop("otps", None) + for key in ("ttft", "otps", "queue"): + summary.pop(key, None) items.append(summary) return self._respond(200, { diff --git a/src/slo_api/static/index.html b/src/slo_api/static/index.html index 6eb493b..e9c8118 100644 --- a/src/slo_api/static/index.html +++ b/src/slo_api/static/index.html @@ -113,15 +113,15 @@