Add guardrail against trying to crossgrade in the middle of another upgrade

Author: nightkr

rust/crd/src/lib.rs

@@ -804,21 +804,40 @@ impl HdfsCluster {
         Ok(result)
     }
 
-    pub fn upgrade_state(&self) -> Option<UpgradeState> {
-        let status = self.status.as_ref()?;
+    pub fn upgrade_state(&self) -> Result<Option<UpgradeState>, UpgradeStateError> {
+        use upgrade_state_error::*;
+        let Some(status) = self.status.as_ref() else {
+            return Ok(None);
+        };
         let requested_version = self.spec.image.product_version();
+        let Some(deployed_version) = status.deployed_product_version.as_deref() else {
+            // If no deployed version, fresh install -> no upgrade
+            return Ok(None);
+        };
+        let current_upgrade_target_version = status.upgrade_target_product_version.as_deref();
 
-        if requested_version != status.deployed_product_version.as_deref()? {
+        if requested_version != deployed_version {
             // If we're requesting a different version than what is deployed, assume that we're upgrading.
             // Could also be a downgrade to an older version, but we don't support downgrades after upgrade finalization.
-            Some(UpgradeState::Upgrading)
-        } else if requested_version != status.upgrade_target_product_version.as_deref()? {
+            match current_upgrade_target_version {
+                Some(upgrading_version) if requested_version != upgrading_version => {
+                    // If we're in an upgrade, do not allow switching to a third version
+                    InvalidCrossgradeSnafu {
+                        requested_version,
+                        deployed_version,
+                        upgrading_version,
+                    }
+                    .fail()
+                }
+                _ => Ok(Some(UpgradeState::Upgrading)),
+            }
+        } else if current_upgrade_target_version.is_some_and(|x| requested_version != x) {
             // If we're requesting the old version mid-upgrade, assume that we're downgrading.
             // We only support downgrading to the exact previous version.
-            Some(UpgradeState::Downgrading)
+            Ok(Some(UpgradeState::Downgrading))
         } else {
             // All three versions match, upgrade was completed without clearing `upgrading_product_version`.
-            None
+            Ok(None)
         }
     }
 
@@ -984,6 +1003,17 @@ pub enum UpgradeState {
     Downgrading,
 }
 
+#[derive(Debug, Snafu)]
+#[snafu(module)]
+pub enum UpgradeStateError {
+    #[snafu(display("requested version {requested_version:?} while still upgrading from {deployed_version:?} to {upgrading_version:?}, please finish the upgrade or downgrade first"))]
+    InvalidCrossgrade {
+        requested_version: String,
+        deployed_version: String,
+        upgrading_version: String,
+    },
+}
+
 #[derive(
     Clone,
     Debug,

rust/operator-binary/src/container.rs

@@ -549,7 +549,7 @@ impl ContainerConfig {
             args.push_str(&Self::export_kerberos_real_env_var_command());
         }
 
-        let upgrade_args = if hdfs.upgrade_state() == Some(UpgradeState::Upgrading)
+        let upgrade_args = if hdfs.upgrade_state().ok() == Some(Some(UpgradeState::Upgrading))
             && *role == HdfsRole::NameNode
         {
             "-rollingUpgrade started"

rust/operator-binary/src/hdfs_controller.rs

@@ -51,7 +51,8 @@ use stackable_operator::{
 use strum::{EnumDiscriminants, IntoEnumIterator, IntoStaticStr};
 
 use stackable_hdfs_crd::{
-    constants::*, AnyNodeConfig, HdfsCluster, HdfsClusterStatus, HdfsPodRef, HdfsRole, UpgradeState,
+    constants::*, AnyNodeConfig, HdfsCluster, HdfsClusterStatus, HdfsPodRef, HdfsRole,
+    UpgradeState, UpgradeStateError,
 };
 
 use crate::{
@@ -86,6 +87,9 @@ pub enum Error {
         source: stackable_operator::product_config_utils::Error,
     },
 
+    #[snafu(display("invalid upgrade state"))]
+    InvalidUpgradeState { source: UpgradeStateError },
+
     #[snafu(display("cannot create rolegroup service {name:?}"))]
     ApplyRoleGroupService {
         source: stackable_operator::cluster_resources::Error,
@@ -326,7 +330,7 @@ pub async fn reconcile_hdfs(hdfs: Arc<HdfsCluster>, ctx: Arc<Ctx>) -> HdfsOperat
     let dfs_replication = hdfs.spec.cluster_config.dfs_replication;
     let mut ss_cond_builder = StatefulSetConditionBuilder::default();
 
-    let upgrade_state = hdfs.upgrade_state();
+    let upgrade_state = hdfs.upgrade_state().context(InvalidUpgradeStateSnafu)?;
     let mut deploy_done = true;
 
     // Roles must be deployed in order during rolling upgrades,