Skip to content

[19/n] report mupdate overrides while fetching target release #8427

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions nexus/db-model/src/target_release.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,13 @@ impl TargetRelease {
pub fn into_external(
&self,
release_source: views::TargetReleaseSource,
mupdate_override: bool,
) -> views::TargetRelease {
views::TargetRelease {
generation: (&self.generation.0).into(),
time_requested: self.time_requested,
release_source,
mupdate_override,
}
}
}
39 changes: 39 additions & 0 deletions nexus/db-queries/src/db/datastore/deployment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1560,6 +1560,23 @@ impl DataStore {
Self::blueprint_current_target_only(&conn).await.map_err(|e| e.into())
}

/// Get the minimum generation for the current target blueprint, if one exists
pub async fn blueprint_target_get_current_min_gen(
&self,
opctx: &OpContext,
) -> Result<Generation, Error> {
opctx.authorize(authz::Action::Read, &authz::BLUEPRINT_CONFIG).await?;
let conn = self.pool_connection_authorized(opctx).await?;
let target = Self::blueprint_current_target_only(&conn).await?;

let authz_blueprint = authz_blueprint_from_id(target.target_id);
Self::blueprint_get_minimum_generation_connection(
&authz_blueprint,
&conn,
)
.await
}

// Helper to fetch the current blueprint target (without fetching the entire
// blueprint for that target).
//
Expand Down Expand Up @@ -1587,6 +1604,28 @@ impl DataStore {

Ok(current_target.into())
}

// Helper to fetch the minimum generation for a blueprint ID (without
// fetching the entire blueprint for that ID.)
async fn blueprint_get_minimum_generation_connection(
authz: &authz::Blueprint,
conn: &async_bb8_diesel::Connection<DbConnection>,
) -> Result<Generation, Error> {
use nexus_db_schema::schema::blueprint::dsl;

let id = authz.id();
let db_blueprint = dsl::blueprint
.filter(dsl::id.eq(id))
.select(DbBlueprint::as_select())
.first_async::<DbBlueprint>(conn)
.await
.optional()
.map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?;
let db_blueprint = db_blueprint.ok_or_else(|| {
Error::not_found_by_id(ResourceType::Blueprint, &id)
})?;
Ok(db_blueprint.target_release_minimum_generation.0)
}
}

// Helper to create an `authz::Blueprint` for a specific blueprint ID
Expand Down
149 changes: 147 additions & 2 deletions nexus/db-queries/src/db/datastore/target_release.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,15 @@ impl DataStore {
}
}
};
Ok(target_release.into_external(release_source))
// We choose to fetch the blueprint directly from the database rather
// than relying on the cached blueprint in Nexus because our APIs try to
// be strongly consistent. This shows up/will show up as a warning in
// the UI, and we don't want the warning to flicker in and out of
// existence based on which Nexus is getting hit.
let min_gen = self.blueprint_target_get_current_min_gen(opctx).await?;
// The semantics of min_gen mean we use a > sign here, not >=.
let mupdate_override = min_gen > target_release.generation.0;
Ok(target_release.into_external(release_source, mupdate_override))
}
}

