From 637c806e2445aee2c5ed0e794b54c8ce44897824 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Tue, 7 Jan 2025 15:50:34 +0100 Subject: [PATCH 1/3] chore: Switch WebUI liveness probe to tcp socket instead of httpGet --- rust/operator-binary/src/container.rs | 71 ++++++++++++--------------- 1 file changed, 32 insertions(+), 39 deletions(-) diff --git a/rust/operator-binary/src/container.rs b/rust/operator-binary/src/container.rs index d854b19e..70b7e9c5 100644 --- a/rust/operator-binary/src/container.rs +++ b/rust/operator-binary/src/container.rs @@ -45,9 +45,8 @@ use stackable_operator::{ k8s_openapi::{ api::core::v1::{ ConfigMapKeySelector, ConfigMapVolumeSource, Container, ContainerPort, - EmptyDirVolumeSource, EnvVar, EnvVarSource, HTTPGetAction, ObjectFieldSelector, - PersistentVolumeClaim, Probe, ResourceRequirements, TCPSocketAction, Volume, - VolumeMount, + EmptyDirVolumeSource, EnvVar, EnvVarSource, ObjectFieldSelector, PersistentVolumeClaim, + Probe, ResourceRequirements, TCPSocketAction, Volume, VolumeMount, }, apimachinery::pkg::util::intstr::IntOrString, }, @@ -958,38 +957,28 @@ wait_for_termination $! initial_delay_seconds: i32, failure_threshold: i32, ) -> Option { - match self { - ContainerConfig::Hdfs { - web_ui_http_port_name, - web_ui_https_port_name, - web_ui_path, - .. - } => { - let http_get_action = if hdfs.has_https_enabled() { - HTTPGetAction { - port: IntOrString::String(web_ui_https_port_name.to_string()), - scheme: Some("HTTPS".into()), - path: Some(web_ui_path.to_string()), - ..HTTPGetAction::default() - } - } else { - HTTPGetAction { - port: IntOrString::String(web_ui_http_port_name.to_string()), - scheme: Some("HTTP".into()), - path: Some(web_ui_path.to_string()), - ..HTTPGetAction::default() - } - }; - Some(Probe { - http_get: Some(http_get_action), - period_seconds: Some(period_seconds), - initial_delay_seconds: Some(initial_delay_seconds), - failure_threshold: Some(failure_threshold), - ..Probe::default() - }) - } - _ => None, - } + let ContainerConfig::Hdfs { + web_ui_http_port_name, + web_ui_https_port_name, + .. + } = self + else { + return None; + }; + + let port = if hdfs.has_https_enabled() { + web_ui_https_port_name + } else { + web_ui_http_port_name + }; + + Some(Probe { + tcp_socket: Some(Self::tcp_socket_action_for_port(*port)), + period_seconds: Some(period_seconds), + initial_delay_seconds: Some(initial_delay_seconds), + failure_threshold: Some(failure_threshold), + ..Probe::default() + }) } /// Creates a probe for the IPC/RPC port @@ -1001,10 +990,7 @@ wait_for_termination $! ) -> Option { match self { ContainerConfig::Hdfs { ipc_port_name, .. } => Some(Probe { - tcp_socket: Some(TCPSocketAction { - port: IntOrString::String(ipc_port_name.to_string()), - ..TCPSocketAction::default() - }), + tcp_socket: Some(Self::tcp_socket_action_for_port(*ipc_port_name)), period_seconds: Some(period_seconds), initial_delay_seconds: Some(initial_delay_seconds), failure_threshold: Some(failure_threshold), @@ -1014,6 +1000,13 @@ wait_for_termination $! } } + fn tcp_socket_action_for_port(port: impl Into) -> TCPSocketAction { + TCPSocketAction { + port: IntOrString::String(port.into()), + ..Default::default() + } + } + /// Return the container volumes. fn volumes( &self, From 28f769a1144f62fe6e47fbf3ca821105ee624769 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Tue, 7 Jan 2025 16:18:26 +0100 Subject: [PATCH 2/3] cleanup --- CHANGELOG.md | 6 ++++++ rust/operator-binary/src/container.rs | 12 ------------ 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a6447465..c2ddf27c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,12 @@ All notable changes to this project will be documented in this file. config property `requestedSecretLifetime`. This helps reducing frequent Pod restarts ([#619]). - Run a `containerdebug` process in the background of each HDFS container to collect debugging information ([#629]). +### Changed + +- Switch the WebUI liveness probe from `httpGet` to checking the tcp socket. + This helps with setups where configOverrides are used to enable security on the HTTP interfaces. + As this results in `401` HTTP responses (instead of `200`), this previously failed the liveness checks. + ### Fixed - BREAKING: Use distinct ServiceAccounts for the Stacklets, so that multiple Stacklets can be diff --git a/rust/operator-binary/src/container.rs b/rust/operator-binary/src/container.rs index 70b7e9c5..8c7e3450 100644 --- a/rust/operator-binary/src/container.rs +++ b/rust/operator-binary/src/container.rs @@ -157,15 +157,6 @@ pub enum ContainerConfig { web_ui_http_port_name: &'static str, /// Port name of the web UI HTTPS port, used for the liveness probe. web_ui_https_port_name: &'static str, - /// Path of the web UI URL; The path defaults to / in Kubernetes - /// and the kubelet follows redirects. The default would work if - /// the location header is set properly but that is not the case - /// for the DataNode. On a TLS-enabled DataNode, calling - /// https://127.0.0.1:9865/ redirects to the non-TLS URL - /// http://127.0.0.1:9865/index.html which causes the liveness - /// probe to fail. So it is best to not rely on the location - /// header but instead provide the resolved path directly. - web_ui_path: &'static str, /// The JMX Exporter metrics port. metrics_port: u16, }, @@ -1383,7 +1374,6 @@ impl From for ContainerConfig { ipc_port_name: SERVICE_PORT_NAME_RPC, web_ui_http_port_name: SERVICE_PORT_NAME_HTTP, web_ui_https_port_name: SERVICE_PORT_NAME_HTTPS, - web_ui_path: "/dfshealth.html", metrics_port: DEFAULT_NAME_NODE_METRICS_PORT, }, HdfsRole::DataNode => Self::Hdfs { @@ -1393,7 +1383,6 @@ impl From for ContainerConfig { ipc_port_name: SERVICE_PORT_NAME_IPC, web_ui_http_port_name: SERVICE_PORT_NAME_HTTP, web_ui_https_port_name: SERVICE_PORT_NAME_HTTPS, - web_ui_path: "/datanode.html", metrics_port: DEFAULT_DATA_NODE_METRICS_PORT, }, HdfsRole::JournalNode => Self::Hdfs { @@ -1403,7 +1392,6 @@ impl From for ContainerConfig { ipc_port_name: SERVICE_PORT_NAME_RPC, web_ui_http_port_name: SERVICE_PORT_NAME_HTTP, web_ui_https_port_name: SERVICE_PORT_NAME_HTTPS, - web_ui_path: "/journalnode.html", metrics_port: DEFAULT_JOURNAL_NODE_METRICS_PORT, }, } From 101270a24b2a96714a07ebb8c1fc462f185b6cdc Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Wed, 8 Jan 2025 13:21:57 +0100 Subject: [PATCH 3/3] Update rust/operator-binary/src/container.rs Co-authored-by: Siegfried Weber --- rust/operator-binary/src/container.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rust/operator-binary/src/container.rs b/rust/operator-binary/src/container.rs index 8c7e3450..2a72cf55 100644 --- a/rust/operator-binary/src/container.rs +++ b/rust/operator-binary/src/container.rs @@ -964,6 +964,7 @@ wait_for_termination $! }; Some(Probe { + // Use tcp_socket instead of http_get so that the probe is independent of the authentication settings. tcp_socket: Some(Self::tcp_socket_action_for_port(*port)), period_seconds: Some(period_seconds), initial_delay_seconds: Some(initial_delay_seconds),