cli connection timeouts (#655)

golemcloud · Jul 9, 2024 · 5e062be · 5e062be
1 parent bd940f6
commit 5e062be
Show file tree

Hide file tree

Showing 5 changed files with 100 additions and 7 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/golem-cli/Cargo.toml b/golem-cli/Cargo.toml
@@ -65,6 +65,7 @@ textwrap = "0.16.1"
 anyhow.workspace = true
 colored = "2.1.0"
 inquire = "0.7.5"
+iso8601 = "0.6.1"
 
 [dev-dependencies]
 golem-test-framework = { path = "../golem-test-framework", version = "0.0.0" }

diff --git a/golem-cli/src/config.rs b/golem-cli/src/config.rs
@@ -20,11 +20,13 @@ use derive_more::FromStr;
 use indoc::printdoc;
 use itertools::Itertools;
 use serde::{Deserialize, Serialize};
+use std::cmp::min;
 use std::collections::HashMap;
 use std::fmt::{Display, Formatter};
 use std::fs::{create_dir_all, File, OpenOptions};
 use std::io::{BufReader, BufWriter};
 use std::path::{Path, PathBuf};
+use std::time::Duration;
 use tracing::warn;
 use url::Url;
 
@@ -277,3 +279,49 @@ impl Config {
         config.store_file(config_dir)
     }
 }
+
+#[derive(Debug, Clone)]
+pub struct HttpClientConfig {
+    pub timeout: Option<Duration>,
+    pub connect_timeout: Option<Duration>,
+    pub read_timeout: Option<Duration>,
+}
+
+impl HttpClientConfig {
+    pub fn env() -> Self {
+        fn env_duration(name: &str) -> Option<Duration> {
+            let duration_str = std::env::var(name).ok()?;
+            Some(iso8601::duration(&duration_str).ok()?.into())
+        }
+
+        let timeout = env_duration("GOLEM_TIMEOUT");
+        let connect_timeout = env_duration("GOLEM_CONNECT_TIMEOUT");
+        let read_timeout = env_duration("GOLEM_READ_TIMEOUT");
+
+        Self {
+            timeout,
+            connect_timeout,
+            read_timeout,
+        }
+    }
+
+    pub fn health_check() -> Self {
+        fn min_opt(d1: Duration, opt_d2: Option<Duration>) -> Duration {
+            match opt_d2 {
+                None => d1,
+                Some(d2) => min(d1, d2),
+            }
+        }
+
+        let from_env = Self::env();
+
+        let timeout = Some(min_opt(Duration::from_secs(2), from_env.timeout));
+        let connect_timeout = Some(min_opt(Duration::from_secs(1), from_env.connect_timeout));
+        let read_timeout = Some(min_opt(Duration::from_secs(1), from_env.read_timeout));
+        Self {
+            timeout,
+            connect_timeout,
+            read_timeout,
+        }
+    }
+}
diff --git a/golem-cli/src/oss/factory.rs b/golem-cli/src/oss/factory.rs
@@ -17,6 +17,7 @@ use crate::clients::api_deployment::ApiDeploymentClient;
 use crate::clients::component::ComponentClient;
 use crate::clients::health_check::HealthCheckClient;
 use crate::clients::worker::WorkerClient;
+use crate::config::HttpClientConfig;
 use crate::factory::{FactoryWithAuth, ServiceFactory};
 use crate::model::GolemError;
 use crate::oss::clients::api_definition::ApiDefinitionClientLive;
@@ -37,27 +38,56 @@ pub struct OssServiceFactory {
 }
 
 impl OssServiceFactory {
-    fn client(&self) -> Result<reqwest::Client, GolemError> {
+    fn client(&self, http_client_config: &HttpClientConfig) -> Result<reqwest::Client, GolemError> {
         let mut builder = reqwest::Client::builder();
         if self.allow_insecure {
             builder = builder.danger_accept_invalid_certs(true);
         }
 
+        if let Some(timeout) = http_client_config.timeout {
+            builder = builder.timeout(timeout);
+        }
+        if let Some(connect_timeout) = http_client_config.connect_timeout {
+            builder = builder.connect_timeout(connect_timeout);
+        }
+        if let Some(read_timeout) = http_client_config.read_timeout {
+            builder = builder.read_timeout(read_timeout);
+        }
+
         Ok(builder.connection_verbose(true).build()?)
     }
 
-    fn component_context(&self) -> Result<Context, GolemError> {
+    fn component_context_base(
+        &self,
+        http_client_config: &HttpClientConfig,
+    ) -> Result<Context, GolemError> {
         Ok(Context {
             base_url: self.component_url.clone(),
-            client: self.client()?,
+            client: self.client(http_client_config)?,
         })
     }
-    fn worker_context(&self) -> Result<Context, GolemError> {
+    fn worker_context_base(
+        &self,
+        http_client_config: &HttpClientConfig,
+    ) -> Result<Context, GolemError> {
         Ok(Context {
             base_url: self.worker_url.clone(),
-            client: self.client()?,
+            client: self.client(http_client_config)?,
         })
     }
+
+    fn component_context(&self) -> Result<Context, GolemError> {
+        self.component_context_base(&HttpClientConfig::env())
+    }
+    fn component_context_health_check(&self) -> Result<Context, GolemError> {
+        self.component_context_base(&HttpClientConfig::health_check())
+    }
+    fn worker_context(&self) -> Result<Context, GolemError> {
+        self.worker_context_base(&HttpClientConfig::env())
+    }
+    fn worker_context_health_check(&self) -> Result<Context, GolemError> {
+        self.worker_context_base(&HttpClientConfig::health_check())
+    }
 }
 
 impl ServiceFactory for OssServiceFactory {
@@ -147,12 +177,12 @@ impl ServiceFactory for OssServiceFactory {
         Ok(vec![
             Box::new(HealthCheckClientLive {
                 client: golem_client::api::HealthCheckClientLive {
-                    context: self.component_context()?,
+                    context: self.component_context_health_check()?,
                 },
             }),
             Box::new(HealthCheckClientLive {
                 client: golem_client::api::HealthCheckClientLive {
-                    context: self.worker_context()?,
+                    context: self.worker_context_health_check()?,
                 },
             }),
         ])

diff --git a/golem-cli/tests/cli.rs b/golem-cli/tests/cli.rs
@@ -129,6 +129,8 @@ impl CliLive {
 
         let output = Command::new(&self.golem_cli_path)
             .env("GOLEM_CONFIG_DIR", self.config_dir.to_str().unwrap())
+            .env("GOLEM_CONNECT_TIMEOUT", "PT10S")
+            .env("GOLEM_READ_TIMEOUT", "PT5M")
             .arg(self.config.arg('F', "format"))
             .arg(self.format.to_string())
             .arg("-v")
@@ -183,6 +185,8 @@ impl Cli for CliLive {
 
         let mut child = Command::new(&self.golem_cli_path)
             .env("GOLEM_CONFIG_DIR", self.config_dir.to_str().unwrap())
+            .env("GOLEM_CONNECT_TIMEOUT", "PT10S")
+            .env("GOLEM_READ_TIMEOUT", "PT5M")
             .arg(self.config.arg('F', "format"))
             .arg(self.format.to_string())
             .args(args)