From 2c579d1b725e28661bb85505ccb2a3e013c18992 Mon Sep 17 00:00:00 2001 From: Fletcher Woodruff Date: Fri, 10 Oct 2025 16:17:01 -0700 Subject: [PATCH] feat: inline commit logic into settings packages There are 30-100ms delays between each systemd service that runs during the settings config phase. Avoid some of these delays by running the settings commit logic as part of the subsequent config phase. --- packages/os/pluto.service | 9 +- packages/os/settings-applier.service | 1 - packages/os/sundog.service | 2 - sources/Cargo.lock | 5 + sources/Cargo.toml | 1 + sources/api/pluto/Cargo.toml | 3 + sources/api/pluto/src/main.rs | 20 ++++ sources/api/settings-committer/src/lib.rs | 118 +++++++++++++++++++++ sources/api/settings-committer/src/main.rs | 112 ++----------------- sources/api/sundog/Cargo.toml | 1 + sources/api/sundog/src/main.rs | 7 ++ sources/api/thar-be-settings/Cargo.toml | 1 + sources/api/thar-be-settings/src/main.rs | 6 ++ 13 files changed, 172 insertions(+), 114 deletions(-) create mode 100644 sources/api/settings-committer/src/lib.rs diff --git a/packages/os/pluto.service b/packages/os/pluto.service index 5e5298fae..5ff22beb9 100644 --- a/packages/os/pluto.service +++ b/packages/os/pluto.service @@ -4,6 +4,8 @@ Description=Generate additional settings for Kubernetes # settings generators, and before settings-applier commits all the settings, renders # config files, and restarts services. After=network-online.target apiserver.service sundog.service +# Use Before here rather than After=pluto.service in settings-applier.service +# since this unit is only present on k8s variants. Before=settings-applier.service Requires=sundog.service # We don't want to restart the unit if the network goes offline or apiserver restarts @@ -15,16 +17,9 @@ RefuseManualStop=true [Service] Type=oneshot -# pluto needs access to any Kubernetes settings supplied through user-data, along with -# network-related settings such as proxy servers. Commit any settings that might have -# been generated during the sundog phase. -ExecStartPre=/usr/bin/settings-committer ExecStart=/usr/bin/pluto RemainAfterExit=true StandardError=journal+console [Install] -# settings-applier requires sundog to succeed as a signal that all settings generators ran -# successfully. Since pluto is an honorary settings generator, settings-applier also needs -# it to succeed before it can start. RequiredBy=settings-applier.service diff --git a/packages/os/settings-applier.service b/packages/os/settings-applier.service index f7d1198dc..1abb844b9 100644 --- a/packages/os/settings-applier.service +++ b/packages/os/settings-applier.service @@ -11,7 +11,6 @@ RefuseManualStop=true [Service] Type=oneshot -ExecStartPre=/usr/bin/settings-committer ExecStart=/usr/bin/thar-be-settings --all RemainAfterExit=true StandardError=journal+console diff --git a/packages/os/sundog.service b/packages/os/sundog.service index 093089e93..ea5776b48 100644 --- a/packages/os/sundog.service +++ b/packages/os/sundog.service @@ -12,8 +12,6 @@ RefuseManualStop=true [Service] Type=oneshot -# Commit first so we can use settings from user data. -ExecStartPre=/usr/bin/settings-committer ExecStart=/usr/bin/sundog RemainAfterExit=true StandardError=journal+console diff --git a/sources/Cargo.lock b/sources/Cargo.lock index 069748f85..058d6979c 100644 --- a/sources/Cargo.lock +++ b/sources/Cargo.lock @@ -4146,9 +4146,12 @@ dependencies = [ "generate-readme", "httptest", "imdsclient", + "log", "rustls 0.23.31", "serde", "serde_json", + "settings-committer", + "simplelog", "snafu", "tempfile", "tokio", @@ -5609,6 +5612,7 @@ dependencies = [ "models", "serde", "serde_json", + "settings-committer", "shlex", "simplelog", "snafu", @@ -5755,6 +5759,7 @@ dependencies = [ "nix 0.26.4", "schnauzer", "serde_json", + "settings-committer", "simplelog", "snafu", "tempfile", diff --git a/sources/Cargo.toml b/sources/Cargo.toml index b8cad87a6..814fdb2a6 100644 --- a/sources/Cargo.toml +++ b/sources/Cargo.toml @@ -96,6 +96,7 @@ models = { version = "0.1", path = "models" } parse-datetime = { version = "0.1", path = "parse-datetime" } pciclient = { version = "0.1", path = "pciclient" } retry-read = { version = "0.1", path = "retry-read" } +settings-committer = { version = "0.1", path = "api/settings-committer" } signpost = { version = "0.1", path = "updater/signpost" } storewolf = { version = "0.1", path = "api/storewolf" } simple-settings-plugin = { version = "0.1", path = "api/simple-settings-plugin" } diff --git a/sources/api/pluto/Cargo.toml b/sources/api/pluto/Cargo.toml index 192be8aae..8189f7f3c 100644 --- a/sources/api/pluto/Cargo.toml +++ b/sources/api/pluto/Cargo.toml @@ -25,9 +25,12 @@ bottlerocket-modeled-types.workspace = true bottlerocket-settings-models.workspace = true constants.workspace = true imdsclient.workspace = true +log.workspace = true rustls.workspace = true serde = { workspace = true, features = ["derive"] } serde_json.workspace = true +settings-committer.workspace = true +simplelog.workspace = true snafu.workspace = true tempfile.workspace = true tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } diff --git a/sources/api/pluto/src/main.rs b/sources/api/pluto/src/main.rs index 0875ca8c4..ad76d3707 100644 --- a/sources/api/pluto/src/main.rs +++ b/sources/api/pluto/src/main.rs @@ -30,6 +30,8 @@ Pluto returns a special exit code of 2 to inform `sundog` that a setting should example, if `max-pods` cannot be generated, we want `sundog` to skip it without failing since a reasonable default is available. */ +#[macro_use] +extern crate log; mod api; mod aws; @@ -37,6 +39,7 @@ mod ec2; mod eks; use api::{settings_view_get, settings_view_set, SettingsViewDelta}; +use simplelog::{Config as LogConfig, LevelFilter, SimpleLogger}; use aws_smithy_experimental::hyper_1_0::CryptoMode; use base64::Engine; use bottlerocket_modeled_types::{KubernetesClusterDnsIp, KubernetesHostnameOverrideSource}; @@ -72,6 +75,7 @@ const PROVIDER: CryptoMode = CryptoMode::AwsLcFips; mod error { use crate::{api, ec2, eks}; + use settings_committer::SettingsCommitterError; use snafu::Snafu; use std::net::AddrParseError; @@ -162,6 +166,12 @@ mod error { #[snafu(display("Unable to create tempdir: {}", source))] Tempdir { source: std::io::Error }, + + #[snafu(display("Unable to commit pending transaction: {}", source))] + Commit { source: SettingsCommitterError }, + + #[snafu(display("Logger setup error: {}", source))] + Logger { source: log::SetLoggerError }, } } @@ -488,6 +498,16 @@ fn set_aws_config(aws_k8s_info: &SettingsViewDelta, filepath: &Path) -> Result<( } async fn run() -> Result<()> { + // SimpleLogger will send errors to stderr and anything less to stdout. + SimpleLogger::init(LevelFilter::Info, LogConfig::default()).context(error::LoggerSnafu)?; + info!("Pluto started"); + + // pluto needs access to any Kubernetes settings supplied through user-data, along with + // network-related settings such as proxy servers. Commit any settings that might have + // been generated during the sundog phase. + settings_committer::commit(constants::API_SOCKET, constants::LAUNCH_TRANSACTION).await.context(error::CommitSnafu)?; + + info!("Retrieving settings values"); let mut client = ImdsClient::new(); let current_settings = api::get_aws_k8s_info().await.context(error::AwsInfoSnafu)?; let mut aws_k8s_info = SettingsViewDelta::from_api_response(current_settings); diff --git a/sources/api/settings-committer/src/lib.rs b/sources/api/settings-committer/src/lib.rs new file mode 100644 index 000000000..fd063674d --- /dev/null +++ b/sources/api/settings-committer/src/lib.rs @@ -0,0 +1,118 @@ +#[macro_use] +extern crate log; + +use snafu::ResultExt; +use std::{collections::HashMap}; + +const API_PENDING_URI_BASE: &str = "/v2/tx"; +const API_COMMIT_URI_BASE: &str = "/tx/commit"; + +pub mod error { + use http::StatusCode; + use snafu::Snafu; + + /// Potential errors during user data management. + #[derive(Debug, Snafu)] + #[snafu(visibility(pub(super)))] + pub enum SettingsCommitterError { + #[snafu(display("Error sending {} to {}: {}", method, uri, source))] + APIRequest { + method: String, + uri: String, + #[snafu(source(from(apiclient::Error, Box::new)))] + source: Box, + }, + + #[snafu(display("Error {} when sending {} to {}: {}", code, method, uri, response_body))] + APIResponse { + method: String, + uri: String, + code: StatusCode, + response_body: String, + }, + } +} +pub use error::SettingsCommitterError; +pub type Result = std::result::Result; + +/// Checks pending settings and logs them. We don't want to prevent a +/// commit if there's a blip in retrieval or parsing of the pending +/// settings. We know the system won't be functional without a commit, +/// but we can live without logging what was committed. +async fn check_pending_settings>(socket_path: S, transaction: &str) { + let uri = format!("{API_PENDING_URI_BASE}?tx={transaction}"); + + debug!("GET-ing {uri} to determine if there are pending settings"); + let get_result = apiclient::raw_request(socket_path.as_ref(), &uri, "GET", None).await; + let response_body = match get_result { + Ok((code, response_body)) => { + if !code.is_success() { + warn!("Got {code} when sending GET to {uri}: {response_body}"); + return; + } + response_body + } + Err(err) => { + warn!("Failed to GET pending settings from {uri}: {err}"); + return; + } + }; + + let pending_result: serde_json::Result> = + serde_json::from_str(&response_body); + match pending_result { + Ok(pending) => { + debug!("Pending settings for tx {}: {:?}", transaction, &pending); + } + Err(err) => { + warn!("Failed to parse response from {uri}: {err}"); + } + } +} + +/// Commits pending settings to live. +async fn commit_pending_settings>(socket_path: S, transaction: &str) -> Result<()> { + let uri = format!("{API_COMMIT_URI_BASE}?tx={transaction}"); + debug!("POST-ing to {uri} to move pending settings to live"); + + if let Err(e) = apiclient::raw_request(socket_path.as_ref(), &uri, "POST", None).await { + match e { + // Some types of response errors are OK for this use. + apiclient::Error::ResponseStatus { code, body, .. } => { + if code.as_u16() == 422 { + info!("settings-committer found no settings changes to commit"); + return Ok(()); + } else { + return error::APIResponseSnafu { + method: "POST", + uri, + code, + response_body: body, + } + .fail(); + } + } + // Any other type of error means we couldn't even make the request. + _ => { + return Err(e).context(error::APIRequestSnafu { + method: "POST", + uri, + }); + } + } + } + Ok(()) +} + +pub async fn commit(socket_path: &str, transaction: &str) -> Result<()> { + if log_enabled!(log::Level::Debug) { + // We log the pending settings at Debug, so only fetch them if they won't be filtered. + info!("Checking pending settings."); + check_pending_settings(socket_path, transaction).await; + } + + info!("Committing settings."); + commit_pending_settings(socket_path, transaction).await?; + + Ok(()) +} diff --git a/sources/api/settings-committer/src/main.rs b/sources/api/settings-committer/src/main.rs index 6eb59d36e..8364e4a91 100644 --- a/sources/api/settings-committer/src/main.rs +++ b/sources/api/settings-committer/src/main.rs @@ -8,116 +8,27 @@ By default, it commits the 'bottlerocket-launch' transaction, which is used to o The `--transaction` argument can be used to specify another transaction. */ -#[macro_use] -extern crate log; - +use settings_committer::{commit}; use simplelog::{Config as LogConfig, LevelFilter, SimpleLogger}; use snafu::ResultExt; use std::str::FromStr; -use std::{collections::HashMap, env, process}; - -const API_PENDING_URI_BASE: &str = "/v2/tx"; -const API_COMMIT_URI_BASE: &str = "/tx/commit"; - -type Result = std::result::Result; +use std::{env, process}; mod error { - use http::StatusCode; + use settings_committer::SettingsCommitterError; use snafu::Snafu; - /// Potential errors during user data management. #[derive(Debug, Snafu)] #[snafu(visibility(pub(super)))] - pub(super) enum SettingsCommitterError { - #[snafu(display("Error sending {} to {}: {}", method, uri, source))] - APIRequest { - method: String, - uri: String, - #[snafu(source(from(apiclient::Error, Box::new)))] - source: Box, - }, - - #[snafu(display("Error {} when sending {} to {}: {}", code, method, uri, response_body))] - APIResponse { - method: String, - uri: String, - code: StatusCode, - response_body: String, - }, + pub(super) enum Error { + #[snafu(display("Error when committing settings: {}", source))] + Commit { source: SettingsCommitterError }, #[snafu(display("Logger setup error: {}", source))] Logger { source: log::SetLoggerError }, } } - -/// Checks pending settings and logs them. We don't want to prevent a -/// commit if there's a blip in retrieval or parsing of the pending -/// settings. We know the system won't be functional without a commit, -/// but we can live without logging what was committed. -async fn check_pending_settings>(socket_path: S, transaction: &str) { - let uri = format!("{API_PENDING_URI_BASE}?tx={transaction}"); - - debug!("GET-ing {uri} to determine if there are pending settings"); - let get_result = apiclient::raw_request(socket_path.as_ref(), &uri, "GET", None).await; - let response_body = match get_result { - Ok((code, response_body)) => { - if !code.is_success() { - warn!("Got {code} when sending GET to {uri}: {response_body}"); - return; - } - response_body - } - Err(err) => { - warn!("Failed to GET pending settings from {uri}: {err}"); - return; - } - }; - - let pending_result: serde_json::Result> = - serde_json::from_str(&response_body); - match pending_result { - Ok(pending) => { - debug!("Pending settings for tx {}: {:?}", transaction, &pending); - } - Err(err) => { - warn!("Failed to parse response from {uri}: {err}"); - } - } -} - -/// Commits pending settings to live. -async fn commit_pending_settings>(socket_path: S, transaction: &str) -> Result<()> { - let uri = format!("{API_COMMIT_URI_BASE}?tx={transaction}"); - debug!("POST-ing to {uri} to move pending settings to live"); - - if let Err(e) = apiclient::raw_request(socket_path.as_ref(), &uri, "POST", None).await { - match e { - // Some types of response errors are OK for this use. - apiclient::Error::ResponseStatus { code, body, .. } => { - if code.as_u16() == 422 { - info!("settings-committer found no settings changes to commit"); - return Ok(()); - } else { - return error::APIResponseSnafu { - method: "POST", - uri, - code, - response_body: body, - } - .fail(); - } - } - // Any other type of error means we couldn't even make the request. - _ => { - return Err(e).context(error::APIRequestSnafu { - method: "POST", - uri, - }); - } - } - } - Ok(()) -} +type Result = std::result::Result; /// Store the args we receive on the command line struct Args { @@ -201,14 +112,7 @@ async fn run() -> Result<()> { // SimpleLogger will send errors to stderr and anything less to stdout. SimpleLogger::init(args.log_level, LogConfig::default()).context(error::LoggerSnafu)?; - if args.log_level >= LevelFilter::Debug { - // We log the pending settings at Debug, so only fetch them if they won't be filtered. - info!("Checking pending settings."); - check_pending_settings(&args.socket_path, &args.transaction).await; - } - - info!("Committing settings."); - commit_pending_settings(&args.socket_path, &args.transaction).await?; + commit(&args.socket_path, &args.transaction).await.context(error::CommitSnafu)?; Ok(()) } diff --git a/sources/api/sundog/Cargo.toml b/sources/api/sundog/Cargo.toml index abd4d9e8e..017f7d7d3 100644 --- a/sources/api/sundog/Cargo.toml +++ b/sources/api/sundog/Cargo.toml @@ -18,6 +18,7 @@ log.workspace = true models.workspace = true serde = { workspace = true, features = ["derive"] } serde_json.workspace = true +settings-committer.workspace = true simplelog.workspace = true shlex.workspace = true snafu.workspace = true diff --git a/sources/api/sundog/src/main.rs b/sources/api/sundog/src/main.rs index 21aa44baa..3aa813367 100644 --- a/sources/api/sundog/src/main.rs +++ b/sources/api/sundog/src/main.rs @@ -31,6 +31,7 @@ const SETTINGS_GENERATOR_TIMEOUT: Duration = Duration::from_secs(360); /// Potential errors during Sundog execution mod error { use http::StatusCode; + use settings_committer::SettingsCommitterError; use snafu::Snafu; use datastore::{self, deserialization, serialization, KeyType}; @@ -162,6 +163,9 @@ mod error { #[snafu(display("Logger setup error: {}", source))] Logger { source: log::SetLoggerError }, + + #[snafu(display("Unable to commit pending transaction: {}", source))] + Commit { source: SettingsCommitterError }, } } @@ -578,6 +582,9 @@ async fn run() -> Result<()> { info!("Sundog started"); + // Commit first so we can use settings from user data. + settings_committer::commit(constants::API_SOCKET, constants::LAUNCH_TRANSACTION).await.context(error::CommitSnafu)?; + info!("Retrieving setting generators"); let generators = get_setting_generators(&args.socket_path).await?; if generators.is_empty() { diff --git a/sources/api/thar-be-settings/Cargo.toml b/sources/api/thar-be-settings/Cargo.toml index 9590501df..f6899a185 100644 --- a/sources/api/thar-be-settings/Cargo.toml +++ b/sources/api/thar-be-settings/Cargo.toml @@ -20,6 +20,7 @@ models.workspace = true nix.workspace = true schnauzer.workspace = true serde_json.workspace = true +settings-committer.workspace = true simplelog.workspace = true snafu.workspace = true tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } diff --git a/sources/api/thar-be-settings/src/main.rs b/sources/api/thar-be-settings/src/main.rs index 6c81bb809..6f9ea1292 100644 --- a/sources/api/thar-be-settings/src/main.rs +++ b/sources/api/thar-be-settings/src/main.rs @@ -14,6 +14,7 @@ use tokio::runtime::Runtime; use thar_be_settings::{config, get_changed_settings, service}; mod error { + use settings_committer::SettingsCommitterError; use snafu::Snafu; #[derive(Debug, Snafu)] @@ -21,6 +22,9 @@ mod error { pub(super) enum Error { #[snafu(display("Logger setup error: {}", source))] Logger { source: log::SetLoggerError }, + + #[snafu(display("Unable to commit pending transaction: {}", source))] + Commit { source: SettingsCommitterError }, } } @@ -155,6 +159,8 @@ async fn run(args: Args) -> Result<(), Box> { info!("thar-be-settings started"); + settings_committer::commit(constants::API_SOCKET, constants::LAUNCH_TRANSACTION).await.context(error::CommitSnafu)?; + match args.mode { RunMode::SpecificKeys => { // Get the settings that changed via stdin