Consider max error when determining if time is synced

clients/ntp-admin-client/Cargo.toml

@@ -15,3 +15,6 @@ schemars.workspace = true
 serde.workspace = true
 slog.workspace = true
 omicron-workspace-hack.workspace = true
+
+[build-dependencies]
+git-stub-vcs.workspace = true

clients/ntp-admin-client/build.rs

@@ -0,0 +1,13 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at https://mozilla.org/MPL/2.0/.
+
+use git_stub_vcs::Materializer;
+
+fn main() {
+    let materializer = Materializer::for_build_script("../../")
+        .expect("detected VCS at repo root");
+    materializer
+        .materialize("openapi/ntp-admin/ntp-admin-1.0.0-aeffc2.json.gitstub")
+        .expect("materialized ntp-admint-client v1 git stub");
+}

clients/ntp-admin-client/src/lib.rs

@@ -20,3 +20,25 @@ progenitor::generate_api!(
     }),
     derives = [schemars::JsonSchema],
 );
+
+pub mod v1 {
+    progenitor::generate_api!(
+        spec = {
+            path = "git-stub-vcs/openapi/ntp-admin/ntp-admin-1.0.0-aeffc2.json",
+            relative_to = OutDir,
+        },
+        interface = Positional,
+        inner_type = slog::Logger,
+        pre_hook = (|log: &slog::Logger, request: &reqwest::Request| {
+            slog::debug!(log, "client request";
+                "method" => %request.method(),
+                "uri" => %request.url(),
+                "body" => ?&request.body(),
+            );
+        }),
+        post_hook = (|log: &slog::Logger, result: &Result<_, _>| {
+            slog::debug!(log, "client response"; "result" => ?result);
+        }),
+        derives = [schemars::JsonSchema],
+    );
+}

nexus/inventory/src/collector.rs

@@ -511,7 +511,7 @@ impl<'a> Collector<'a> {
                 let url = format!("http://{addr}");
                 let log = self.log.new(o!("ntp_admin_url" => url.clone()));
 
-                (cfg.id, ntp_admin_client::Client::new(&url, log))
+                (cfg.id, ntp_admin_client::v1::Client::new(&url, log))
             })
             .collect();
 
@@ -537,7 +537,7 @@ impl<'a> Collector<'a> {
     async fn collect_one_timesync(
         log: &slog::Logger,
         zone_id: OmicronZoneUuid,
-        client: &ntp_admin_client::Client,
+        client: &ntp_admin_client::v1::Client,
         in_progress: &mut CollectionBuilder,
     ) -> Result<(), anyhow::Error> {
         let sled_agent_url = client.baseurl();

ntp-admin/api/src/lib.rs

@@ -4,35 +4,21 @@
 
 use dropshot::{HttpError, HttpResponseOk, RequestContext};
 use dropshot_api_manager_types::api_versions;
-use ntp_admin_types_versions::latest;
+use ntp_admin_types_versions::{latest, v1};
 
 api_versions!([
-    // WHEN CHANGING THE API (part 1 of 2):
+    // Do not create new versions of this client-side versioned API.
+    // https://github.com/oxidecomputer/omicron/issues/9290
     //
-    // +- Pick a new semver and define it in the list below.  The list MUST
-    // |  remain sorted, which generally means that your version should go at
-    // |  the very top.
-    // |
-    // |  Duplicate this line, uncomment the *second* copy, update that copy for
-    // |  your new API version, and leave the first copy commented out as an
-    // |  example for the next person.
-    // v
-    // (next_int, IDENT),
+    // NOTE: Version 2 of this API was added to address
+    // //github.com/oxidecomputer/omicron/issues/7668, a bad failure mode in
+    // which CRDB panics early during control plane startup because the clocks
+    // are not synchronized well-enough. We're adding this as part of a
+    // two-phase rollout to get around #9290 for now.
+    (2, ADD_MAX_ERROR_AND_OFFSET),
     (1, INITIAL),
 ]);
 
-// WHEN CHANGING THE API (part 2 of 2):
-//
-// The call to `api_versions!` above defines constants of type
-// `semver::Version` that you can use in your Dropshot API definition to specify
-// the version when a particular endpoint was added or removed.  For example, if
-// you used:
-//
-//     (2, ADD_FOOBAR)
-//
-// Then you could use `VERSION_ADD_FOOBAR` as the version in which endpoints
-// were added or removed.
-
 #[dropshot::api_description]
 pub trait NtpAdminApi {
     type Context;
@@ -41,8 +27,22 @@ pub trait NtpAdminApi {
     #[endpoint {
         method = GET,
         path = "/timesync",
+        versions = VERSION_ADD_MAX_ERROR_AND_OFFSET..
     }]
     async fn timesync(
         rqctx: RequestContext<Self::Context>,
     ) -> Result<HttpResponseOk<latest::timesync::TimeSync>, HttpError>;
+
+    /// Query for the state of time synchronization
+    #[endpoint {
+        method = GET,
+        path = "/timesync",
+        operation_id = "timesync",
+        versions = VERSION_INITIAL..VERSION_ADD_MAX_ERROR_AND_OFFSET,
+    }]
+    async fn timesync_v1(
+        rqctx: RequestContext<Self::Context>,
+    ) -> Result<HttpResponseOk<v1::timesync::TimeSync>, HttpError> {
+        Ok(Self::timesync(rqctx).await?.map(|ts| ts.into()))
+    }
 }