Skip to content
Merged
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
6 changes: 6 additions & 0 deletions sled-agent/config-reconciler/src/reconciler_task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,12 @@ impl ReconcilerTask {
// and also we want to report it as part of each reconciler result).
let timesync_status = self.zones.check_timesync().await;

// Call back into sled-agent and let it do any work that needs to happen
// once time is sync'd (e.g., rewrite `uptime`).
if timesync_status.is_synchronized() {
sled_agent_facilities.on_time_sync();
}

// We conservatively refuse to start any new zones if any zones have
// failed to shut down cleanly. This could be more precise, but we want
// to avoid wandering into some really weird cases, such as:
Expand Down
2 changes: 1 addition & 1 deletion sled-agent/config-reconciler/src/reconciler_task/zones.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1129,7 +1129,7 @@ mod tests {
&self.underlay_vnic
}

async fn on_time_sync(&self) {}
fn on_time_sync(&self) {}

async fn start_omicron_zone(
&self,
Expand Down
2 changes: 1 addition & 1 deletion sled-agent/config-reconciler/src/sled_agent_facilities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub trait SledAgentFacilities: Send + Sync + 'static {
// currently implemented by `ServiceManager` and does a couple one-time
// setup things (like rewrite the OS boot time). We could probably absorb
// that work and remove this callback.
fn on_time_sync(&self) -> impl Future<Output = ()> + Send;
fn on_time_sync(&self);

/// Method to start a zone.
// TODO-cleanup This is implemented by
Expand Down
8 changes: 4 additions & 4 deletions sled-agent/src/services.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3118,14 +3118,14 @@ impl ServiceManager {
///
/// This function only executes the out-of-band actions once, once the
/// synchronization state has shifted to true.
pub(crate) async fn on_time_sync(&self) {
pub(crate) fn on_time_sync(&self) {
if self
.inner
.time_synced
.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed)
.is_ok()
{
debug!(self.inner.log, "Time is now synchronized");
info!(self.inner.log, "Time is now synchronized");
// We only want to rewrite the boot time once, so we do it here
// when we know the time is synchronized.
self.boottime_rewrite();
Expand All @@ -3138,11 +3138,11 @@ impl ServiceManager {
// https://github.com/oxidecomputer/omicron/issues/8022.
let queue = self.metrics_queue();
match queue.notify_time_synced_sled(self.sled_id()) {
Ok(_) => debug!(
Ok(_) => info!(
self.inner.log,
"Notified metrics task that time is now synced",
),
Err(e) => error!(
Err(e) => warn!(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why switch to a warn here?
Will we retry this? (I know that this is not part of the goal of the PR, just wondering)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will not retry this, but I don't think it's particularly detrimental to sled-agent itself, right? I can change it back if you think error! is better. I have a vague sense that those are pretty rare and usually near-fatal.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Merging to get the fix into main, but happy to open a PR switching this back if you disagree)

self.inner.log,
"Failed to notify metrics task that \
time is now synced, metrics may not be produced.";
Expand Down
4 changes: 2 additions & 2 deletions sled-agent/src/sled_agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1336,8 +1336,8 @@ impl SledAgentFacilities for ReconcilerFacilities {
&self.etherstub_vnic
}

async fn on_time_sync(&self) {
self.service_manager.on_time_sync().await
fn on_time_sync(&self) {
self.service_manager.on_time_sync();
}

async fn start_omicron_zone(
Expand Down
Loading