From c930bd20aa1e7507f312afad75f5eb6ecdffc849 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Mon, 6 Nov 2023 10:14:29 +0100 Subject: [PATCH] feat: Support graceful shutdown (#740) * feat: Support graceful shutdown * docs * changelog * Update docs/modules/zookeeper/pages/usage_guide/operations/graceful-shutdown.adoc Co-authored-by: Malte Sander * Use formatdoc! * Update docs/modules/zookeeper/pages/usage_guide/operations/graceful-shutdown.adoc Co-authored-by: Malte Sander --------- Co-authored-by: Malte Sander --- CHANGELOG.md | 3 +- Cargo.lock | 7 +++ Cargo.toml | 1 + deploy/helm/zookeeper-operator/crds/crds.yaml | 8 ++++ .../operations/graceful-shutdown.adoc | 15 ++++-- rust/crd/src/lib.rs | 11 +++++ rust/operator-binary/Cargo.toml | 9 ++-- .../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 + 11 files changed, 113 insertions(+), 16 deletions(-) create mode 100644 rust/operator-binary/src/operations/graceful_shutdown.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index c6a4a26c..d9ee3613 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 ([#740]). ### 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]). - [BREAKING]: removed the `logging` layer in the `clusterConfig` CRD to make logging config consistent with other operators ([#739]). @@ -30,6 +30,7 @@ All notable changes to this project will be documented in this file. [#731]: https://github.com/stackabletech/zookeeper-operator/pull/731 [#732]: https://github.com/stackabletech/zookeeper-operator/pull/732 [#739]: https://github.com/stackabletech/zookeeper-operator/pull/739 +[#740]: https://github.com/stackabletech/zookeeper-operator/pull/740 ## [23.7.0] - 2023-07-14 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/deploy/helm/zookeeper-operator/crds/crds.yaml b/deploy/helm/zookeeper-operator/crds/crds.yaml index e354188c..3646e120 100644 --- a/deploy/helm/zookeeper-operator/crds/crds.yaml +++ b/deploy/helm/zookeeper-operator/crds/crds.yaml @@ -628,6 +628,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 @@ -4132,6 +4136,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..37823faf 100644 --- a/docs/modules/zookeeper/pages/usage_guide/operations/graceful-shutdown.adoc +++ b/docs/modules/zookeeper/pages/usage_guide/operations/graceful-shutdown.adoc @@ -1,7 +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, 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. + +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]). + +However, there is no acknowledge message in the log indicating a graceful shutdown. diff --git a/rust/crd/src/lib.rs b/rust/crd/src/lib.rs index 8eacf902..6ad0739f 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"); } @@ -210,12 +213,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)] @@ -292,6 +302,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/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/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..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}, @@ -42,6 +43,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 +55,7 @@ use stackable_operator::{ statefulset::StatefulSetConditionBuilder, }, time::Duration, + utils::COMMON_BASH_TRAP_FUNCTIONS, }; use stackable_zookeeper_crd::{ security::ZookeeperSecurity, Container, ZookeeperCluster, ZookeeperClusterStatus, @@ -66,7 +69,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 +226,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 +269,7 @@ impl ReconcilerError for Error { Error::FailedToInitializeSecurityContext { .. } => None, Error::FailedToResolveConfig { .. } => None, Error::FailedToCreatePdb { .. } => None, + Error::GracefulShutdown { .. } => None, } } } @@ -725,8 +734,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 +770,26 @@ 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![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 @@ -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