From 260fc97c1254f2ebf247a468cb393621a619f171 Mon Sep 17 00:00:00 2001 From: chesedo Date: Thu, 23 Nov 2023 17:32:47 +0000 Subject: [PATCH 01/19] tests: deleting a project that has resources --- Cargo.lock | 4 + common-tests/src/lib.rs | 1 + common-tests/src/resource_recorder.rs | 116 ++++++++++++++++++++++++++ gateway/Cargo.toml | 4 + gateway/src/api/latest.rs | 11 +++ gateway/src/args.rs | 3 + gateway/src/lib.rs | 103 ++++++++++++++++++++++- gateway/src/project.rs | 3 + gateway/src/service.rs | 12 +++ 9 files changed, 254 insertions(+), 3 deletions(-) create mode 100644 common-tests/src/resource_recorder.rs diff --git a/Cargo.lock b/Cargo.lock index 51a5157ac..1752bbb99 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5955,6 +5955,7 @@ dependencies = [ "chrono", "clap", "colored", + "flate2", "fqdn", "futures", "http", @@ -5974,16 +5975,19 @@ dependencies = [ "rcgen", "reqwest", "ring 0.17.5", + "rmp-serde", "rustls", "rustls-pemfile", "serde", "serde_json", "shuttle-common", + "shuttle-common-tests", "shuttle-orchestrator", "shuttle-proto", "snailquote", "sqlx", "strum 0.25.0", + "tar", "tempfile", "test-context", "tokio", diff --git a/common-tests/src/lib.rs b/common-tests/src/lib.rs index db33ef72a..3a69e1123 100644 --- a/common-tests/src/lib.rs +++ b/common-tests/src/lib.rs @@ -1,6 +1,7 @@ pub mod builder; pub mod cargo_shuttle; pub mod logger; +pub mod resource_recorder; use shuttle_common::claims::{AccountTier, Claim, Scope}; diff --git a/common-tests/src/resource_recorder.rs b/common-tests/src/resource_recorder.rs new file mode 100644 index 000000000..573cd8ece --- /dev/null +++ b/common-tests/src/resource_recorder.rs @@ -0,0 +1,116 @@ +use std::{ + net::{Ipv4Addr, SocketAddr}, + sync::Mutex, +}; + +use portpicker::pick_unused_port; +use shuttle_proto::resource_recorder::{ + resource_recorder_server::{ResourceRecorder, ResourceRecorderServer}, + ProjectResourcesRequest, RecordRequest, Resource, ResourceIds, ResourceResponse, + ResourcesResponse, ResultResponse, ServiceResourcesRequest, +}; +use tonic::{async_trait, transport::Server, Request, Response, Status}; + +struct MockedResourceRecorder { + resources: Mutex>, +} + +#[async_trait] +impl ResourceRecorder for MockedResourceRecorder { + async fn record_resources( + &self, + request: Request, + ) -> Result, Status> { + println!("recording resources"); + let RecordRequest { + project_id, + service_id, + resources, + } = request.into_inner(); + + let mut resources = resources + .into_iter() + .map(|r| Resource { + project_id: project_id.clone(), + service_id: service_id.clone(), + r#type: r.r#type, + config: r.config, + data: r.data, + is_active: true, + created_at: None, + last_updated: None, + }) + .collect(); + + self.resources.lock().unwrap().append(&mut resources); + + Ok(Response::new(ResultResponse { + success: true, + message: Default::default(), + })) + } + + async fn get_project_resources( + &self, + _request: Request, + ) -> Result, Status> { + println!("getting project resources"); + Ok(Response::new(Default::default())) + } + + async fn get_service_resources( + &self, + request: Request, + ) -> Result, Status> { + println!("getting service resources"); + + let ServiceResourcesRequest { service_id } = request.into_inner(); + let resources = self + .resources + .lock() + .unwrap() + .iter() + .filter(|r| r.service_id == service_id) + .cloned() + .collect(); + + Ok(Response::new(ResourcesResponse { + success: true, + message: Default::default(), + resources, + })) + } + + async fn get_resource( + &self, + _request: tonic::Request, + ) -> Result, Status> { + println!("getting resources"); + Ok(Response::new(Default::default())) + } + + async fn delete_resource( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new(Default::default())) + } +} + +/// Start a mocked resource recorder and return the address it started on +pub async fn start_mocked_resource_recorder() -> u16 { + let resource_recorder = MockedResourceRecorder { + resources: Mutex::new(Vec::new()), + }; + + let port = pick_unused_port().unwrap(); + let resource_recorder_addr = SocketAddr::new(Ipv4Addr::UNSPECIFIED.into(), port); + tokio::spawn(async move { + Server::builder() + .add_service(ResourceRecorderServer::new(resource_recorder)) + .serve(resource_recorder_addr) + .await + }); + + port +} diff --git a/gateway/Cargo.toml b/gateway/Cargo.toml index 8493d8f84..4e66c8abe 100644 --- a/gateway/Cargo.toml +++ b/gateway/Cargo.toml @@ -61,9 +61,13 @@ ulid = { workspace = true, features = ["serde"] } [dev-dependencies] anyhow = { workspace = true } colored = { workspace = true } +flate2 = { workspace = true } jsonwebtoken = { workspace = true } portpicker = { workspace = true } ring = { workspace = true } +rmp-serde = { workspace = true } +shuttle-common-tests = { workspace = true } snailquote = "0.3.1" +tar = { workspace = true } tempfile = { workspace = true } test-context = "0.1.4" diff --git a/gateway/src/api/latest.rs b/gateway/src/api/latest.rs index d052cb4bd..e8154125b 100644 --- a/gateway/src/api/latest.rs +++ b/gateway/src/api/latest.rs @@ -1402,6 +1402,17 @@ pub mod tests { Ok(()) } + #[test_context(TestProject)] + #[tokio::test] + async fn api_delete_project_that_has_resources( + project: &mut TestProject, + ) -> anyhow::Result<()> { + project.deploy("../examples/rocket/secrets").await; + project.router_call(Method::DELETE, "/delete").await; + + Ok(()) + } + #[tokio::test(flavor = "multi_thread")] async fn status() { let world = World::new().await; diff --git a/gateway/src/args.rs b/gateway/src/args.rs index 4448a3108..607f5dea8 100644 --- a/gateway/src/args.rs +++ b/gateway/src/args.rs @@ -62,6 +62,9 @@ pub struct ContextArgs { /// Address to reach the authentication service at #[arg(long, default_value = "http://127.0.0.1:8008")] pub auth_uri: Uri, + /// Address to reach the resource recorder service at + #[arg(long, default_value = "http://resource-recorder:8000")] + pub resource_recorder_uri: Uri, /// The Docker Network name in which to deploy user runtimes #[arg(long, default_value = "shuttle_default")] pub network_name: String, diff --git a/gateway/src/lib.rs b/gateway/src/lib.rs index 396a613e8..c7da5a06d 100644 --- a/gateway/src/lib.rs +++ b/gateway/src/lib.rs @@ -254,6 +254,7 @@ pub trait Refresh: Sized { pub mod tests { use std::collections::HashMap; use std::env; + use std::fs::{canonicalize, read_dir}; use std::net::SocketAddr; use std::str::FromStr; use std::sync::{Arc, Mutex}; @@ -265,6 +266,8 @@ pub mod tests { use axum::routing::get; use axum::{extract, Router, TypedHeader}; use bollard::Docker; + use flate2::write::GzEncoder; + use flate2::Compression; use fqdn::FQDN; use futures::prelude::*; use http::Method; @@ -277,7 +280,9 @@ pub mod tests { use ring::signature::{self, Ed25519KeyPair, KeyPair}; use shuttle_common::backends::auth::ConvertResponse; use shuttle_common::claims::{AccountTier, Claim, Scope}; - use shuttle_common::models::project; + use shuttle_common::models::deployment::DeploymentRequest; + use shuttle_common::models::{project, service}; + use shuttle_common_tests::resource_recorder::start_mocked_resource_recorder; use sqlx::sqlite::SqliteConnectOptions; use sqlx::SqlitePool; use test_context::AsyncTestContext; @@ -514,6 +519,7 @@ pub mod tests { let bouncer = format!("127.0.0.1:{bouncer}").parse().unwrap(); let auth: SocketAddr = format!("0.0.0.0:{auth_port}").parse().unwrap(); let auth_uri: Uri = format!("http://{auth}").parse().unwrap(); + let resource_recorder_port = start_mocked_resource_recorder().await; let auth_service = AuthService::new(auth); auth_service @@ -528,7 +534,7 @@ pub mod tests { ); let image = env::var("SHUTTLE_TESTS_RUNTIME_IMAGE") - .unwrap_or_else(|_| "public.ecr.aws/shuttle/deployer:latest".to_string()); + .unwrap_or_else(|_| "public.ecr.aws/shuttle-dev/deployer:latest".to_string()); let network_name = env::var("SHUTTLE_TESTS_NETWORK").unwrap_or_else(|_| "shuttle_default".to_string()); @@ -563,6 +569,11 @@ pub mod tests { auth_uri: format!("http://host.docker.internal:{auth_port}") .parse() .unwrap(), + resource_recorder_uri: format!( + "http://host.docker.internal:{resource_recorder_port}" + ) + .parse() + .unwrap(), network_name, proxy_fqdn: FQDN::from_str("test.shuttleapp.rs").unwrap(), deploys_api_key: "gateway".to_string(), @@ -866,6 +877,87 @@ pub mod tests { .await .unwrap(); } + + /// Deploy the code at the path to the project + pub async fn deploy(&mut self, path: &str) { + let path = canonicalize(path).expect("deploy path to be valid"); + let name = path.file_name().unwrap().to_str().unwrap(); + let enc = GzEncoder::new(Vec::new(), Compression::fast()); + let mut tar = tar::Builder::new(enc); + + for dir_entry in read_dir(&path).unwrap() { + let dir_entry = dir_entry.unwrap(); + if dir_entry.file_name() != "target" { + let path = format!("{name}/{}", dir_entry.file_name().to_str().unwrap()); + + if dir_entry.file_type().unwrap().is_dir() { + tar.append_dir_all(path, dir_entry.path()).unwrap(); + } else { + tar.append_path_with_name(dir_entry.path(), path).unwrap(); + } + } + } + + let enc = tar.into_inner().unwrap(); + let bytes = enc.finish().unwrap(); + + println!("{name}: finished getting archive for test"); + + let project_name = &self.project_name; + let deployment_req = rmp_serde::to_vec(&DeploymentRequest { + data: bytes, + no_test: true, + ..Default::default() + }) + .expect("to serialize DeploymentRequest as a MessagePack byte vector"); + + self.router + .call( + Request::builder() + .method(Method::POST) + .header("Transfer-Encoding", "chunked") + .uri(format!("/projects/{project_name}/services/{project_name}")) + .body(deployment_req.into()) + .unwrap() + .with_header(&self.authorization), + ) + .map_ok(|resp| { + assert_eq!(resp.status(), StatusCode::OK); + }) + .await + .unwrap(); + + // Wait for deployment to be up + let mut tries = 0; + + loop { + let resp = self + .router + .call( + Request::get(format!("/projects/{project_name}/services/{project_name}")) + .with_header(&self.authorization) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(resp.status(), StatusCode::OK); + let body = hyper::body::to_bytes(resp.into_body()).await.unwrap(); + let service: service::Summary = serde_json::from_slice(&body).unwrap(); + + if service.deployment.is_some() { + break; + } + + tries += 1; + if tries > 240 { + panic!("timed out waiting for deployment"); + } + + sleep(Duration::from_secs(1)).await; + } + } } #[async_trait] @@ -880,7 +972,12 @@ pub mod tests { } async fn teardown(mut self) { - assert!(self.is_missing().await, "test left a dangling project"); + let dangling = !self.is_missing().await; + + if dangling { + self.router_call(Method::DELETE, "/delete").await; + panic!("test left a dangling project which you might need to clean manually"); + } } } diff --git a/gateway/src/project.rs b/gateway/src/project.rs index b13d16489..71ee07238 100644 --- a/gateway/src/project.rs +++ b/gateway/src/project.rs @@ -795,6 +795,7 @@ impl ProjectCreating { provisioner_host, builder_host, auth_uri, + resource_recorder_uri, fqdn: public, extra_hosts, .. @@ -849,6 +850,8 @@ impl ProjectCreating { auth_uri, "--builder-uri", format!("http://{builder_host}:8000"), + "--resource-recorder", + resource_recorder_uri, "--project-id", self.project_id.to_string() ], diff --git a/gateway/src/service.rs b/gateway/src/service.rs index bbb74135a..c0e00a304 100644 --- a/gateway/src/service.rs +++ b/gateway/src/service.rs @@ -63,6 +63,7 @@ pub struct ContainerSettingsBuilder { provisioner: Option, builder: Option, auth_uri: Option, + resource_recorder_uri: Option, network_name: Option, fqdn: Option, extra_hosts: Option>, @@ -82,6 +83,7 @@ impl ContainerSettingsBuilder { provisioner: None, builder: None, auth_uri: None, + resource_recorder_uri: None, network_name: None, fqdn: None, extra_hosts: None, @@ -95,6 +97,7 @@ impl ContainerSettingsBuilder { provisioner_host, builder_host, auth_uri, + resource_recorder_uri, image, proxy_fqdn, extra_hosts, @@ -105,6 +108,7 @@ impl ContainerSettingsBuilder { .provisioner_host(provisioner_host) .builder_host(builder_host) .auth_uri(auth_uri) + .resource_recorder_uri(resource_recorder_uri) .network_name(network_name) .fqdn(proxy_fqdn) .extra_hosts(extra_hosts) @@ -137,6 +141,11 @@ impl ContainerSettingsBuilder { self } + pub fn resource_recorder_uri(mut self, resource_recorder_uri: S) -> Self { + self.resource_recorder_uri = Some(resource_recorder_uri.to_string()); + self + } + pub fn network_name(mut self, name: S) -> Self { self.network_name = Some(name.to_string()); self @@ -158,6 +167,7 @@ impl ContainerSettingsBuilder { let provisioner_host = self.provisioner.take().unwrap(); let builder_host = self.builder.take().unwrap(); let auth_uri = self.auth_uri.take().unwrap(); + let resource_recorder_uri = self.resource_recorder_uri.take().unwrap(); let extra_hosts = self.extra_hosts.take().unwrap(); let network_name = self.network_name.take().unwrap(); @@ -169,6 +179,7 @@ impl ContainerSettingsBuilder { provisioner_host, builder_host, auth_uri, + resource_recorder_uri, network_name, fqdn, extra_hosts, @@ -183,6 +194,7 @@ pub struct ContainerSettings { pub provisioner_host: String, pub builder_host: String, pub auth_uri: String, + pub resource_recorder_uri: String, pub network_name: String, pub fqdn: String, pub extra_hosts: Vec, From 72edee86b91ad9d016889b58e973837856c4ea67 Mon Sep 17 00:00:00 2001 From: chesedo Date: Thu, 23 Nov 2023 22:36:58 +0000 Subject: [PATCH 02/19] feat: remove resources when deleting project --- common-tests/src/resource_recorder.rs | 45 ++++++++++++++++++++++++--- gateway/src/api/latest.rs | 42 ++++++++++++++++--------- gateway/src/lib.rs | 24 ++++++++++++++ 3 files changed, 91 insertions(+), 20 deletions(-) diff --git a/common-tests/src/resource_recorder.rs b/common-tests/src/resource_recorder.rs index 573cd8ece..bea588289 100644 --- a/common-tests/src/resource_recorder.rs +++ b/common-tests/src/resource_recorder.rs @@ -22,6 +22,7 @@ impl ResourceRecorder for MockedResourceRecorder { request: Request, ) -> Result, Status> { println!("recording resources"); + let RecordRequest { project_id, service_id, @@ -83,17 +84,51 @@ impl ResourceRecorder for MockedResourceRecorder { async fn get_resource( &self, - _request: tonic::Request, + request: tonic::Request, ) -> Result, Status> { - println!("getting resources"); - Ok(Response::new(Default::default())) + println!("getting resource"); + + let ResourceIds { + project_id, + service_id, + r#type, + } = request.into_inner(); + let resource = self + .resources + .lock() + .unwrap() + .iter() + .find(|r| { + r.project_id == project_id && r.service_id == service_id && r.r#type == r#type + }) + .cloned(); + + Ok(Response::new(ResourceResponse { + success: resource.is_some(), + message: Default::default(), + resource, + })) } async fn delete_resource( &self, - _request: tonic::Request, + request: tonic::Request, ) -> Result, Status> { - Ok(Response::new(Default::default())) + println!("delete resource"); + + let ResourceIds { + project_id, + service_id, + r#type, + } = request.into_inner(); + self.resources.lock().unwrap().retain(|r| { + !(r.project_id == project_id && r.service_id == service_id && r.r#type == r#type) + }); + + Ok(Response::new(ResultResponse { + success: true, + message: Default::default(), + })) } } diff --git a/gateway/src/api/latest.rs b/gateway/src/api/latest.rs index e8154125b..6e1c2a17f 100644 --- a/gateway/src/api/latest.rs +++ b/gateway/src/api/latest.rs @@ -14,7 +14,7 @@ use axum::routing::{any, delete, get, post}; use axum::{Json as AxumJson, Router}; use fqdn::FQDN; use futures::Future; -use http::{StatusCode, Uri}; +use http::{Method, StatusCode, Uri}; use instant_acme::{AccountCredentials, ChallengeType}; use serde::{Deserialize, Serialize}; use shuttle_common::backends::auth::{AuthPublicKey, JwtAuthenticationLayer, ScopedLayer}; @@ -370,7 +370,7 @@ async fn delete_project( } // check if database in resources - let mut rb = hyper::Request::builder(); + let mut rb = Request::builder(); rb.headers_mut().unwrap().clone_from(req.headers()); let resource_req = rb .uri( @@ -381,7 +381,7 @@ async fn delete_project( .method("GET") .body(hyper::Body::empty()) .unwrap(); - let res = route_project(State(state.clone()), scoped_user, resource_req).await?; + let res = route_project(State(state.clone()), scoped_user.clone(), resource_req).await?; // 404 == no service == no resources if res.status() != StatusCode::NOT_FOUND { if res.status() != StatusCode::OK { @@ -394,20 +394,31 @@ async fn delete_project( serde_json::from_slice(&body_bytes) .map_err(|e| Error::source(ErrorKind::Internal, e))?; - let resources = resources - .into_iter() - .filter(|resource| { - matches!( - resource.r#type, - shuttle_common::resource::Type::Database(_) - | shuttle_common::resource::Type::Secrets + let mut delete_fails = Vec::new(); + + for resource in resources { + let resource_type = resource.r#type.to_string(); + let mut rb = Request::builder(); + rb.headers_mut().unwrap().clone_from(req.headers()); + let resource_req = rb + .uri( + format!("/projects/{project_name}/services/{project_name}/resources/{resource_type}") + .parse::() + .unwrap(), ) - }) - .map(|resource| resource.r#type.to_string()) - .collect::>(); + .method(Method::DELETE) + .body(hyper::Body::empty()) + .unwrap(); + let res = + route_project(State(state.clone()), scoped_user.clone(), resource_req).await?; + + if res.status() != StatusCode::OK { + delete_fails.push(resource_type) + } + } - if !resources.is_empty() { - return Err(Error::from_kind(ErrorKind::ProjectHasResources(resources))); + if !delete_fails.is_empty() { + println!("failed to remove some resources"); } } @@ -1408,6 +1419,7 @@ pub mod tests { project: &mut TestProject, ) -> anyhow::Result<()> { project.deploy("../examples/rocket/secrets").await; + project.stop_service().await; project.router_call(Method::DELETE, "/delete").await; Ok(()) diff --git a/gateway/src/lib.rs b/gateway/src/lib.rs index c7da5a06d..c6a5c067c 100644 --- a/gateway/src/lib.rs +++ b/gateway/src/lib.rs @@ -958,6 +958,30 @@ pub mod tests { sleep(Duration::from_secs(1)).await; } } + + /// Stop a service running in a project + pub async fn stop_service(&mut self) { + let TestProject { + router, + authorization, + project_name, + } = self; + + router + .call( + Request::builder() + .method("DELETE") + .uri(format!("/projects/{project_name}/services/{project_name}")) + .body(Body::empty()) + .unwrap() + .with_header(authorization), + ) + .map_ok(|resp| { + assert_eq!(resp.status(), StatusCode::OK); + }) + .await + .unwrap(); + } } #[async_trait] From 134c65eb2c6737f26a012a16c0a061e245c7673e Mon Sep 17 00:00:00 2001 From: chesedo Date: Thu, 23 Nov 2023 22:55:20 +0000 Subject: [PATCH 03/19] tests: deleting a project when resource removal fails --- common-tests/src/resource_recorder.rs | 11 ++++++++++ gateway/src/api/latest.rs | 30 ++++++++++++++++++++++++--- gateway/src/lib.rs | 8 +++---- 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/common-tests/src/resource_recorder.rs b/common-tests/src/resource_recorder.rs index bea588289..0bac8da9b 100644 --- a/common-tests/src/resource_recorder.rs +++ b/common-tests/src/resource_recorder.rs @@ -121,6 +121,15 @@ impl ResourceRecorder for MockedResourceRecorder { service_id, r#type, } = request.into_inner(); + + // Fail to delete a metadata resource if requested + if r#type == "metadata" { + return Ok(Response::new(ResultResponse { + success: false, + message: Default::default(), + })); + } + self.resources.lock().unwrap().retain(|r| { !(r.project_id == project_id && r.service_id == service_id && r.r#type == r#type) }); @@ -133,6 +142,8 @@ impl ResourceRecorder for MockedResourceRecorder { } /// Start a mocked resource recorder and return the address it started on +/// This mock will function like a normal resource recorder. However, it will always fail to delete metadata resources +/// if any tests need to simulate a failure. pub async fn start_mocked_resource_recorder() -> u16 { let resource_recorder = MockedResourceRecorder { resources: Mutex::new(Vec::new()), diff --git a/gateway/src/api/latest.rs b/gateway/src/api/latest.rs index 6e1c2a17f..a4f6022fd 100644 --- a/gateway/src/api/latest.rs +++ b/gateway/src/api/latest.rs @@ -1399,7 +1399,10 @@ pub mod tests { #[test_context(TestProject)] #[tokio::test] async fn api_delete_project_that_is_ready(project: &mut TestProject) -> anyhow::Result<()> { - project.router_call(Method::DELETE, "/delete").await; + assert_eq!( + project.router_call(Method::DELETE, "/delete").await, + StatusCode::OK + ); Ok(()) } @@ -1408,7 +1411,10 @@ pub mod tests { #[tokio::test] async fn api_delete_project_that_is_destroyed(project: &mut TestProject) -> anyhow::Result<()> { project.destroy_project().await; - project.router_call(Method::DELETE, "/delete").await; + assert_eq!( + project.router_call(Method::DELETE, "/delete").await, + StatusCode::OK + ); Ok(()) } @@ -1420,7 +1426,25 @@ pub mod tests { ) -> anyhow::Result<()> { project.deploy("../examples/rocket/secrets").await; project.stop_service().await; - project.router_call(Method::DELETE, "/delete").await; + assert_eq!( + project.router_call(Method::DELETE, "/delete").await, + StatusCode::OK + ); + + Ok(()) + } + + #[test_context(TestProject)] + #[tokio::test] + async fn api_delete_project_that_has_resources_but_fails_to_remove_them( + project: &mut TestProject, + ) -> anyhow::Result<()> { + project.deploy("../examples/axum/metadata").await; + project.stop_service().await; + assert_eq!( + project.router_call(Method::DELETE, "/delete").await, + StatusCode::INTERNAL_SERVER_ERROR + ); Ok(()) } diff --git a/gateway/src/lib.rs b/gateway/src/lib.rs index c6a5c067c..97530a83d 100644 --- a/gateway/src/lib.rs +++ b/gateway/src/lib.rs @@ -859,7 +859,7 @@ pub mod tests { } /// Send a request to the router for this project - pub async fn router_call(&mut self, method: Method, sub_path: &str) { + pub async fn router_call(&mut self, method: Method, sub_path: &str) -> StatusCode { let project_name = &self.project_name; self.router @@ -871,11 +871,9 @@ pub mod tests { .unwrap() .with_header(&self.authorization), ) - .map_ok(|resp| { - assert_eq!(resp.status(), StatusCode::OK); - }) + .map_ok(|resp| resp.status()) .await - .unwrap(); + .unwrap() } /// Deploy the code at the path to the project From 4ace7523194710c08eac60e80291d14a39e6dcad Mon Sep 17 00:00:00 2001 From: chesedo Date: Thu, 23 Nov 2023 22:59:28 +0000 Subject: [PATCH 04/19] refactor: better error message --- common/src/models/error.rs | 4 ++-- gateway/src/api/latest.rs | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/common/src/models/error.rs b/common/src/models/error.rs index d4ee09104..979fd37e1 100644 --- a/common/src/models/error.rs +++ b/common/src/models/error.rs @@ -97,8 +97,8 @@ impl From for ApiError { ErrorKind::ProjectHasResources(resources) => { let resources = resources.join(", "); return Self { - message: format!("Project has resources: {}. Use `cargo shuttle resource list` and `cargo shuttle resource delete ` to delete them.", resources), - status_code: StatusCode::FORBIDDEN.as_u16(), + message: format!("Could not automatically delete the following resources: {}. Reach out to our support and ask them for a helping hand.", resources), + status_code: StatusCode::INTERNAL_SERVER_ERROR.as_u16(), } } ErrorKind::InvalidProjectName(err) => { diff --git a/gateway/src/api/latest.rs b/gateway/src/api/latest.rs index a4f6022fd..ebba8a3d5 100644 --- a/gateway/src/api/latest.rs +++ b/gateway/src/api/latest.rs @@ -418,7 +418,9 @@ async fn delete_project( } if !delete_fails.is_empty() { - println!("failed to remove some resources"); + return Err(Error::from_kind(ErrorKind::ProjectHasResources( + delete_fails, + ))); } } From 63fe8e1a6be00713029b2b26c74d61424ed6d914 Mon Sep 17 00:00:00 2001 From: chesedo Date: Thu, 23 Nov 2023 23:03:06 +0000 Subject: [PATCH 05/19] refactor: don't panic if project is not deleted --- gateway/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gateway/src/lib.rs b/gateway/src/lib.rs index 97530a83d..4b298c128 100644 --- a/gateway/src/lib.rs +++ b/gateway/src/lib.rs @@ -998,7 +998,7 @@ pub mod tests { if dangling { self.router_call(Method::DELETE, "/delete").await; - panic!("test left a dangling project which you might need to clean manually"); + eprintln!("test left a dangling project which you might need to clean manually"); } } } From 8d747cf8e5768645cf3cf8639803d6ab7952a610 Mon Sep 17 00:00:00 2001 From: chesedo Date: Thu, 23 Nov 2023 23:18:02 +0000 Subject: [PATCH 06/19] refactor: remove unneeded `Result` return type --- gateway/src/api/latest.rs | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/gateway/src/api/latest.rs b/gateway/src/api/latest.rs index ebba8a3d5..86b6570cb 100644 --- a/gateway/src/api/latest.rs +++ b/gateway/src/api/latest.rs @@ -1400,55 +1400,48 @@ pub mod tests { #[test_context(TestProject)] #[tokio::test] - async fn api_delete_project_that_is_ready(project: &mut TestProject) -> anyhow::Result<()> { + async fn api_delete_project_that_is_ready(project: &mut TestProject) { assert_eq!( project.router_call(Method::DELETE, "/delete").await, StatusCode::OK ); - - Ok(()) } #[test_context(TestProject)] #[tokio::test] - async fn api_delete_project_that_is_destroyed(project: &mut TestProject) -> anyhow::Result<()> { + async fn api_delete_project_that_is_destroyed(project: &mut TestProject) { project.destroy_project().await; + assert_eq!( project.router_call(Method::DELETE, "/delete").await, StatusCode::OK ); - - Ok(()) } #[test_context(TestProject)] #[tokio::test] - async fn api_delete_project_that_has_resources( - project: &mut TestProject, - ) -> anyhow::Result<()> { + async fn api_delete_project_that_has_resources(project: &mut TestProject) { project.deploy("../examples/rocket/secrets").await; project.stop_service().await; + assert_eq!( project.router_call(Method::DELETE, "/delete").await, StatusCode::OK ); - - Ok(()) } #[test_context(TestProject)] #[tokio::test] async fn api_delete_project_that_has_resources_but_fails_to_remove_them( project: &mut TestProject, - ) -> anyhow::Result<()> { + ) { project.deploy("../examples/axum/metadata").await; project.stop_service().await; + assert_eq!( project.router_call(Method::DELETE, "/delete").await, StatusCode::INTERNAL_SERVER_ERROR ); - - Ok(()) } #[tokio::test(flavor = "multi_thread")] From e3e2b6929d1fef9055b05ef8bf24d2cd99e8f7ae Mon Sep 17 00:00:00 2001 From: chesedo Date: Thu, 23 Nov 2023 23:22:02 +0000 Subject: [PATCH 07/19] refactor: helpful comment --- gateway/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/gateway/src/lib.rs b/gateway/src/lib.rs index 4b298c128..a447a2899 100644 --- a/gateway/src/lib.rs +++ b/gateway/src/lib.rs @@ -949,6 +949,7 @@ pub mod tests { } tries += 1; + // We should consider making a mock deployer image to be able to "deploy" (aka fake deploy) things instantly for tests if tries > 240 { panic!("timed out waiting for deployment"); } From 732767ea2c07c74dce04606697f8bd5c2bad938f Mon Sep 17 00:00:00 2001 From: chesedo Date: Thu, 23 Nov 2023 23:27:22 +0000 Subject: [PATCH 08/19] refactor: clippy errors --- common-tests/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common-tests/Cargo.toml b/common-tests/Cargo.toml index de1f74858..632d98f6d 100644 --- a/common-tests/Cargo.toml +++ b/common-tests/Cargo.toml @@ -8,7 +8,7 @@ publish = false [dependencies] cargo-shuttle = { path = "../cargo-shuttle" } -shuttle-proto = { workspace = true, features = ["builder", "logger"] } +shuttle-proto = { workspace = true, features = ["builder", "logger", "resource-recorder"] } hyper = { workspace = true } portpicker = { workspace = true } From c8a452906d3df32447628eb4b2e29c83f09bb18b Mon Sep 17 00:00:00 2001 From: chesedo Date: Sun, 26 Nov 2023 17:25:06 +0000 Subject: [PATCH 09/19] tests: delete a project with a running deployment --- gateway/src/api/latest.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/gateway/src/api/latest.rs b/gateway/src/api/latest.rs index 86b6570cb..1f474824e 100644 --- a/gateway/src/api/latest.rs +++ b/gateway/src/api/latest.rs @@ -1444,6 +1444,17 @@ pub mod tests { ); } + #[test_context(TestProject)] + #[tokio::test] + async fn api_delete_project_that_has_running_deployment(project: &mut TestProject) { + project.deploy("../examples/axum/hello-world").await; + + assert_eq!( + project.router_call(Method::DELETE, "/delete").await, + StatusCode::OK + ); + } + #[tokio::test(flavor = "multi_thread")] async fn status() { let world = World::new().await; From ae48ec96ea8b3e302057b1e88189305ab05b2784 Mon Sep 17 00:00:00 2001 From: chesedo Date: Sun, 26 Nov 2023 17:30:08 +0000 Subject: [PATCH 10/19] feat: auto stop running deployment when deleting project --- common/src/models/error.rs | 4 ++-- gateway/src/api/latest.rs | 18 +++++++++++++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/common/src/models/error.rs b/common/src/models/error.rs index 979fd37e1..61908b405 100644 --- a/common/src/models/error.rs +++ b/common/src/models/error.rs @@ -91,8 +91,8 @@ impl From for ApiError { (StatusCode::FORBIDDEN, "You cannot create more projects. Delete some projects first.") }, ErrorKind::ProjectHasRunningDeployment => ( - StatusCode::FORBIDDEN, - "A deployment is running. Stop it with `cargo shuttle stop` first." + StatusCode::INTERNAL_SERVER_ERROR, + "Could not automatically stop the running deployment for the project. Reach out to our support and ask them for a helping hand." ), ErrorKind::ProjectHasResources(resources) => { let resources = resources.join(", "); diff --git a/gateway/src/api/latest.rs b/gateway/src/api/latest.rs index 1f474824e..db2cfa123 100644 --- a/gateway/src/api/latest.rs +++ b/gateway/src/api/latest.rs @@ -364,8 +364,24 @@ async fn delete_project( .map_err(|e| Error::source(ErrorKind::Internal, e))?; let summary: shuttle_common::models::service::Summary = serde_json::from_slice(&body_bytes) .map_err(|e| Error::source(ErrorKind::Internal, e))?; + if summary.deployment.is_some() { - return Err(Error::from_kind(ErrorKind::ProjectHasRunningDeployment)); + let mut rb = Request::builder(); + rb.headers_mut().unwrap().clone_from(req.headers()); + let service_req = rb + .uri( + format!("/projects/{project_name}/services/{project_name}") + .parse::() + .unwrap(), + ) + .method("DELETE") + .body(hyper::Body::empty()) + .unwrap(); + let res = route_project(State(state.clone()), scoped_user.clone(), service_req).await?; + + if res.status() != StatusCode::OK { + return Err(Error::from_kind(ErrorKind::ProjectHasRunningDeployment)); + } } } From 94646ab3155ed4f1e4e279ee3df7258d259297ac Mon Sep 17 00:00:00 2001 From: chesedo Date: Sun, 26 Nov 2023 21:57:20 +0000 Subject: [PATCH 11/19] refactor: make gateway calls to project structured --- common/src/models/service.rs | 2 +- gateway/src/api/latest.rs | 114 ++++++-------------------- gateway/src/api/mod.rs | 1 + gateway/src/api/project_caller.rs | 129 ++++++++++++++++++++++++++++++ 4 files changed, 155 insertions(+), 91 deletions(-) create mode 100644 gateway/src/api/project_caller.rs diff --git a/common/src/models/service.rs b/common/src/models/service.rs index a43013184..bc5c52555 100644 --- a/common/src/models/service.rs +++ b/common/src/models/service.rs @@ -18,7 +18,7 @@ pub struct Response { pub name: String, } -#[derive(Deserialize, Serialize)] +#[derive(Deserialize, Serialize, Default)] #[cfg_attr(feature = "openapi", derive(ToSchema))] #[cfg_attr(feature = "openapi", schema(as = shuttle_common::models::service::Summary))] pub struct Summary { diff --git a/gateway/src/api/latest.rs b/gateway/src/api/latest.rs index db2cfa123..daa75aad9 100644 --- a/gateway/src/api/latest.rs +++ b/gateway/src/api/latest.rs @@ -14,7 +14,7 @@ use axum::routing::{any, delete, get, post}; use axum::{Json as AxumJson, Router}; use fqdn::FQDN; use futures::Future; -use http::{Method, StatusCode, Uri}; +use http::{StatusCode, Uri}; use instant_acme::{AccountCredentials, ChallengeType}; use serde::{Deserialize, Serialize}; use shuttle_common::backends::auth::{AuthPublicKey, JwtAuthenticationLayer, ScopedLayer}; @@ -58,6 +58,7 @@ use crate::worker::WORKER_QUEUE_SIZE; use crate::{Error, AUTH_CLIENT}; use super::auth_layer::ShuttleAuthLayer; +use super::project_caller::ProjectCaller; pub const SVC_DEGRADED_THRESHOLD: usize = 128; pub const SHUTTLE_GATEWAY_VARIANT: &str = "shuttle-gateway"; @@ -341,105 +342,38 @@ async fn delete_project( let service = state.service.clone(); let sender = state.sender.clone(); + let project_caller = + ProjectCaller::new(state.clone(), scoped_user.clone(), req.headers()).await?; + // check that deployment is not running - let mut rb = Request::builder(); - rb.headers_mut().unwrap().clone_from(req.headers()); - let service_req = rb - .uri( - format!("/projects/{project_name}/services/{project_name}") - .parse::() - .unwrap(), - ) - .method("GET") - .body(hyper::Body::empty()) - .unwrap(); - let res = route_project(State(state.clone()), scoped_user.clone(), service_req).await?; - // 404 == no service == no deployments - if res.status() != StatusCode::NOT_FOUND { - if res.status() != StatusCode::OK { - return Err(Error::from_kind(ErrorKind::Internal)); - } - let body_bytes = hyper::body::to_bytes(res.into_body()) - .await - .map_err(|e| Error::source(ErrorKind::Internal, e))?; - let summary: shuttle_common::models::service::Summary = serde_json::from_slice(&body_bytes) - .map_err(|e| Error::source(ErrorKind::Internal, e))?; - - if summary.deployment.is_some() { - let mut rb = Request::builder(); - rb.headers_mut().unwrap().clone_from(req.headers()); - let service_req = rb - .uri( - format!("/projects/{project_name}/services/{project_name}") - .parse::() - .unwrap(), - ) - .method("DELETE") - .body(hyper::Body::empty()) - .unwrap(); - let res = route_project(State(state.clone()), scoped_user.clone(), service_req).await?; + let summary = project_caller.get_service_summary().await?; + if summary.deployment.is_some() { + let res = project_caller.stop_active_deployment().await?; - if res.status() != StatusCode::OK { - return Err(Error::from_kind(ErrorKind::ProjectHasRunningDeployment)); - } + if res.status() != StatusCode::OK { + return Err(Error::from_kind(ErrorKind::ProjectHasRunningDeployment)); } } - // check if database in resources - let mut rb = Request::builder(); - rb.headers_mut().unwrap().clone_from(req.headers()); - let resource_req = rb - .uri( - format!("/projects/{project_name}/services/{project_name}/resources") - .parse::() - .unwrap(), - ) - .method("GET") - .body(hyper::Body::empty()) - .unwrap(); - let res = route_project(State(state.clone()), scoped_user.clone(), resource_req).await?; - // 404 == no service == no resources - if res.status() != StatusCode::NOT_FOUND { - if res.status() != StatusCode::OK { - return Err(Error::from_kind(ErrorKind::Internal)); - } - let body_bytes = hyper::body::to_bytes(res.into_body()) - .await - .map_err(|e| Error::source(ErrorKind::Internal, e))?; - let resources: Vec = - serde_json::from_slice(&body_bytes) - .map_err(|e| Error::source(ErrorKind::Internal, e))?; - - let mut delete_fails = Vec::new(); - - for resource in resources { - let resource_type = resource.r#type.to_string(); - let mut rb = Request::builder(); - rb.headers_mut().unwrap().clone_from(req.headers()); - let resource_req = rb - .uri( - format!("/projects/{project_name}/services/{project_name}/resources/{resource_type}") - .parse::() - .unwrap(), - ) - .method(Method::DELETE) - .body(hyper::Body::empty()) - .unwrap(); - let res = - route_project(State(state.clone()), scoped_user.clone(), resource_req).await?; + // check if any resources exist + let resources = project_caller.get_resources().await?; + let mut delete_fails = Vec::new(); - if res.status() != StatusCode::OK { - delete_fails.push(resource_type) - } - } + for resource in resources { + let resource_type = resource.r#type.to_string(); + let res = project_caller.delete_resource(&resource_type).await?; - if !delete_fails.is_empty() { - return Err(Error::from_kind(ErrorKind::ProjectHasResources( - delete_fails, - ))); + if res.status() != StatusCode::OK { + delete_fails.push(resource_type) } } + if !delete_fails.is_empty() { + return Err(Error::from_kind(ErrorKind::ProjectHasResources( + delete_fails, + ))); + } + if dry_run.is_some_and(|d| d) { return Ok(AxumJson("project not deleted due to dry run".to_owned())); } diff --git a/gateway/src/api/mod.rs b/gateway/src/api/mod.rs index 3165903a4..93b20907d 100644 --- a/gateway/src/api/mod.rs +++ b/gateway/src/api/mod.rs @@ -1,3 +1,4 @@ mod auth_layer; pub mod latest; +mod project_caller; diff --git a/gateway/src/api/project_caller.rs b/gateway/src/api/project_caller.rs new file mode 100644 index 000000000..1fb288fe5 --- /dev/null +++ b/gateway/src/api/project_caller.rs @@ -0,0 +1,129 @@ +use std::sync::Arc; + +use axum::response::Response; +use http::{HeaderMap, Method, Request, StatusCode, Uri}; +use hyper::Body; +use serde::de::DeserializeOwned; +use shuttle_common::{ + models::{error::ErrorKind, project::ProjectName, service}, + resource, +}; + +use crate::{auth::ScopedUser, project::Project, service::GatewayService, AccountName, Error}; + +use super::latest::RouterState; + +/// Helper to easily make requests to a project +pub struct ProjectCaller { + project: Project, + project_name: ProjectName, + account_name: AccountName, + service: Arc, + headers: HeaderMap, +} + +impl ProjectCaller { + /// Make a new project caller to easily make requests to this project + pub async fn new( + state: RouterState, + scoped_user: ScopedUser, + headers: &HeaderMap, + ) -> Result { + let RouterState { + service, sender, .. + } = state; + let project_name = scoped_user.scope; + let project = service.find_or_start_project(&project_name, sender).await?; + + Ok(Self { + project: project.state, + project_name, + account_name: scoped_user.user.name, + service, + headers: headers.clone(), + }) + } + + /// Make a simple request call to get the response + pub async fn call(&self, uri: &str, method: Method) -> Result, Error> { + let mut rb = Request::builder(); + rb.headers_mut().unwrap().clone_from(&self.headers); + let req = rb + .uri(uri.parse::().unwrap()) + .method(method) + .body(hyper::Body::empty()) + .unwrap(); + + self.service + .route(&self.project, &self.project_name, &self.account_name, req) + .await + } + + /// Make a request call and deserialize the body to the generic type + async fn call_deserialize( + &self, + uri: &str, + method: Method, + ) -> Result { + let res = self.call(uri, method).await?; + + if res.status() != StatusCode::NOT_FOUND { + if res.status() != StatusCode::OK { + return Err(Error::from_kind(ErrorKind::Internal)); + } + let body_bytes = hyper::body::to_bytes(res.into_body()) + .await + .map_err(|e| Error::source(ErrorKind::Internal, e))?; + let body = serde_json::from_slice(&body_bytes) + .map_err(|e| Error::source(ErrorKind::Internal, e))?; + + Ok(body) + } else { + Ok(Default::default()) + } + } + + /// Get the service summary for the project + pub async fn get_service_summary(&self) -> Result { + let project_name = &self.project_name; + + self.call_deserialize( + &format!("/projects/{project_name}/services/{project_name}"), + Method::GET, + ) + .await + } + + /// Stop the active deployment of the project + pub async fn stop_active_deployment(&self) -> Result, Error> { + let project_name = &self.project_name; + + self.call( + &format!("/projects/{project_name}/services/{project_name}"), + Method::DELETE, + ) + .await + } + + /// Get all the resources the project is using + pub async fn get_resources(&self) -> Result, Error> { + let project_name = &self.project_name; + + self.call_deserialize( + &format!("/projects/{project_name}/services/{project_name}/resources"), + Method::GET, + ) + .await + } + + /// Delete a resource used by the project + pub async fn delete_resource(&self, r#type: &str) -> Result, Error> { + let project_name = &self.project_name; + + self.call( + &format!("/projects/{project_name}/services/{project_name}/resources/{type}"), + Method::DELETE, + ) + .await + } +} From 7ccdf6cf1553ade82a6021c322f727e7f70b7ae3 Mon Sep 17 00:00:00 2001 From: chesedo Date: Mon, 27 Nov 2023 13:25:52 +0000 Subject: [PATCH 12/19] refactor: reach out to support error message --- common/src/models/error.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/src/models/error.rs b/common/src/models/error.rs index 61908b405..05a8f334a 100644 --- a/common/src/models/error.rs +++ b/common/src/models/error.rs @@ -92,12 +92,12 @@ impl From for ApiError { }, ErrorKind::ProjectHasRunningDeployment => ( StatusCode::INTERNAL_SERVER_ERROR, - "Could not automatically stop the running deployment for the project. Reach out to our support and ask them for a helping hand." + "Could not automatically stop the running deployment for the project. Please reach out to Shuttle support for help." ), ErrorKind::ProjectHasResources(resources) => { let resources = resources.join(", "); return Self { - message: format!("Could not automatically delete the following resources: {}. Reach out to our support and ask them for a helping hand.", resources), + message: format!("Could not automatically delete the following resources: {}. Please reach out to Shuttle support for help.", resources), status_code: StatusCode::INTERNAL_SERVER_ERROR.as_u16(), } } From 7d036f1ba8844fa4508668e52ec6ced6719b9d6c Mon Sep 17 00:00:00 2001 From: chesedo Date: Mon, 27 Nov 2023 15:08:30 +0000 Subject: [PATCH 13/19] refactor: remove dry-run option --- cargo-shuttle/src/client.rs | 7 ++----- cargo-shuttle/src/lib.rs | 4 ++-- gateway/src/api/latest.rs | 10 ++++------ 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/cargo-shuttle/src/client.rs b/cargo-shuttle/src/client.rs index f0aef6a90..b4e5eb555 100644 --- a/cargo-shuttle/src/client.rs +++ b/cargo-shuttle/src/client.rs @@ -168,11 +168,8 @@ impl Client { self.delete(path).await } - pub async fn delete_project(&self, project: &str, dry_run: bool) -> Result { - let path = format!( - "/projects/{project}/delete{}", - if dry_run { "?dry_run=true" } else { "" } - ); + pub async fn delete_project(&self, project: &str) -> Result { + let path = format!("/projects/{project}/delete"); self.delete(path).await } diff --git a/cargo-shuttle/src/lib.rs b/cargo-shuttle/src/lib.rs index 9552a2237..820ed7335 100644 --- a/cargo-shuttle/src/lib.rs +++ b/cargo-shuttle/src/lib.rs @@ -1860,7 +1860,7 @@ impl Shuttle { let client = self.client.as_ref().unwrap(); // If a check fails, print the returned error - client.delete_project(self.ctx.project_name(), true).await.map_err(|err| { + client.delete_project(self.ctx.project_name()).await.map_err(|err| { if let Some(api_error) = err.downcast_ref::() { // If the returned error string changes, this could break if api_error.message.contains("not ready") { @@ -1908,7 +1908,7 @@ impl Shuttle { } client - .delete_project(self.ctx.project_name(), false) + .delete_project(self.ctx.project_name()) .await .map_err(|err| { suggestions::project::project_request_failure( diff --git a/gateway/src/api/latest.rs b/gateway/src/api/latest.rs index daa75aad9..6bc423212 100644 --- a/gateway/src/api/latest.rs +++ b/gateway/src/api/latest.rs @@ -297,6 +297,10 @@ async fn destroy_project( #[derive(Deserialize, IntoParams)] struct DeleteProjectParams { + // Was added in v0.30.0 + // We have not needed it since 0.35.0, but have to keep in for any old CLI users + #[deprecated(since = "0.35.0", note = "was added in 0.30.0")] + #[allow(dead_code)] dry_run: Option, } @@ -311,13 +315,11 @@ struct DeleteProjectParams { ), params( ("project_name" = String, Path, description = "The name of the project."), - DeleteProjectParams, ) )] async fn delete_project( State(state): State, scoped_user: ScopedUser, - Query(DeleteProjectParams { dry_run }): Query, req: Request, ) -> Result, Error> { let project_name = scoped_user.scope.clone(); @@ -374,10 +376,6 @@ async fn delete_project( ))); } - if dry_run.is_some_and(|d| d) { - return Ok(AxumJson("project not deleted due to dry run".to_owned())); - } - let task = service .new_task() .project(project_name.clone()) From c0a8809ae55815ce211560147bf9ebc20b37f927 Mon Sep 17 00:00:00 2001 From: chesedo Date: Mon, 27 Nov 2023 15:58:50 +0000 Subject: [PATCH 14/19] refactor: handle deployments that might be in the building state --- common/src/models/error.rs | 5 +++ gateway/src/api/latest.rs | 51 +++++++++++++++++++++++++--- gateway/src/api/project_caller.rs | 56 ++++++++++++++++++++----------- gateway/src/lib.rs | 11 ++++-- 4 files changed, 96 insertions(+), 27 deletions(-) diff --git a/common/src/models/error.rs b/common/src/models/error.rs index 05a8f334a..45f2aac7b 100644 --- a/common/src/models/error.rs +++ b/common/src/models/error.rs @@ -51,6 +51,7 @@ pub enum ErrorKind { TooManyProjects, ProjectHasResources(Vec), ProjectHasRunningDeployment, + ProjectHasBuildingDeployment, CustomDomainNotFound, InvalidCustomDomain, CustomDomainAlreadyExists, @@ -94,6 +95,10 @@ impl From for ApiError { StatusCode::INTERNAL_SERVER_ERROR, "Could not automatically stop the running deployment for the project. Please reach out to Shuttle support for help." ), + ErrorKind::ProjectHasBuildingDeployment => ( + StatusCode::BAD_REQUEST, + "Project currently has a deployment that is busy building. Use `cargo shuttle deployment list` to see it and wait for it to finish" + ), ErrorKind::ProjectHasResources(resources) => { let resources = resources.join(", "); return Self { diff --git a/gateway/src/api/latest.rs b/gateway/src/api/latest.rs index 6bc423212..f0c392a78 100644 --- a/gateway/src/api/latest.rs +++ b/gateway/src/api/latest.rs @@ -29,7 +29,7 @@ use shuttle_common::models::{ project::{self, ProjectName}, stats, }; -use shuttle_common::{request_span, VersionInfo}; +use shuttle_common::{deployment, request_span, VersionInfo}; use shuttle_proto::provisioner::provisioner_client::ProvisionerClient; use shuttle_proto::provisioner::Ping; use tokio::sync::mpsc::Sender; @@ -347,10 +347,36 @@ async fn delete_project( let project_caller = ProjectCaller::new(state.clone(), scoped_user.clone(), req.headers()).await?; - // check that deployment is not running - let summary = project_caller.get_service_summary().await?; - if summary.deployment.is_some() { - let res = project_caller.stop_active_deployment().await?; + // check that a deployment is not running + let mut deployments = project_caller.get_deployment_list().await?; + deployments.sort_by_key(|d| d.last_update); + + // Make sure no deployment is in the building pipeline + let has_bad_state = deployments + .iter() + .find(|d| { + !matches!( + d.state, + deployment::State::Running + | deployment::State::Completed + | deployment::State::Crashed + | deployment::State::Stopped + ) + }) + .is_some(); + + if has_bad_state { + return Err(Error::from_kind(ErrorKind::ProjectHasBuildingDeployment)); + } + + let running_deployments = deployments + .into_iter() + .filter(|d| d.state == deployment::State::Running); + + for running_deployment in running_deployments { + let res = project_caller + .stop_deployment(&running_deployment.id) + .await?; if res.status() != StatusCode::OK { return Err(Error::from_kind(ErrorKind::ProjectHasRunningDeployment)); @@ -1086,6 +1112,7 @@ pub mod tests { use test_context::test_context; use tokio::sync::mpsc::channel; use tokio::sync::oneshot; + use tokio::time::sleep; use tower::Service; use super::*; @@ -1403,6 +1430,20 @@ pub mod tests { ); } + #[test_context(TestProject)] + #[tokio::test] + async fn api_delete_project_that_is_building(project: &mut TestProject) { + project.just_deploy("../examples/axum/hello-world").await; + + // Wait a bit to it to progress in the queue + sleep(Duration::from_secs(2)).await; + + assert_eq!( + project.router_call(Method::DELETE, "/delete").await, + StatusCode::BAD_REQUEST + ); + } + #[tokio::test(flavor = "multi_thread")] async fn status() { let world = World::new().await; diff --git a/gateway/src/api/project_caller.rs b/gateway/src/api/project_caller.rs index 1fb288fe5..ff990f906 100644 --- a/gateway/src/api/project_caller.rs +++ b/gateway/src/api/project_caller.rs @@ -5,9 +5,10 @@ use http::{HeaderMap, Method, Request, StatusCode, Uri}; use hyper::Body; use serde::de::DeserializeOwned; use shuttle_common::{ - models::{error::ErrorKind, project::ProjectName, service}, + models::{deployment, error::ErrorKind, project::ProjectName}, resource, }; +use uuid::Uuid; use crate::{auth::ScopedUser, project::Project, service::GatewayService, AccountName, Error}; @@ -60,11 +61,12 @@ impl ProjectCaller { } /// Make a request call and deserialize the body to the generic type - async fn call_deserialize( + /// Returns `None` when the request was successful but found nothing + async fn call_deserialize( &self, uri: &str, method: Method, - ) -> Result { + ) -> Result, Error> { let res = self.call(uri, method).await?; if res.status() != StatusCode::NOT_FOUND { @@ -77,29 +79,36 @@ impl ProjectCaller { let body = serde_json::from_slice(&body_bytes) .map_err(|e| Error::source(ErrorKind::Internal, e))?; - Ok(body) + Ok(Some(body)) } else { - Ok(Default::default()) + return Ok(None); } } - /// Get the service summary for the project - pub async fn get_service_summary(&self) -> Result { + /// Get the deployments for the project + pub async fn get_deployment_list(&self) -> Result, Error> { let project_name = &self.project_name; - self.call_deserialize( - &format!("/projects/{project_name}/services/{project_name}"), - Method::GET, - ) - .await + let deployments = self + .call_deserialize( + &format!("/projects/{project_name}/deployments"), + Method::GET, + ) + .await?; + + if let Some(deployments) = deployments { + Ok(deployments) + } else { + Ok(Vec::new()) + } } - /// Stop the active deployment of the project - pub async fn stop_active_deployment(&self) -> Result, Error> { + /// Stop a deployment of the project + pub async fn stop_deployment(&self, deployment_id: &Uuid) -> Result, Error> { let project_name = &self.project_name; self.call( - &format!("/projects/{project_name}/services/{project_name}"), + &format!("/projects/{project_name}/deployments/{deployment_id}"), Method::DELETE, ) .await @@ -109,11 +118,18 @@ impl ProjectCaller { pub async fn get_resources(&self) -> Result, Error> { let project_name = &self.project_name; - self.call_deserialize( - &format!("/projects/{project_name}/services/{project_name}/resources"), - Method::GET, - ) - .await + let resources = self + .call_deserialize( + &format!("/projects/{project_name}/services/{project_name}/resources"), + Method::GET, + ) + .await?; + + if let Some(resources) = resources { + Ok(resources) + } else { + Ok(Vec::new()) + } } /// Delete a resource used by the project diff --git a/gateway/src/lib.rs b/gateway/src/lib.rs index a447a2899..5e08504aa 100644 --- a/gateway/src/lib.rs +++ b/gateway/src/lib.rs @@ -876,8 +876,8 @@ pub mod tests { .unwrap() } - /// Deploy the code at the path to the project - pub async fn deploy(&mut self, path: &str) { + /// Just deploy the code at the path and don't wait for it to finish + pub async fn just_deploy(&mut self, path: &str) { let path = canonicalize(path).expect("deploy path to be valid"); let name = path.file_name().unwrap().to_str().unwrap(); let enc = GzEncoder::new(Vec::new(), Compression::fast()); @@ -924,6 +924,13 @@ pub mod tests { }) .await .unwrap(); + } + + /// Deploy the code at the path to the project and wait for it to finish + pub async fn deploy(&mut self, path: &str) { + self.just_deploy(path).await; + + let project_name = &self.project_name; // Wait for deployment to be up let mut tries = 0; From f4386d16d801f686f54c550deb7b2b9a388f8576 Mon Sep 17 00:00:00 2001 From: chesedo Date: Mon, 27 Nov 2023 16:02:35 +0000 Subject: [PATCH 15/19] refactor: less `if` nesting --- gateway/src/api/project_caller.rs | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/gateway/src/api/project_caller.rs b/gateway/src/api/project_caller.rs index ff990f906..b2d3bfd7d 100644 --- a/gateway/src/api/project_caller.rs +++ b/gateway/src/api/project_caller.rs @@ -69,19 +69,18 @@ impl ProjectCaller { ) -> Result, Error> { let res = self.call(uri, method).await?; - if res.status() != StatusCode::NOT_FOUND { - if res.status() != StatusCode::OK { - return Err(Error::from_kind(ErrorKind::Internal)); + match res.status() { + StatusCode::NOT_FOUND => Ok(None), + StatusCode::OK => { + let body_bytes = hyper::body::to_bytes(res.into_body()) + .await + .map_err(|e| Error::source(ErrorKind::Internal, e))?; + let body = serde_json::from_slice(&body_bytes) + .map_err(|e| Error::source(ErrorKind::Internal, e))?; + + Ok(Some(body)) } - let body_bytes = hyper::body::to_bytes(res.into_body()) - .await - .map_err(|e| Error::source(ErrorKind::Internal, e))?; - let body = serde_json::from_slice(&body_bytes) - .map_err(|e| Error::source(ErrorKind::Internal, e))?; - - Ok(Some(body)) - } else { - return Ok(None); + _ => Err(Error::from_kind(ErrorKind::Internal)), } } From d621134d8e037d25b550a51d327a4ba570946ff9 Mon Sep 17 00:00:00 2001 From: chesedo Date: Mon, 27 Nov 2023 16:13:06 +0000 Subject: [PATCH 16/19] refactor: don't delete twice --- cargo-shuttle/src/lib.rs | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/cargo-shuttle/src/lib.rs b/cargo-shuttle/src/lib.rs index 820ed7335..d019353ce 100644 --- a/cargo-shuttle/src/lib.rs +++ b/cargo-shuttle/src/lib.rs @@ -1859,29 +1859,6 @@ impl Shuttle { async fn project_delete(&self) -> Result { let client = self.client.as_ref().unwrap(); - // If a check fails, print the returned error - client.delete_project(self.ctx.project_name()).await.map_err(|err| { - if let Some(api_error) = err.downcast_ref::() { - // If the returned error string changes, this could break - if api_error.message.contains("not ready") { - println!("{}", "Project delete failed".red()); - println!(); - println!("{}", "Try restarting the project with `cargo shuttle project restart` first.".yellow()); - println!("{}", "This is needed to check for any resources linked to it.".yellow()); - println!("{}", "For more help with deleting projects, visit https://docs.shuttle.rs/support/delete-project".yellow()); - println!(); - return err; - } - } - println!("{}", "For more help with deleting projects, visit https://docs.shuttle.rs/support/delete-project".yellow()); - suggestions::project::project_request_failure( - err, - "Project delete failed", - true, - "deleting the project or getting project status fails repeatedly", - ) - })?; - println!( "{}", formatdoc!( From 634b8488d54f217b3bf8f8ca90d741806427bee7 Mon Sep 17 00:00:00 2001 From: chesedo Date: Mon, 27 Nov 2023 16:16:10 +0000 Subject: [PATCH 17/19] refactor: clippy suggestion --- gateway/src/api/latest.rs | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/gateway/src/api/latest.rs b/gateway/src/api/latest.rs index f0c392a78..ead17591b 100644 --- a/gateway/src/api/latest.rs +++ b/gateway/src/api/latest.rs @@ -352,18 +352,15 @@ async fn delete_project( deployments.sort_by_key(|d| d.last_update); // Make sure no deployment is in the building pipeline - let has_bad_state = deployments - .iter() - .find(|d| { - !matches!( - d.state, - deployment::State::Running - | deployment::State::Completed - | deployment::State::Crashed - | deployment::State::Stopped - ) - }) - .is_some(); + let has_bad_state = deployments.iter().any(|d| { + !matches!( + d.state, + deployment::State::Running + | deployment::State::Completed + | deployment::State::Crashed + | deployment::State::Stopped + ) + }); if has_bad_state { return Err(Error::from_kind(ErrorKind::ProjectHasBuildingDeployment)); From 5b1273ba7ccb189c108ac81ebd1c0b2f85869252 Mon Sep 17 00:00:00 2001 From: chesedo Date: Mon, 27 Nov 2023 16:25:51 +0000 Subject: [PATCH 18/19] refactor: more clippy errors --- gateway/src/api/project_caller.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gateway/src/api/project_caller.rs b/gateway/src/api/project_caller.rs index b2d3bfd7d..a086a3e81 100644 --- a/gateway/src/api/project_caller.rs +++ b/gateway/src/api/project_caller.rs @@ -15,7 +15,7 @@ use crate::{auth::ScopedUser, project::Project, service::GatewayService, Account use super::latest::RouterState; /// Helper to easily make requests to a project -pub struct ProjectCaller { +pub(crate) struct ProjectCaller { project: Project, project_name: ProjectName, account_name: AccountName, From e2d7d72b93cc69588c9c0dcb5c5a3fcb1ff2a544 Mon Sep 17 00:00:00 2001 From: chesedo Date: Mon, 27 Nov 2023 17:24:30 +0000 Subject: [PATCH 19/19] refactor: better default handling --- common/src/models/service.rs | 2 +- gateway/src/api/project_caller.rs | 12 ++---------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/common/src/models/service.rs b/common/src/models/service.rs index bc5c52555..a43013184 100644 --- a/common/src/models/service.rs +++ b/common/src/models/service.rs @@ -18,7 +18,7 @@ pub struct Response { pub name: String, } -#[derive(Deserialize, Serialize, Default)] +#[derive(Deserialize, Serialize)] #[cfg_attr(feature = "openapi", derive(ToSchema))] #[cfg_attr(feature = "openapi", schema(as = shuttle_common::models::service::Summary))] pub struct Summary { diff --git a/gateway/src/api/project_caller.rs b/gateway/src/api/project_caller.rs index a086a3e81..4b6ac6f50 100644 --- a/gateway/src/api/project_caller.rs +++ b/gateway/src/api/project_caller.rs @@ -95,11 +95,7 @@ impl ProjectCaller { ) .await?; - if let Some(deployments) = deployments { - Ok(deployments) - } else { - Ok(Vec::new()) - } + Ok(deployments.unwrap_or_default()) } /// Stop a deployment of the project @@ -124,11 +120,7 @@ impl ProjectCaller { ) .await?; - if let Some(resources) = resources { - Ok(resources) - } else { - Ok(Vec::new()) - } + Ok(resources.unwrap_or_default()) } /// Delete a resource used by the project