Skip to content

Commit

Permalink
[easy] [sled-agent] make omicron_zones_list internally infallible (#6699
Browse files Browse the repository at this point in the history
)

Small cleanup -- we're not promising that it's infallible in the API, but
internally infallible makes some upcoming logic slightly easier.
  • Loading branch information
sunshowers authored Sep 28, 2024
1 parent ea2f4d2 commit 1bae5ae
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 29 deletions.
2 changes: 1 addition & 1 deletion sled-agent/src/http_entrypoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ impl SledAgentApi for SledAgentImpl {
rqctx: RequestContext<Self::Context>,
) -> Result<HttpResponseOk<OmicronZonesConfig>, HttpError> {
let sa = rqctx.context();
Ok(HttpResponseOk(sa.omicron_zones_list().await?))
Ok(HttpResponseOk(sa.omicron_zones_list().await))
}

async fn omicron_zones_put(
Expand Down
36 changes: 12 additions & 24 deletions sled-agent/src/services.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3316,9 +3316,7 @@ impl ServiceManager {
}

/// Returns the current Omicron zone configuration
pub async fn omicron_zones_list(
&self,
) -> Result<OmicronZonesConfig, Error> {
pub async fn omicron_zones_list(&self) -> OmicronZonesConfig {
let log = &self.inner.log;

// We need to take the lock in order for the information in the ledger
Expand All @@ -3337,7 +3335,7 @@ impl ServiceManager {
None => OmicronZonesConfigLocal::initial(),
};

Ok(ledger_data.to_omicron_zones_config())
ledger_data.to_omicron_zones_config()
}

/// Ensures that particular Omicron zones are running
Expand Down Expand Up @@ -5102,8 +5100,7 @@ mod test {
.await;

let v1 = Generation::new();
let found =
mgr.omicron_zones_list().await.expect("failed to list zones");
let found = mgr.omicron_zones_list().await;
assert_eq!(found.generation, v1);
assert!(found.zones.is_empty());

Expand All @@ -5117,8 +5114,7 @@ mod test {
)
.await;

let found =
mgr.omicron_zones_list().await.expect("failed to list zones");
let found = mgr.omicron_zones_list().await;
assert_eq!(found.generation, v2);
assert_eq!(found.zones.len(), 1);
assert_eq!(found.zones[0].id, id);
Expand Down Expand Up @@ -5181,8 +5177,7 @@ mod test {
.await;

let v1 = Generation::new();
let found =
mgr.omicron_zones_list().await.expect("failed to list zones");
let found = mgr.omicron_zones_list().await;
assert_eq!(found.generation, v1);
assert!(found.zones.is_empty());

Expand Down Expand Up @@ -5270,8 +5265,7 @@ mod test {
ensure_new_service(&mgr, id, v2, dir.clone()).await;
let v3 = v2.next();
ensure_existing_service(&mgr, id, v3, dir).await;
let found =
mgr.omicron_zones_list().await.expect("failed to list zones");
let found = mgr.omicron_zones_list().await;
assert_eq!(found.generation, v3);
assert_eq!(found.zones.len(), 1);
assert_eq!(found.zones[0].id, id);
Expand Down Expand Up @@ -5400,8 +5394,7 @@ mod test {
.await;
illumos_utils::USE_MOCKS.store(false, Ordering::SeqCst);

let found =
mgr.omicron_zones_list().await.expect("failed to list zones");
let found = mgr.omicron_zones_list().await;
assert_eq!(found.generation, v2);
assert_eq!(found.zones.len(), 1);
assert_eq!(found.zones[0].id, id);
Expand Down Expand Up @@ -5469,8 +5462,7 @@ mod test {
)
.await;

let found =
mgr.omicron_zones_list().await.expect("failed to list zones");
let found = mgr.omicron_zones_list().await;
assert_eq!(found.generation, v1);
assert!(found.zones.is_empty());

Expand Down Expand Up @@ -5526,8 +5518,7 @@ mod test {
.await
.unwrap();

let found =
mgr.omicron_zones_list().await.expect("failed to list zones");
let found = mgr.omicron_zones_list().await;
assert_eq!(found.generation, v2);
assert_eq!(found.zones.len(), 1);
assert_eq!(found.zones[0].id, id1);
Expand Down Expand Up @@ -5562,8 +5553,7 @@ mod test {
Error::RequestedConfigOutdated { requested, current }
if requested == v1 && current == v2
));
let found2 =
mgr.omicron_zones_list().await.expect("failed to list zones");
let found2 = mgr.omicron_zones_list().await;
assert_eq!(found, found2);

// Now try to apply that list with the same generation number that we
Expand All @@ -5579,8 +5569,7 @@ mod test {
error,
Error::RequestedConfigConflicts(vr) if vr == v2
));
let found3 =
mgr.omicron_zones_list().await.expect("failed to list zones");
let found3 = mgr.omicron_zones_list().await;
assert_eq!(found, found3);

// But we should be able to apply this new list of zones as long as we
Expand All @@ -5592,8 +5581,7 @@ mod test {
)
.await
.expect("failed to remove all zones in a new generation");
let found4 =
mgr.omicron_zones_list().await.expect("failed to list zones");
let found4 = mgr.omicron_zones_list().await;
assert_eq!(found4.generation, v3);
let mut our_zones = zones;
our_zones.sort_by(|a, b| a.id.cmp(&b.id));
Expand Down
6 changes: 2 additions & 4 deletions sled-agent/src/sled_agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -918,10 +918,8 @@ impl SledAgent {
}

/// List the Omicron zone configuration that's currently running
pub async fn omicron_zones_list(
&self,
) -> Result<OmicronZonesConfig, Error> {
Ok(self.inner.services.omicron_zones_list().await?)
pub async fn omicron_zones_list(&self) -> OmicronZonesConfig {
self.inner.services.omicron_zones_list().await
}

/// Ensures that the specific set of Omicron zones are running as configured
Expand Down

0 comments on commit 1bae5ae

Please sign in to comment.