Skip to content

Commit 87a4772

Browse files
committed
Add (failing test) -> TDD :)
1 parent 8ff284f commit 87a4772

File tree

2 files changed

+123
-12
lines changed

2 files changed

+123
-12
lines changed

rust/operator-binary/src/config/jvm.rs

Lines changed: 122 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -47,17 +47,15 @@ pub fn construct_role_specific_jvm_args(
4747
role: role.to_string(),
4848
}
4949
})?;
50-
jvm_args.push(format!(
51-
"-Xmx{}",
52-
(memory_limit * JVM_HEAP_FACTOR)
53-
.scale_to(BinaryMultiple::Kibi)
54-
.format_for_java()
55-
.with_context(|_| {
56-
InvalidJavaHeapConfigSnafu {
57-
role: role.to_string(),
58-
}
59-
})?
60-
));
50+
let heap = memory_limit.scale_to(BinaryMultiple::Mebi) * JVM_HEAP_FACTOR;
51+
let heap = heap
52+
.format_for_java()
53+
.with_context(|_| InvalidJavaHeapConfigSnafu {
54+
role: role.to_string(),
55+
})?;
56+
57+
jvm_args.push(format!("-Xms{heap}"));
58+
jvm_args.push(format!("-Xmx{heap}"));
6159
}
6260

6361
jvm_args.extend([format!(
@@ -73,3 +71,116 @@ pub fn construct_role_specific_jvm_args(
7371
// TODO: Handle user input
7472
Ok(jvm_args.join(" "))
7573
}
74+
75+
#[cfg(test)]
76+
mod tests {
77+
use stackable_hdfs_crd::{constants::DEFAULT_NAME_NODE_METRICS_PORT, HdfsCluster};
78+
79+
use crate::container::ContainerConfig;
80+
81+
use super::*;
82+
83+
#[test]
84+
fn test_global_jvm_args() {
85+
assert_eq!(construct_global_jvm_args(false), "");
86+
assert_eq!(
87+
construct_global_jvm_args(true),
88+
"-Djava.security.krb5.conf=/stackable/kerberos/krb5.conf"
89+
);
90+
}
91+
92+
#[test]
93+
fn test_jvm_config_defaults_without_kerberos() {
94+
let input = r#"
95+
apiVersion: hdfs.stackable.tech/v1alpha1
96+
kind: HdfsCluster
97+
metadata:
98+
name: hdfs
99+
spec:
100+
image:
101+
productVersion: 3.4.0
102+
clusterConfig:
103+
zookeeperConfigMapName: hdfs-zk
104+
nameNodes:
105+
roleGroups:
106+
default:
107+
replicas: 1
108+
"#;
109+
let jvm_config = construct_test_role_specific_jvm_args(input, false);
110+
111+
assert_eq!(
112+
jvm_config,
113+
"-Xms819m \
114+
-Xmx819m \
115+
-Djava.security.properties=/stackable/config/security.properties \
116+
-javaagent:/stackable/jmx/jmx_prometheus_javaagent.jar=8183:/stackable/jmx/namenode.yaml"
117+
);
118+
}
119+
120+
#[test]
121+
fn test_jvm_config_jvm_argument_overrides() {
122+
let input = r#"
123+
apiVersion: hdfs.stackable.tech/v1alpha1
124+
kind: HdfsCluster
125+
metadata:
126+
name: hdfs
127+
spec:
128+
image:
129+
productVersion: 3.4.0
130+
clusterConfig:
131+
zookeeperConfigMapName: hdfs-zk
132+
nameNodes:
133+
config:
134+
resources:
135+
memory:
136+
limit: 42Gi
137+
jvmArgumentOverrides:
138+
add:
139+
- -Dhttps.proxyHost=proxy.my.corp
140+
- -Dhttps.proxyPort=8080
141+
- -Djava.net.preferIPv4Stack=true
142+
roleGroups:
143+
default:
144+
replicas: 1
145+
jvmArgumentOverrides:
146+
# We need more memory!
147+
removeRegex:
148+
- -Xmx.*
149+
- -Dhttps.proxyPort=.*
150+
add:
151+
- -Xmx40000m
152+
- -Dhttps.proxyPort=1234
153+
"#;
154+
let jvm_config = construct_test_role_specific_jvm_args(input, true);
155+
156+
assert_eq!(
157+
jvm_config,
158+
"-Xms34406m \
159+
-Djava.security.properties=/stackable/config/security.properties \
160+
-javaagent:/stackable/jmx/jmx_prometheus_javaagent.jar=8183:/stackable/jmx/namenode.yaml \
161+
-Djava.security.krb5.conf=/stackable/kerberos/krb5.conf \
162+
-Dhttps.proxyHost=proxy.my.corp \
163+
-Djava.net.preferIPv4Stack=true \
164+
-Xmx40000m \
165+
-Dhttps.proxyPort=1234"
166+
);
167+
}
168+
169+
fn construct_test_role_specific_jvm_args(hdfs_cluster: &str, kerberos_enabled: bool) -> String {
170+
let hdfs: HdfsCluster = serde_yaml::from_str(hdfs_cluster).expect("illegal test input");
171+
172+
let role = HdfsRole::NameNode;
173+
let merged_config = role.merged_config(&hdfs, "default").unwrap();
174+
let container_config = ContainerConfig::from(role);
175+
let resources = container_config.resources(&merged_config);
176+
177+
construct_role_specific_jvm_args(
178+
&role,
179+
kerberos_enabled,
180+
resources.as_ref(),
181+
"/stackable/config",
182+
DEFAULT_NAME_NODE_METRICS_PORT,
183+
)
184+
.unwrap()
185+
}
186+
}

rust/operator-binary/src/container.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -913,7 +913,7 @@ wait_for_termination $!
913913
}
914914

915915
/// Returns the container resources.
916-
fn resources(&self, merged_config: &AnyNodeConfig) -> Option<ResourceRequirements> {
916+
pub fn resources(&self, merged_config: &AnyNodeConfig) -> Option<ResourceRequirements> {
917917
match self {
918918
// Namenode sidecar containers
919919
ContainerConfig::Zkfc { .. } => Some(

0 commit comments

Comments
 (0)