Skip to content

Commit 8cdd096

Browse files
razvanclaudeNickLarsenNZ
authored
feat: better startup and readiness probes (#976)
* feat: add exec-based NiFi 2.x management-server probe builders Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat: gate NiFi startup/readiness on management-server health endpoints * feat: pin NiFi management-server address explicitly via JVM property Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test: assert NiFi 2.x management-server probes and readiness gating in cluster_operation kuttl test * fix: set explicit timeout_seconds on management-server exec probes Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: revert retry-unsafe NotReady polling assertion in cluster_operation kuttl test * fix: increase probe timeout_seconds from 3 to 5 for real headroom Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs: changelog entry for NiFi 2.x management-server health probes * refactor: relocate MANAGEMENT_SERVER_PORT, document override caveat, drop dead readiness delay Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Update the changelog and cleanups * Move liveliness probe to probes.rs * Fix the kuttl smoke test * Extract the management IP into a constant * Update CHANGELOG.md Co-authored-by: Nick <10092581+NickLarsenNZ@users.noreply.github.com> * Implement review feedback --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Nick <10092581+NickLarsenNZ@users.noreply.github.com>
1 parent d0c53fe commit 8cdd096

9 files changed

Lines changed: 230 additions & 30 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ All notable changes to this project will be documented in this file.
1414
- BREAKING: The `nodes` role is now required by the CRD; a NifiCluster without it was
1515
previously accepted by the API server but failed reconciliation ([#966]).
1616
- All product containers now run with `securityContext.runAsNonRoot` set to `true` to improve security ([#975]).
17+
- NiFi startup and readiness probes now use the local management server's `/health` and
18+
`/health/cluster` endpoints instead of a bare TCP check ([#976]).
1719

1820
### Fixed
1921

@@ -25,6 +27,7 @@ All notable changes to this project will be documented in this file.
2527
[#966]: https://github.com/stackabletech/nifi-operator/pull/966
2628
[#970]: https://github.com/stackabletech/nifi-operator/pull/970
2729
[#975]: https://github.com/stackabletech/nifi-operator/pull/975
30+
[#976]: https://github.com/stackabletech/nifi-operator/pull/976
2831

2932
## [26.7.0] - 2026-07-21
3033

‎rust/operator-binary/src/controller/build.rs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,19 @@ pub const PROTOCOL_PORT: Port = Port(9088);
4646
pub const BALANCE_PORT_NAME: &str = "balance";
4747
pub const BALANCE_PORT: Port = Port(6243);
4848

49+
/// Loopback address NiFi's management server binds to by default
50+
/// (`org.apache.nifi.management.server.address`, see
51+
/// `ManagementServerProvider.MANAGEMENT_SERVER_DEFAULT_ADDRESS` upstream).
52+
/// The operator pins the JVM system property to this same value (see
53+
/// `build::jvm`), so this constant is the single source of truth for it.
54+
/// Note: a user-supplied `jvmArgumentOverrides` that removes or repoints the
55+
/// corresponding `-D` property would desync this from the actual
56+
/// management-server address, silently breaking both probes.
57+
pub const MANAGEMENT_SERVER_ADDRESS: &str = "127.0.0.1";
58+
/// Port NiFi's management server binds to by default, see
59+
/// [`MANAGEMENT_SERVER_ADDRESS`].
60+
pub const MANAGEMENT_SERVER_PORT: u16 = 52020;
61+
4962
// Filesystem paths shared by multiple builders. Single-consumer paths live in their builder.
5063
pub const NIFI_CONFIG_DIRECTORY: &str = "/stackable/nifi/conf";
5164
pub const NIFI_PYTHON_WORKING_DIRECTORY: &str = "/nifi-python-working-directory";

‎rust/operator-binary/src/controller/build/jvm.rs‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@ use stackable_operator::{
99
use crate::{
1010
controller::{
1111
ValidatedNifiConfig,
12-
build::{NIFI_CONFIG_DIRECTORY, properties::ConfigFileName},
12+
build::{
13+
MANAGEMENT_SERVER_ADDRESS, MANAGEMENT_SERVER_PORT, NIFI_CONFIG_DIRECTORY,
14+
properties::ConfigFileName,
15+
},
1316
},
1417
security::{
1518
authentication::{STACKABLE_SERVER_TLS_DIR, STACKABLE_TLS_STORE_PASSWORD},
@@ -86,6 +89,12 @@ pub fn build_merged_jvm_config(
8689
"-Djava.security.properties={NIFI_CONFIG_DIRECTORY}/{}",
8790
ConfigFileName::SecurityProperties
8891
),
92+
// Pin the NiFi 2.x management server (used by the startup/readiness
93+
// probes in resource::probes) to a known address instead of relying
94+
// on its undocumented upstream default.
95+
format!(
96+
"-Dorg.apache.nifi.management.server.address={MANAGEMENT_SERVER_ADDRESS}:{MANAGEMENT_SERVER_PORT}"
97+
),
8998
];
9099

91100
// Add JVM truststore properties when OPA TLS is enabled
@@ -170,6 +179,28 @@ mod tests {
170179
);
171180
}
172181

182+
/// The management-server bind address is pinned explicitly rather than
183+
/// relying on NiFi's undocumented upstream default, so the probes in
184+
/// `resource::probes` always target the right port even if that default
185+
/// ever changes upstream.
186+
#[test]
187+
fn management_server_address_is_pinned_explicitly() {
188+
let cluster = minimal_validated_cluster();
189+
let args = build_merged_jvm_config(
190+
&default_rg(&cluster).config,
191+
&JvmArgumentOverrides::default(),
192+
None,
193+
)
194+
.expect("jvm config should build");
195+
196+
assert!(
197+
args.contains(
198+
&"-Dorg.apache.nifi.management.server.address=127.0.0.1:52020".to_string()
199+
),
200+
"expected an explicit management-server address JVM property, got: {args:?}"
201+
);
202+
}
203+
173204
/// Without OPA TLS, no truststore properties are emitted.
174205
#[test]
175206
fn without_opa_tls_no_truststore_properties() {

‎rust/operator-binary/src/controller/build/properties/bootstrap_conf.rs‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ mod tests {
122122
java.arg.10=-Djavax.security.auth.useSubjectCredsOnly=true
123123
java.arg.11=-Dzookeeper.admin.enableServer=false
124124
java.arg.12=-Djava.security.properties=/stackable/nifi/conf/security.properties
125+
java.arg.13=-Dorg.apache.nifi.management.server.address=127.0.0.1:52020
125126
java.arg.2=-Xms3276m
126127
java.arg.3=-XX:+UseG1GC
127128
java.arg.4=-Djava.awt.headless=true
@@ -187,10 +188,11 @@ mod tests {
187188
java=java
188189
java.arg.1=-Xms34406m
189190
java.arg.10=-Djava.security.properties=/stackable/nifi/conf/security.properties
190-
java.arg.11=-Dhttps.proxyHost=proxy.my.corp
191-
java.arg.12=-Djava.net.preferIPv4Stack=true
192-
java.arg.13=-Xmx40000m
193-
java.arg.14=-Dhttps.proxyPort=1234
191+
java.arg.11=-Dorg.apache.nifi.management.server.address=127.0.0.1:52020
192+
java.arg.12=-Dhttps.proxyHost=proxy.my.corp
193+
java.arg.13=-Djava.net.preferIPv4Stack=true
194+
java.arg.14=-Xmx40000m
195+
java.arg.15=-Dhttps.proxyPort=1234
194196
java.arg.2=-Djava.awt.headless=true
195197
java.arg.3=-Dorg.apache.jasper.compiler.disablejsr199=true
196198
java.arg.4=-Djava.net.preferIPv4Stack=true

‎rust/operator-binary/src/controller/build/resource/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
pub mod config_map;
66
pub mod listener;
77
pub mod pdb;
8+
pub mod probes;
89
pub mod rbac;
910
pub mod service;
1011
pub mod statefulset;
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
//! Builds the exec-based startup and readiness probes that check NiFi 2.x's
2+
//! local, unauthenticated management-server endpoints (`/health` and
3+
//! `/health/cluster`).
4+
//!
5+
//! The management server binds `127.0.0.1` only, so these must be `exec`
6+
//! probes using `curl` from inside the container - a `httpGet` probe cannot
7+
//! reach a loopback-only address.
8+
9+
use stackable_operator::k8s_openapi::{
10+
api::core::v1::{ExecAction, Probe, TCPSocketAction},
11+
apimachinery::pkg::util::intstr::IntOrString,
12+
};
13+
14+
use crate::controller::build::{
15+
HTTPS_PORT_NAME, MANAGEMENT_SERVER_ADDRESS, MANAGEMENT_SERVER_PORT,
16+
};
17+
18+
fn management_health_exec(path: &str) -> ExecAction {
19+
ExecAction {
20+
command: Some(vec![
21+
"/bin/bash".to_string(),
22+
"-euo".to_string(),
23+
"pipefail".to_string(),
24+
"-c".to_string(),
25+
format!(
26+
"curl --fail --silent --show-error --output /dev/null http://{MANAGEMENT_SERVER_ADDRESS}:{MANAGEMENT_SERVER_PORT}{path}"
27+
),
28+
]),
29+
}
30+
}
31+
pub fn management_startup_probe() -> Probe {
32+
Probe {
33+
initial_delay_seconds: Some(10),
34+
period_seconds: Some(10),
35+
timeout_seconds: Some(5),
36+
failure_threshold: Some(20 * 6),
37+
exec: Some(management_health_exec("/health")),
38+
..Probe::default()
39+
}
40+
}
41+
pub fn management_readiness_probe() -> Probe {
42+
Probe {
43+
period_seconds: Some(10),
44+
timeout_seconds: Some(5),
45+
failure_threshold: Some(3),
46+
exec: Some(management_health_exec("/health/cluster")),
47+
..Probe::default()
48+
}
49+
}
50+
51+
pub fn tcp_liveliness_probe() -> Probe {
52+
Probe {
53+
initial_delay_seconds: Some(10),
54+
period_seconds: Some(10),
55+
tcp_socket: Some(TCPSocketAction {
56+
port: IntOrString::String(HTTPS_PORT_NAME.to_string()),
57+
..TCPSocketAction::default()
58+
}),
59+
..Probe::default()
60+
}
61+
}
62+
63+
#[cfg(test)]
64+
mod tests {
65+
use super::*;
66+
67+
#[test]
68+
fn startup_probe_execs_curl_against_health_endpoint() {
69+
let probe = management_startup_probe();
70+
71+
let command = probe
72+
.exec
73+
.expect("startup probe must be an exec probe")
74+
.command
75+
.expect("exec action must have a command");
76+
let script = command.last().expect("bash -c script argument");
77+
78+
assert!(
79+
script.contains("http://127.0.0.1:52020/health") && !script.contains("/health/cluster"),
80+
"expected curl against /health, got: {script}"
81+
);
82+
assert_eq!(probe.failure_threshold, Some(120));
83+
assert_eq!(probe.timeout_seconds, Some(5));
84+
assert!(
85+
probe.tcp_socket.is_none(),
86+
"must not fall back to tcp_socket"
87+
);
88+
}
89+
90+
#[test]
91+
fn readiness_probe_execs_curl_against_cluster_health_endpoint() {
92+
let probe = management_readiness_probe();
93+
94+
let command = probe
95+
.exec
96+
.expect("readiness probe must be an exec probe")
97+
.command
98+
.expect("exec action must have a command");
99+
let script = command.last().expect("bash -c script argument");
100+
101+
let cluster_health_url =
102+
format!("http://{MANAGEMENT_SERVER_ADDRESS}:{MANAGEMENT_SERVER_PORT}/health/cluster");
103+
assert!(
104+
script.contains(&cluster_health_url),
105+
"expected curl against /health/cluster, got: {script}"
106+
);
107+
assert_eq!(probe.failure_threshold, Some(3));
108+
assert_eq!(probe.timeout_seconds, Some(5));
109+
assert_eq!(
110+
probe.initial_delay_seconds, None,
111+
"readiness probe delay is redundant: k8s already suppresses readiness checks \
112+
until the startup probe succeeds"
113+
);
114+
}
115+
116+
#[test]
117+
fn probes_use_bash_pipefail_wrapper_not_bare_curl_argv() {
118+
for probe in [management_startup_probe(), management_readiness_probe()] {
119+
let command = probe.exec.unwrap().command.unwrap();
120+
assert_eq!(
121+
command[..4],
122+
[
123+
"/bin/bash".to_string(),
124+
"-euo".to_string(),
125+
"pipefail".to_string(),
126+
"-c".to_string(),
127+
],
128+
"exec command must follow the repo's bash -euo pipefail -c convention"
129+
);
130+
}
131+
}
132+
}

‎rust/operator-binary/src/controller/build/resource/statefulset.rs‎

Lines changed: 14 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@ use stackable_operator::{
2121
apps::v1::{StatefulSet, StatefulSetSpec, StatefulSetUpdateStrategy},
2222
core::v1::{
2323
ConfigMapKeySelector, ConfigMapVolumeSource, EmptyDirVolumeSource, EnvVar,
24-
EnvVarSource, ObjectFieldSelector, PersistentVolumeClaim, Probe,
25-
SecretVolumeSource, TCPSocketAction, Volume,
24+
EnvVarSource, ObjectFieldSelector, PersistentVolumeClaim, SecretVolumeSource,
25+
Volume,
2626
},
2727
},
28-
apimachinery::pkg::{apis::meta::v1::LabelSelector, util::intstr::IntOrString},
28+
apimachinery::pkg::apis::meta::v1::LabelSelector,
2929
},
3030
memory::{BinaryMultiple, MemoryQuantity},
3131
product_logging::{
@@ -54,9 +54,14 @@ use crate::{
5454
graceful_shutdown::add_graceful_shutdown_config,
5555
object_meta,
5656
properties::ConfigFileName,
57-
resource::listener::{
58-
LISTENER_VOLUME_DIR, LISTENER_VOLUME_NAME, build_group_listener_pvc,
59-
group_listener_name,
57+
resource::{
58+
listener::{
59+
LISTENER_VOLUME_DIR, LISTENER_VOLUME_NAME, build_group_listener_pvc,
60+
group_listener_name,
61+
},
62+
probes::{
63+
management_readiness_probe, management_startup_probe, tcp_liveliness_probe,
64+
},
6065
},
6166
},
6267
},
@@ -435,25 +440,9 @@ pub(crate) fn build_node_rolegroup_statefulset(
435440
.add_container_port(HTTPS_PORT_NAME, HTTPS_PORT.into())
436441
.add_container_port(PROTOCOL_PORT_NAME, PROTOCOL_PORT.into())
437442
.add_container_port(BALANCE_PORT_NAME, BALANCE_PORT.into())
438-
.liveness_probe(Probe {
439-
initial_delay_seconds: Some(10),
440-
period_seconds: Some(10),
441-
tcp_socket: Some(TCPSocketAction {
442-
port: IntOrString::String(HTTPS_PORT_NAME.to_string()),
443-
..TCPSocketAction::default()
444-
}),
445-
..Probe::default()
446-
})
447-
.startup_probe(Probe {
448-
initial_delay_seconds: Some(10),
449-
period_seconds: Some(10),
450-
failure_threshold: Some(20 * 6),
451-
tcp_socket: Some(TCPSocketAction {
452-
port: IntOrString::String(HTTPS_PORT_NAME.to_string()),
453-
..TCPSocketAction::default()
454-
}),
455-
..Probe::default()
456-
})
443+
.liveness_probe(tcp_liveliness_probe())
444+
.startup_probe(management_startup_probe())
445+
.readiness_probe(management_readiness_probe())
457446
.resources(merged_config.resources.clone().into());
458447

459448
let mut pod_builder = PodBuilder::new();

‎tests/templates/kuttl/cluster_operation/20-assert.yaml‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,31 @@ metadata:
1212
status:
1313
readyReplicas: 2
1414
replicas: 2
15+
---
16+
apiVersion: apps/v1
17+
kind: StatefulSet
18+
metadata:
19+
name: test-nifi-node-default
20+
spec:
21+
template:
22+
spec:
23+
containers:
24+
- name: nifi
25+
startupProbe:
26+
failureThreshold: 120
27+
exec:
28+
command:
29+
- /bin/bash
30+
- -euo
31+
- pipefail
32+
- -c
33+
- curl --fail --silent --show-error --output /dev/null http://127.0.0.1:52020/health
34+
readinessProbe:
35+
failureThreshold: 3
36+
exec:
37+
command:
38+
- /bin/bash
39+
- -euo
40+
- pipefail
41+
- -c
42+
- curl --fail --silent --show-error --output /dev/null http://127.0.0.1:52020/health/cluster

‎tests/templates/kuttl/smoke/35-assert.yaml.j2‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ commands:
3636
java.arg.10=-Djavax.security.auth.useSubjectCredsOnly=true
3737
java.arg.11=-Dzookeeper.admin.enableServer=false
3838
java.arg.12=-Djava.security.properties=/stackable/nifi/conf/security.properties
39+
java.arg.13=-Dorg.apache.nifi.management.server.address=127.0.0.1:52020
3940
java.arg.2=-Xms3276m
4041
java.arg.3=-XX:+UseG1GC
4142
java.arg.4=-Djava.awt.headless=true

0 commit comments

Comments
 (0)