From 0aec37736460ad2692fd6c4d6582cdc32749779d Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 3 Nov 2023 11:36:10 +0100 Subject: [PATCH 1/6] feat: Support graceful shutdown --- CHANGELOG.md | 2 +- deploy/helm/zookeeper-operator/crds/crds.yaml | 8 ++++ .../operations/graceful-shutdown.adoc | 18 ++++++++ rust/crd/src/lib.rs | 11 +++++ .../src/operations/graceful_shutdown.rs | 26 +++++++++++ rust/operator-binary/src/operations/mod.rs | 1 + rust/operator-binary/src/zk_controller.rs | 46 ++++++++++++++++--- tests/templates/kuttl/smoke/10-assert.yaml.j2 | 2 + 8 files changed, 106 insertions(+), 8 deletions(-) create mode 100644 rust/operator-binary/src/operations/graceful_shutdown.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index c4a6e1a8..6a676fc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,11 +10,11 @@ All notable changes to this project will be documented in this file. - Configuration overrides for the JVM security properties, such as DNS caching ([#715]). - Support PodDisruptionBudgets ([#730], [#731]). - Support for ZooKeeper 3.8.3 added ([#732]). +- Support graceful shutdown ([#XXX]). ### Changed - `vector` `0.26.0` -> `0.33.0` ([#709], [#732]). -- `operator-rs` `0.44.0` -> `0.52.1` ([#711], [#730], [#731]). - Let secret-operator handle certificate conversion ([#695]). ## Removed diff --git a/deploy/helm/zookeeper-operator/crds/crds.yaml b/deploy/helm/zookeeper-operator/crds/crds.yaml index 8f6cca95..71a4ce6f 100644 --- a/deploy/helm/zookeeper-operator/crds/crds.yaml +++ b/deploy/helm/zookeeper-operator/crds/crds.yaml @@ -633,6 +633,10 @@ spec: type: array type: object type: object + gracefulShutdownTimeout: + description: Time period Pods have to gracefully shut down, e.g. `30m`, `1h` or `2d`. Consult the operator documentation for details. + nullable: true + type: string initLimit: format: uint32 minimum: 0.0 @@ -4137,6 +4141,10 @@ spec: type: array type: object type: object + gracefulShutdownTimeout: + description: Time period Pods have to gracefully shut down, e.g. `30m`, `1h` or `2d`. Consult the operator documentation for details. + nullable: true + type: string initLimit: format: uint32 minimum: 0.0 diff --git a/docs/modules/zookeeper/pages/usage_guide/operations/graceful-shutdown.adoc b/docs/modules/zookeeper/pages/usage_guide/operations/graceful-shutdown.adoc index fb9689f5..ceb8fda7 100644 --- a/docs/modules/zookeeper/pages/usage_guide/operations/graceful-shutdown.adoc +++ b/docs/modules/zookeeper/pages/usage_guide/operations/graceful-shutdown.adoc @@ -5,3 +5,21 @@ or we have not implemented it yet. The efforts of implementing graceful shutdown for all products that have this functionality is tracked in https://github.com/stackabletech/issues/issues/357 + + + +bin/zkServer.sh stop + +stop) + echo -n "Stopping zookeeper ... " + if [ ! -f "$ZOOPIDFILE" ] + then + echo "no zookeeper to stop (could not find file $ZOOPIDFILE)" + else + $KILL $(cat "$ZOOPIDFILE") + rm "$ZOOPIDFILE" + sleep 1 + echo STOPPED + fi + exit 0 + ;; diff --git a/rust/crd/src/lib.rs b/rust/crd/src/lib.rs index a7516ff4..7fa219e3 100644 --- a/rust/crd/src/lib.rs +++ b/rust/crd/src/lib.rs @@ -25,6 +25,7 @@ use stackable_operator::{ role_utils::{GenericRoleConfig, Role, RoleGroup, RoleGroupRef}, schemars::{self, JsonSchema}, status::condition::{ClusterCondition, HasStatusCondition}, + time::Duration, }; use strum::{Display, EnumIter, EnumString, IntoEnumIterator}; @@ -67,6 +68,8 @@ const JVM_HEAP_FACTOR: f32 = 0.8; pub const DOCKER_IMAGE_BASE_NAME: &str = "zookeeper"; +const DEFAULT_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT: Duration = Duration::from_minutes_unchecked(2); + mod built_info { pub const CARGO_PKG_VERSION: &str = env!("CARGO_PKG_VERSION"); } @@ -218,12 +221,19 @@ pub struct ZookeeperConfig { pub sync_limit: Option, pub tick_time: Option, pub myid_offset: u16, + #[fragment_attrs(serde(default))] pub resources: Resources, + #[fragment_attrs(serde(default))] pub logging: Logging, + #[fragment_attrs(serde(default))] pub affinity: StackableAffinity, + + /// Time period Pods have to gracefully shut down, e.g. `30m`, `1h` or `2d`. Consult the operator documentation for details. + #[fragment_attrs(serde(default))] + pub graceful_shutdown_timeout: Option, } #[derive(Clone, Debug, Default, JsonSchema, PartialEq, Fragment)] @@ -300,6 +310,7 @@ impl ZookeeperConfig { }, logging: product_logging::spec::default_logging(), affinity: get_affinity(cluster_name, role), + graceful_shutdown_timeout: Some(DEFAULT_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT), } } } diff --git a/rust/operator-binary/src/operations/graceful_shutdown.rs b/rust/operator-binary/src/operations/graceful_shutdown.rs new file mode 100644 index 00000000..760f7a15 --- /dev/null +++ b/rust/operator-binary/src/operations/graceful_shutdown.rs @@ -0,0 +1,26 @@ +use snafu::{ResultExt, Snafu}; +use stackable_operator::builder::PodBuilder; +use stackable_zookeeper_crd::ZookeeperConfig; + +#[derive(Debug, Snafu)] +pub enum Error { + #[snafu(display("Failed to set terminationGracePeriod"))] + SetTerminationGracePeriod { + source: stackable_operator::builder::pod::Error, + }, +} + +pub fn add_graceful_shutdown_config( + merged_config: &ZookeeperConfig, + pod_builder: &mut PodBuilder, +) -> Result<(), Error> { + // This must be always set by the merge mechanism, as we provide a default value, + // users can not disable graceful shutdown. + if let Some(graceful_shutdown_timeout) = merged_config.graceful_shutdown_timeout { + pod_builder + .termination_grace_period(&graceful_shutdown_timeout) + .context(SetTerminationGracePeriodSnafu)?; + } + + Ok(()) +} diff --git a/rust/operator-binary/src/operations/mod.rs b/rust/operator-binary/src/operations/mod.rs index d3cf6e9c..92ca2ec7 100644 --- a/rust/operator-binary/src/operations/mod.rs +++ b/rust/operator-binary/src/operations/mod.rs @@ -1 +1,2 @@ +pub mod graceful_shutdown; pub mod pdb; diff --git a/rust/operator-binary/src/zk_controller.rs b/rust/operator-binary/src/zk_controller.rs index 4940fff2..e3f636d1 100644 --- a/rust/operator-binary/src/zk_controller.rs +++ b/rust/operator-binary/src/zk_controller.rs @@ -42,6 +42,7 @@ use stackable_operator::{ product_config_utils::{transform_all_roles_to_config, validate_all_roles_and_groups_config}, product_logging::{ self, + framework::{create_vector_shutdown_file_command, remove_vector_shutdown_file_command}, spec::{ ConfigMapLogConfig, ContainerLogConfig, ContainerLogConfigChoice, CustomContainerLogConfig, @@ -53,6 +54,7 @@ use stackable_operator::{ statefulset::StatefulSetConditionBuilder, }, time::Duration, + utils::COMMON_BASH_TRAP_FUNCTIONS, }; use stackable_zookeeper_crd::{ security::ZookeeperSecurity, Container, ZookeeperCluster, ZookeeperClusterStatus, @@ -66,7 +68,7 @@ use strum::{EnumDiscriminants, IntoStaticStr}; use crate::{ command::create_init_container_command_args, discovery::{self, build_discovery_configmaps}, - operations::pdb::add_pdbs, + operations::{graceful_shutdown::add_graceful_shutdown_config, pdb::add_pdbs}, product_logging::{extend_role_group_config_map, resolve_vector_aggregator_address}, utils::build_recommended_labels, ObjectRef, APP_NAME, OPERATOR_NAME, @@ -223,6 +225,11 @@ pub enum Error { FailedToCreatePdb { source: crate::operations::pdb::Error, }, + + #[snafu(display("failed to configure graceful shutdown"))] + GracefulShutdown { + source: crate::operations::graceful_shutdown::Error, + }, } type Result = std::result::Result; @@ -261,6 +268,7 @@ impl ReconcilerError for Error { Error::FailedToInitializeSecurityContext { .. } => None, Error::FailedToResolveConfig { .. } => None, Error::FailedToCreatePdb { .. } => None, + Error::GracefulShutdown { .. } => None, } } } @@ -725,8 +733,14 @@ fn build_server_rolegroup_statefulset( let container_prepare = cb_prepare .image_from_product_image(resolved_product_image) - .command(vec!["bash".to_string(), "-c".to_string()]) - .args(vec![args.join(" && ")]) + .command(vec![ + "/bin/bash".to_string(), + "-x".to_string(), + "-euo".to_string(), + "pipefail".to_string(), + "-c".to_string(), + ]) + .args(vec![args.join("\n")]) .add_env_vars(env_vars.clone()) .add_env_vars(vec![EnvVar { name: "POD_NAME".to_string(), @@ -755,11 +769,27 @@ fn build_server_rolegroup_statefulset( let container_zk = cb_zookeeper .image_from_product_image(resolved_product_image) - .args(vec![ - "bin/zkServer.sh".to_string(), - "start-foreground".to_string(), - format!("{dir}/zoo.cfg", dir = STACKABLE_RW_CONFIG_DIR), + .command(vec![ + "/bin/bash".to_string(), + "-x".to_string(), + "-euo".to_string(), + "pipefail".to_string(), + "-c".to_string(), ]) + .args(vec![format!( + "\ +{COMMON_BASH_TRAP_FUNCTIONS} +{remove_vector_shutdown_file_command} +prepare_signal_handlers +bin/zkServer.sh start-foreground {STACKABLE_RW_CONFIG_DIR}/zoo.cfg & +wait_for_termination $! +{create_vector_shutdown_file_command} +", + remove_vector_shutdown_file_command = + remove_vector_shutdown_file_command(STACKABLE_LOG_DIR), + create_vector_shutdown_file_command = + create_vector_shutdown_file_command(STACKABLE_LOG_DIR), + )]) .add_env_vars(env_vars) // Only allow the global load balancing service to send traffic to pods that are members of the quorum // This also acts as a hint to the StatefulSet controller to wait for each pod to enter quorum before taking down the next @@ -876,6 +906,8 @@ fn build_server_rolegroup_statefulset( )); } + add_graceful_shutdown_config(config, &mut pod_builder).context(GracefulShutdownSnafu)?; + let mut pod_template = pod_builder.build_template(); pod_template.merge_from(role.config.pod_overrides.clone()); pod_template.merge_from(rolegroup.config.pod_overrides.clone()); diff --git a/tests/templates/kuttl/smoke/10-assert.yaml.j2 b/tests/templates/kuttl/smoke/10-assert.yaml.j2 index d39462c7..22ebd65a 100644 --- a/tests/templates/kuttl/smoke/10-assert.yaml.j2 +++ b/tests/templates/kuttl/smoke/10-assert.yaml.j2 @@ -24,6 +24,7 @@ spec: {% if lookup('env', 'VECTOR_AGGREGATOR') %} - name: vector {% endif %} + terminationGracePeriodSeconds: 120 status: readyReplicas: 2 replicas: 2 @@ -47,6 +48,7 @@ spec: {% if lookup('env', 'VECTOR_AGGREGATOR') %} - name: vector {% endif %} + terminationGracePeriodSeconds: 120 status: readyReplicas: 1 replicas: 1 From 4cd411c187e671af852c54d0e3f633459acdc2b5 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 3 Nov 2023 12:55:51 +0100 Subject: [PATCH 2/6] docs --- .../operations/graceful-shutdown.adoc | 25 ++++++------------- 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/docs/modules/zookeeper/pages/usage_guide/operations/graceful-shutdown.adoc b/docs/modules/zookeeper/pages/usage_guide/operations/graceful-shutdown.adoc index ceb8fda7..48318128 100644 --- a/docs/modules/zookeeper/pages/usage_guide/operations/graceful-shutdown.adoc +++ b/docs/modules/zookeeper/pages/usage_guide/operations/graceful-shutdown.adoc @@ -1,25 +1,14 @@ = Graceful shutdown -Graceful shutdown of Zookeeper nodes is either not supported by the product itself -or we have not implemented it yet. +You can configure the graceful shutdown as described in xref:concepts:operations/graceful_shutdown.adoc[]. -The efforts of implementing graceful shutdown for all products that have this functionality is tracked in -https://github.com/stackabletech/issues/issues/357 +== Servers +As a default, server have `2 minutes` to shut down gracefully. +The Zookeeper server process will receive a `SIGTERM` signal when Kubernetes wants to terminate the Pod. +After the graceful shutdown timeout runs out, and the process still didn't exit, Kubernetes will issue a `SIGKILL` signal. -bin/zkServer.sh stop +This is equivalent to executing the `bin/zkServer.sh stop` command, which internally executes `kill ` (https://github.com/apache/zookeeper/blob/74db005175a4ec545697012f9069cb9dcc8cdda7/bin/zkServer.sh#L219[code]). -stop) - echo -n "Stopping zookeeper ... " - if [ ! -f "$ZOOPIDFILE" ] - then - echo "no zookeeper to stop (could not find file $ZOOPIDFILE)" - else - $KILL $(cat "$ZOOPIDFILE") - rm "$ZOOPIDFILE" - sleep 1 - echo STOPPED - fi - exit 0 - ;; +However, there is no acknowledge message in the log indicating a graceful shutdown. From 04214b0721a8b60e302eb460db162fefa3b1755e Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 3 Nov 2023 12:57:26 +0100 Subject: [PATCH 3/6] changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a676fc1..95400781 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ All notable changes to this project will be documented in this file. - Configuration overrides for the JVM security properties, such as DNS caching ([#715]). - Support PodDisruptionBudgets ([#730], [#731]). - Support for ZooKeeper 3.8.3 added ([#732]). -- Support graceful shutdown ([#XXX]). +- Support graceful shutdown ([#740]). ### Changed @@ -28,6 +28,7 @@ All notable changes to this project will be documented in this file. [#730]: https://github.com/stackabletech/zookeeper-operator/pull/730 [#731]: https://github.com/stackabletech/zookeeper-operator/pull/731 [#732]: https://github.com/stackabletech/zookeeper-operator/pull/732 +[#740]: https://github.com/stackabletech/zookeeper-operator/pull/740 ## [23.7.0] - 2023-07-14 From 5b74ffe5537719031d5ee9583888d74080fcbb4c Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 3 Nov 2023 13:53:47 +0100 Subject: [PATCH 4/6] Update docs/modules/zookeeper/pages/usage_guide/operations/graceful-shutdown.adoc Co-authored-by: Malte Sander --- .../pages/usage_guide/operations/graceful-shutdown.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/modules/zookeeper/pages/usage_guide/operations/graceful-shutdown.adoc b/docs/modules/zookeeper/pages/usage_guide/operations/graceful-shutdown.adoc index 48318128..716b7a84 100644 --- a/docs/modules/zookeeper/pages/usage_guide/operations/graceful-shutdown.adoc +++ b/docs/modules/zookeeper/pages/usage_guide/operations/graceful-shutdown.adoc @@ -6,7 +6,7 @@ You can configure the graceful shutdown as described in xref:concepts:operations As a default, server have `2 minutes` to shut down gracefully. -The Zookeeper server process will receive a `SIGTERM` signal when Kubernetes wants to terminate the Pod. +The ZooKeeper server process will receive a `SIGTERM` signal when Kubernetes wants to terminate the Pod. After the graceful shutdown timeout runs out, and the process still didn't exit, Kubernetes will issue a `SIGKILL` signal. This is equivalent to executing the `bin/zkServer.sh stop` command, which internally executes `kill ` (https://github.com/apache/zookeeper/blob/74db005175a4ec545697012f9069cb9dcc8cdda7/bin/zkServer.sh#L219[code]). From 514452d778c5851d2503293abd856ed1b95317ff Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 3 Nov 2023 13:57:49 +0100 Subject: [PATCH 5/6] Use formatdoc! --- Cargo.lock | 7 +++++++ Cargo.toml | 1 + rust/operator-binary/Cargo.toml | 9 +++++---- rust/operator-binary/src/zk_controller.rs | 20 ++++++++++---------- 4 files changed, 23 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 23e335d9..069272e3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -923,6 +923,12 @@ dependencies = [ "hashbrown 0.14.0", ] +[[package]] +name = "indoc" +version = "2.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1e186cfbae8084e513daff4240b4797e342f988cecda4fb6c939150f96315fd8" + [[package]] name = "instant" version = "0.1.12" @@ -2075,6 +2081,7 @@ dependencies = [ "failure", "fnv", "futures 0.3.28", + "indoc", "pin-project", "product-config", "rstest", diff --git a/Cargo.toml b/Cargo.toml index ec653773..dc2b8657 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,6 +15,7 @@ clap = "4.3" failure = "0.1" fnv = "1.0" futures = { version = "0.3", features = ["compat"] } +indoc = "2.0" pin-project = "1.1" rstest = "0.18" semver = "1.0" diff --git a/rust/operator-binary/Cargo.toml b/rust/operator-binary/Cargo.toml index 2f0ea986..5e98c8fa 100644 --- a/rust/operator-binary/Cargo.toml +++ b/rust/operator-binary/Cargo.toml @@ -17,16 +17,17 @@ clap.workspace = true failure.workspace = true fnv.workspace = true futures.workspace = true +indoc.workspace = true +pin-project.workspace = true +product-config.workspace = true semver.workspace = true serde.workspace = true snafu.workspace = true +stackable-operator.workspace = true strum.workspace = true -tokio.workspace = true tokio-zookeeper.workspace = true +tokio.workspace = true tracing.workspace = true -pin-project.workspace = true -stackable-operator.workspace = true -product-config.workspace = true [dev-dependencies] rstest.workspace = true diff --git a/rust/operator-binary/src/zk_controller.rs b/rust/operator-binary/src/zk_controller.rs index e3f636d1..a334993c 100644 --- a/rust/operator-binary/src/zk_controller.rs +++ b/rust/operator-binary/src/zk_controller.rs @@ -8,6 +8,7 @@ use std::{ }; use fnv::FnvHasher; +use indoc::formatdoc; use product_config::{ types::PropertyNameKind, writer::{to_java_properties_string, PropertiesWriterError}, @@ -776,20 +777,19 @@ fn build_server_rolegroup_statefulset( "pipefail".to_string(), "-c".to_string(), ]) - .args(vec![format!( - "\ -{COMMON_BASH_TRAP_FUNCTIONS} -{remove_vector_shutdown_file_command} -prepare_signal_handlers -bin/zkServer.sh start-foreground {STACKABLE_RW_CONFIG_DIR}/zoo.cfg & -wait_for_termination $! -{create_vector_shutdown_file_command} -", + .args(vec![formatdoc! {" + {COMMON_BASH_TRAP_FUNCTIONS} + {remove_vector_shutdown_file_command} + prepare_signal_handlers + bin/zkServer.sh start-foreground {STACKABLE_RW_CONFIG_DIR}/zoo.cfg & + wait_for_termination $! + {create_vector_shutdown_file_command} + ", remove_vector_shutdown_file_command = remove_vector_shutdown_file_command(STACKABLE_LOG_DIR), create_vector_shutdown_file_command = create_vector_shutdown_file_command(STACKABLE_LOG_DIR), - )]) + }]) .add_env_vars(env_vars) // Only allow the global load balancing service to send traffic to pods that are members of the quorum // This also acts as a hint to the StatefulSet controller to wait for each pod to enter quorum before taking down the next From d0914d81f8d68325427e6e5572a992154e086a8d Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Mon, 6 Nov 2023 09:50:44 +0100 Subject: [PATCH 6/6] Update docs/modules/zookeeper/pages/usage_guide/operations/graceful-shutdown.adoc Co-authored-by: Malte Sander --- .../pages/usage_guide/operations/graceful-shutdown.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/modules/zookeeper/pages/usage_guide/operations/graceful-shutdown.adoc b/docs/modules/zookeeper/pages/usage_guide/operations/graceful-shutdown.adoc index 716b7a84..37823faf 100644 --- a/docs/modules/zookeeper/pages/usage_guide/operations/graceful-shutdown.adoc +++ b/docs/modules/zookeeper/pages/usage_guide/operations/graceful-shutdown.adoc @@ -4,7 +4,7 @@ You can configure the graceful shutdown as described in xref:concepts:operations == Servers -As a default, server have `2 minutes` to shut down gracefully. +As a default, ZooKeeper servers have `2 minutes` to shut down gracefully. The ZooKeeper server process will receive a `SIGTERM` signal when Kubernetes wants to terminate the Pod. After the graceful shutdown timeout runs out, and the process still didn't exit, Kubernetes will issue a `SIGKILL` signal.