WEBDIENSTE/NLnetLabs-krill
1
0
Fork 0
mirror of https://github.com/NLnetLabs/krill.git synced 2026-09-21 00:47:41 +02:00
Code Issues Packages Projects Releases Wiki Activity

Crash Krill if a fatal error is encountered by the task scheduler. (#1132)

Browse Source 
* Crash Krill if a fatal error is encountered by the task scheduler.
* Do not finish running task when just adding a new task.
This commit is contained in:
Tim Bruijnzeels
2023-10-17 15:31:11 +02:00
parent 9d55e12826
commit 4f4eafdfd3
5 changed files with 155 additions and 103 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Cargo.lock
Generated
+6 -6
View File
@@ -1129,9 +1129,9 @@ dependencies = [
[[package]]
name = "kvx"
version = "0.8.0"
version = "0.9.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "31ba9dfb918a4c8e2f326d72a86891ace7c83d13c550e40c543d4cec57a9bf34"
checksum = "dbd3053d98daca52860c508c21f32107c174396af5772ea54051110807d040cb"
dependencies = [
"kvx_macros",
"kvx_types",
@@ -1148,9 +1148,9 @@ dependencies = [
[[package]]
name = "kvx_macros"
version = "0.8.0"
version = "0.9.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e87fdea2c4bf35291921c3a224b4cc0abf86cadb6818a4c09facefc755f2e3dd"
checksum = "ed06dbcd44cd4f7af95fb5e0c8d020d14e64cbbd2ceacfdd9ed07f2e6d81e87f"
dependencies = [
"kvx_types",
"proc-macro-error",
@@ -1161,9 +1161,9 @@ dependencies = [
[[package]]
name = "kvx_types"
version = "0.8.0"
version = "0.9.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "0569372a25dd8a7177baf7fb0575709d1121f4ec829623c424150d653200f8d7"
checksum = "fbff677c127336a6e93df7d54a036824312d4c1e9d165bd284e25b024cc83b06"
dependencies = [
"postgres",
"postgres-types",
Cargo.toml
+2 -2
View File
@@ -38,8 +38,8 @@ jmespatch = { version = "^0.3", features = ["sync"], optional = true }
kmip = { version = "0.4.2", package = "kmip-protocol", features = [
"tls-with-openssl",
], optional = true }
kvx = { version = "0.8.0", features = ["macros"] }
# kvx = { version = "0.6.0", git = "https://github.com/nlnetlabs/kvx", features = ["macros"] }
kvx = { version = "0.9.0", features = ["macros"] }
# kvx = { version = "0.8.0", git = "https://github.com/nlnetlabs/kvx", branch = "schedule-without-finish", features = [ "macros"] }
libflate = "^1"
log = "^0.4"
once_cell = { version = "^1.7.2", optional = true }
src/commons/error.rs
+17
View File
@@ -151,6 +151,23 @@ impl From<Error> for ApiAuthError {
}
}
//------------ FatalError --------------------------------------------------
/// Wraps an error so horrible to contemplate that it should result in
/// a server crash, as it would have lost its reason to live.
///
/// Note that we do not provide any From<Error> for this in an attempt
/// to ensure that this is only ever used explicitly and when it is
/// appropriate.
#[derive(Debug)]
pub struct FatalError(pub Error);
impl fmt::Display for FatalError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
self.0.fmt(f)
}
}
//------------ Error -------------------------------------------------------
#[derive(Debug)]
src/daemon/mq.rs
+29 -24
View File
@@ -216,7 +216,7 @@ impl TaskQueue {
trace!(
"fnd task: {} with priority: {}",
pending.name,
Priority(pending.timestamp as i64)
Priority::from_timestamp_ms(pending.timestamp_millis)
);
Some(pending)
}
@@ -227,14 +227,16 @@ impl TaskQueue {
/// was already present, then it will get the highest of the two
/// priorities.
///
/// Many tasks are planned with a high priority - e.g. if they are
/// triggered through CA events. Other tasks may be planned for the
/// future (e.g. sync with parent tomorrow). The latter can be moved
/// forward when circumstances dictate.
///
/// Recurring tasks will typically be re-added by the Scheduler when
/// needed (and can then be moved forward if needed).
/// This will NOT finish any possible running task by the same
/// name.
pub fn schedule(&self, task: Task, priority: Priority) -> KrillResult<()> {
self.schedule_task(task, ScheduleMode::ReplaceExistingSoonest, priority)
}
/// Schedules a task for the given priority. If the equivalent task
/// was already present, then it will get the highest of the two
/// priorities.
pub fn schedule_and_finish_existing(&self, task: Task, priority: Priority) -> KrillResult<()> {
self.schedule_task(task, ScheduleMode::FinishOrReplaceExistingSoonest, priority)
}
@@ -244,24 +246,26 @@ impl TaskQueue {
fn schedule_task(&self, task: Task, mode: ScheduleMode, priority: Priority) -> KrillResult<()> {
let task_name = task.name()?;
trace!("add task: {} with priority: {}", task_name, priority.to_string());
debug!("add task: {} with priority: {}", task_name, priority.to_string());
let json = serde_json::to_value(&task)
.map_err(|e| Error::Custom(format!("could not serialize task {}. error: {}", task_name, e)))?;
self.q
.schedule_task(task_name, json, Some(priority.into()), mode)
.schedule_task(task_name, json, Some(priority.to_millis()), mode)
.map_err(Error::from)
}
/// Finish a running task, without rescheduling it.
pub fn finish(&self, task: &kvx::Key) -> KrillResult<()> {
debug!("Finish task: {}", task);
self.q.finish_running_task(task).map_err(Error::from)
}
/// Reschedule a running task, without finishing it.
pub fn reschedule(&self, task: &kvx::Key, priority: Priority) -> KrillResult<()> {
debug!("Reschedule task: {} to: {}", task, priority);
self.q
.reschedule_running_task(task, Some(priority.into()))
.reschedule_running_task(task, Some(priority.to_millis()))
.map_err(Error::from)
}
@@ -442,7 +446,7 @@ impl eventsourcing::PostSaveEventListener<CertAuth> for TaskQueue {
match event {
CertAuthEvent::ChildUpdatedResources { child, .. } | CertAuthEvent::ChildKeyRevoked { child, .. } => {
debug!("Schedule a sync from the child to this CA as their parent. This will be a no-op for remote children.");
if let Err(e) = self.schedule(
if let Err(e) = self.schedule_and_finish_existing(
Task::SyncParent {
ca_handle: child.convert(),
ca_version: 0, // no need to wait for updated child
@@ -541,12 +545,21 @@ impl eventsourcing::PostSaveEventListener<TrustAnchorProxy> for TaskQueue {
#[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize)]
pub struct Priority(i64);
pub fn now() -> Priority {
Time::now().into()
impl Priority {
/// Convenience function, but note that we don't
/// have ms granularity here, so this is rounded
/// down to seconds.
pub fn from_timestamp_ms(millis: u128) -> Self {
Priority((millis / 1000) as i64)
}
pub fn to_millis(&self) -> u128 {
(self.0 * 1000) as u128
}
}
pub fn in_millis(millis: i64) -> Priority {
(Time::now() + chrono::Duration::milliseconds(millis)).into()
pub fn now() -> Priority {
Time::now().into()
}
pub fn in_seconds(secs: i64) -> Priority {
@@ -604,11 +617,3 @@ impl From<Time> for Priority {
Priority(time.timestamp())
}
}
impl From<Priority> for u64 {
fn from(p: Priority) -> Self {
// even though we use an i64 for the timestamp,
// we know that this can never be negative
p.0 as u64
}
}
src/daemon/scheduler.rs
+101 -71
View File
@@ -18,8 +18,9 @@ use crate::{
api::Timestamp,
bgp::BgpAnalyser,
crypto::dispatch::signerinfo::SignerInfo,
error::FatalError,
eventsourcing::{Aggregate, AggregateStore, WalStore, WalSupport},
KrillResult,
util::KrillVersion,
},
constants::{
CASERVER_NS, PROPERTIES_NS, PUBSERVER_CONTENT_NS, PUBSERVER_NS, SCHEDULER_INTERVAL_RENEW_MINS,
@@ -93,25 +94,27 @@ impl Scheduler {
// So, if we ever change the content of tasks then we should make sure
// that Krill is either backward compatible, or the task queue is migrated
// on upgrade.
error!("Fatal error. Cannot parse task: {}. Error: {}", task_key, e);
return; // stops the server.
error!("Fatal error parsing task: {}. Krill will now stop! This may be because this task is not for this Krill version ({}). If this issue persists, then try deleting this task from storage, it will appear in the 'tasks' dir if you use disk storage. The error was {}", task_key, KrillVersion::code_version(), e);
std::process::exit(1);
}
Ok(task) => {
if let Err(e) = match self.process_task(task).await {
Ok(result) => match result {
Ok(task) => match self.process_task(task).await {
Ok(result) => {
if let Err(e) = match result {
TaskResult::Done => self.tasks.finish(&task_key),
TaskResult::FollowUp(task, priority) => self.tasks.schedule(task, priority),
TaskResult::FollowUp(task, priority) => {
self.tasks.schedule_and_finish_existing(task, priority)
}
TaskResult::Reschedule(priority) => self.tasks.reschedule(&task_key, priority),
},
Err(e) => Err(e),
} {
// We really should not get any errors at this stage. If we do, then
// this is most likely because of an issue with the key value store.
// In this case we should probably just log the error and leave the
// task to be cleaned up later.
error!("Error processing task: {}. Error: {}", task_key, e);
} {
error!("Error finishing / scheduling task {}. Krill will stop as there is no good way to recover from this. When Krill starts it will try to reschedule any missing tasks. Error was: {}", task_key, e);
std::process::exit(1);
}
}
}
Err(e) => {
error!("Error processing task: {}. Tasks are only allowed to return fatal errors. Krill will stop as there is no good way to recover from this. When Krill starts it will try to reschedule any missing tasks. Error was: {}", task_key, e);
std::process::exit(1);
}
},
}
}
@@ -120,7 +123,11 @@ impl Scheduler {
}
/// Process a single task
async fn process_task(&self, task: Task) -> KrillResult<TaskResult> {
///
/// May only return fatal errors. Temporary, or suspected temporary, issues
/// such as not being able to contact a parent CA should result in an
/// Ok(TaskResult::Reschedule) instead.
async fn process_task(&self, task: Task) -> Result<TaskResult, FatalError> {
match task {
Task::QueueStartTasks => self.queue_start_tasks().await, // return error and stop server on failure
@@ -175,7 +182,7 @@ impl Scheduler {
}
/// Queues missing tasks for background jobs when the server is started
async fn queue_start_tasks(&self) -> KrillResult<TaskResult> {
async fn queue_start_tasks(&self) -> Result<TaskResult, FatalError> {
// The task queue is persistent starting with Krill 0.14.0
//
// Tasks should not disappear. But.. to make sure that:
@@ -195,14 +202,14 @@ impl Scheduler {
// to avoid a thundering herd. Note that the operator can always
// choose to run bulk operations manually if they know that they
// cannot wait.
let ca_list = self.ca_manager.ca_list(&self.system_actor)?;
let ca_list = self.ca_manager.ca_list(&self.system_actor).map_err(FatalError)?;
let cas = ca_list.cas();
debug!("Adding missing tasks at start up");
// When multi-node set ups with a shared queue are
// supported then we can no longer safely reschedule
// ALL running tests. See issue: #1112
self.tasks.reschedule_tasks_at_startup()?;
self.tasks.reschedule_tasks_at_startup().map_err(FatalError)?;
// If we have many CAs then we need to apply some jitter
// in the priority of CA to parent and CA to repository
@@ -211,21 +218,23 @@ impl Scheduler {
let use_jitter = cas.len() >= SCHEDULER_USE_JITTER_CAS_THRESHOLD;
for summary in cas {
let ca = self.ca_manager.get_ca(summary.handle()).await?;
let ca = self.ca_manager.get_ca(summary.handle()).await.map_err(FatalError)?;
let ca_handle = ca.handle();
let ca_version = ca.version();
trace!("Adding tasks for CA {}, using jitter: {}", ca.handle(), use_jitter);
debug!("Adding tasks for CA {}, using jitter: {}", ca.handle(), use_jitter);
for parent in ca.parents() {
self.tasks.schedule_missing(
Task::SyncParent {
ca_handle: ca_handle.clone(),
ca_version,
parent: parent.clone(),
},
self.config.ca_refresh_start_up(use_jitter),
)?;
self.tasks
.schedule_missing(
Task::SyncParent {
ca_handle: ca_handle.clone(),
ca_version,
parent: parent.clone(),
},
self.config.ca_refresh_start_up(use_jitter),
)
.map_err(FatalError)?;
}
// Plan a sync with the repo. But only in case we only have a handful
@@ -235,13 +244,15 @@ impl Scheduler {
// then it will be scheduled accordingly. Furthermore, users can use the
// 'bulk' function to explicitly force schedule a sync.
if cas.len() <= SCHEDULER_RESYNC_REPO_CAS_THRESHOLD {
self.tasks.schedule_missing(
Task::SyncRepo {
ca_handle: ca_handle.clone(),
ca_version,
},
now(),
)?;
self.tasks
.schedule_missing(
Task::SyncRepo {
ca_handle: ca_handle.clone(),
ca_version,
},
now(),
)
.map_err(FatalError)?;
}
// If suspension is enabled then plan a task for it. Since this is
@@ -250,35 +261,49 @@ impl Scheduler {
// importantly.. by adding this task we ensure that it will keep being
// re-scheduled when it's done.
if self.config.suspend_child_after_inactive_seconds().is_some() {
self.tasks.schedule_missing(
Task::SuspendChildrenIfNeeded {
ca_handle: ca_handle.clone(),
},
now(),
)?;
self.tasks
.schedule_missing(
Task::SuspendChildrenIfNeeded {
ca_handle: ca_handle.clone(),
},
now(),
)
.map_err(FatalError)?;
}
}
self.tasks.schedule_missing(Task::RepublishIfNeeded, now())?;
self.tasks.schedule_missing(Task::RenewObjectsIfNeeded, now())?;
self.tasks.schedule_missing(Task::RefreshAnnouncementsInfo, now())?;
self.tasks
.schedule_missing(Task::RepublishIfNeeded, now())
.map_err(FatalError)?;
self.tasks
.schedule_missing(Task::RenewObjectsIfNeeded, now())
.map_err(FatalError)?;
self.tasks
.schedule_missing(Task::RefreshAnnouncementsInfo, now())
.map_err(FatalError)?;
#[cfg(feature = "multi-user")]
self.tasks.schedule_missing(Task::SweepLoginCache, in_minutes(1))?;
self.tasks
.schedule_missing(Task::SweepLoginCache, in_minutes(1))
.map_err(FatalError)?;
// Plan updating snapshots soon after a restart.
// This also ensures that this task gets triggered in long
// running tests, such as functional_parent_child.rs.
self.tasks.schedule_missing(Task::UpdateSnapshots, now())?;
self.tasks
.schedule_missing(Task::UpdateSnapshots, now())
.map_err(FatalError)?;
if self.config.testbed().is_some() {
self.tasks.schedule_missing(Task::RenewTestbedTa, now())?;
self.tasks
.schedule_missing(Task::RenewTestbedTa, now())
.map_err(FatalError)?;
}
Ok(TaskResult::Done)
}
async fn sync_repo(&self, ca: CaHandle, version: u64) -> KrillResult<TaskResult> {
async fn sync_repo(&self, ca: CaHandle, version: u64) -> Result<TaskResult, FatalError> {
info!("Synchronize CA {} with repository", ca);
match self
@@ -306,8 +331,8 @@ impl Scheduler {
}
/// Try to synchronize a CA with a specific parent, reschedule if this fails
async fn sync_parent(&self, ca: CaHandle, ca_version: u64, parent: ParentHandle) -> KrillResult<TaskResult> {
if self.ca_manager.has_ca(&ca)? {
async fn sync_parent(&self, ca: CaHandle, ca_version: u64, parent: ParentHandle) -> Result<TaskResult, FatalError> {
if self.ca_manager.has_ca(&ca).map_err(FatalError)? {
info!("Synchronize CA '{}' with its parent '{}'", ca, parent);
match self
.ca_manager
@@ -352,7 +377,7 @@ impl Scheduler {
}
/// Resync the testbed TA signer and proxy
async fn renew_testbed_ta(&self) -> KrillResult<TaskResult> {
async fn renew_testbed_ta(&self) -> Result<TaskResult, FatalError> {
if let Err(e) = self.ca_manager.ta_renew_testbed_ta().await {
error!("There was an issue renewing the testbed TA: {}", e);
}
@@ -362,7 +387,7 @@ impl Scheduler {
/// Try to synchronise the Trust Anchor Proxy with the *local* Signer - if it exists
/// in this server.
async fn sync_ta_proxy_signer_if_possible(&self) -> KrillResult<TaskResult> {
async fn sync_ta_proxy_signer_if_possible(&self) -> Result<TaskResult, FatalError> {
debug!("Synchronise Trust Anchor Proxy with Signer - if Signer is local.");
if let Err(e) = self.ca_manager.sync_ta_proxy_signer_if_possible().await {
error!("There was an issue synchronising the TA Proxy and Signer: {}", e);
@@ -371,8 +396,8 @@ impl Scheduler {
}
/// Try to suspend children for a CA
async fn suspend_children_if_needed(&self, ca_handle: CaHandle) -> KrillResult<TaskResult> {
if self.ca_manager.has_ca(&ca_handle)? {
async fn suspend_children_if_needed(&self, ca_handle: CaHandle) -> Result<TaskResult, FatalError> {
if self.ca_manager.has_ca(&ca_handle).map_err(FatalError)? {
debug!("Verify if CA '{}' has children that need to be suspended", ca_handle);
self.ca_manager
.ca_suspend_inactive_children(&ca_handle, self.started, &self.system_actor)
@@ -389,7 +414,7 @@ impl Scheduler {
}
/// Let CAs that need it republish their CRL/MFT
async fn republish_if_needed(&self) -> KrillResult<TaskResult> {
async fn republish_if_needed(&self) -> Result<TaskResult, FatalError> {
// Note that CRL/MFT re-issuance is handled by the `CaObjects` companion
// struct, rather than the event-sourced `CertAuth`. Meaning... that we
// do not get to see an event in case there is an actual update and
@@ -397,13 +422,15 @@ impl Scheduler {
//
// Instead we get back a list of CAs that had changes, and we need to
// schedule a synchronisation for each of them here.
let cas = self.ca_manager.republish_all(false).await?; // can only fail on critical errors
let cas = self.ca_manager.republish_all(false).await.map_err(FatalError)?;
for ca_handle in cas {
info!("Re-issued MFT and CRL for CA: {}", ca_handle);
let ca_version = 0; // we use 0 because we don't need to wait for an updated CertAuth
self.tasks.schedule(Task::SyncRepo { ca_handle, ca_version }, now())?;
self.tasks
.schedule(Task::SyncRepo { ca_handle, ca_version }, now())
.map_err(FatalError)?;
}
// check again in a short while.. no jitter needed as this is a cheap operation
@@ -416,7 +443,7 @@ impl Scheduler {
}
/// Update announcement info
async fn announcements_refresh(&self) -> KrillResult<TaskResult> {
async fn announcements_refresh(&self) -> Result<TaskResult, FatalError> {
if let Err(e) = self.bgp_analyser.update().await {
error!("Failed to update BGP announcements: {}", e)
}
@@ -427,8 +454,11 @@ impl Scheduler {
}
/// Let CAs that need it re-issue signed objects
async fn renew_objects_if_needed(&self) -> KrillResult<TaskResult> {
self.ca_manager.renew_objects_all(&self.system_actor).await?; // only fails on fatal errors
async fn renew_objects_if_needed(&self) -> Result<TaskResult, FatalError> {
self.ca_manager
.renew_objects_all(&self.system_actor)
.await
.map_err(FatalError)?;
// check again in a short while.. note that this is usually a cheap no-op
Ok(TaskResult::FollowUp(
@@ -438,7 +468,7 @@ impl Scheduler {
}
#[cfg(feature = "multi-user")]
fn sweep_login_cache(&self) -> KrillResult<TaskResult> {
fn sweep_login_cache(&self) -> Result<TaskResult, FatalError> {
if let Err(e) = self.login_session_cache.sweep() {
error!("Background sweep of session decryption cache failed: {}", e);
}
@@ -447,7 +477,7 @@ impl Scheduler {
}
// Call update_snapshots on all AggregateStores and WalStores
fn update_snapshots(&self) -> KrillResult<TaskResult> {
fn update_snapshots(&self) -> Result<TaskResult, FatalError> {
fn update_aggregate_store_snapshots<A: Aggregate>(storage_uri: &Url, namespace: &Namespace) {
match AggregateStore::<A>::create(storage_uri, namespace, false) {
Err(e) => {
@@ -506,7 +536,7 @@ impl Scheduler {
Ok(TaskResult::FollowUp(Task::UpdateSnapshots, in_hours(24)))
}
fn update_rrdp_if_needed(&self) -> KrillResult<TaskResult> {
fn update_rrdp_if_needed(&self) -> Result<TaskResult, FatalError> {
match self.repo_manager.update_rrdp_if_needed() {
Err(e) => {
error!("Could not update RRDP deltas! Error: {}", e);
@@ -533,7 +563,7 @@ impl Scheduler {
parent: ParentHandle,
rcn: ResourceClassName,
revocation_requests: Vec<RevocationRequest>,
) -> KrillResult<TaskResult> {
) -> Result<TaskResult, FatalError> {
info!(
"Trigger send revoke requests for removed RC for '{}' under '{}'",
ca_handle, parent
@@ -541,8 +571,8 @@ impl Scheduler {
let requests = HashMap::from([(rcn, revocation_requests)]);
if self.ca_manager.has_ca(&ca_handle)? {
let ca = self.ca_manager.get_ca(&ca_handle).await?;
if self.ca_manager.has_ca(&ca_handle).map_err(FatalError)? {
let ca = self.ca_manager.get_ca(&ca_handle).await.map_err(FatalError)?;
if ca.version() < ca_version {
// premature, we need to wait for the CA to be committed.
Ok(TaskResult::Reschedule(in_seconds(1)))
@@ -570,14 +600,14 @@ impl Scheduler {
ca_version: u64,
rcn: ResourceClassName,
revocation_request: RevocationRequest,
) -> KrillResult<TaskResult> {
if self.ca_manager.has_ca(&ca_handle)? {
) -> Result<TaskResult, FatalError> {
if self.ca_manager.has_ca(&ca_handle).map_err(FatalError)? {
info!(
"Trigger sending revocation requests for unexpected key with id '{}' in RC '{}'",
revocation_request.key(),
rcn
);
let ca = self.ca_manager.get_ca(&ca_handle).await?;
let ca = self.ca_manager.get_ca(&ca_handle).await.map_err(FatalError)?;
if ca.version() < ca_version {
debug!("reschedule premature task");