fix: Fix upgrade path 3.3.x -> 4.0.x

CHANGELOG.md

@@ -21,12 +21,14 @@ All notable changes to this project will be documented in this file.
 
 - BREAKING: The fields `connection` and `host` on `S3Connection` as well as `bucketName` on `S3Bucket`are now mandatory ([#518]).
 - An invalid `HiveCluster` doesn't cause the operator to stop functioning ([#523]).
+- Fix upgrade path from HMS `3.3.x` to `4.0.x`. Previously the schemaTool would try to re-create the database tables and would therefore fail. Starting with version `4.0.0` the schemaTool has the flag `-initOrUpgradeSchema`, which we use to resolve that problem ([#539]).
 
 [#505]: https://github.com/stackabletech/hive-operator/pull/505
 [#508]: https://github.com/stackabletech/hive-operator/pull/508
 [#518]: https://github.com/stackabletech/hive-operator/pull/518
 [#522]: https://github.com/stackabletech/hive-operator/pull/522
 [#523]: https://github.com/stackabletech/hive-operator/pull/523
+[#539]: https://github.com/stackabletech/hive-operator/pull/539
 
 ## [24.7.0] - 2024-07-24
 

rust/crd/src/lib.rs

@@ -332,7 +332,6 @@ impl MetaStoreConfig {
     pub const CONNECTION_PASSWORD: &'static str = "javax.jdo.option.ConnectionPassword";
     pub const METASTORE_METRICS_ENABLED: &'static str = "hive.metastore.metrics.enabled";
     pub const METASTORE_WAREHOUSE_DIR: &'static str = "hive.metastore.warehouse.dir";
-    pub const DB_TYPE_CLI: &'static str = "dbType";
     // S3
     pub const S3_ENDPOINT: &'static str = "fs.s3a.endpoint";
     pub const S3_ACCESS_KEY: &'static str = "fs.s3a.access.key";
@@ -406,12 +405,6 @@ pub enum DbType {
     Mssql,
 }
 
-impl Default for DbType {
-    fn default() -> Self {
-        Self::Derby
-    }
-}
-
 impl DbType {
     pub fn get_jdbc_driver_class(&self) -> &str {
         match self {
@@ -425,7 +418,7 @@ impl DbType {
 }
 
 /// Database connection specification for the metadata database.
-#[derive(Clone, Debug, Default, Deserialize, Eq, Hash, JsonSchema, PartialEq, Serialize)]
+#[derive(Clone, Debug, Deserialize, Eq, Hash, JsonSchema, PartialEq, Serialize)]
 #[serde(rename_all = "camelCase")]
 pub struct DatabaseConnectionSpec {
     /// A connection string for the database. For example:
@@ -469,15 +462,10 @@ impl Configuration for MetaStoreConfigFragment {
 
     fn compute_cli(
         &self,
-        hive: &Self::Configurable,
+        _hive: &Self::Configurable,
         _role_name: &str,
     ) -> Result<BTreeMap<String, Option<String>>, product_config_utils::Error> {
-        let mut result = BTreeMap::new();
-        result.insert(
-            MetaStoreConfig::DB_TYPE_CLI.to_string(),
-            Some(hive.spec.cluster_config.database.db_type.to_string()),
-        );
-        Ok(result)
+        Ok(BTreeMap::new())
     }
 
     fn compute_files(
@@ -511,14 +499,7 @@ impl Configuration for MetaStoreConfigFragment {
                 );
                 result.insert(
                     MetaStoreConfig::CONNECTION_DRIVER_NAME.to_string(),
-                    Some(
-                        hive.spec
-                            .cluster_config
-                            .database
-                            .db_type
-                            .get_jdbc_driver_class()
-                            .to_string(),
-                    ),
+                    Some(hive.db_type().get_jdbc_driver_class().to_string()),
                 );
 
                 result.insert(
@@ -657,6 +638,10 @@ impl HiveCluster {
             .map(|k| k.secret_class.clone())
     }
 
+    pub fn db_type(&self) -> &DbType {
+        &self.spec.cluster_config.database.db_type
+    }
+
     /// Retrieve and merge resource configs for role and role groups
     pub fn merged_config(
         &self,

rust/operator-binary/src/controller.rs

@@ -3,7 +3,6 @@ use std::{
     borrow::Cow,
     collections::{BTreeMap, HashMap},
     hash::Hasher,
-    str::FromStr,
     sync::Arc,
 };
 
@@ -16,10 +15,10 @@ use product_config::{
 };
 use snafu::{OptionExt, ResultExt, Snafu};
 use stackable_hive_crd::{
-    Container, DbType, HiveCluster, HiveClusterStatus, HiveRole, MetaStoreConfig, APP_NAME,
-    CORE_SITE_XML, DB_PASSWORD_ENV, DB_USERNAME_ENV, HADOOP_HEAPSIZE, HIVE_ENV_SH, HIVE_PORT,
-    HIVE_PORT_NAME, HIVE_SITE_XML, JVM_HEAP_FACTOR, JVM_SECURITY_PROPERTIES_FILE, METRICS_PORT,
-    METRICS_PORT_NAME, STACKABLE_CONFIG_DIR, STACKABLE_CONFIG_DIR_NAME, STACKABLE_CONFIG_MOUNT_DIR,
+    Container, HiveCluster, HiveClusterStatus, HiveRole, MetaStoreConfig, APP_NAME, CORE_SITE_XML,
+    DB_PASSWORD_ENV, DB_USERNAME_ENV, HADOOP_HEAPSIZE, HIVE_ENV_SH, HIVE_PORT, HIVE_PORT_NAME,
+    HIVE_SITE_XML, JVM_HEAP_FACTOR, JVM_SECURITY_PROPERTIES_FILE, METRICS_PORT, METRICS_PORT_NAME,
+    STACKABLE_CONFIG_DIR, STACKABLE_CONFIG_DIR_NAME, STACKABLE_CONFIG_MOUNT_DIR,
     STACKABLE_CONFIG_MOUNT_DIR_NAME, STACKABLE_LOG_CONFIG_MOUNT_DIR,
     STACKABLE_LOG_CONFIG_MOUNT_DIR_NAME, STACKABLE_LOG_DIR, STACKABLE_LOG_DIR_NAME,
 };
@@ -188,12 +187,6 @@ pub enum Error {
         source: stackable_operator::client::Error,
     },
 
-    #[snafu(display("failed to parse db type {db_type}"))]
-    InvalidDbType {
-        source: strum::ParseError,
-        db_type: String,
-    },
-
     #[snafu(display("failed to configure S3 connection"))]
     ConfigureS3 { source: S3Error },
 
@@ -828,36 +821,25 @@ fn build_metastore_rolegroup_statefulset(
         .rolegroup(rolegroup_ref)
         .context(InternalOperatorSnafu)?;
 
-    let mut db_type: Option<DbType> = None;
     let mut container_builder =
         ContainerBuilder::new(APP_NAME).context(FailedToCreateHiveContainerSnafu {
             name: APP_NAME.to_string(),
         })?;
 
     for (property_name_kind, config) in metastore_config {
-        match property_name_kind {
-            PropertyNameKind::Env => {
-                // overrides
-                for (property_name, property_value) in config {
-                    if property_name.is_empty() {
-                        warn!("Received empty property_name for ENV... skipping");
-                        continue;
-                    }
-                    container_builder.add_env_var(property_name, property_value);
-                }
-            }
-            PropertyNameKind::Cli => {
-                for (property_name, property_value) in config {
-                    if property_name == MetaStoreConfig::DB_TYPE_CLI {
-                        db_type = Some(DbType::from_str(property_value).with_context(|_| {
-                            InvalidDbTypeSnafu {
-                                db_type: property_value.to_string(),
-                            }
-                        })?);
-                    }
+        if property_name_kind == &PropertyNameKind::Env {
+            // overrides
+            for (property_name, property_value) in config {
+                if property_name.is_empty() {
+                    warn!(
+                        property_name,
+                        property_value,
+                        "The env variable had an empty name, not adding it to the container"
+                    );
+                    continue;
                 }
+                container_builder.add_env_var(property_name, property_value);
             }
-            _ => {}
         }
     }
 
@@ -894,6 +876,26 @@ fn build_metastore_rolegroup_statefulset(
         }
     }
 
+    let db_type = hive.db_type();
+    let start_command = if resolved_product_image.product_version.starts_with("3.") {
+        // The schematool version in 3.1.x does *not* support the `-initOrUpgradeSchema` flag yet, so we can not use that.
+        // As we *only* support HMS 3.1.x (or newer) since SDP release 23.11, we can safely assume we are always coming
+        // from an existing 3.1.x installation. There is no need to upgrade the schema, we can just check if the schema
+        // is already there and create it if it isn't.
+        // The script `bin/start-metastore` is buggy (e.g. around version upgrades), but it's sufficient for that job :)
+        //
+        // TODO: Once we drop support for HMS 3.1.x we can remove this condition and very likely get rid of the
+        // "bin/start-metastore" script.
+        format!("bin/start-metastore --config {STACKABLE_CONFIG_DIR} --db-type {db_type} --hive-bin-dir bin &")
+    } else {
+        // schematool versions 4.0.x (and above) support the `-initOrUpgradeSchema`, which is exactly what we need :)
+        // Some docs for the schemaTool can be found here: https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=34835119
+        formatdoc! {"
+            bin/base --config \"{STACKABLE_CONFIG_DIR}\" --service schemaTool -dbType \"{db_type}\" -initOrUpgradeSchema
+            bin/base --config \"{STACKABLE_CONFIG_DIR}\" --service metastore &
+        "}
+    };
+
     let container_builder = container_builder
         .image_from_product_image(resolved_product_image)
         .command(vec![
@@ -905,23 +907,22 @@ fn build_metastore_rolegroup_statefulset(
         ])
         .args(build_container_command_args(
             hive,
-              formatdoc! {"
+            formatdoc! {"
             {kerberos_container_start_commands}
 
             {COMMON_BASH_TRAP_FUNCTIONS}
             {remove_vector_shutdown_file_command}
             prepare_signal_handlers
-            bin/start-metastore --config {STACKABLE_CONFIG_DIR} --db-type {db_type} --hive-bin-dir bin &
+            {start_command}
             wait_for_termination $!
             {create_vector_shutdown_file_command}
             ",
-            kerberos_container_start_commands = kerberos_container_start_commands(hive),
-            remove_vector_shutdown_file_command =
-                remove_vector_shutdown_file_command(STACKABLE_LOG_DIR),
-            create_vector_shutdown_file_command =
-                create_vector_shutdown_file_command(STACKABLE_LOG_DIR),
-                  db_type = &db_type.unwrap_or_default().to_string(),
-        },
+                kerberos_container_start_commands = kerberos_container_start_commands(hive),
+                remove_vector_shutdown_file_command =
+                    remove_vector_shutdown_file_command(STACKABLE_LOG_DIR),
+                create_vector_shutdown_file_command =
+                    create_vector_shutdown_file_command(STACKABLE_LOG_DIR),
+            },
             s3_connection,
         ))
         .add_volume_mount(STACKABLE_CONFIG_DIR_NAME, STACKABLE_CONFIG_DIR)