add clarifying comments

Author: jgallagher

sled-agent/config-reconciler/src/ledger.rs

@@ -141,6 +141,21 @@ impl LedgerTaskHandle {
         unimplemented!()
     }
 
+    /// Confirm that a new [`ArtifactConfig`] is valid given the contents of the
+    /// currently-ledgered [`OmicronSledConfig`].
+    ///
+    /// In particular, this confirms that a new artifact config does not
+    /// _remove_ any artifacts needed by zones described by the current sled
+    /// config.
+    ///
+    /// The artifact store in sled agent and this task need to coordinate with
+    /// each other whenever changes are made to either kind of config. This
+    /// method provides a path for the artifact store to validate its incoming
+    /// artifact configs against this task, and this task uses the
+    /// implementation of [`SledAgentArtifactStore`] to validate its incoming
+    /// sled configs against the artifact store. Validation is always performed
+    /// by this task, which enforces serialization of the checks in the event of
+    /// requests arriving concurrently to change both configs.
     pub async fn validate_artifact_config(
         &self,
         _new_config: ArtifactConfig,

sled-agent/config-reconciler/src/sled_agent_facilities.rs

@@ -20,8 +20,12 @@ pub trait SledAgentFacilities: Send + 'static {
     type MetricsUntrackZoneLinksError;
     type ZoneBundleCreateError;
 
-    /// Called once time is synchronized.
-    // TODO-cleanup should we do this work ourselves instead?
+    /// Called by the reconciler task to inform sled-agent that time is
+    /// sychronized. May be called multiple times.
+    // TODO-cleanup should we do this work ourselves instead? This is
+    // 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.
     async fn on_time_sync(&self);
 
     /// Method to start a zone.