Skip to content

Commit 96da51f

Browse files
committed
Unify is_valid_upgrade_version from Cloud
The plan is to use this public static function in the Cloud repo. Borrows most of the same logic, but allows upgrades from/to dev versions.
1 parent 13a8710 commit 96da51f

File tree

1 file changed

+87
-18
lines changed

1 file changed

+87
-18
lines changed

src/cloud-resources/src/crd/materialize.rs

Lines changed: 87 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,6 @@ use crate::crd::generated::cert_manager::certificates::{
3030
CertificateIssuerRef, CertificateSecretTemplate,
3131
};
3232

33-
const V26_0_0: Version = Version::new(26, 0, 0);
34-
3533
pub const LAST_KNOWN_ACTIVE_GENERATION_ANNOTATION: &str =
3634
"materialize.cloud/last-known-active-generation";
3735

@@ -481,14 +479,47 @@ pub mod v1alpha1 {
481479
}
482480
}
483481

484-
/// Checks if the current environmentd image ref is within the upgrade window of the last
485-
/// successful rollout.
486-
///
487482
/// This check isn't strictly required since environmentd will still be able to determine
488483
/// if the upgrade is allowed or not. However, doing this check allows us to provide
489484
/// the error as soon as possible and in a more user friendly way.
485+
pub fn is_valid_upgrade_version(active_version: &Version, next_version: &Version) -> bool {
486+
// Don't allow rolling back
487+
// Note: semver comparison handles RC versions correctly:
488+
// v26.0.0-rc.1 < v26.0.0-rc.2 < v26.0.0
489+
if next_version < active_version {
490+
return false;
491+
}
492+
493+
if active_version.major == 0 {
494+
// Self managed 25.2 to 26.0
495+
if next_version.major != active_version.major {
496+
if next_version.major == 26
497+
&& active_version.major == 0
498+
&& active_version.minor == 164
499+
{
500+
return true;
501+
} else {
502+
return false;
503+
}
504+
}
505+
// Self managed 25.1 to 25.2
506+
if next_version.minor == 147 && active_version.minor == 130 {
507+
return true;
508+
}
509+
// only allow upgrading a single minor version at a time
510+
return next_version.minor <= active_version.minor + 1;
511+
} else if active_version.major >= 26 {
512+
// For versions 26.X.X and onwards, we deny upgrades past 1 major version of the active version
513+
return next_version.major <= active_version.major + 1;
514+
}
515+
516+
true
517+
}
518+
519+
/// Checks if the current environmentd image ref is within the upgrade window of the last
520+
/// successful rollout.
490521
pub fn within_upgrade_window(&self) -> bool {
491-
let current_environmentd_version = self
522+
let active_environmentd_version = self
492523
.status
493524
.as_ref()
494525
.and_then(|status| {
@@ -498,19 +529,19 @@ pub mod v1alpha1 {
498529
})
499530
.and_then(|image_ref| parse_image_ref(image_ref));
500531

501-
if let (Some(new_environmentd_version), Some(current_environmentd_version)) = (
532+
if let (Some(next_environmentd_version), Some(active_environmentd_version)) = (
502533
parse_image_ref(&self.spec.environmentd_image_ref),
503-
current_environmentd_version,
534+
active_environmentd_version,
504535
) {
505-
if current_environmentd_version >= V26_0_0 {
506-
// We deny upgrades past 1 major version of the last successful rollout
507-
return new_environmentd_version.major
508-
<= current_environmentd_version.major + 1;
509-
}
536+
Self::is_valid_upgrade_version(
537+
&active_environmentd_version,
538+
&next_environmentd_version,
539+
)
540+
} else {
541+
// If we fail to parse either version,
542+
// we still allow the upgrade since environmentd will still error if the upgrade is not allowed.
543+
true
510544
}
511-
// If we fail any of the preconditions for the check (e.g. we couldn't parse either version),
512-
// we still allow the upgrade since environmentd will still error if the upgrade is not allowed.
513-
true
514545
}
515546

516547
pub fn managed_resource_meta(&self, name: String) -> ObjectMeta {
@@ -711,13 +742,51 @@ mod tests {
711742
"materialize/environmentd:v28.0.1.not_a_valid_version".to_owned();
712743
assert!(mz.within_upgrade_window());
713744

714-
// Pass: upgrading from 0.147.5 to 26.1.0 (any version before 26.0.0 passes)
745+
// Pass: upgrading from 0.164.0 to 26.1.0 (self managed 25.2 to 26.0)
715746
mz.status
716747
.as_mut()
717748
.unwrap()
718749
.last_completed_rollout_environmentd_image_ref =
719-
Some("materialize/environmentd:v0.147.5".to_owned());
750+
Some("materialize/environmentd:v0.164.0".to_owned());
720751
mz.spec.environmentd_image_ref = "materialize/environmentd:v26.1.0".to_owned();
721752
assert!(mz.within_upgrade_window());
722753
}
754+
755+
#[mz_ore::test]
756+
fn is_valid_upgrade_version() {
757+
let success_tests = [
758+
(Version::new(0, 83, 0), Version::new(0, 83, 0)),
759+
(Version::new(0, 83, 0), Version::new(0, 84, 0)),
760+
(Version::new(0, 9, 0), Version::new(0, 10, 0)),
761+
(Version::new(0, 99, 0), Version::new(0, 100, 0)),
762+
(Version::new(0, 83, 0), Version::new(0, 83, 1)),
763+
(Version::new(0, 83, 0), Version::new(0, 83, 2)),
764+
(Version::new(0, 83, 2), Version::new(0, 83, 10)),
765+
(Version::new(0, 164, 0), Version::new(26, 0, 0)),
766+
(Version::new(26, 0, 0), Version::new(26, 1, 0)),
767+
(Version::new(26, 5, 3), Version::new(26, 10, 0)),
768+
(Version::new(0, 130, 0), Version::new(0, 147, 0)),
769+
];
770+
for (active_version, next_version) in success_tests {
771+
assert!(
772+
Materialize::is_valid_upgrade_version(&active_version, &next_version),
773+
"v{active_version} can upgrade to v{next_version}"
774+
);
775+
}
776+
777+
let failure_tests = [
778+
(Version::new(0, 83, 0), Version::new(0, 82, 0)),
779+
(Version::new(0, 83, 3), Version::new(0, 83, 2)),
780+
(Version::new(0, 83, 3), Version::new(1, 83, 3)),
781+
(Version::new(0, 83, 0), Version::new(0, 85, 0)),
782+
(Version::new(26, 0, 0), Version::new(28, 0, 0)),
783+
(Version::new(0, 130, 0), Version::new(26, 1, 0)),
784+
];
785+
for (active_version, next_version) in failure_tests {
786+
assert!(
787+
!Materialize::is_valid_upgrade_version(&active_version, &next_version),
788+
"v{active_version} can't upgrade to v{next_version}"
789+
);
790+
}
791+
}
723792
}

0 commit comments

Comments
 (0)