diff --git a/CHANGELOG.md b/CHANGELOG.md index d0453547..ca116453 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ config property `requestedSecretLifetime`. This helps reducing frequent Pod restarts ([#598]). - Run a `containerdebug` process in the background of each HBase container to collect debugging information ([#605]). - Aggregate emitted Kubernetes events on the CustomResources ([#612]). +- Support configuring JVM arguments ([#620]). + +### Removed + +- BREAKING: The field `config.hbaseOpts` has been removed. Use JVM argument overrides instead to configure additional JVM arguments ([#620]). ### Changed @@ -17,6 +22,7 @@ [#605]: https://github.com/stackabletech/hbase-operator/pull/605 [#611]: https://github.com/stackabletech/hbase-operator/pull/611 [#612]: https://github.com/stackabletech/hbase-operator/pull/612 +[#620]: https://github.com/stackabletech/hbase-operator/pull/620 ## [24.11.1] - 2025-01-09 diff --git a/deploy/helm/hbase-operator/crds/crds.yaml b/deploy/helm/hbase-operator/crds/crds.yaml index 40106961..73445bbb 100644 --- a/deploy/helm/hbase-operator/crds/crds.yaml +++ b/deploy/helm/hbase-operator/crds/crds.yaml @@ -207,9 +207,6 @@ spec: 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 - hbaseOpts: - nullable: true - type: string hbaseRootdir: nullable: true type: string @@ -354,6 +351,32 @@ spec: default: {} description: '`envOverrides` configure environment variables to be set in the Pods. It is a map from strings to strings - environment variables and the value to set. Read the [environment variable overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#env-overrides) for more information and consult the operator specific usage guide to find out about the product specific environment variables that are available.' type: object + jvmArgumentOverrides: + default: + add: [] + remove: [] + removeRegex: [] + description: Allows overriding JVM arguments. Please read on the [JVM argument overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#jvm-argument-overrides) for details on the usage. + properties: + add: + default: [] + description: JVM arguments to be added + items: + type: string + type: array + remove: + default: [] + description: JVM arguments to be removed by exact match + items: + type: string + type: array + removeRegex: + default: [] + description: JVM arguments matching any of this regexes will be removed + items: + type: string + type: array + type: object podOverrides: default: {} description: In the `podOverrides` property you can define a [PodTemplateSpec](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#podtemplatespec-v1-core) to override any property that can be set on a Kubernetes Pod. Read the [Pod overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#pod-overrides) for more information. @@ -434,9 +457,6 @@ spec: 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 - hbaseOpts: - nullable: true - type: string hbaseRootdir: nullable: true type: string @@ -581,6 +601,32 @@ spec: default: {} description: '`envOverrides` configure environment variables to be set in the Pods. It is a map from strings to strings - environment variables and the value to set. Read the [environment variable overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#env-overrides) for more information and consult the operator specific usage guide to find out about the product specific environment variables that are available.' type: object + jvmArgumentOverrides: + default: + add: [] + remove: [] + removeRegex: [] + description: Allows overriding JVM arguments. Please read on the [JVM argument overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#jvm-argument-overrides) for details on the usage. + properties: + add: + default: [] + description: JVM arguments to be added + items: + type: string + type: array + remove: + default: [] + description: JVM arguments to be removed by exact match + items: + type: string + type: array + removeRegex: + default: [] + description: JVM arguments matching any of this regexes will be removed + items: + type: string + type: array + type: object podOverrides: default: {} description: In the `podOverrides` property you can define a [PodTemplateSpec](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#podtemplatespec-v1-core) to override any property that can be set on a Kubernetes Pod. Read the [Pod overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#pod-overrides) for more information. @@ -642,9 +688,6 @@ spec: 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 - hbaseOpts: - nullable: true - type: string hbaseRootdir: nullable: true type: string @@ -789,6 +832,32 @@ spec: default: {} description: '`envOverrides` configure environment variables to be set in the Pods. It is a map from strings to strings - environment variables and the value to set. Read the [environment variable overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#env-overrides) for more information and consult the operator specific usage guide to find out about the product specific environment variables that are available.' type: object + jvmArgumentOverrides: + default: + add: [] + remove: [] + removeRegex: [] + description: Allows overriding JVM arguments. Please read on the [JVM argument overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#jvm-argument-overrides) for details on the usage. + properties: + add: + default: [] + description: JVM arguments to be added + items: + type: string + type: array + remove: + default: [] + description: JVM arguments to be removed by exact match + items: + type: string + type: array + removeRegex: + default: [] + description: JVM arguments matching any of this regexes will be removed + items: + type: string + type: array + type: object podOverrides: default: {} description: In the `podOverrides` property you can define a [PodTemplateSpec](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#podtemplatespec-v1-core) to override any property that can be set on a Kubernetes Pod. Read the [Pod overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#pod-overrides) for more information. @@ -869,9 +938,6 @@ spec: 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 - hbaseOpts: - nullable: true - type: string hbaseRootdir: nullable: true type: string @@ -1016,6 +1082,32 @@ spec: default: {} description: '`envOverrides` configure environment variables to be set in the Pods. It is a map from strings to strings - environment variables and the value to set. Read the [environment variable overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#env-overrides) for more information and consult the operator specific usage guide to find out about the product specific environment variables that are available.' type: object + jvmArgumentOverrides: + default: + add: [] + remove: [] + removeRegex: [] + description: Allows overriding JVM arguments. Please read on the [JVM argument overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#jvm-argument-overrides) for details on the usage. + properties: + add: + default: [] + description: JVM arguments to be added + items: + type: string + type: array + remove: + default: [] + description: JVM arguments to be removed by exact match + items: + type: string + type: array + removeRegex: + default: [] + description: JVM arguments matching any of this regexes will be removed + items: + type: string + type: array + type: object podOverrides: default: {} description: In the `podOverrides` property you can define a [PodTemplateSpec](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#podtemplatespec-v1-core) to override any property that can be set on a Kubernetes Pod. Read the [Pod overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#pod-overrides) for more information. @@ -1077,9 +1169,6 @@ spec: 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 - hbaseOpts: - nullable: true - type: string hbaseRootdir: nullable: true type: string @@ -1224,6 +1313,32 @@ spec: default: {} description: '`envOverrides` configure environment variables to be set in the Pods. It is a map from strings to strings - environment variables and the value to set. Read the [environment variable overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#env-overrides) for more information and consult the operator specific usage guide to find out about the product specific environment variables that are available.' type: object + jvmArgumentOverrides: + default: + add: [] + remove: [] + removeRegex: [] + description: Allows overriding JVM arguments. Please read on the [JVM argument overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#jvm-argument-overrides) for details on the usage. + properties: + add: + default: [] + description: JVM arguments to be added + items: + type: string + type: array + remove: + default: [] + description: JVM arguments to be removed by exact match + items: + type: string + type: array + removeRegex: + default: [] + description: JVM arguments matching any of this regexes will be removed + items: + type: string + type: array + type: object podOverrides: default: {} description: In the `podOverrides` property you can define a [PodTemplateSpec](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#podtemplatespec-v1-core) to override any property that can be set on a Kubernetes Pod. Read the [Pod overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#pod-overrides) for more information. @@ -1304,9 +1419,6 @@ spec: 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 - hbaseOpts: - nullable: true - type: string hbaseRootdir: nullable: true type: string @@ -1451,6 +1563,32 @@ spec: default: {} description: '`envOverrides` configure environment variables to be set in the Pods. It is a map from strings to strings - environment variables and the value to set. Read the [environment variable overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#env-overrides) for more information and consult the operator specific usage guide to find out about the product specific environment variables that are available.' type: object + jvmArgumentOverrides: + default: + add: [] + remove: [] + removeRegex: [] + description: Allows overriding JVM arguments. Please read on the [JVM argument overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#jvm-argument-overrides) for details on the usage. + properties: + add: + default: [] + description: JVM arguments to be added + items: + type: string + type: array + remove: + default: [] + description: JVM arguments to be removed by exact match + items: + type: string + type: array + removeRegex: + default: [] + description: JVM arguments matching any of this regexes will be removed + items: + type: string + type: array + type: object podOverrides: default: {} description: In the `podOverrides` property you can define a [PodTemplateSpec](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#podtemplatespec-v1-core) to override any property that can be set on a Kubernetes Pod. Read the [Pod overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#pod-overrides) for more information. diff --git a/docs/modules/hbase/pages/usage-guide/overrides.adoc b/docs/modules/hbase/pages/usage-guide/overrides.adoc index 346c1100..7199676b 100644 --- a/docs/modules/hbase/pages/usage-guide/overrides.adoc +++ b/docs/modules/hbase/pages/usage-guide/overrides.adoc @@ -95,3 +95,21 @@ The HBaseCluster Stacklet does not support environment variable overrides with t The HBase Stacklet and operator also support Pod overrides, allowing you to override any property that you can set on a Kubernetes Pod. Read the xref:concepts:overrides.adoc#pod-overrides[Pod overrides documentation] to learn more about this feature. + +== JVM argument overrides + +Stackable operators automatically determine the set of needed JVM arguments, such as memory settings or trust- and keystores. +Using JVM argument overrides you can configure the JVM arguments xref:concepts:overrides.adoc#jvm-argument-overrides[according to the concepts page]. + +One thing that is different for Kafka, is that all heap-related arguments will be passed in via the env variable `HBASE_HEAPSIZE`, all the other ones via `HBASE_OPTS`, `HBASE_MASTER_OPTS`, `HBASE_REGIONSERVER_OPTS` and `HBASE_REST_OPTS`. +The `HBASE_HEAPSIZE` variable is documented as follows in the https://cwiki.apache.org/confluence/display/HADOOP2/Hbase+FAQ+Operations[HBase FAQs]: + +> Set the HBASE_HEAPSIZE environment variable in ${HBASE_HOME}/conf/hbase-env.sh if your install needs to run with a larger heap. +> HBASE_HEAPSIZE is like HADOOP_HEAPSIZE in that its value is the desired heap size in MB. +> The surrounding '-Xmx' and 'm' needed to make up the maximum heap size java option are added by the hbase start script +> (See how HBASE_HEAPSIZE is used in the ${HBASE_HOME}/bin/hbase script for clarification). + +Looking at `bin/hbase`, you can actually add the `m` suffix to make the unit more clear, the script will detect this https://github.com/apache/hbase/blob/777010361abb203b8b17673d84acf4f7f1d0283a/bin/hbase#L165[here] and work correctly. + +Because of this, it is not possible to change `-XmS` and `-XmX` via JVM argument overrides. +You need to envOverride `HBASE_HEAPSIZE` instead. diff --git a/rust/crd/src/lib.rs b/rust/crd/src/lib.rs index 27387a27..c42f9625 100644 --- a/rust/crd/src/lib.rs +++ b/rust/crd/src/lib.rs @@ -21,9 +21,7 @@ use stackable_operator::{ kube::{runtime::reflector::ObjectRef, CustomResource, ResourceExt}, product_config_utils::Configuration, product_logging::{self, spec::Logging}, - role_utils::{ - GenericProductSpecificCommonConfig, GenericRoleConfig, Role, RoleGroup, RoleGroupRef, - }, + role_utils::{GenericRoleConfig, JavaCommonConfig, Role, RoleGroup, RoleGroupRef}, schemars::{self, JsonSchema}, status::condition::{ClusterCondition, HasStatusCondition}, time::Duration, @@ -51,16 +49,10 @@ pub const HBASE_SITE_XML: &str = "hbase-site.xml"; pub const SSL_SERVER_XML: &str = "ssl-server.xml"; pub const SSL_CLIENT_XML: &str = "ssl-client.xml"; -pub const HBASE_MANAGES_ZK: &str = "HBASE_MANAGES_ZK"; -pub const HBASE_MASTER_OPTS: &str = "HBASE_MASTER_OPTS"; -pub const HBASE_REGIONSERVER_OPTS: &str = "HBASE_REGIONSERVER_OPTS"; -pub const HBASE_REST_OPTS: &str = "HBASE_REST_OPTS"; - pub const HBASE_CLUSTER_DISTRIBUTED: &str = "hbase.cluster.distributed"; pub const HBASE_ROOTDIR: &str = "hbase.rootdir"; pub const HBASE_UNSAFE_REGIONSERVER_HOSTNAME_DISABLE_MASTER_REVERSEDNS: &str = "hbase.unsafe.regionserver.hostname.disable.master.reversedns"; -pub const HBASE_HEAPSIZE: &str = "HBASE_HEAPSIZE"; pub const HBASE_UI_PORT_NAME_HTTP: &str = "ui-http"; pub const HBASE_UI_PORT_NAME_HTTPS: &str = "ui-https"; @@ -81,8 +73,6 @@ pub const HBASE_REST_UI_PORT: u16 = 8085; // Newer versions use the same port as the UI because Hbase provides it's own metrics API pub const METRICS_PORT: u16 = 9100; -pub const JVM_HEAP_FACTOR: f32 = 0.8; - #[derive(Snafu, Debug)] pub enum Error { #[snafu(display("the role [{role}] is invalid and does not exist in HBase"))] @@ -137,15 +127,15 @@ pub struct HbaseClusterSpec { /// The HBase master process is responsible for assigning regions to region servers and /// manages the cluster. #[serde(default, skip_serializing_if = "Option::is_none")] - pub masters: Option>, + pub masters: Option>, /// Region servers hold the data and handle requests from clients for their region. #[serde(default, skip_serializing_if = "Option::is_none")] - pub region_servers: Option>, + pub region_servers: Option>, /// Rest servers provide a REST API to interact with. #[serde(default, skip_serializing_if = "Option::is_none")] - pub rest_servers: Option>, + pub rest_servers: Option>, } #[derive(Clone, Debug, Default, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] @@ -325,7 +315,6 @@ impl HbaseRole { HbaseConfigFragment { hbase_rootdir: None, - hbase_opts: None, resources, logging: product_logging::spec::default_logging(), affinity: get_affinity(cluster_name, self, hdfs_discovery_cm_name), @@ -413,12 +402,13 @@ pub enum Container { pub struct HbaseConfig { #[serde(default, skip_serializing_if = "Option::is_none")] pub hbase_rootdir: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub hbase_opts: Option, + #[fragment_attrs(serde(default))] pub resources: Resources, + #[fragment_attrs(serde(default))] pub logging: Logging, + #[fragment_attrs(serde(default))] pub affinity: StackableAffinity, @@ -538,7 +528,10 @@ impl HbaseCluster { } } - pub fn get_role(&self, role: &HbaseRole) -> Option<&Role> { + pub fn get_role( + &self, + role: &HbaseRole, + ) -> Option<&Role> { match role { HbaseRole::Master => self.spec.masters.as_ref(), HbaseRole::RegionServer => self.spec.region_servers.as_ref(), @@ -550,7 +543,7 @@ impl HbaseCluster { pub fn get_role_group( &self, rolegroup_ref: &RoleGroupRef, - ) -> Result<&RoleGroup, Error> { + ) -> Result<&RoleGroup, Error> { let role_variant = HbaseRole::from_str(&rolegroup_ref.role).with_context(|_| InvalidRoleSnafu { role: rolegroup_ref.role.to_owned(), diff --git a/rust/operator-binary/src/config/jvm.rs b/rust/operator-binary/src/config/jvm.rs new file mode 100644 index 00000000..8a40368e --- /dev/null +++ b/rust/operator-binary/src/config/jvm.rs @@ -0,0 +1,275 @@ +use snafu::{OptionExt, ResultExt, Snafu}; +use stackable_hbase_crd::{ + HbaseConfig, HbaseConfigFragment, HbaseRole, CONFIG_DIR_NAME, JVM_SECURITY_PROPERTIES_FILE, + METRICS_PORT, +}; +use stackable_operator::{ + memory::{BinaryMultiple, MemoryQuantity}, + role_utils::{self, GenericRoleConfig, JavaCommonConfig, JvmArgumentOverrides, Role}, +}; + +const JAVA_HEAP_FACTOR: f32 = 0.8; + +#[derive(Snafu, Debug)] +pub enum Error { + #[snafu(display("invalid memory resource configuration - missing default or value in crd?"))] + MissingMemoryResourceConfig, + + #[snafu(display("invalid memory config"))] + InvalidMemoryConfig { + source: stackable_operator::memory::Error, + }, + + #[snafu(display("failed to merge jvm argument overrides"))] + MergeJvmArgumentOverrides { source: role_utils::Error }, +} + +// Applies to both the servers and the CLI +pub fn construct_global_jvm_args(kerberos_enabled: bool) -> String { + let mut jvm_args = Vec::new(); + + if kerberos_enabled { + jvm_args.push("-Djava.security.krb5.conf=/stackable/kerberos/krb5.conf"); + } + + // We do *not* add user overrides to the global JVM args, but only the role specific JVM arguments. + // This allows users to configure stuff for the server (probably what they want to do), without + // also influencing e.g. startup scripts. + // + // However, this is just an assumption. If it is wrong users can still envOverride the global + // JVM args. + // + // Please feel absolutely free to change this behavior! + jvm_args.join(" ") +} + +/// JVM arguments that specifically for the role (server), so will *not* be used e.g. by CLI tools +fn construct_role_specific_jvm_args( + hbase_role: &HbaseRole, + role: &Role, + role_group: &str, + product_version: &str, + kerberos_enabled: bool, +) -> Result, Error> { + let mut jvm_args = vec![format!( + "-Djava.security.properties={CONFIG_DIR_NAME}/{JVM_SECURITY_PROPERTIES_FILE}" + )]; + + // Starting with HBase 2.6 the JVM exporter is not needed anymore + if product_version.starts_with("2.4") || product_version.starts_with("2.5") { + jvm_args.push( + format!("-javaagent:/stackable/jmx/jmx_prometheus_javaagent.jar={METRICS_PORT}:/stackable/jmx/{hbase_role}.yaml") + ); + } + if kerberos_enabled { + jvm_args.push("-Djava.security.krb5.conf=/stackable/kerberos/krb5.conf".to_owned()); + } + + let operator_generated = JvmArgumentOverrides::new_with_only_additions(jvm_args); + let merged = role + .get_merged_jvm_argument_overrides(role_group, &operator_generated) + .context(MergeJvmArgumentOverridesSnafu)?; + Ok(merged + .effective_jvm_config_after_merging() + // Sorry for the clone, that's how operator-rs is currently modelled :P + .clone()) +} + +/// Arguments that go into `HBASE_OPTS`, so *not* the heap settings (which go into `HBASE_HEAPSIZE`). +pub fn construct_role_specific_non_heap_jvm_args( + hbase_role: &HbaseRole, + role: &Role, + role_group: &str, + product_version: &str, + kerberos_enabled: bool, +) -> Result { + let mut jvm_args = construct_role_specific_jvm_args( + hbase_role, + role, + role_group, + product_version, + kerberos_enabled, + )?; + jvm_args.retain(|arg| !is_heap_jvm_argument(arg)); + + Ok(jvm_args.join(" ")) +} + +/// This will be put into `HBASE_HEAPSIZE`, which is just the heap size in megabytes (with the `m` +/// unit prepended). +/// +/// The `bin/hbase` script will use this to set the needed JVM arguments. +/// Looking at `bin/hbase`, you can actually add the `m` suffix to make the unit more clear, the +/// script will detect this [here](https://github.com/apache/hbase/blob/777010361abb203b8b17673d84acf4f7f1d0283a/bin/hbase#L165) +/// and work correctly. +pub fn construct_hbase_heapsize_env(merged_config: &HbaseConfig) -> Result { + let heap_size = MemoryQuantity::try_from( + merged_config + .resources + .memory + .limit + .as_ref() + .context(MissingMemoryResourceConfigSnafu)?, + ) + .context(InvalidMemoryConfigSnafu)? + .scale_to(BinaryMultiple::Mebi) + * JAVA_HEAP_FACTOR; + + heap_size + .format_for_java() + .context(InvalidMemoryConfigSnafu) +} + +fn is_heap_jvm_argument(jvm_argument: &str) -> bool { + let lowercase = jvm_argument.to_lowercase(); + + lowercase.starts_with("-xms") || lowercase.starts_with("-xmx") +} + +#[cfg(test)] +mod tests { + use stackable_hbase_crd::{HbaseCluster, HbaseRole}; + + use super::*; + + #[test] + fn test_construct_jvm_arguments_defaults() { + let input = r#" + apiVersion: hbase.stackable.tech/v1alpha1 + kind: HbaseCluster + metadata: + name: simple-hbase + spec: + image: + productVersion: 2.6.1 + clusterConfig: + hdfsConfigMapName: simple-hdfs + zookeeperConfigMapName: simple-znode + masters: + roleGroups: + default: + replicas: 1 + regionServers: + roleGroups: + default: + replicas: 1 + "#; + let (hbase_role, merged_config, role, role_group, product_version) = + construct_boilerplate(input); + let kerberos_enabled = false; + + let global_jvm_args = construct_global_jvm_args(kerberos_enabled); + let role_specific_non_heap_jvm_args = construct_role_specific_non_heap_jvm_args( + &hbase_role, + &role, + &role_group, + &product_version, + kerberos_enabled, + ) + .unwrap(); + let hbase_heapsize_env = construct_hbase_heapsize_env(&merged_config).unwrap(); + + assert_eq!(global_jvm_args, ""); + assert_eq!( + role_specific_non_heap_jvm_args, + "-Djava.security.properties=/stackable/conf/security.properties" + ); + assert_eq!(hbase_heapsize_env, "819m"); + } + + #[test] + fn test_construct_jvm_argument_overrides() { + let input = r#" + apiVersion: hbase.stackable.tech/v1alpha1 + kind: HbaseCluster + metadata: + name: simple-hbase + spec: + image: + productVersion: 2.4.18 + clusterConfig: + hdfsConfigMapName: simple-hdfs + zookeeperConfigMapName: simple-znode + masters: + roleGroups: + default: + replicas: 1 + regionServers: + config: + resources: + memory: + limit: 42Gi + jvmArgumentOverrides: + add: + - -Dhttps.proxyHost=proxy.my.corp + - -Dhttps.proxyPort=8080 + - -Djava.net.preferIPv4Stack=true + roleGroups: + default: + replicas: 1 + jvmArgumentOverrides: + removeRegex: + - -Dhttps.proxyPort=.* + add: + - -Xmx40000m # This has no effect! + - -Dhttps.proxyPort=1234 + "#; + let (hbase_role, merged_config, role, role_group, product_version) = + construct_boilerplate(input); + let kerberos_enabled = true; + + let global_jvm_args = construct_global_jvm_args(kerberos_enabled); + let role_specific_non_heap_jvm_args = construct_role_specific_non_heap_jvm_args( + &hbase_role, + &role, + &role_group, + &product_version, + kerberos_enabled, + ) + .unwrap(); + let hbase_heapsize_env = construct_hbase_heapsize_env(&merged_config).unwrap(); + + assert_eq!( + global_jvm_args, + "-Djava.security.krb5.conf=/stackable/kerberos/krb5.conf" + ); + assert_eq!( + role_specific_non_heap_jvm_args, + "-Djava.security.properties=/stackable/conf/security.properties \ + -javaagent:/stackable/jmx/jmx_prometheus_javaagent.jar=9100:/stackable/jmx/regionserver.yaml \ + -Djava.security.krb5.conf=/stackable/kerberos/krb5.conf \ + -Dhttps.proxyHost=proxy.my.corp \ + -Djava.net.preferIPv4Stack=true \ + -Dhttps.proxyPort=1234" + ); + assert_eq!(hbase_heapsize_env, "34406m"); + } + + fn construct_boilerplate( + hbase_cluster: &str, + ) -> ( + HbaseRole, + HbaseConfig, + Role, + String, + String, + ) { + let hbase: HbaseCluster = serde_yaml::from_str(hbase_cluster).expect("illegal test input"); + + let hbase_role = HbaseRole::RegionServer; + let merged_config = hbase + .merged_config(&hbase_role, "default", "my-hdfs") + .unwrap(); + let role: Role = + hbase.spec.region_servers.unwrap(); + let product_version = hbase.spec.image.product_version().to_owned(); + + ( + hbase_role, + merged_config, + role, + "default".to_owned(), + product_version, + ) + } +} diff --git a/rust/operator-binary/src/config/mod.rs b/rust/operator-binary/src/config/mod.rs new file mode 100644 index 00000000..271c6d99 --- /dev/null +++ b/rust/operator-binary/src/config/mod.rs @@ -0,0 +1 @@ +pub mod jvm; diff --git a/rust/operator-binary/src/hbase_controller.rs b/rust/operator-binary/src/hbase_controller.rs index 454eb7d4..3c9d8916 100644 --- a/rust/operator-binary/src/hbase_controller.rs +++ b/rust/operator-binary/src/hbase_controller.rs @@ -56,7 +56,7 @@ use stackable_operator::{ CustomContainerLogConfig, }, }, - role_utils::{GenericRoleConfig, Role, RoleGroupRef}, + role_utils::{GenericRoleConfig, JavaCommonConfig, Role, RoleGroupRef}, status::condition::{ compute_conditions, operations::ClusterOperationsConditionBuilder, statefulset::StatefulSetConditionBuilder, @@ -68,14 +68,16 @@ use strum::{EnumDiscriminants, IntoStaticStr, ParseError}; use stackable_hbase_crd::{ merged_env, Container, HbaseCluster, HbaseClusterStatus, HbaseConfig, HbaseConfigFragment, - HbaseRole, APP_NAME, CONFIG_DIR_NAME, HBASE_ENV_SH, HBASE_HEAPSIZE, HBASE_MANAGES_ZK, - HBASE_MASTER_OPTS, HBASE_REGIONSERVER_OPTS, HBASE_REST_OPTS, HBASE_REST_PORT_NAME_HTTP, - HBASE_REST_PORT_NAME_HTTPS, HBASE_SITE_XML, JVM_HEAP_FACTOR, JVM_SECURITY_PROPERTIES_FILE, - METRICS_PORT, SSL_CLIENT_XML, SSL_SERVER_XML, + HbaseRole, APP_NAME, CONFIG_DIR_NAME, HBASE_ENV_SH, HBASE_REST_PORT_NAME_HTTP, + HBASE_REST_PORT_NAME_HTTPS, HBASE_SITE_XML, JVM_SECURITY_PROPERTIES_FILE, SSL_CLIENT_XML, + SSL_SERVER_XML, }; -use crate::product_logging::STACKABLE_LOG_DIR; -use crate::security::opa::HbaseOpaConfig; +use crate::{config::jvm::construct_hbase_heapsize_env, product_logging::STACKABLE_LOG_DIR}; +use crate::{ + config::jvm::{construct_global_jvm_args, construct_role_specific_non_heap_jvm_args}, + security::opa::HbaseOpaConfig, +}; use crate::{ discovery::build_discovery_configmap, kerberos::{ @@ -127,6 +129,9 @@ pub enum Error { #[snafu(display("object defines no master role"))] NoMasterRole, + #[snafu(display("the HBase role [{role}] is missing from spec"))] + MissingHbaseRole { role: String }, + #[snafu(display("object defines no regionserver role"))] NoRegionServerRole, @@ -232,15 +237,6 @@ pub enum Error { #[snafu(display("failed to resolve and merge config for role and role group"))] FailedToResolveConfig { source: stackable_hbase_crd::Error }, - #[snafu(display("invalid java heap config - missing default or value in crd?"))] - InvalidJavaHeapConfig, - - #[snafu(display("failed to convert java heap config to unit [{unit}]"))] - FailedToConvertJavaHeap { - source: stackable_operator::memory::Error, - unit: String, - }, - #[snafu(display("failed to resolve the Vector aggregator address"))] ResolveVectorAggregatorAddress { source: crate::product_logging::Error, @@ -316,6 +312,12 @@ pub enum Error { InvalidHBaseCluster { source: error_boundary::InvalidObject, }, + + #[snafu(display("failed to construct HBASE_HEAPSIZE env variable"))] + ConstructHbaseHeapsizeEnv { source: crate::config::jvm::Error }, + + #[snafu(display("failed to construct JVM arguments"))] + ConstructJvmArgument { source: crate::config::jvm::Error }, } type Result = std::result::Result; @@ -427,6 +429,11 @@ pub async fn reconcile_hbase( role: role_name.to_string(), })?; for (rolegroup_name, rolegroup_config) in group_config.iter() { + let role = hbase + .get_role(&hbase_role) + .with_context(|| MissingHbaseRoleSnafu { + role: hbase_role.to_string(), + })?; let rolegroup = hbase.server_rolegroup_ref(role_name, rolegroup_name); let merged_config = hbase @@ -442,6 +449,7 @@ pub async fn reconcile_hbase( let rg_configmap = build_rolegroup_config_map( hbase, &client.kubernetes_cluster_info, + role, &rolegroup, rolegroup_config, &zookeeper_connection_information, @@ -569,10 +577,11 @@ pub fn build_region_server_role_service( fn build_rolegroup_config_map( hbase: &HbaseCluster, cluster_info: &KubernetesClusterInfo, + role: &Role, rolegroup: &RoleGroupRef, rolegroup_config: &HashMap>, zookeeper_connection_information: &ZookeeperConnectionInformation, - hbase_config: &HbaseConfig, + merged_config: &HbaseConfig, resolved_product_image: &ResolvedProductImage, hbase_opa_config: Option<&HbaseOpaConfig>, vector_aggregator_address: Option<&str>, @@ -582,7 +591,7 @@ fn build_rolegroup_config_map( let mut ssl_server_xml = String::new(); let mut ssl_client_xml = String::new(); - let role = + let hbase_role = HbaseRole::from_str(rolegroup.role.as_ref()).with_context(|_| UnknownHbaseRoleSnafu { role: rolegroup.role.clone(), })?; @@ -611,9 +620,12 @@ fn build_rolegroup_config_map( } PropertyNameKind::File(file_name) if file_name == HBASE_ENV_SH => { let mut hbase_env_config = build_hbase_env_sh( - hbase_config, - &role, - resolved_product_image.product_version.as_ref(), + hbase, + merged_config, + &hbase_role, + role, + &rolegroup.role_group, + &resolved_product_image.product_version, )?; // configOverride come last @@ -701,7 +713,7 @@ fn build_rolegroup_config_map( extend_role_group_config_map( rolegroup, vector_aggregator_address, - &hbase_config.logging, + &merged_config.logging, &mut builder, &resolved_product_image.product_version, ) @@ -1083,7 +1095,15 @@ fn build_rolegroup_statefulset( #[allow(clippy::type_complexity)] fn build_roles( hbase: &HbaseCluster, -) -> Result, Role)>> { +) -> Result< + HashMap< + String, + ( + Vec, + Role, + ), + >, +> { let config_types = vec![ PropertyNameKind::Env, PropertyNameKind::File(HBASE_ENV_SH.to_string()), @@ -1167,72 +1187,57 @@ pub fn build_recommended_labels<'a>( /// The content of the HBase `hbase-env.sh` file. fn build_hbase_env_sh( - hbase_config: &HbaseConfig, - role: &HbaseRole, - hbase_version: &str, + hbase: &HbaseCluster, + merged_config: &HbaseConfig, + hbase_role: &HbaseRole, + role: &Role, + role_group: &str, + product_version: &str, ) -> Result, Error> { let mut result = BTreeMap::new(); - result.insert(HBASE_MANAGES_ZK.to_string(), "false".to_string()); - - // We always enable `-Djava.security.krb5.conf` even if it's not used. - let all_hbase_opts = [ - format!("-Djava.security.properties={CONFIG_DIR_NAME}/{JVM_SECURITY_PROPERTIES_FILE}"), - String::from("-Djava.security.krb5.conf=/stackable/kerberos/krb5.conf"), - ] - .iter() - .chain(jmx_system_properties(role, hbase_version).as_slice()) // Add the JMX options - .chain(hbase_config.hbase_opts.as_slice()) // Add the user defined options - .cloned() - .collect::>() - .join(" "); - - match role { + result.insert("HBASE_MANAGES_ZK".to_string(), "false".to_string()); + + result.insert( + "HBASE_HEAPSIZE".to_owned(), + construct_hbase_heapsize_env(merged_config).context(ConstructHbaseHeapsizeEnvSnafu)?, + ); + result.insert( + "HBASE_OPTS".to_owned(), + construct_global_jvm_args(hbase.has_kerberos_enabled()), + ); + let role_specific_non_heap_jvm_args = construct_role_specific_non_heap_jvm_args( + hbase_role, + role, + role_group, + product_version, + hbase.has_kerberos_enabled(), + ) + .context(ConstructJvmArgumentSnafu)?; + match hbase_role { HbaseRole::Master => { - result.insert(HBASE_MASTER_OPTS.to_string(), all_hbase_opts); + result.insert( + "HBASE_MASTER_OPTS".to_string(), + role_specific_non_heap_jvm_args, + ); } HbaseRole::RegionServer => { - result.insert(HBASE_REGIONSERVER_OPTS.to_string(), all_hbase_opts); + result.insert( + "HBASE_REGIONSERVER_OPTS".to_string(), + role_specific_non_heap_jvm_args, + ); } HbaseRole::RestServer => { - result.insert(HBASE_REST_OPTS.to_string(), all_hbase_opts); + result.insert( + "HBASE_REST_OPTS".to_string(), + role_specific_non_heap_jvm_args, + ); } } - let memory_limit = MemoryQuantity::try_from( - hbase_config - .resources - .memory - .limit - .as_ref() - .context(InvalidJavaHeapConfigSnafu)?, - ) - .context(FailedToConvertJavaHeapSnafu { - unit: BinaryMultiple::Mebi.to_java_memory_unit(), - })?; - let heap_in_mebi = (memory_limit * JVM_HEAP_FACTOR) - .scale_to(BinaryMultiple::Mebi) - .format_for_java() - .context(FailedToConvertJavaHeapSnafu { - unit: BinaryMultiple::Mebi.to_java_memory_unit(), - })?; - result.insert(HBASE_HEAPSIZE.to_string(), heap_in_mebi); - Ok(result) } -/// Return the JVM system properties for the JMX exporter. -/// Starting with HBase 2.6 these are not needed anymore -fn jmx_system_properties(role: &HbaseRole, hbase_version: &str) -> Option { - if hbase_version.starts_with(r"2.4") { - let role_name = role.to_string(); - - Some(format!("-javaagent:/stackable/jmx/jmx_prometheus_javaagent.jar={METRICS_PORT}:/stackable/jmx/{role_name}.yaml")) - } else { - None - } -} - /// Ensures that no authorization is configured for HBase versions that do not support it. /// In the future, such validations should be moved to the CRD CEL rules which are much more flexible /// and have to added benefit that invalid CRs are rejected by the API server. diff --git a/rust/operator-binary/src/kerberos.rs b/rust/operator-binary/src/kerberos.rs index f6a293dc..e19aa33a 100644 --- a/rust/operator-binary/src/kerberos.rs +++ b/rust/operator-binary/src/kerberos.rs @@ -255,11 +255,6 @@ pub fn add_kerberos_pod_config( // Needed env vars cb.add_env_var("KRB5_CONFIG", "/stackable/kerberos/krb5.conf"); - // This env var does not only affect the servers, but also the hbase shell - cb.add_env_var( - "HBASE_OPTS", - "-Djava.security.krb5.conf=/stackable/kerberos/krb5.conf", - ); } if let Some(https_secret_class) = hbase.https_secret_class() { diff --git a/rust/operator-binary/src/main.rs b/rust/operator-binary/src/main.rs index 673670aa..71bd8c33 100644 --- a/rust/operator-binary/src/main.rs +++ b/rust/operator-binary/src/main.rs @@ -1,11 +1,3 @@ -mod discovery; -mod hbase_controller; -mod kerberos; -mod operations; -mod product_logging; -mod security; -mod zookeeper; - use clap::Parser; use futures::StreamExt; use hbase_controller::FULL_HBASE_CONTROLLER_NAME; @@ -25,6 +17,15 @@ use stackable_operator::{ }; use std::sync::Arc; +mod config; +mod discovery; +mod hbase_controller; +mod kerberos; +mod operations; +mod product_logging; +mod security; +mod zookeeper; + mod built_info { include!(concat!(env!("OUT_DIR"), "/built.rs")); } diff --git a/tests/templates/kuttl/omid/40-install-omid.yaml.j2 b/tests/templates/kuttl/omid/40-install-omid.yaml.j2 index 7b6d7a1d..123169d4 100644 --- a/tests/templates/kuttl/omid/40-install-omid.yaml.j2 +++ b/tests/templates/kuttl/omid/40-install-omid.yaml.j2 @@ -80,7 +80,7 @@ spec: - name: HADOOP_CONF_DIR value: /stackable/conf/hdfs - name: JVM_FLAGS - value: "-Xmx3g -Xms3g" + value: "-Xms3g -Xmx3g" ports: # See also hbase-omid-client-config.yml above where the client is configured with this port - containerPort: 24758