JVM config overrides (#272)

* wip

* Update changelog.

* First working version

* Update smoke test

* Fix path to security.properties for histo server.

* Update docs.

* Fix logging test.

* Remove redundant java options.

* Update smoke test.

* Remove debug=ssl property.

* Revert "Remove redundant java options."

This reverts commit e8d79f3.

* Remove SPARK_DAEMON_JAVA_OPTS

* Remove javax.net.debug

Author: razvan

CHANGELOG.md

@@ -7,15 +7,18 @@ All notable changes to this project will be documented in this file.
 ### Added
 
 - Default stackableVersion to operator version. It is recommended to remove `spec.image.stackableVersion` from your custom resources ([#267], [#268]).
+- Configuration overrides for the JVM security properties, such as DNS caching ([#272]).
 
 ### Changed
 
 - `vector` `0.26.0` -> `0.31.0` ([#269]).
 - `operator-rs` `0.44.0` -> `0.45.1` ([#267]).
+- Removed usages of SPARK_DAEMON_JAVA_OPTS since it's not a reliable way to pass extra JVM options ([#272]).
 
 [#267]: https://github.com/stackabletech/spark-k8s-operator/pull/267
 [#268]: https://github.com/stackabletech/spark-k8s-operator/pull/268
 [#269]: https://github.com/stackabletech/spark-k8s-operator/pull/269
+[#272]: https://github.com/stackabletech/spark-k8s-operator/pull/272
 
 ## [23.7.0] - 2023-07-14
 

deploy/config-spec/properties.yaml

@@ -3,4 +3,41 @@
 version: 0.1.0
 spec:
   units: []
-properties: []
+properties:
+  - property: &jvmDnsCacheTtl
+      propertyNames:
+        - name: "networkaddress.cache.ttl"
+          kind:
+            type: "file"
+            file: "security.properties"
+      datatype:
+        type: "integer"
+        min: "0"
+      recommendedValues:
+        - fromVersion: "0.0.0"
+          value: "30"
+      roles:
+        - name: "node"
+          required: true
+      asOfVersion: "0.0.0"
+      comment: "History server - TTL for successfully resolved domain names."
+      description: "History server - TTL for successfully resolved domain names."
+
+  - property: &jvmDnsCacheNegativeTtl
+      propertyNames:
+        - name: "networkaddress.cache.negative.ttl"
+          kind:
+            type: "file"
+            file: "security.properties"
+      datatype:
+        type: "integer"
+        min: "0"
+      recommendedValues:
+        - fromVersion: "0.0.0"
+          value: "0"
+      roles:
+        - name: "node"
+          required: true
+      asOfVersion: "0.0.0"
+      comment: "History server - TTL for domain names that cannot be resolved."
+      description: "History server - TTL for domain names that cannot be resolved."

deploy/helm/spark-k8s-operator/configs/properties.yaml

@@ -3,4 +3,41 @@
 version: 0.1.0
 spec:
   units: []
-properties: []
+properties:
+  - property: &jvmDnsCacheTtl
+      propertyNames:
+        - name: "networkaddress.cache.ttl"
+          kind:
+            type: "file"
+            file: "security.properties"
+      datatype:
+        type: "integer"
+        min: "0"
+      recommendedValues:
+        - fromVersion: "0.0.0"
+          value: "30"
+      roles:
+        - name: "node"
+          required: true
+      asOfVersion: "0.0.0"
+      comment: "History server - TTL for successfully resolved domain names."
+      description: "History server - TTL for successfully resolved domain names."
+
+  - property: &jvmDnsCacheNegativeTtl
+      propertyNames:
+        - name: "networkaddress.cache.negative.ttl"
+          kind:
+            type: "file"
+            file: "security.properties"
+      datatype:
+        type: "integer"
+        min: "0"
+      recommendedValues:
+        - fromVersion: "0.0.0"
+          value: "0"
+      roles:
+        - name: "node"
+          required: true
+      asOfVersion: "0.0.0"
+      comment: "History server - TTL for domain names that cannot be resolved."
+      description: "History server - TTL for domain names that cannot be resolved."

deploy/helm/spark-k8s-operator/crds/crds.yaml

@@ -527,6 +527,12 @@ spec:
                               type: array
                           type: object
                       type: object
+                    jvmSecurity:
+                      additionalProperties:
+                        nullable: true
+                        type: string
+                      default: {}
+                      type: object
                     logging:
                       default:
                         enableVectorAgent: null
@@ -4015,6 +4021,12 @@ spec:
                       minimum: 0.0
                       nullable: true
                       type: integer
+                    jvmSecurity:
+                      additionalProperties:
+                        nullable: true
+                        type: string
+                      default: {}
+                      type: object
                     logging:
                       default:
                         enableVectorAgent: null

docs/modules/spark-k8s/pages/crd-reference.adoc

@@ -116,4 +116,10 @@ Below are listed the CRD fields that can be defined by the user:
 |`spec.logFileDirectory.prefix`
 |Prefix to use when storing events for the Spark History server.
 
+|`spec.driver.jvmSecurity`
+|A list JVM security properties to pass on to the driver VM. The TTL of DNS caches are especially important.
+
+|`spec.executor.jvmSecurity`
+|A list JVM security properties to pass on to the executor VM. The TTL of DNS caches are especially important.
+
 |===

docs/modules/spark-k8s/pages/usage-guide/history-server.adoc

@@ -31,7 +31,7 @@ The secret with S3 credentials must contain at least the following two keys:
 
 Any other entries of the Secret are ignored by the operator.
 
-== Application configuration
+== Spark application configuration
 
 
 The example below demonstrates how to configure Spark applications to write log events to an S3 bucket.
@@ -65,3 +65,28 @@ spark-history-node-cleaner   NodePort    10.96.203.43    <none>        18080:325
 By setting up port forwarding on 18080 the UI can be opened by pointing your browser to `http://localhost:18080`:
 
 image::history-server-ui.png[History Server Console]
+
+== Configuration Properties
+
+For a role group of the Spark history server, you can specify: `configOverrides` for the following files:
+
+- `security.properties`
+
+=== The security.properties file
+
+The `security.properties` file is used to configure JVM security properties. It is very seldom that users need to tweak any of these, but there is one use-case that stands out, and that users need to be aware of: the JVM DNS cache.
+
+The JVM manages it's own cache of successfully resolved host names as well as a cache of host names that cannot be resolved. Some products of the Stackable platform are very sensible to the contents of these caches and their performance is heavily affected by them. As of version 3.4.0, Apache Spark may perform poorly if the positive cache is disabled. To cache resolved host names, and thus speeding up queries you can configure the TTL of entries in the positive cache like this:
+
+[source,yaml]
+----
+  nodes:
+    configOverrides:
+      security.properties:
+        networkaddress.cache.ttl: "30"
+        networkaddress.cache.negative.ttl: "0"
+----
+
+NOTE: The operator configures DNS caching by default as shown in the example above.
+
+For details on the JVM security see https://docs.oracle.com/en/java/javase/11/security/java-security-overview1.html

rust/crd/src/constants.rs

@@ -12,6 +12,7 @@ pub const VOLUME_MOUNT_PATH_EXECUTOR_POD_TEMPLATES: &str =
 pub const POD_TEMPLATE_FILE: &str = "template.yaml";
 
 pub const VOLUME_MOUNT_NAME_CONFIG: &str = "config";
+pub const VOLUME_MOUNT_PATH_CONFIG: &str = "/stackable/spark/conf";
 
 pub const VOLUME_MOUNT_NAME_JOB: &str = "job-files";
 pub const VOLUME_MOUNT_PATH_JOB: &str = "/stackable/spark/jobs";
@@ -27,6 +28,8 @@ pub const VOLUME_MOUNT_PATH_LOG: &str = "/stackable/log";
 
 pub const LOG4J2_CONFIG_FILE: &str = "log4j2.properties";
 
+pub const JVM_SECURITY_PROPERTIES_FILE: &str = "security.properties";
+
 pub const ACCESS_KEY_ID: &str = "accessKey";
 pub const SECRET_ACCESS_KEY: &str = "secretKey";
 pub const S3_SECRET_DIR_NAME: &str = "/stackable/secrets";
@@ -67,8 +70,7 @@ pub const HISTORY_ROLE_NAME: &str = "node";
 
 pub const HISTORY_IMAGE_BASE_NAME: &str = "spark-k8s";
 
-pub const HISTORY_CONFIG_FILE_NAME: &str = "spark-defaults.conf";
-pub const HISTORY_CONFIG_FILE_NAME_FULL: &str = "/stackable/spark/conf/spark-defaults.conf";
+pub const SPARK_DEFAULTS_FILE_NAME: &str = "spark-defaults.conf";
 
 pub const SPARK_CLUSTER_ROLE: &str = "spark-k8s-clusterrole";
 pub const SPARK_UID: i64 = 1000;

rust/crd/src/history.rs

@@ -200,7 +200,10 @@ impl SparkHistoryServer {
         > = vec![(
             HISTORY_ROLE_NAME.to_string(),
             (
-                vec![PropertyNameKind::File(HISTORY_CONFIG_FILE_NAME.to_string())],
+                vec![
+                    PropertyNameKind::File(SPARK_DEFAULTS_FILE_NAME.to_string()),
+                    PropertyNameKind::File(JVM_SECURITY_PROPERTIES_FILE.to_string()),
+                ],
                 self.spec.nodes.clone(),
             ),
         )]

rust/crd/src/lib.rs

@@ -538,11 +538,27 @@ impl SparkApplication {
             }
         }
 
-        // s3 with TLS
+        // Extra JVM opts:
+        // - java security properties
+        // - s3 with TLS
+        let mut extra_java_opts = vec![format!(
+            "-Djava.security.properties={VOLUME_MOUNT_PATH_LOG_CONFIG}/{JVM_SECURITY_PROPERTIES_FILE}"
+        )];
         if tlscerts::tls_secret_names(s3conn, s3_log_dir).is_some() {
-            submit_cmd.push(format!("--conf spark.driver.extraJavaOptions=\"-Djavax.net.ssl.trustStore={STACKABLE_TRUST_STORE}/truststore.p12 -Djavax.net.ssl.trustStorePassword={STACKABLE_TLS_STORE_PASSWORD} -Djavax.net.ssl.trustStoreType=pkcs12 -Djavax.net.debug=ssl,handshake\""));
-            submit_cmd.push(format!("--conf spark.executor.extraJavaOptions=\"-Djavax.net.ssl.trustStore={STACKABLE_TRUST_STORE}/truststore.p12 -Djavax.net.ssl.trustStorePassword={STACKABLE_TLS_STORE_PASSWORD}  -Djavax.net.ssl.trustStoreType=pkcs12 -Djavax.net.debug=ssl,handshake\""));
+            extra_java_opts.extend(
+                vec![
+                    format!("-Djavax.net.ssl.trustStore={STACKABLE_TRUST_STORE}/truststore.p12"),
+                    format!("-Djavax.net.ssl.trustStorePassword={STACKABLE_TLS_STORE_PASSWORD}"),
+                    format!("-Djavax.net.ssl.trustStoreType=pkcs12"),
+                ]
+                .into_iter(),
+            );
         }
+        let str_extra_java_opts = extra_java_opts.join(" ");
+        submit_cmd.extend(vec![
+            format!("--conf spark.driver.extraJavaOptions=\"{str_extra_java_opts}\""),
+            format!("--conf spark.executor.extraJavaOptions=\"{str_extra_java_opts}\""),
+        ]);
 
         // repositories and packages arguments
         if let Some(deps) = self.spec.deps.clone() {
@@ -642,18 +658,6 @@ impl SparkApplication {
                 value_from: None,
             });
         }
-        if let Some(s3logdir) = s3logdir {
-            if tlscerts::tls_secret_name(&s3logdir.bucket.connection).is_some() {
-                e.push(EnvVar {
-                    name: "SPARK_DAEMON_JAVA_OPTS".to_string(),
-                    value: Some(format!(
-                        "-Djavax.net.ssl.trustStore={STACKABLE_TRUST_STORE}/truststore.p12 -Djavax.net.ssl.trustStorePassword={STACKABLE_TLS_STORE_PASSWORD} -Djavax.net.ssl.trustStoreType=pkcs12"
-                    )),
-                    value_from: None,
-                });
-            }
-        }
-
         e
     }
 
@@ -957,6 +961,8 @@ pub struct DriverConfig {
     #[fragment_attrs(serde(default))]
     #[fragment_attrs(schemars(schema_with = "pod_overrides_schema"))]
     pub pod_overrides: PodTemplateSpec,
+    #[fragment_attrs(serde(default))]
+    pub jvm_security: HashMap<String, Option<String>>,
 }
 
 impl DriverConfig {
@@ -977,6 +983,18 @@ impl DriverConfig {
             volume_mounts: Some(VolumeMounts::default()),
             affinity: StackableAffinityFragment::default(),
             pod_overrides: PodTemplateSpec::default(),
+            jvm_security: vec![
+                (
+                    "networkaddress.cache.ttl".to_string(),
+                    Some("30".to_string()),
+                ),
+                (
+                    "networkaddress.cache.negative.ttl".to_string(),
+                    Some("0".to_string()),
+                ),
+            ]
+            .into_iter()
+            .collect(),
         }
     }
 }
@@ -1011,6 +1029,8 @@ pub struct ExecutorConfig {
     #[fragment_attrs(serde(default))]
     #[fragment_attrs(schemars(schema_with = "pod_overrides_schema"))]
     pub pod_overrides: PodTemplateSpec,
+    #[fragment_attrs(serde(default))]
+    pub jvm_security: HashMap<String, Option<String>>,
 }
 
 impl ExecutorConfig {
@@ -1033,6 +1053,18 @@ impl ExecutorConfig {
             node_selector: Default::default(),
             affinity: Default::default(),
             pod_overrides: PodTemplateSpec::default(),
+            jvm_security: vec![
+                (
+                    "networkaddress.cache.ttl".to_string(),
+                    Some("30".to_string()),
+                ),
+                (
+                    "networkaddress.cache.negative.ttl".to_string(),
+                    Some("0".to_string()),
+                ),
+            ]
+            .into_iter()
+            .collect(),
         }
     }
 }
@@ -1053,7 +1085,7 @@ mod tests {
     };
     use stackable_operator::k8s_openapi::api::core::v1::PodTemplateSpec;
     use stackable_operator::product_logging::spec::Logging;
-    use std::collections::BTreeMap;
+    use std::collections::{BTreeMap, HashMap};
     use std::str::FromStr;
 
     #[test]
@@ -1419,6 +1451,7 @@ spec:
             volume_mounts: None,
             affinity: StackableAffinity::default(),
             pod_overrides: PodTemplateSpec::default(),
+            jvm_security: HashMap::new(),
         };
 
         let mut props = BTreeMap::new();
@@ -1474,6 +1507,7 @@ spec:
             node_selector: None,
             affinity: StackableAffinity::default(),
             pod_overrides: PodTemplateSpec::default(),
+            jvm_security: HashMap::new(),
         };
 
         let mut props = BTreeMap::new();

rust/crd/src/s3logdir.rs

@@ -95,7 +95,7 @@ impl S3LogDir {
     }
 
     /// Constructs the properties needed for loading event logs from S3.
-    /// These properties are later written in the `HISTORY_CONFIG_FILE_NAME_FULL` file.
+    /// These properties are later written in the `SPARK_DEFAULTS_FILE_NAME` file.
     ///
     /// The following properties related to credentials are not included:
     /// * spark.hadoop.fs.s3a.aws.credentials.provider