Expand All @@ -135,6 +143,12 @@ mod test {
use crate::db::model::{Generation, TargetReleaseSource};
use crate::db::pub_test_utils::TestDatabase;
use chrono::{TimeDelta, Utc};
use nexus_inventory::now_db_precision;
use nexus_reconfigurator_planning::blueprint_builder::BlueprintBuilder;
use nexus_reconfigurator_planning::example::{
ExampleSystemBuilder, SimRngState,
};
use nexus_types::deployment::BlueprintTarget;
use omicron_common::api::external::{
TufArtifactMeta, TufRepoDescription, TufRepoMeta,
};
Expand All @@ -145,7 +159,8 @@ mod test {

#[tokio::test]
async fn target_release_datastore() {
let logctx = dev::test_setup_log("target_release_datastore");
const TEST_NAME: &str = "target_release_datastore";
let logctx = dev::test_setup_log(TEST_NAME);
let db = TestDatabase::new_with_datastore(&logctx.log).await;
let (opctx, datastore) = (db.opctx(), db.datastore());

Expand All @@ -163,6 +178,56 @@ mod test {
);
assert!(initial_target_release.tuf_repo_id.is_none());

// Set up an initial blueprint and make it the target. This models real
// systems which always have a target blueprint.
let mut rng = SimRngState::from_seed(TEST_NAME);
let (system, mut blueprint) = ExampleSystemBuilder::new_with_rng(
&logctx.log,
rng.next_system_rng(),
)
.build();
assert_eq!(
blueprint.target_release_minimum_generation,
1.into(),
"initial blueprint should have minimum generation of 1",
);
// Treat this blueprint as the initial one for the system.
blueprint.parent_blueprint_id = None;

datastore
.blueprint_insert(&opctx, &blueprint)
.await
.expect("inserted blueprint");
datastore
.blueprint_target_set_current(
opctx,
BlueprintTarget {
target_id: blueprint.id,
// enabled = true or false shouldn't matter for this.
enabled: true,
time_made_target: now_db_precision(),
},
)
.await
.expect("set blueprint target");

// We should always be able to get a view of the target release.
let initial_target_release_view = datastore
.target_release_view(opctx, &initial_target_release)
.await
.expect("got target release");
eprintln!(
"initial target release view: {:#?}",
initial_target_release_view
);

// This target release should not have the mupdate override set, because
// the generation is <= the minimum generation in the target blueprint.
assert!(
!initial_target_release_view.mupdate_override,
"mupdate_override should be false for initial target release"
);

// We should be able to set a new generation just like the first.
// We allow some slack in the timestamp comparison because the
// database only stores timestamps with μsec precision.
Expand Down Expand Up @@ -256,6 +321,86 @@ mod test {
);
assert_eq!(target_release.tuf_repo_id, Some(tuf_repo_id));

// Generate a new blueprint with a greater target release generation.
let mut builder = BlueprintBuilder::new_based_on(
&logctx.log,
&blueprint,
&system.input,
&system.collection,
TEST_NAME,
)
.expect("created blueprint builder");
builder.set_rng(rng.next_planner_rng());
builder
.set_target_release_minimum_generation(
blueprint.target_release_minimum_generation,
5.into(),
)
.expect("set target release minimum generation");
let bp2 = builder.build();

datastore
.blueprint_insert(&opctx, &bp2)
.await
.expect("inserted blueprint");
datastore
.blueprint_target_set_current(
opctx,
BlueprintTarget {
target_id: bp2.id,
// enabled = true or false shouldn't matter for this.
enabled: true,
time_made_target: now_db_precision(),
},
)
.await
.expect("set blueprint target");

// Fetch the target release again.
let target_release = datastore
.target_release_get_current(opctx)
.await
.expect("got target release");
let target_release_view_2 = datastore
.target_release_view(opctx, &target_release)
.await
.expect("got target release");

eprintln!("target release view 2: {target_release_view_2:#?}");

assert!(
target_release_view_2.mupdate_override,
"mupdate override is set",
);

// Now set the target release again -- this should cause the mupdate
// override to disappear.
let before = Utc::now();
let target_release = datastore
.target_release_insert(
opctx,
TargetRelease::new_system_version(&target_release, tuf_repo_id),
)
.await
.unwrap();
let after = Utc::now();

assert_eq!(target_release.generation, Generation(5.into()));
assert!(target_release.time_requested >= before);
assert!(target_release.time_requested <= after);

let target_release_view_3 = datastore
.target_release_view(opctx, &target_release)
.await
.expect("got target release");

eprintln!("target release view 3: {target_release_view_3:#?}");

assert!(
!target_release_view_3.mupdate_override,
"mupdate override is not set",
);

// Clean up.
db.terminate().await;
logctx.cleanup_successful();
Expand Down
11 changes: 11 additions & 0 deletions nexus/types/src/external_api/views.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1488,6 +1488,17 @@ pub struct TargetRelease {

/// The source of the target release.
pub release_source: TargetReleaseSource,

/// If true, indicates that at least one sled in the system has been updated
/// through the recovery (MUPdate) path since the last time the target
/// release was set.
///
/// In this case, the system will ignore the currently-set target release,
/// on the assumption that continuing an update may reintroduce or
/// exacerbate whatever problem caused the recovery path to be used. An
/// operator must set the target release again in order to resume automated
/// updates.
pub mupdate_override: bool,
}

fn expected_one_of<T: strum::VariantArray + fmt::Display>() -> String {
Expand Down
5 changes: 5 additions & 0 deletions openapi/nexus.json
Original file line number Diff line number Diff line change
Expand Up @@ -24802,6 +24802,10 @@
"type": "integer",
"format": "int64"
},
"mupdate_override": {
"description": "If true, indicates that at least one sled in the system has been updated through the recovery (MUPdate) path since the last time the target release was set.\n\nIn this case, the system will ignore the currently-set target release, on the assumption that continuing an update may reintroduce or exacerbate whatever problem caused the recovery path to be used. An operator must set the target release again in order to resume automated updates.",
"type": "boolean"
},
"release_source": {
"description": "The source of the target release.",
"allOf": [
Expand All @@ -24818,6 +24822,7 @@
},
"required": [
"generation",
"mupdate_override",
"release_source",
"time_requested"
]
Expand Down
Loading