layout: make optional version mandatory in apply layout changes

This commit is contained in:
Alex Auvolat 2025-03-11 10:05:02 +01:00
parent 5f308bd688
commit 1f645830a4
3 changed files with 9 additions and 20 deletions

View file

@ -404,7 +404,7 @@ impl RequestHandler for PreviewClusterLayoutChangesRequest {
) -> Result<PreviewClusterLayoutChangesResponse, Error> {
let layout = garage.system.cluster_layout().inner().clone();
let new_ver = layout.current().version + 1;
match layout.apply_staged_changes(Some(new_ver)) {
match layout.apply_staged_changes(new_ver) {
Err(GarageError::Message(error)) => {
Ok(PreviewClusterLayoutChangesResponse::Error { error })
}
@ -426,7 +426,7 @@ impl RequestHandler for ApplyClusterLayoutRequest {
_admin: &Admin,
) -> Result<ApplyClusterLayoutResponse, Error> {
let layout = garage.system.cluster_layout().inner().clone();
let (layout, msg) = layout.apply_staged_changes(Some(self.version))?;
let (layout, msg) = layout.apply_staged_changes(self.version)?;
garage
.system

View file

@ -267,20 +267,9 @@ impl LayoutHistory {
changed
}
pub fn apply_staged_changes(mut self, version: Option<u64>) -> Result<(Self, Message), Error> {
match version {
None => {
let error = r#"
Please pass the new layout version number to ensure that you are writing the correct version of the cluster layout.
To know the correct value of the new layout version, invoke `garage layout show` and review the proposed changes.
"#;
return Err(Error::Message(error.into()));
}
Some(v) => {
if v != self.current().version + 1 {
return Err(Error::Message("Invalid new layout version".into()));
}
}
pub fn apply_staged_changes(mut self, version: u64) -> Result<(Self, Message), Error> {
if version != self.current().version + 1 {
return Err(Error::Message("Invalid new layout version".into()));
}
// Compute new version and add it to history

View file

@ -124,7 +124,7 @@ fn test_assignment() {
let mut cl = LayoutHistory::new(ReplicationFactor::new(3).unwrap());
update_layout(&mut cl, &node_capacity_vec, &node_zone_vec, 3);
let v = cl.current().version;
let (mut cl, msg) = cl.apply_staged_changes(Some(v + 1)).unwrap();
let (mut cl, msg) = cl.apply_staged_changes(v + 1).unwrap();
show_msg(&msg);
assert_eq!(cl.check(), Ok(()));
assert!(check_against_naive(cl.current()).unwrap());
@ -133,7 +133,7 @@ fn test_assignment() {
node_zone_vec = vec!["A", "B", "C", "C", "C", "B", "G", "H", "I"];
update_layout(&mut cl, &node_capacity_vec, &node_zone_vec, 2);
let v = cl.current().version;
let (mut cl, msg) = cl.apply_staged_changes(Some(v + 1)).unwrap();
let (mut cl, msg) = cl.apply_staged_changes(v + 1).unwrap();
show_msg(&msg);
assert_eq!(cl.check(), Ok(()));
assert!(check_against_naive(cl.current()).unwrap());
@ -141,7 +141,7 @@ fn test_assignment() {
node_capacity_vec = vec![4000, 1000, 2000, 7000, 1000, 1000, 2000, 10000, 2000];
update_layout(&mut cl, &node_capacity_vec, &node_zone_vec, 3);
let v = cl.current().version;
let (mut cl, msg) = cl.apply_staged_changes(Some(v + 1)).unwrap();
let (mut cl, msg) = cl.apply_staged_changes(v + 1).unwrap();
show_msg(&msg);
assert_eq!(cl.check(), Ok(()));
assert!(check_against_naive(cl.current()).unwrap());
@ -151,7 +151,7 @@ fn test_assignment() {
];
update_layout(&mut cl, &node_capacity_vec, &node_zone_vec, 1);
let v = cl.current().version;
let (cl, msg) = cl.apply_staged_changes(Some(v + 1)).unwrap();
let (cl, msg) = cl.apply_staged_changes(v + 1).unwrap();
show_msg(&msg);
assert_eq!(cl.check(), Ok(()));
assert!(check_against_naive(cl.current()).unwrap());