diff --git a/docs/operator-user-guide.md b/docs/operator-user-guide.md index 9b8af60..734d314 100644 --- a/docs/operator-user-guide.md +++ b/docs/operator-user-guide.md @@ -326,7 +326,7 @@ Key fields: | `name` | Pool name used in labels, StatefulSet names, and peer DNS. Must be unique in the Tenant. | | `servers` | Number of RustFS pods in the pool. Must be greater than `0`. Immutable after creation. | | `persistence.volumesPerServer` | Number of PVCs mounted into each server. Must be greater than `0`. Immutable after creation. | -| `persistence.volumeClaimTemplate` | PVC spec used for each generated volume. Set storage size, access modes, and StorageClass here. | +| `persistence.volumeClaimTemplate` | PVC spec used for each generated volume. Set storage size, access modes, and StorageClass at creation; these fields are immutable afterward. | | `persistence.path` | Base mount path. Defaults to `/data`; mounted paths become `{path}/rustfs0`, `{path}/rustfs1`, and so on. | | `nodeSelector`, `affinity`, `tolerations`, `topologySpreadConstraints` | Pool-level scheduling controls. | | `resources` | Container resource requests and limits for the pool. | @@ -1001,7 +1001,7 @@ The operator reconciles StatefulSets and reports rollout status in Tenant condit ### Change Storage Capacity -PVC expansion depends on the StorageClass and Kubernetes environment. Do not change immutable pool shape fields (`servers` and `volumesPerServer`) in place. To add capacity, add a new pool when appropriate and follow RustFS decommission and migration procedures. +Do not change an existing pool's `volumeClaimTemplate` storage request, access modes, or StorageClass in place; the StatefulSet template is immutable. To add capacity, add a new pool when appropriate and follow RustFS decommission and migration procedures. ### Restart Tenant Pods diff --git a/docs/operator-user-guide.zh-CN.md b/docs/operator-user-guide.zh-CN.md index 0330513..9902395 100644 --- a/docs/operator-user-guide.zh-CN.md +++ b/docs/operator-user-guide.zh-CN.md @@ -317,7 +317,7 @@ Tenant 名称必须兼容 DNS-1035，且长度不超过 55 个字符，因为 Op | `name` | Pool 名称，用于 label、StatefulSet 名称和 peer DNS。同一个 Tenant 内必须唯一。 | | `servers` | 该 pool 的 RustFS Pod 数量。必须大于 `0`。创建后不可变。 | | `persistence.volumesPerServer` | 每个 server 挂载的 PVC 数量。必须大于 `0`。创建后不可变。 | -| `persistence.volumeClaimTemplate` | 每个数据卷的 PVC spec，可设置容量、access mode 和 StorageClass。 | +| `persistence.volumeClaimTemplate` | 每个数据卷的 PVC spec，可在创建时设置容量、access mode 和 StorageClass；创建后这些字段不可变。 | | `persistence.path` | 数据卷挂载基础路径。默认 `/data`，最终路径为 `{path}/rustfs0`、`{path}/rustfs1` 等。 | | `nodeSelector`、`affinity`、`tolerations`、`topologySpreadConstraints` | Pool 级调度控制。 | | `resources` | Pool 容器资源 request 和 limit。 | @@ -977,7 +977,7 @@ Operator 会 reconcile StatefulSet，并通过 Tenant condition 和 pool status ### 修改存储容量 -PVC 扩容取决于 StorageClass 和 Kubernetes 环境。不要原地修改不可变的 pool 形态字段（`servers` 和 `volumesPerServer`）。需要扩容时，可按需新增 pool，并结合 RustFS decommission 和迁移流程操作。 +不要原地修改已有 pool 的 `volumeClaimTemplate` 存储请求、access mode 或 StorageClass；StatefulSet 模板不可变。需要扩容时，可按需新增 pool，并结合 RustFS decommission 和迁移流程操作。 ### 重启 Tenant Pod diff --git a/src/types/v1alpha1/tenant/workloads.rs b/src/types/v1alpha1/tenant/workloads.rs index 5b75d92..81df872 100755 --- a/src/types/v1alpha1/tenant/workloads.rs +++ b/src/types/v1alpha1/tenant/workloads.rs @@ -28,6 +28,7 @@ use crate::types::v1alpha1::tls::{TlsPlan, http_probe}; use k8s_openapi::DeepMerge; use k8s_openapi::api::apps::v1; use k8s_openapi::api::core::v1 as corev1; +use k8s_openapi::apimachinery::pkg::api::resource::Quantity; use k8s_openapi::apimachinery::pkg::apis::meta::v1 as metav1; const LOCAL_KMS_KEY_DIR_ENV: &str = "RUSTFS_KMS_KEY_DIR"; @@ -472,6 +473,146 @@ fn volume_claim_template_name(shard: i32) -> String { format!("{VOLUME_CLAIM_TEMPLATE_PREFIX}-{shard}") } +const MAX_QUANTITY_NANOUNITS: i128 = (i64::MAX as i128) * 1_000_000_000; + +// Kubernetes compares Quantity values as fixed-point numbers, rounds sub-nanounit +// precision away from zero, and caps magnitudes at i64::MAX. Keep the comparison +// exact instead of routing through floating point, which loses precision for large PVCs. +fn multiply_decimal_digits(digits: &str, multiplier: u64) -> Option { + let mut result = Vec::with_capacity(digits.len().saturating_add(19)); + let mut carry = 0_u128; + + for byte in digits.bytes().rev() { + let digit = byte.checked_sub(b'0')?; + if digit > 9 { + return None; + } + let product = u128::from(digit) * u128::from(multiplier) + carry; + result.push(b'0' + (product % 10) as u8); + carry = product / 10; + } + + while carry > 0 { + result.push(b'0' + (carry % 10) as u8); + carry /= 10; + } + + result.reverse(); + String::from_utf8(result).ok() +} + +fn scale_quantity_to_nanounits(digits: &str, exponent: i32) -> Option { + let max_digits = MAX_QUANTITY_NANOUNITS.to_string().len(); + + if exponent >= 0 { + let zero_count = usize::try_from(exponent).ok()?; + let scaled_len = digits.len().checked_add(zero_count)?; + if scaled_len > max_digits { + return Some(MAX_QUANTITY_NANOUNITS); + } + + let mut scaled = String::with_capacity(scaled_len); + scaled.push_str(digits); + scaled.extend(std::iter::repeat_n('0', zero_count)); + return scaled + .parse::() + .ok() + .map(|value| value.min(MAX_QUANTITY_NANOUNITS)); + } + + let divisor_digits = usize::try_from(exponent.checked_neg()?).ok()?; + let (whole, fraction) = if divisor_digits >= digits.len() { + ("0", digits) + } else { + digits.split_at(digits.len() - divisor_digits) + }; + + if whole.len() > max_digits { + return Some(MAX_QUANTITY_NANOUNITS); + } + + let mut rounded = whole.parse::().ok()?; + if fraction.bytes().any(|byte| byte != b'0') { + rounded = rounded.checked_add(1)?; + } + Some(rounded.min(MAX_QUANTITY_NANOUNITS)) +} + +fn parse_quantity_nanounits(quantity: &Quantity) -> Option { + let (negative, unsigned) = match quantity.0.as_str() { + value if value.starts_with('-') => (true, &value[1..]), + value if value.starts_with('+') => (false, &value[1..]), + value => (false, value), + }; + let number_end = unsigned + .find(|character: char| !character.is_ascii_digit() && character != '.') + .unwrap_or(unsigned.len()); + let (number, suffix) = unsigned.split_at(number_end); + let (whole, fraction) = match number.split_once('.') { + Some((whole, fraction)) if !fraction.contains('.') => (whole, fraction), + None => (number, ""), + _ => return None, + }; + if whole.is_empty() && fraction.is_empty() { + return None; + } + + let mut digits = String::with_capacity(whole.len().saturating_add(fraction.len())); + digits.push_str(whole); + digits.push_str(fraction); + if !digits.bytes().all(|byte| byte.is_ascii_digit()) { + return None; + } + let digits = digits.trim_start_matches('0'); + + let (multiplier, suffix_exponent) = match suffix { + "" => (1, 0), + "n" => (1, -9), + "u" => (1, -6), + "m" => (1, -3), + "k" => (1, 3), + "M" => (1, 6), + "G" => (1, 9), + "T" => (1, 12), + "P" => (1, 15), + "E" => (1, 18), + "Ki" => (1 << 10, 0), + "Mi" => (1 << 20, 0), + "Gi" => (1 << 30, 0), + "Ti" => (1 << 40, 0), + "Pi" => (1 << 50, 0), + "Ei" => (1 << 60, 0), + value if value.starts_with('e') || value.starts_with('E') => { + (1, value[1..].parse::().ok()?) + } + _ => return None, + }; + let fraction_digits = i32::try_from(fraction.len()).ok()?; + let exponent = suffix_exponent + .checked_add(9)? + .checked_sub(fraction_digits)?; + if digits.is_empty() { + return Some(0); + } + let digits = multiply_decimal_digits(digits, multiplier)?; + let magnitude = scale_quantity_to_nanounits(&digits, exponent)?; + + Some(if negative { -magnitude } else { magnitude }) +} + +fn quantities_semantically_equal(existing: Option<&Quantity>, desired: Option<&Quantity>) -> bool { + match (existing, desired) { + (Some(existing), Some(desired)) => match ( + parse_quantity_nanounits(existing), + parse_quantity_nanounits(desired), + ) { + (Some(existing), Some(desired)) => existing == desired, + _ => existing == desired, + }, + _ => existing == desired, + } +} + fn container_env_values<'a>(container: &'a corev1::Container, name: &str) -> Vec<&'a str> { container .env @@ -1712,9 +1853,7 @@ impl Tenant { }); } - // Validate volumeClaimTemplates are unchanged (immutable field) - // Note: This is a simplified check. In reality, you can only change certain fields - // like storage size (depending on storage class), but template structure and names cannot change. + // Validate volumeClaimTemplates are unchanged (immutable field). let existing_vcts = existing_spec.volume_claim_templates.as_ref(); let desired_vcts = desired_spec.volume_claim_templates.as_ref(); @@ -1752,15 +1891,60 @@ impl Tenant { }); } + let existing_vct_spec = existing_vct.spec.as_ref(); + let desired_vct_spec = desired_vct.spec.as_ref(); + let existing_storage = existing_vct_spec + .and_then(|spec| spec.resources.as_ref()) + .and_then(|resources| resources.requests.as_ref()) + .and_then(|requests| requests.get("storage")); + let desired_storage = desired_vct_spec + .and_then(|spec| spec.resources.as_ref()) + .and_then(|resources| resources.requests.as_ref()) + .and_then(|requests| requests.get("storage")); + + if !quantities_semantically_equal(existing_storage, desired_storage) { + return Err(types::error::Error::ImmutableFieldModified { + name: ss_name.clone(), + field: format!( + "spec.volumeClaimTemplates[{}].spec.resources.requests.storage", + i + ), + message: format!( + "Storage request changed from '{}' to '{}'. Restore the original value; add a new pool to expand capacity.", + existing_storage + .map(|quantity| quantity.0.as_str()) + .unwrap_or(""), + desired_storage + .map(|quantity| quantity.0.as_str()) + .unwrap_or("") + ), + }); + } + + let existing_access_modes = + existing_vct_spec.and_then(|spec| spec.access_modes.as_ref()); + let desired_access_modes = + desired_vct_spec.and_then(|spec| spec.access_modes.as_ref()); + + if existing_access_modes != desired_access_modes { + return Err(types::error::Error::ImmutableFieldModified { + name: ss_name.clone(), + field: format!("spec.volumeClaimTemplates[{}].spec.accessModes", i), + message: format!( + "Access modes changed from '{}' to '{}'. Restore the original value or add a new pool with the required access modes.", + existing_access_modes + .map(|modes| modes.join(", ")) + .unwrap_or_else(|| "".to_string()), + desired_access_modes + .map(|modes| modes.join(", ")) + .unwrap_or_else(|| "".to_string()) + ), + }); + } + // Check if storage class changed (also problematic) - let existing_sc = existing_vct - .spec - .as_ref() - .and_then(|s| s.storage_class_name.as_ref()); - let desired_sc = desired_vct - .spec - .as_ref() - .and_then(|s| s.storage_class_name.as_ref()); + let existing_sc = existing_vct_spec.and_then(|s| s.storage_class_name.as_ref()); + let desired_sc = desired_vct_spec.and_then(|s| s.storage_class_name.as_ref()); if existing_sc != desired_sc { return Err(types::error::Error::ImmutableFieldModified { @@ -1785,8 +1969,8 @@ mod tests { use super::{ DEFAULT_FS_GROUP, DEFAULT_RUN_AS_GROUP, DEFAULT_RUN_AS_USER, MAX_APP_ARMOR_LOCALHOST_PROFILE_LENGTH, RUNTIME_DEFAULT_IMAGE_ACK_ANNOTATION, - uses_unpartitioned_rolling_update, validate_declared_app_armor_profile, - validate_declared_seccomp_profile, + quantities_semantically_equal, uses_unpartitioned_rolling_update, + validate_declared_app_armor_profile, validate_declared_seccomp_profile, }; use crate::types::v1alpha1::encryption::{ EncryptionConfig, KmsBackendType, LocalKmsConfig, LocalKmsMasterKeySecretRef, @@ -1797,6 +1981,7 @@ mod tests { use crate::types::v1alpha1::tls::{SecretKeyReference, TlsPlan}; use k8s_openapi::api::apps::v1; use k8s_openapi::api::core::v1 as corev1; + use k8s_openapi::apimachinery::pkg::api::resource::Quantity; fn image_pull_secret(name: &str) -> corev1::LocalObjectReference { corev1::LocalObjectReference { @@ -1804,6 +1989,29 @@ mod tests { } } + fn volume_claim_template_spec( + storage: &str, + access_modes: &[&str], + ) -> corev1::PersistentVolumeClaimSpec { + let requests = std::collections::BTreeMap::from([( + "storage".to_string(), + k8s_openapi::apimachinery::pkg::api::resource::Quantity(storage.to_string()), + )]); + corev1::PersistentVolumeClaimSpec { + access_modes: Some( + access_modes + .iter() + .map(|mode| (*mode).to_string()) + .collect(), + ), + resources: Some(corev1::VolumeResourceRequirements { + requests: Some(requests), + ..Default::default() + }), + ..Default::default() + } + } + fn tls_plan(hash: &str) -> TlsPlan { TlsPlan::for_test("server-tls", hash) } @@ -4495,6 +4703,129 @@ mod tests { } } + #[test] + fn test_statefulset_storage_request_change_rejected() { + let mut tenant = crate::tests::create_test_tenant(None, None); + let pool = &tenant.spec.pools[0]; + let statefulset = tenant + .new_statefulset(pool) + .expect("Should create StatefulSet"); + + tenant.spec.pools[0].persistence.volume_claim_template = + Some(volume_claim_template_spec("20Gi", &["ReadWriteOnce"])); + let pool = &tenant.spec.pools[0]; + + let err = tenant + .validate_statefulset_update(&statefulset, pool) + .expect_err("Validation should reject storage request changes"); + + match err { + crate::types::error::Error::ImmutableFieldModified { field, message, .. } => { + assert_eq!( + field, + "spec.volumeClaimTemplates[0].spec.resources.requests.storage" + ); + assert!(message.contains("10Gi")); + assert!(message.contains("20Gi")); + assert!(message.contains("add a new pool")); + } + _ => panic!("Expected ImmutableFieldModified error"), + } + } + + #[test] + fn test_statefulset_semantically_equal_storage_requests_allowed() { + for (tenant_storage, persisted_storage) in [("1024Mi", "1Gi"), ("1.5Gi", "1536Mi")] { + let mut tenant = crate::tests::create_test_tenant(None, None); + tenant.spec.pools[0].persistence.volume_claim_template = Some( + volume_claim_template_spec(tenant_storage, &["ReadWriteOnce"]), + ); + let pool = &tenant.spec.pools[0]; + let mut statefulset = tenant + .new_statefulset(pool) + .expect("Should create StatefulSet"); + + statefulset + .spec + .as_mut() + .and_then(|spec| spec.volume_claim_templates.as_mut()) + .and_then(|templates| templates.first_mut()) + .and_then(|template| template.spec.as_mut()) + .and_then(|spec| spec.resources.as_mut()) + .and_then(|resources| resources.requests.as_mut()) + .and_then(|requests| requests.get_mut("storage")) + .expect("StatefulSet should contain a storage request") + .0 = persisted_storage.to_string(); + + tenant + .validate_statefulset_update(&statefulset, pool) + .expect("Semantically equal storage requests should be allowed"); + } + } + + #[test] + fn test_quantity_semantic_comparison_preserves_integer_precision() { + let existing = Quantity("9223372036854775806".to_string()); + let desired = Quantity("9223372036854775807".to_string()); + + assert!(!quantities_semantically_equal( + Some(&existing), + Some(&desired) + )); + } + + #[test] + fn test_quantity_semantic_comparison_handles_kubernetes_formats() { + for (existing, desired) in [ + ("1024Mi", "1Gi"), + ("1.5Gi", "1536Mi"), + ("1000M", "1G"), + ("1e3", "1k"), + (".5Gi", "512Mi"), + ("0.1n", "1n"), + ] { + assert!(quantities_semantically_equal( + Some(&Quantity(existing.to_string())), + Some(&Quantity(desired.to_string())) + )); + } + + assert!(!quantities_semantically_equal( + Some(&Quantity("1000Mi".to_string())), + Some(&Quantity("1Gi".to_string())) + )); + assert!(!quantities_semantically_equal( + Some(&Quantity("0invalid".to_string())), + Some(&Quantity("0".to_string())) + )); + } + + #[test] + fn test_statefulset_access_modes_change_rejected() { + let mut tenant = crate::tests::create_test_tenant(None, None); + let pool = &tenant.spec.pools[0]; + let statefulset = tenant + .new_statefulset(pool) + .expect("Should create StatefulSet"); + + tenant.spec.pools[0].persistence.volume_claim_template = + Some(volume_claim_template_spec("10Gi", &["ReadWriteMany"])); + let pool = &tenant.spec.pools[0]; + + let err = tenant + .validate_statefulset_update(&statefulset, pool) + .expect_err("Validation should reject access mode changes"); + + match err { + crate::types::error::Error::ImmutableFieldModified { field, message, .. } => { + assert_eq!(field, "spec.volumeClaimTemplates[0].spec.accessModes"); + assert!(message.contains("ReadWriteOnce")); + assert!(message.contains("ReadWriteMany")); + } + _ => panic!("Expected ImmutableFieldModified error"), + } + } + // Test: StatefulSet validation - safe update allowed #[test] fn test_statefulset_safe_update_allowed() {