From f0d6fd6971c6c1560e701471acad9a459f2035b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6nke=20Liebau?= Date: Mon, 14 Dec 2020 17:06:07 +0100 Subject: [PATCH 1/4] Added rendering of template strings to environment variables. --- src/provider/states/starting.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/provider/states/starting.rs b/src/provider/states/starting.rs index 7c743a1..0d3c0a4 100644 --- a/src/provider/states/starting.rs +++ b/src/provider/states/starting.rs @@ -65,9 +65,11 @@ impl State for Starting { .map(|env_var| { ( String::from(&env_var.name), - String::from( + CreatingConfig::render_config_template( + &template_data, &env_var.value.clone().unwrap_or_else(|| String::from("")), - ), + ) + .unwrap_or(String::from("")), ) }) .collect::>() From 479a1e8b323d3107d461abee5fbcd34e6fe9a685 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6nke=20Liebau?= Date: Mon, 14 Dec 2020 18:23:56 +0100 Subject: [PATCH 2/4] Added error handling to variable rendering. --- src/provider/states/starting.rs | 34 +++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/src/provider/states/starting.rs b/src/provider/states/starting.rs index 0d3c0a4..de03bf6 100644 --- a/src/provider/states/starting.rs +++ b/src/provider/states/starting.rs @@ -56,23 +56,37 @@ impl State for Starting { "Starting command: {:?} with arguments {:?}", binary, os_args ); + + // Check if environment variables were stated for the container let env_variables = if let Some(vars) = container.env() { debug!( "Got environment vars: {:?} service {}", vars, pod_state.service_name ); - vars.iter() + match vars + .iter() .map(|env_var| { - ( - String::from(&env_var.name), - CreatingConfig::render_config_template( - &template_data, - &env_var.value.clone().unwrap_or_else(|| String::from("")), - ) - .unwrap_or(String::from("")), - ) + let name = String::from(&env_var.name); + let value_result = CreatingConfig::render_config_template( + &template_data, + &env_var.value.clone().unwrap_or_else(|| String::from("")), + ); + match value_result { + Ok(value) => Ok((name, value)), + Err(error) => Err(error), + } }) - .collect::>() + .collect() + { + // All variables were rendered correctly, continue + Ok(rendered_value) => rendered_value, + // One or more variables failed to render, transition this pod to a failed + // state + Err(error) => { + error!("Failed to render value for env var due to: {:?}", error); + return Transition::Complete(Err(anyhow::Error::from(error))); + } + } } else { debug!( "No environment vars set for service {}", From 172fcfe60160d84b43e801b8a20f2422f8054dbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6nke=20Liebau?= Date: Tue, 15 Dec 2020 11:51:09 +0100 Subject: [PATCH 3/4] Adressed review comments. --- src/provider/states/starting.rs | 37 +++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/src/provider/states/starting.rs b/src/provider/states/starting.rs index 6cbf461..de57462 100644 --- a/src/provider/states/starting.rs +++ b/src/provider/states/starting.rs @@ -57,37 +57,42 @@ impl State for Starting { binary, os_args ); - // Check if environment variables were stated for the container + // Check if environment variables are set on the container - if some are present + // we render all values as templates to replace configroot, packageroot and logroot + // directories in case they are referenced in the values + // + // If even one of these renderings fails the entire pod will be failed and + // transitioned to a complete state with the error that occurred. + // If all renderings work, the vec<(String,String)> is returned as value and used + // later when starting the process let env_variables = if let Some(vars) = container.env() { debug!( "Got environment vars: {:?} service {}", vars, pod_state.service_name ); - match vars + let render_result = vars .iter() .map(|env_var| { - let name = String::from(&env_var.name); - let value_result = CreatingConfig::render_config_template( + // Replace variables in value + CreatingConfig::render_config_template( &template_data, - &env_var.value.clone().unwrap_or_else(|| String::from("")), - ); - match value_result { - Ok(value) => Ok((name, value)), - Err(error) => Err(error), - } + &env_var.value.as_deref().unwrap_or_default(), + ) + .map(|value| (&env_var.name, value)) }) - .collect() - { - // All variables were rendered correctly, continue - Ok(rendered_value) => rendered_value, - // One or more variables failed to render, transition this pod to a failed - // state + .collect(); + + // If any single rendering failed, the overall result for the map will have + // collected the Err which we can check for here + match render_result { + Ok(rendered_values) => rendered_values, Err(error) => { error!("Failed to render value for env var due to: {:?}", error); return Transition::Complete(Err(anyhow::Error::from(error))); } } } else { + // No environment variables present for this container -> empty vec debug!( "No environment vars set for service {}", pod_state.service_name From fee81c6e0c04220f596ed3972ac6e193143c85e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6nke=20Liebau?= Date: Tue, 15 Dec 2020 21:21:50 +0100 Subject: [PATCH 4/4] Added extra comment explaining how Result collection works. --- src/provider/states/starting.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/provider/states/starting.rs b/src/provider/states/starting.rs index de57462..6c81541 100644 --- a/src/provider/states/starting.rs +++ b/src/provider/states/starting.rs @@ -65,6 +65,11 @@ impl State for Starting { // transitioned to a complete state with the error that occurred. // If all renderings work, the vec<(String,String)> is returned as value and used // later when starting the process + // This works because Result implements + // (FromIterator)[https://doc.rust-lang.org/std/result/enum.Result.html#method.from_iter] + // which returns a Result that is Ok(..) if none of the internal results contained + // an Error. If any error occurred, iteration stops on the first error and returns + // that in the outer result. let env_variables = if let Some(vars) = container.env() { debug!( "Got environment vars: {:?} service {}",