From ba991ce4b40d34a7f3c424a84c5d36ed27d0a6d8 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Fri, 21 May 2021 14:14:54 +0200 Subject: [PATCH 1/7] zbus version upgraded to prevent deadlock --- Cargo.lock | 74 +++++++++++++-------- Cargo.toml | 11 ++- src/provider/systemdmanager/manager.rs | 8 ++- src/provider/systemdmanager/systemd1_api.rs | 18 ++--- 4 files changed, 69 insertions(+), 42 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c51b6c7..6f1fc16 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -46,6 +46,28 @@ version = "1.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f8cb5d814eb646a863c4f24978cff2880c4be96ad8cde2c0f0678732902e271" +[[package]] +name = "async-broadcast" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8501c1102158e8cf7a73cab03102be0d2651b0f574a33a749ebc3aec72fcde9d" +dependencies = [ + "easy-parallel", + "event-listener", + "futures-core", +] + +[[package]] +name = "async-channel" +version = "1.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2114d64672151c0c5eaa5e131ec84a74f06e1e559830dabba01ca30605d66319" +dependencies = [ + "concurrent-queue", + "event-listener", + "futures-core", +] + [[package]] name = "async-compression" version = "0.3.8" @@ -505,6 +527,12 @@ version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ee2626afccd7561a06cf1367e2950c4718ea04565e20fb5029b6c7d8ad09abcf" +[[package]] +name = "easy-parallel" +version = "3.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1dd4afd79212583ff429b913ad6605242ed7eec277e950b1438f300748f948f4" + [[package]] name = "either" version = "1.6.1" @@ -858,9 +886,9 @@ dependencies = [ [[package]] name = "getrandom" -version = "0.2.2" +version = "0.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c9495705279e7140bf035dde1f6e750c162df8b625267cd52cc44e0b156732c8" +checksum = "7fcd999463524c52659517fe2cea98493cfe485d10565e7b0fb07dbba7ad2753" dependencies = [ "cfg-if 1.0.0", "libc", @@ -1627,18 +1655,6 @@ dependencies = [ "winapi 0.3.9", ] -[[package]] -name = "nix" -version = "0.19.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b2ccba0cfe4fdf15982d1674c69b1fd80bad427d293849982668dfe454bd61f2" -dependencies = [ - "bitflags", - "cc", - "cfg-if 1.0.0", - "libc", -] - [[package]] name = "nix" version = "0.20.0" @@ -1998,9 +2014,9 @@ checksum = "bc881b2c22681370c6a780e47af9840ef841837bc98118431d4e1868bd0c1086" [[package]] name = "proc-macro2" -version = "1.0.26" +version = "1.0.27" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a152013215dca273577e18d2bf00fa862b89b24169fb78c4c95aeb07992c9cec" +checksum = "f0d8caf72986c1a598726adc988bb5984792ef84f5ee5aa50209145ee8077038" dependencies = [ "unicode-xid", ] @@ -2165,7 +2181,7 @@ version = "0.6.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "34cf66eb183df1c5876e2dcf6b13d57340741e8dc255b48e40a26de954d06ae7" dependencies = [ - "getrandom 0.2.2", + "getrandom 0.2.3", ] [[package]] @@ -2222,7 +2238,7 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "528532f3d801c87aec9def2add9ca802fe569e44a544afe633765267840abe64" dependencies = [ - "getrandom 0.2.2", + "getrandom 0.2.3", "redox_syscall", ] @@ -2714,7 +2730,7 @@ dependencies = [ "kubelet", "lazy_static", "log", - "nix 0.20.0", + "nix", "oci-distribution", "regex", "reqwest", @@ -3460,7 +3476,7 @@ version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bc5cf98d8186244414c848017f0e2676b3fcb46807f6668a97dfe67359a3c4b7" dependencies = [ - "getrandom 0.2.2", + "getrandom 0.2.3", ] [[package]] @@ -3766,9 +3782,10 @@ dependencies = [ [[package]] name = "zbus" version = "2.0.0-beta.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3e1e656194618d167524f97e88ff9bf87f2b1e8bf58f357b2a7abfdff8cc85c9" +source = "git+https://gitlab.freedesktop.org/dbus/zbus.git?rev=9c551554e665532abc76469cdc73c1943bfb6285#9c551554e665532abc76469cdc73c1943bfb6285" dependencies = [ + "async-broadcast", + "async-channel", "async-io", "async-lock", "byteorder", @@ -3779,7 +3796,7 @@ dependencies = [ "futures-sink", "futures-util", "hex", - "nix 0.19.1", + "nix", "once_cell", "rand 0.8.3", "scoped-tls", @@ -3787,6 +3804,7 @@ dependencies = [ "serde_repr", "sha1", "slotmap", + "static_assertions", "zbus_macros", "zvariant", ] @@ -3794,8 +3812,7 @@ dependencies = [ [[package]] name = "zbus_macros" version = "2.0.0-beta.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fcd4cb372bc2cade3f2323e4104112dceb6819f5dd9afa98515b4e821d232932" +source = "git+https://gitlab.freedesktop.org/dbus/zbus.git?rev=9c551554e665532abc76469cdc73c1943bfb6285#9c551554e665532abc76469cdc73c1943bfb6285" dependencies = [ "proc-macro-crate", "proc-macro2", @@ -3806,20 +3823,19 @@ dependencies = [ [[package]] name = "zvariant" version = "2.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "678e7262502a135f49b1ece65010526649be7ee68acb80e1fc5377fc71fef878" +source = "git+https://gitlab.freedesktop.org/dbus/zbus.git?rev=9c551554e665532abc76469cdc73c1943bfb6285#9c551554e665532abc76469cdc73c1943bfb6285" dependencies = [ "byteorder", "enumflags2", "serde", + "static_assertions", "zvariant_derive", ] [[package]] name = "zvariant_derive" version = "2.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "27d7c34325a35020b94343389cc9391e0f8ac245cca9155429c4022d93141241" +source = "git+https://gitlab.freedesktop.org/dbus/zbus.git?rev=9c551554e665532abc76469cdc73c1943bfb6285#9c551554e665532abc76469cdc73c1943bfb6285" dependencies = [ "proc-macro-crate", "proc-macro2", diff --git a/Cargo.toml b/Cargo.toml index 1aceba8..ba50987 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,8 +42,15 @@ tar = "0.4" thiserror = "1.0" tokio = { version = "1.6", features = ["macros", "rt-multi-thread", "time"] } url = "2.2" -zbus = "2.0.0-beta.3" -zvariant = "2.6" +# The current zbus version 2.0.0-beta.3 causes deadlocks (see +# https://gitlab.freedesktop.org/dbus/zbus/-/issues/150). So the +# dependency is pinned to the current HEAD of the main branch until the +# next version is released. +zbus = { git = "https://gitlab.freedesktop.org/dbus/zbus.git", rev = "9c551554e665532abc76469cdc73c1943bfb6285" } +# The current zvariant version 2.6.0 is not compatible with the pinned +# zbus version. So the dependency is pinned to the current HEAD of the +# main branch until the next version is released. +zvariant = { git = "https://gitlab.freedesktop.org/dbus/zbus.git", rev = "9c551554e665532abc76469cdc73c1943bfb6285" } [dev-dependencies] indoc = "1.0" diff --git a/src/provider/systemdmanager/manager.rs b/src/provider/systemdmanager/manager.rs index 8dbf7bf..00e7b9c 100644 --- a/src/provider/systemdmanager/manager.rs +++ b/src/provider/systemdmanager/manager.rs @@ -57,8 +57,12 @@ impl SystemdManager { })? }; - let proxy = - AsyncManagerProxy::new(&connection).map_err(|e| RuntimeError { msg: e.to_string() })?; + // The maximum number of queued DBus messages must be higher + // than the number of containers which can be started and + // stopped simultaneously. + let connection = connection.set_max_queued(1024); + + let proxy = AsyncManagerProxy::new(&connection); // Depending on whether we are supposed to run in user space or system-wide // we'll pick the default directory to initialize the systemd manager with diff --git a/src/provider/systemdmanager/systemd1_api.rs b/src/provider/systemdmanager/systemd1_api.rs index c2154a7..0dc9eaf 100644 --- a/src/provider/systemdmanager/systemd1_api.rs +++ b/src/provider/systemdmanager/systemd1_api.rs @@ -80,7 +80,7 @@ macro_rules! impl_type_for_enum { } /// Type of an entry in a changes list -#[derive(Debug, Display, EnumString, EnumVariantNames, Eq, PartialEq)] +#[derive(Clone, Debug, Display, EnumString, EnumVariantNames, Eq, PartialEq)] #[strum(serialize_all = "kebab-case")] pub enum ChangeType { Symlink, @@ -91,7 +91,7 @@ impl_deserialize_for_enum!(ChangeType); impl_type_for_enum!(ChangeType); /// Entry of a changes list -#[derive(Debug, Type, Deserialize)] +#[derive(Clone, Debug, Type, Deserialize)] pub struct Change { pub change_type: ChangeType, pub filename: String, @@ -102,7 +102,7 @@ pub struct Change { type Changes = Vec; /// Mode in which a unit will be started -#[derive(Debug, Display, AsRefStr)] +#[derive(Clone, Debug, Display, AsRefStr)] #[strum(serialize_all = "kebab-case")] pub enum StartMode { /// The unit and its dependencies will be started, possibly @@ -134,7 +134,7 @@ impl_serialize_for_enum!(StartMode); impl_type_for_enum!(StartMode); /// Mode in which a unit will be stopped -#[derive(Debug, Display, AsRefStr)] +#[derive(Clone, Debug, Display, AsRefStr)] #[strum(serialize_all = "kebab-case")] pub enum StopMode { /// The unit and its dependencies will be stopped, possibly @@ -268,14 +268,14 @@ trait Manager { /// .map(|message| message.body::().unwrap()); /// # }); /// ``` -#[derive(Debug, Display, Eq, PartialEq, IntoStaticStr)] +#[derive(Clone, Debug, Display, Eq, PartialEq, IntoStaticStr)] pub enum ManagerSignals { /// Sent out each time a job is dequeued JobRemoved, } /// Result in the `JobRemoved` signal. -#[derive(Debug, Display, EnumString, EnumVariantNames, Eq, PartialEq)] +#[derive(Clone, Debug, Display, EnumString, EnumVariantNames, Eq, PartialEq)] #[strum(serialize_all = "kebab-case")] pub enum JobRemovedResult { /// Indicates successful execution of a job @@ -305,7 +305,7 @@ impl_deserialize_for_enum!(JobRemovedResult); impl_type_for_enum!(JobRemovedResult); /// Message body of [`ManagerSignals::JobRemoved`] -#[derive(Debug, Deserialize, Type)] +#[derive(Clone, Debug, Deserialize, Type)] pub struct JobRemovedSignal { /// Numeric job ID pub id: u32, @@ -322,7 +322,7 @@ pub struct JobRemovedSignal { /// ActiveState contains a state value that reflects whether the unit is /// currently active or not. -#[derive(Debug, Display, EnumString, Eq, PartialEq)] +#[derive(Clone, Debug, Display, EnumString, Eq, PartialEq)] #[strum(serialize_all = "kebab-case")] pub enum ActiveState { /// The unit is active. @@ -358,7 +358,7 @@ impl TryFrom for ActiveState { } /// Unique ID for a runtime cycle of a unit -#[derive(Debug, Eq, PartialEq)] +#[derive(Clone, Debug, Eq, PartialEq)] pub struct InvocationId(Vec); impl TryFrom for InvocationId { From 2349d8b766c6ef29ca7cb493197ee3fba8069687 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Fri, 21 May 2021 14:21:50 +0200 Subject: [PATCH 2/7] Version set to 0.2.1 and changelog updated --- CHANGELOG.adoc | 8 +++++++- Cargo.lock | 2 +- Cargo.toml | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 5f3bb5e..1c46ba4 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -1,6 +1,12 @@ = Changelog -== 0.3.0 - unreleased +== 0.2.1 - 2021-05-25 + +:168: https://github.com/stackabletech/agent/pull/168[#168] + +=== Fixed +* Deadlock fixed which occurred when multiple pods were started or + stopped simultaneously ({168}). == 0.2.0 - 2021-05-20 diff --git a/Cargo.lock b/Cargo.lock index 6f1fc16..7631c92 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2712,7 +2712,7 @@ checksum = "6e63cff320ae2c57904679ba7cb63280a3dc4613885beafb148ee7bf9aa9042d" [[package]] name = "stackable-agent" -version = "0.3.0-nightly" +version = "0.2.1" dependencies = [ "Inflector", "anyhow", diff --git a/Cargo.toml b/Cargo.toml index ba50987..58ad99e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "stackable-agent" description = "The component of the Stackable Platform that manages installation of services on the workers" -version = "0.3.0-nightly" +version = "0.2.1" authors = ["Sönke Liebau "] edition = "2018" license = "Apache-2.0" From 5433342dea610a74072ca3d2813dce8379a53032 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Fri, 21 May 2021 14:35:41 +0200 Subject: [PATCH 3/7] Doctests fixed --- src/provider/systemdmanager/systemd1_api.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/provider/systemdmanager/systemd1_api.rs b/src/provider/systemdmanager/systemd1_api.rs index 0dc9eaf..06c0e24 100644 --- a/src/provider/systemdmanager/systemd1_api.rs +++ b/src/provider/systemdmanager/systemd1_api.rs @@ -172,7 +172,7 @@ impl_type_for_enum!(StopMode); /// ``` /// # use stackable_agent::provider::systemdmanager::systemd1_api::*; /// let connection = zbus::Connection::new_system().unwrap(); -/// let manager = ManagerProxy::new(&connection).unwrap(); +/// let manager = ManagerProxy::new(&connection); /// let unit = manager.load_unit("dbus.service").unwrap(); /// ``` /// @@ -182,7 +182,7 @@ impl_type_for_enum!(StopMode); /// # use stackable_agent::provider::systemdmanager::systemd1_api::*; /// # tokio::runtime::Runtime::new().unwrap().block_on(async { /// let connection = zbus::azync::Connection::new_system().await.unwrap(); -/// let manager = AsyncManagerProxy::new(&connection).unwrap(); +/// let manager = AsyncManagerProxy::new(&connection); /// let unit = manager.load_unit("dbus.service").await.unwrap(); /// # }); /// ``` @@ -262,7 +262,7 @@ trait Manager { /// /// # tokio::runtime::Runtime::new().unwrap().block_on(async { /// let connection = zbus::azync::Connection::new_system().await.unwrap(); -/// let manager = AsyncManagerProxy::new(&connection).unwrap(); +/// let manager = AsyncManagerProxy::new(&connection); /// let signals = manager /// .receive_signal(ManagerSignals::JobRemoved.into()).await.unwrap() /// .map(|message| message.body::().unwrap()); From 4565b25fb4ebcb4f5ac265a7f2b730fcbced50b2 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Thu, 27 May 2021 12:06:40 +0200 Subject: [PATCH 4/7] DBus message queue size made dependent on maximum number of pods --- src/bin/stackable-agent.rs | 1 + src/provider/mod.rs | 3 ++- src/provider/systemdmanager/manager.rs | 4 ++-- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/bin/stackable-agent.rs b/src/bin/stackable-agent.rs index 326bfc4..dc46adb 100644 --- a/src/bin/stackable-agent.rs +++ b/src/bin/stackable-agent.rs @@ -95,6 +95,7 @@ async fn main() -> anyhow::Result<()> { agent_config.log_directory.clone(), agent_config.session, agent_config.pod_cidr, + krustlet_config.max_pods, ) .await .expect("Error initializing provider."); diff --git a/src/provider/mod.rs b/src/provider/mod.rs index 2c1e2d6..a8263d3 100644 --- a/src/provider/mod.rs +++ b/src/provider/mod.rs @@ -175,8 +175,9 @@ impl StackableProvider { log_directory: PathBuf, session: bool, pod_cidr: String, + max_pods: u16, ) -> Result { - let systemd_manager = Arc::new(SystemdManager::new(session).await?); + let systemd_manager = Arc::new(SystemdManager::new(session, max_pods).await?); let provider_state = ProviderState { handles: Default::default(), diff --git a/src/provider/systemdmanager/manager.rs b/src/provider/systemdmanager/manager.rs index 00e7b9c..05bf3d1 100644 --- a/src/provider/systemdmanager/manager.rs +++ b/src/provider/systemdmanager/manager.rs @@ -39,7 +39,7 @@ pub struct SystemdManager { impl SystemdManager { /// Creates a new instance, takes a flag whether to run within the /// user session or manage services system-wide. - pub async fn new(user_mode: bool) -> Result { + pub async fn new(user_mode: bool, max_pods: u16) -> Result { // Connect to session or system bus depending on the value of [user_mode] let connection = if user_mode { Connection::new_session().await.map_err(|e| RuntimeError { @@ -60,7 +60,7 @@ impl SystemdManager { // The maximum number of queued DBus messages must be higher // than the number of containers which can be started and // stopped simultaneously. - let connection = connection.set_max_queued(1024); + let connection = connection.set_max_queued(max_pods as usize * 2); let proxy = AsyncManagerProxy::new(&connection); From a8323efc1dccf943a02906ca2baabc5187fbead9 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Thu, 27 May 2021 13:47:06 +0200 Subject: [PATCH 5/7] Changelog updated --- CHANGELOG.adoc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 1c46ba4..24c434a 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -1,12 +1,12 @@ = Changelog -== 0.2.1 - 2021-05-25 +== 0.2.1 - 2021-05-27 -:168: https://github.com/stackabletech/agent/pull/168[#168] +:176: https://github.com/stackabletech/agent/pull/176[#176] === Fixed * Deadlock fixed which occurred when multiple pods were started or - stopped simultaneously ({168}). + stopped simultaneously ({176}). == 0.2.0 - 2021-05-20 From bdfb51f84cfbf80719a418a5571f698b7c190503 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Thu, 27 May 2021 14:08:14 +0200 Subject: [PATCH 6/7] Changelog updated --- CHANGELOG.adoc | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 24c434a..d8bad5f 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -1,9 +1,19 @@ = Changelog -== 0.2.1 - 2021-05-27 +== 0.3.0 - 2021-05-27 +:165: https://github.com/stackabletech/agent/pull/165[#165] +:169: https://github.com/stackabletech/agent/pull/169[#169] +:173: https://github.com/stackabletech/agent/pull/176[#173] :176: https://github.com/stackabletech/agent/pull/176[#176] +=== Added +* Artifacts for merge requests are created ({169}, {173}). + +=== Changed +* Structure of the documentation changed so that it can be incorporated + into the overall Stackable documentation ({165}). + === Fixed * Deadlock fixed which occurred when multiple pods were started or stopped simultaneously ({176}). From fd4c6c0dbdd6c9288004491097c93ea321c1e747 Mon Sep 17 00:00:00 2001 From: Siegfried Weber Date: Thu, 27 May 2021 14:24:29 +0200 Subject: [PATCH 7/7] Version set to 0.3.0 --- Cargo.lock | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7631c92..eb29358 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2712,7 +2712,7 @@ checksum = "6e63cff320ae2c57904679ba7cb63280a3dc4613885beafb148ee7bf9aa9042d" [[package]] name = "stackable-agent" -version = "0.2.1" +version = "0.3.0" dependencies = [ "Inflector", "anyhow", diff --git a/Cargo.toml b/Cargo.toml index 58ad99e..a509a49 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "stackable-agent" description = "The component of the Stackable Platform that manages installation of services on the workers" -version = "0.2.1" +version = "0.3.0" authors = ["Sönke Liebau "] edition = "2018" license = "Apache-2.0"