From 702e6a8596b751492d7df031ae5ad3ab7fb41e08 Mon Sep 17 00:00:00 2001 From: DoL <145818730+dolzhuying@users.noreply.github.com> Date: Sun, 4 May 2025 13:24:00 +0800 Subject: [PATCH] fix(api): some api's logic and response status code (#92) --- crates/provider/src/handlers/delete.rs | 52 +++++++------ crates/provider/src/handlers/deploy.rs | 73 +++++++++---------- crates/provider/src/handlers/function_list.rs | 39 +++++----- .../provider/src/handlers/invoke_resolver.rs | 4 +- crates/provider/src/proxy/proxy_handler.rs | 26 +++---- 5 files changed, 96 insertions(+), 98 deletions(-) diff --git a/crates/provider/src/handlers/delete.rs b/crates/provider/src/handlers/delete.rs index b8cd6e1..6d8499b 100644 --- a/crates/provider/src/handlers/delete.rs +++ b/crates/provider/src/handlers/delete.rs @@ -2,10 +2,14 @@ use crate::{ consts, handlers::{function_get::get_function, utils::CustomError}, }; -use actix_web::{HttpResponse, Responder, ResponseError, error, web}; +use actix_web::{HttpResponse, Responder, web}; use serde::{Deserialize, Serialize}; use service::containerd_manager::ContainerdManager; +use super::function_list::Function; + +// 参考响应状态：https://github.com/openfaas/faas/blob/7803ea1861f2a22adcbcfa8c79ed539bc6506d5b/api-docs/spec.openapi.yml#L141C2-L162C45 +// 请求体反序列化失败，自动返回400错误 pub async fn delete_handler(info: web::Json) -> impl Responder { let function_name = info.function_name.clone(); let namespace = info @@ -13,44 +17,44 @@ pub async fn delete_handler(info: web::Json) -> impl Respon .clone() .unwrap_or_else(|| consts::DEFAULT_FUNCTION_NAMESPACE.to_string()); - match delete(&function_name, &namespace).await { + let namespaces = ContainerdManager::list_namespaces().await.unwrap(); + if !namespaces.contains(&namespace.to_string()) { + return HttpResponse::NotFound().body(format!("Namespace '{}' does not exist", namespace)); + } + + let function = match get_function(&function_name, &namespace).await { + Ok(function) => function, + Err(e) => { + log::error!("Failed to get function: {}", e); + return HttpResponse::NotFound() + .body(format!("Function '{}' not found ", function_name)); + } + }; + + match delete(&function, &namespace).await { Ok(()) => { HttpResponse::Ok().body(format!("Function {} deleted successfully.", function_name)) } - Err(e) => e.error_response(), + Err(e) => { + HttpResponse::InternalServerError().body(format!("Failed to delete function: {}", e)) + } } } -async fn delete(function_name: &str, namespace: &str) -> Result<(), CustomError> { - let namespaces = ContainerdManager::list_namespaces().await.unwrap(); - if !namespaces.contains(&namespace.to_string()) { - return Err(CustomError::ActixError(error::ErrorBadRequest(format!( - "Namespace '{}' not valid or does not exist", - namespace - )))); - } - let function = get_function(function_name, namespace).await.map_err(|e| { - log::error!("Failed to get function: {}", e); - CustomError::ActixError(error::ErrorNotFound(format!( - "Function '{}' not found in namespace '{}'", - function_name, namespace - ))) - })?; +async fn delete(function: &Function, namespace: &str) -> Result<(), CustomError> { + let function_name = function.name.clone(); if function.replicas != 0 { log::info!("function.replicas: {:?}", function.replicas); - cni::delete_cni_network(namespace, function_name); + cni::delete_cni_network(namespace, &function_name); log::info!("delete_cni_network ok"); } else { log::info!("function.replicas: {:?}", function.replicas); } - ContainerdManager::delete_container(function_name, namespace) + ContainerdManager::delete_container(&function_name, namespace) .await .map_err(|e| { log::error!("Failed to delete container: {}", e); - CustomError::ActixError(error::ErrorInternalServerError(format!( - "Failed to delete container: {}", - e - ))) + CustomError::OtherError(format!("Failed to delete container: {}", e)) })?; Ok(()) } diff --git a/crates/provider/src/handlers/deploy.rs b/crates/provider/src/handlers/deploy.rs index e706ad4..ced7e24 100644 --- a/crates/provider/src/handlers/deploy.rs +++ b/crates/provider/src/handlers/deploy.rs @@ -1,12 +1,10 @@ -use crate::{ - consts, - handlers::utils::CustomError, - types::function_deployment::{DeployFunctionInfo, FunctionDeployment}, -}; +use crate::{consts, handlers::utils::CustomError, types::function_deployment::DeployFunctionInfo}; use actix_web::{HttpResponse, Responder, web}; use service::{containerd_manager::ContainerdManager, image_manager::ImageManager}; +// 参考响应状态 https://github.com/openfaas/faas/blob/7803ea1861f2a22adcbcfa8c79ed539bc6506d5b/api-docs/spec.openapi.yml#L121C1-L140C45 +// 请求体反序列化失败，自动返回400错误 pub async fn deploy_handler(info: web::Json) -> impl Responder { let image = info.image.clone(); let function_name = info.function_name.clone(); @@ -15,69 +13,64 @@ pub async fn deploy_handler(info: web::Json) -> impl Respond .clone() .unwrap_or(consts::DEFAULT_FUNCTION_NAMESPACE.to_string()); - let config = FunctionDeployment { - service: function_name, - image, - namespace: Some(namespace), + log::info!("Namespace '{}' validated.", &namespace); + + let container_list = match ContainerdManager::list_container_into_string(&namespace).await { + Ok(container_list) => container_list, + Err(e) => { + log::error!("Failed to list container: {}", e); + return HttpResponse::InternalServerError() + .body(format!("Failed to list container: {}", e)); + } }; - match deploy(&config).await { + if container_list.contains(&function_name) { + return HttpResponse::BadRequest().body(format!( + "Function '{}' already exists in namespace '{}'", + function_name, namespace + )); + } + + match deploy(&function_name, &image, &namespace).await { Ok(()) => HttpResponse::Accepted().body(format!( - "Function {} deployment initiated successfully.", - config.service + "Function {} deployment initiated successfully .", + function_name )), - Err(e) => HttpResponse::InternalServerError().body(format!( + Err(e) => HttpResponse::BadRequest().body(format!( "failed to deploy function {}, because {}", - config.service, e + function_name, e )), } } -async fn deploy(config: &FunctionDeployment) -> Result<(), CustomError> { - let namespace = config.namespace.clone().unwrap(); - - log::info!( - "Namespace '{}' validated.", - config.namespace.clone().unwrap() - ); - - let container_list = ContainerdManager::list_container_into_string(&namespace) - .await - .map_err(|e| CustomError::OtherError(format!("failed to list container:{}", e)))?; - - if container_list.contains(&config.service) { - return Err(CustomError::OtherError( - "container has been existed".to_string(), - )); - } - - ImageManager::prepare_image(&config.image, &namespace, true) +async fn deploy(function_name: &str, image: &str, namespace: &str) -> Result<(), CustomError> { + ImageManager::prepare_image(image, namespace, true) .await .map_err(CustomError::from)?; - log::info!("Image '{}' validated ,", &config.image); + log::info!("Image '{}' validated ,", image); - ContainerdManager::create_container(&config.image, &config.service, &namespace) + ContainerdManager::create_container(image, function_name, namespace) .await .map_err(|e| CustomError::OtherError(format!("failed to create container:{}", e)))?; log::info!( "Container {} created using image {} in namespace {}", - &config.service, - &config.image, + function_name, + image, namespace ); - ContainerdManager::new_task(&config.service, &namespace) + ContainerdManager::new_task(function_name, namespace) .await .map_err(|e| { CustomError::OtherError(format!( "failed to start task for container {},{}", - &config.service, e + function_name, e )) })?; log::info!( "Task for container {} was created successfully", - &config.service + function_name ); Ok(()) diff --git a/crates/provider/src/handlers/function_list.rs b/crates/provider/src/handlers/function_list.rs index 397e0bf..54f53d6 100644 --- a/crates/provider/src/handlers/function_list.rs +++ b/crates/provider/src/handlers/function_list.rs @@ -23,43 +23,44 @@ pub struct Function { pub created_at: SystemTime, } +// openfaas API文档和faasd源码的响应不能完全对齐，这里参考源码的响应码设置 +// 考虑到部分操作可能返回500错误，但是faasd并没有做internal server error的处理（可能上层有中间件捕获），这里应该需要做500的处理 pub async fn function_list_handler(req: HttpRequest) -> impl Responder { let namespace = req.match_info().get("namespace").unwrap_or(""); if namespace.is_empty() { return HttpResponse::BadRequest().body("provide namespace in path"); } - match get_function_list(namespace).await { - Ok(functions) => HttpResponse::Ok().body(serde_json::to_string(&functions).unwrap()), - Err(e) => HttpResponse::from_error(e), - } -} - -async fn get_function_list(namespace: &str) -> Result, CustomError> { let namespaces = match ContainerdManager::list_namespaces().await { Ok(namespace) => namespace, Err(e) => { - return Err(CustomError::OtherError(format!( - "Failed to list namespaces:{}", - e - ))); + return HttpResponse::InternalServerError() + .body(format!("Failed to list namespaces:{}", e)); } }; if !namespaces.contains(&namespace.to_string()) { - return Err(CustomError::OtherError(format!( - "Namespace '{}' not valid or does not exist", - namespace - ))); + return HttpResponse::BadRequest() + .body(format!("Namespace '{}' does not exist", namespace)); } + let container_list = match ContainerdManager::list_container_into_string(namespace).await { Ok(container_list) => container_list, Err(e) => { - return Err(CustomError::OtherError(format!( - "Failed to list container:{}", - e - ))); + return HttpResponse::InternalServerError() + .body(format!("Failed to list container:{}", e)); } }; log::info!("container_list: {:?}", container_list); + + match get_function_list(container_list, namespace).await { + Ok(functions) => HttpResponse::Ok().body(serde_json::to_string(&functions).unwrap()), + Err(e) => HttpResponse::BadRequest().body(format!("Failed to get function list: {}", e)), + } +} + +async fn get_function_list( + container_list: Vec, + namespace: &str, +) -> Result, CustomError> { let mut functions: Vec = Vec::new(); for cid in container_list { log::info!("cid: {}", cid); diff --git a/crates/provider/src/handlers/invoke_resolver.rs b/crates/provider/src/handlers/invoke_resolver.rs index 487a875..bef03fd 100644 --- a/crates/provider/src/handlers/invoke_resolver.rs +++ b/crates/provider/src/handlers/invoke_resolver.rs @@ -1,6 +1,6 @@ use crate::consts::DEFAULT_FUNCTION_NAMESPACE; use crate::handlers::function_get::get_function; -use actix_web::{Error, error::ErrorInternalServerError}; +use actix_web::{Error, error::ErrorInternalServerError, error::ErrorServiceUnavailable}; use log; use url::Url; @@ -23,7 +23,7 @@ impl InvokeResolver { Ok(function) => function, Err(e) => { log::error!("Failed to get function:{}", e); - return Err(ErrorInternalServerError("Failed to get function")); + return Err(ErrorServiceUnavailable("Failed to get function")); } }; log::info!("Function:{:?}", function); diff --git a/crates/provider/src/proxy/proxy_handler.rs b/crates/provider/src/proxy/proxy_handler.rs index 4fcce1b..67e5da8 100644 --- a/crates/provider/src/proxy/proxy_handler.rs +++ b/crates/provider/src/proxy/proxy_handler.rs @@ -2,13 +2,19 @@ use crate::handlers::invoke_resolver::InvokeResolver; use crate::proxy::builder::build_proxy_request; use crate::proxy::client::new_proxy_client_from_config; use crate::types::config::FaaSConfig; -use actix_web::{Error, HttpRequest, HttpResponse, Responder, http::Method, web}; +use actix_web::{ + Error, HttpRequest, HttpResponse, + error::{ErrorBadRequest, ErrorInternalServerError, ErrorMethodNotAllowed}, + http::Method, + web, +}; +// 主要参考源码的响应设置 pub async fn proxy_handler( config: web::Data, req: HttpRequest, payload: web::Payload, -) -> impl Responder { +) -> Result { let proxy_client = new_proxy_client_from_config(config.as_ref()).await; log::info!("proxy_client : {:?}", proxy_client); @@ -19,11 +25,8 @@ pub async fn proxy_handler( | Method::GET | Method::PATCH | Method::HEAD - | Method::OPTIONS => match proxy_request(&req, payload, &proxy_client).await { - Ok(resp) => resp, - Err(e) => HttpResponse::from_error(e), - }, - _ => HttpResponse::MethodNotAllowed().body("method not allowed"), + | Method::OPTIONS => proxy_request(&req, payload, &proxy_client).await, + _ => Err(ErrorMethodNotAllowed("method not allowed")), } } @@ -35,13 +38,10 @@ async fn proxy_request( ) -> Result { let function_name = req.match_info().get("name").unwrap_or(""); if function_name.is_empty() { - return Ok(HttpResponse::BadRequest().body("provide function name in path")); + return Err(ErrorBadRequest("function name is required")); } - let function_addr = match InvokeResolver::resolve_function_url(function_name).await { - Ok(function_addr) => function_addr, - Err(e) => return Ok(HttpResponse::BadRequest().body(e.to_string())), - }; + let function_addr = InvokeResolver::resolve_function_url(function_name).await?; let proxy_req = build_proxy_request(req, &function_addr, proxy_client, payload).await?; @@ -58,6 +58,6 @@ async fn proxy_request( Ok(client_resp.body(body)) } - Err(e) => Ok(HttpResponse::BadGateway().body(e.to_string())), + Err(e) => Err(ErrorInternalServerError(e)), } }