From 7e4e33becfcc0ed9e3176cc26d6c6cd1cbe483a5 Mon Sep 17 00:00:00 2001 From: Wenyu Huang Date: Thu, 24 Nov 2022 11:22:24 -0500 Subject: [PATCH 1/2] nydusd: automatically retry registry http scheme Signed-off-by: Wenyu Huang --- docs/containerd-env-setup.md | 3 +- docs/nydusd.md | 4 +- storage/src/backend/registry.rs | 101 ++++++++++++++++++++++++++++---- 3 files changed, 92 insertions(+), 16 deletions(-) diff --git a/docs/containerd-env-setup.md b/docs/containerd-env-setup.md index f5232ddba56..edb2716ae12 100644 --- a/docs/containerd-env-setup.md +++ b/docs/containerd-env-setup.md @@ -29,7 +29,8 @@ $ sudo tee /etc/nydusd-config.json > /dev/null << EOF "backend": { "type": "registry", "config": { - "scheme": "http", + // Registry url scheme, leave empty to automatically detect, otherwise specify to https or http. + "scheme": "", "skip_verify": false, "timeout": 5, "connect_timeout": 5, diff --git a/docs/nydusd.md b/docs/nydusd.md index 93c40b43afa..9fa3407d198 100644 --- a/docs/nydusd.md +++ b/docs/nydusd.md @@ -181,8 +181,8 @@ We are working on enabling cloud-hypervisor support for nydus. "type": "registry", "config": { ... - // Registry url scheme, https or http - "scheme": "http", + // Registry url scheme, leave empty to automatically detect, otherwise specify to https or http. + "scheme": "", // Registry hostname with format `$host:$port` "host": "my-registry:5000", // Skip SSL certificate validation for HTTPS scheme diff --git a/storage/src/backend/registry.rs b/storage/src/backend/registry.rs index e7d7094fcfe..ba769edc9b4 100644 --- a/storage/src/backend/registry.rs +++ b/storage/src/backend/registry.rs @@ -4,8 +4,10 @@ //! Storage backend driver to access blobs on container image registry. use std::collections::HashMap; -use std::io::{Error, Read, Result}; -use std::sync::atomic::Ordering; +use std::error::Error; +use std::fmt::Display; +use std::io::{Read, Result}; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, RwLock}; use std::thread; use std::time::{Duration, SystemTime, UNIX_EPOCH}; @@ -43,7 +45,7 @@ pub enum RegistryError { Scheme(String), Auth(String), ResponseHead(String), - Response(Error), + Response(std::io::Error), Transport(reqwest::Error), } @@ -134,9 +136,27 @@ enum Auth { Bearer(BearerAuth), } +pub struct Scheme(AtomicBool); + +impl Scheme { + fn new(value: bool) -> Self { + Scheme(AtomicBool::new(value)) + } +} + +impl Display for Scheme { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if self.0.load(Ordering::Relaxed) { + write!(f, "https") + } else { + write!(f, "http") + } + } +} + struct RegistryState { // HTTP scheme like: https, http - scheme: String, + scheme: Scheme, host: String, // Image repo name like: library/ubuntu repo: String, @@ -166,6 +186,16 @@ struct RegistryState { cached_bearer_auth: ArcSwapOption, } +fn needs_fallback_http(e: &dyn Error) -> bool { + match e.source() { + Some(err) => match err.source() { + Some(err) => err.to_string().contains("error:1408F10B"), + None => false, + }, + None => false, + } +} + impl RegistryState { fn url(&self, path: &str, query: &[&str]) -> std::result::Result { let path = if query.is_empty() { @@ -304,6 +334,12 @@ impl RegistryState { _ => None, } } + + fn fallback_http(&self) { + if self.scheme.0.load(Ordering::Relaxed) { + self.scheme.0.store(false, Ordering::Relaxed); + } + } } struct RegistryReader { @@ -487,8 +523,28 @@ impl RegistryReader { return self._try_read(buf, offset, false); } } else { - resp = - self.request::<&[u8]>(Method::GET, url.as_str(), None, headers.clone(), false)?; + resp = match self.request::<&[u8]>( + Method::GET, + url.as_str(), + None, + headers.clone(), + false, + ) { + Ok(res) => res, + Err(RegistryError::Request(ConnectionError::Common(e))) + if needs_fallback_http(&e) => + { + self.state.fallback_http(); + let url = self + .state + .url(format!("/blobs/sha256:{}", self.blob_id).as_str(), &[]) + .map_err(RegistryError::Url)?; + self.request::<&[u8]>(Method::GET, url.as_str(), None, headers.clone(), false)? + } + Err(e) => { + return Err(e); + } + }; let status = resp.status(); // Handle redirect request and cache redirect url @@ -559,8 +615,24 @@ impl BlobReader for RegistryReader { .state .url(&format!("/blobs/sha256:{}", self.blob_id), &[]) .map_err(RegistryError::Url)?; + let resp = - self.request::<&[u8]>(Method::HEAD, url.as_str(), None, HeaderMap::new(), true)?; + match self.request::<&[u8]>(Method::HEAD, url.as_str(), None, HeaderMap::new(), true) { + Ok(res) => res, + Err(RegistryError::Request(ConnectionError::Common(e))) + if needs_fallback_http(&e) => + { + self.state.fallback_http(); + let url = self + .state + .url(format!("/blobs/sha256:{}", self.blob_id).as_str(), &[]) + .map_err(RegistryError::Url)?; + self.request::<&[u8]>(Method::HEAD, url.as_str(), None, HeaderMap::new(), true)? + } + Err(e) => { + return Err(BackendError::Registry(e)); + } + }; let content_length = resp .headers() .get(CONTENT_LENGTH) @@ -620,8 +692,14 @@ impl Registry { Cache::new(String::new()) }; + let scheme = if !config.scheme.is_empty() && config.scheme == "http" { + Scheme::new(false) + } else { + Scheme::new(true) + }; + let state = Arc::new(RegistryState { - scheme: config.scheme, + scheme, host: config.host, repo: config.repo, auth, @@ -796,8 +874,8 @@ mod tests { #[test] fn test_state_url() { - let mut state = RegistryState { - scheme: "http".to_string(), + let state = RegistryState { + scheme: Scheme::new(false), host: "alibaba-inc.com".to_string(), repo: "nydus".to_string(), auth: None, @@ -820,9 +898,6 @@ mod tests { state.url("image", &[]).unwrap(), "http://alibaba-inc.com/v2/nydusimage".to_owned() ); - - state.scheme = "unknown_schema".to_owned(); - assert!(state.url("image", &[]).is_err()); } #[test] From 6f7c8e5a20fe8ed1ac35c7a578b167a44fd51f51 Mon Sep 17 00:00:00 2001 From: Yan Song Date: Wed, 30 Nov 2022 03:07:27 +0000 Subject: [PATCH 2/2] storage: enhance retryable registry http scheme Check the tls connection error with `wrong version number` keywords, it's more reliable than the specific error code. Signed-off-by: Yan Song --- docs/containerd-env-setup.md | 3 +-- storage/src/backend/registry.rs | 41 +++++++++++++++++++++------------ 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/docs/containerd-env-setup.md b/docs/containerd-env-setup.md index edb2716ae12..dff95996d61 100644 --- a/docs/containerd-env-setup.md +++ b/docs/containerd-env-setup.md @@ -29,7 +29,6 @@ $ sudo tee /etc/nydusd-config.json > /dev/null << EOF "backend": { "type": "registry", "config": { - // Registry url scheme, leave empty to automatically detect, otherwise specify to https or http. "scheme": "", "skip_verify": false, "timeout": 5, @@ -59,7 +58,7 @@ EOF Note: -- You might have to change the scheme from `http` to `https` according to you registry configuration. +- The `scheme` is registry url scheme, leave empty to automatically detect, otherwise specify to `https` or `http` according to your registry server configuration. - The `auth` is base64 encoded `username:password`. It is required by `nydusd` to lazily pull image data from registry which is authentication enabled. - `containerd-nydus-grpc` will automatically read docker login auth from the configuration `$HOME/.docker/config.json`, otherwise please copy it to replace `YOUR_LOGIN_AUTH=`. diff --git a/storage/src/backend/registry.rs b/storage/src/backend/registry.rs index ba769edc9b4..47612482b1c 100644 --- a/storage/src/backend/registry.rs +++ b/storage/src/backend/registry.rs @@ -186,16 +186,6 @@ struct RegistryState { cached_bearer_auth: ArcSwapOption, } -fn needs_fallback_http(e: &dyn Error) -> bool { - match e.source() { - Some(err) => match err.source() { - Some(err) => err.to_string().contains("error:1408F10B"), - None => false, - }, - None => false, - } -} - impl RegistryState { fn url(&self, path: &str, query: &[&str]) -> std::result::Result { let path = if query.is_empty() { @@ -210,6 +200,29 @@ impl RegistryState { Ok(url.to_string()) } + fn needs_fallback_http(&self, e: &dyn Error) -> bool { + match e.source() { + Some(err) => match err.source() { + Some(err) => { + if !self.scheme.0.load(Ordering::Relaxed) { + return false; + } + let msg = err.to_string().to_lowercase(); + // As far as we can observe, if we try to establish a tls connection + // with the http registry server, we will encounter this type of error: + // https://github.com/openssl/openssl/blob/6b3d28757620e0781bb1556032bb6961ee39af63/crypto/err/openssl.txt#L1574 + let fallback = msg.contains("wrong version number"); + if fallback { + warn!("fallback to http due to tls connection error: {}", err); + } + fallback + } + None => false, + }, + None => false, + } + } + /// Request registry authentication server to get bearer token fn get_token(&self, auth: BearerAuth, connection: &Arc) -> Result { // The information needed for getting token needs to be placed both in @@ -336,9 +349,7 @@ impl RegistryState { } fn fallback_http(&self) { - if self.scheme.0.load(Ordering::Relaxed) { - self.scheme.0.store(false, Ordering::Relaxed); - } + self.scheme.0.store(false, Ordering::Relaxed); } } @@ -532,7 +543,7 @@ impl RegistryReader { ) { Ok(res) => res, Err(RegistryError::Request(ConnectionError::Common(e))) - if needs_fallback_http(&e) => + if self.state.needs_fallback_http(&e) => { self.state.fallback_http(); let url = self @@ -620,7 +631,7 @@ impl BlobReader for RegistryReader { match self.request::<&[u8]>(Method::HEAD, url.as_str(), None, HeaderMap::new(), true) { Ok(res) => res, Err(RegistryError::Request(ConnectionError::Common(e))) - if needs_fallback_http(&e) => + if self.state.needs_fallback_http(&e) => { self.state.fallback_http(); let url = self