Use precondition failed code in delete_timeline when tenant is missing (#3884)

This allows client to differentiate between missing tenant and missing
timeline cases

libs/utils/src/http/error.rs

@@ -20,6 +20,9 @@ pub enum ApiError {
     #[error("Conflict: {0}")]
     Conflict(String),
 
+    #[error("Precondition failed: {0}")]
+    PreconditionFailed(&'static str),
+
     #[error(transparent)]
     InternalServerError(anyhow::Error),
 }
@@ -44,6 +47,10 @@ impl ApiError {
             ApiError::Conflict(_) => {
                 HttpErrorBody::response_from_msg_and_status(self.to_string(), StatusCode::CONFLICT)
             }
+            ApiError::PreconditionFailed(_) => HttpErrorBody::response_from_msg_and_status(
+                self.to_string(),
+                StatusCode::PRECONDITION_FAILED,
+            ),
             ApiError::InternalServerError(err) => HttpErrorBody::response_from_msg_and_status(
                 err.to_string(),
                 StatusCode::INTERNAL_SERVER_ERROR,

pageserver/src/http/openapi_spec.yml

@@ -189,6 +189,13 @@ paths:
             application/json:
               schema:
                 $ref: "#/components/schemas/NotFoundError"
+        "412":
+          description: Tenant is missing
+          content:
+            application/json:
+              schema:
+                $ref: "#/components/schemas/PreconditionFailedError"
+
         "500":
           description: Generic operation error
           content:
@@ -958,6 +965,13 @@ components:
       properties:
         msg:
           type: string
+    PreconditionFailedError:
+      type: object
+      required:
+        - msg
+      properties:
+        msg:
+          type: string
 
 security:
   - JWT: []

pageserver/src/http/routes.rs

@@ -148,6 +148,11 @@ impl From<crate::tenant::mgr::DeleteTimelineError> for ApiError {
     fn from(value: crate::tenant::mgr::DeleteTimelineError) -> Self {
         use crate::tenant::mgr::DeleteTimelineError::*;
         match value {
+            // Report Precondition failed so client can distinguish between
+            // "tenant is missing" case from "timeline is missing"
+            Tenant(TenantStateError::NotFound(..)) => {
+                ApiError::PreconditionFailed("Requested tenant is missing")
+            }
             Tenant(t) => ApiError::from(t),
             Timeline(t) => ApiError::from(t),
         }

test_runner/regress/test_timeline_delete.py

@@ -10,7 +10,7 @@ def test_timeline_delete(neon_simple_env: NeonEnv):
     env.pageserver.allowed_errors.append(".*Timeline .* was not found.*")
     env.pageserver.allowed_errors.append(".*timeline not found.*")
     env.pageserver.allowed_errors.append(".*Cannot delete timeline which has child timelines.*")
-    env.pageserver.allowed_errors.append(".*NotFound: tenant .*")
+    env.pageserver.allowed_errors.append(".*Precondition failed: Requested tenant is missing.*")
 
     ps_http = env.pageserver.http_client()
 
@@ -24,11 +24,11 @@ def test_timeline_delete(neon_simple_env: NeonEnv):
     invalid_tenant_id = TenantId.generate()
     with pytest.raises(
         PageserverApiException,
-        match=f"NotFound: tenant {invalid_tenant_id}",
+        match="Precondition failed: Requested tenant is missing",
     ) as exc:
         ps_http.timeline_delete(tenant_id=invalid_tenant_id, timeline_id=invalid_timeline_id)
 
-    assert exc.value.status_code == 404
+    assert exc.value.status_code == 412
 
     # construct pair of branches to validate that pageserver prohibits
     # deletion of ancestor timelines when they have child branches