Skip to content

Commit

Permalink
[wicketd] allow starting multiple updates with one API call (#4039)
Browse files Browse the repository at this point in the history
Extend `post_start_update` to allow starting updates on several sleds at
once. This is not currently used (the TUI always updates one sled at a
time), but will be used for command-line driven mupdates.

If we're issuing updates on several sleds at once, we can encounter
different kinds of errors for each sled. So instead of returning
immediately, we collect errors into a vector and then return them all at
once.

This also required some refactoring in `update_tracker.rs`. To take care
of all possible situations:

1. Add a new `SpawnUpdateDriver` trait, which has two methods: one
   to perform a one-time setup, and one to perform a spawn operation for
   each SP.
2. Add three implementations of `SpawnUpdateDriver`:
   `RealUpdateDriver` which is the actual implementation,
   `FakeUpdateDriver` which is used for tests, and `NeverUpdateDriver`
   which is an uninhabited type (empty enum, can never be constructed)
   and is used to perform pre-update checks but not the update itself.

Happy to hear suggestions about how to make this better.

One path I went down but rejected is using a typestate to indicate that
update checks had passed -- then the caller could decide whether to
perform the update or not. The problem is that for the typestate to be
valid it would have to hold on to the `MutexGuard` (otherwise something
could come in between and replace the task that we thought was
finished), and that seems a bit fraught as you could accidentally
attempt to lock the update data again. A callback-like approach, which
was the previous implementation and which has been retained in this PR,
does not have that pitfall.

I tested this by spinning up sp-sim, mgs, and wicketd, and it worked
as expected. Errors (e.g. no inventory present) were caught as
expected.
  • Loading branch information
sunshowers authored Sep 29, 2023
1 parent 8fb68a2 commit b03dd6b
Show file tree
Hide file tree
Showing 9 changed files with 595 additions and 302 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

97 changes: 52 additions & 45 deletions openapi/wicketd.json
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,33 @@
}
}
},
"/update": {
"post": {
"summary": "An endpoint to start updating one or more sleds, switches and PSCs.",
"operationId": "post_start_update",
"requestBody": {
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/StartUpdateParams"
}
}
},
"required": true
},
"responses": {
"204": {
"description": "resource updated"
},
"4XX": {
"$ref": "#/components/responses/Error"
},
"5XX": {
"$ref": "#/components/responses/Error"
}
}
}
},
"/update/{type}/{slot}": {
"get": {
"summary": "An endpoint to get the status of any update being performed or recently",
Expand Down Expand Up @@ -641,51 +668,6 @@
"$ref": "#/components/responses/Error"
}
}
},
"post": {
"summary": "An endpoint to start updating a sled.",
"operationId": "post_start_update",
"parameters": [
{
"in": "path",
"name": "slot",
"required": true,
"schema": {
"type": "integer",
"format": "uint32",
"minimum": 0
}
},
{
"in": "path",
"name": "type",
"required": true,
"schema": {
"$ref": "#/components/schemas/SpType"
}
}
],
"requestBody": {
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/StartUpdateOptions"
}
}
},
"required": true
},
"responses": {
"204": {
"description": "resource updated"
},
"4XX": {
"$ref": "#/components/responses/Error"
},
"5XX": {
"$ref": "#/components/responses/Error"
}
}
}
}
},
Expand Down Expand Up @@ -2761,6 +2743,31 @@
"skip_sp_version_check"
]
},
"StartUpdateParams": {
"type": "object",
"properties": {
"options": {
"description": "Options for the update.",
"allOf": [
{
"$ref": "#/components/schemas/StartUpdateOptions"
}
]
},
"targets": {
"description": "The SP identifiers to start the update with. Must be non-empty.",
"type": "array",
"items": {
"$ref": "#/components/schemas/SpIdentifier"
},
"uniqueItems": true
}
},
"required": [
"options",
"targets"
]
},
"StepComponentSummaryForGenericSpec": {
"type": "object",
"properties": {
Expand Down
11 changes: 6 additions & 5 deletions wicket/src/wicketd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use tokio::time::{interval, Duration, MissedTickBehavior};
use wicketd_client::types::{
AbortUpdateOptions, ClearUpdateStateOptions, GetInventoryParams,
GetInventoryResponse, GetLocationResponse, IgnitionCommand, SpIdentifier,
SpType, StartUpdateOptions,
SpType, StartUpdateOptions, StartUpdateParams,
};

use crate::events::EventReportMap;
Expand Down Expand Up @@ -164,10 +164,11 @@ impl WicketdManager {
tokio::spawn(async move {
let update_client =
create_wicketd_client(&log, addr, WICKETD_TIMEOUT);
let sp: SpIdentifier = component_id.into();
let response = match update_client
.post_start_update(sp.type_, sp.slot, &options)
.await
let params = StartUpdateParams {
targets: vec![component_id.into()],
options,
};
let response = match update_client.post_start_update(&params).await
{
Ok(_) => Ok(()),
Err(error) => Err(error.to_string()),
Expand Down
1 change: 1 addition & 0 deletions wicketd/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ hubtools.workspace = true
http.workspace = true
hyper.workspace = true
illumos-utils.workspace = true
itertools.workspace = true
reqwest.workspace = true
schemars.workspace = true
serde.workspace = true
Expand Down
41 changes: 41 additions & 0 deletions wicketd/src/helpers.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// 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/.

//! Helpers and utility functions for wicketd.
use std::fmt;

use gateway_client::types::{SpIdentifier, SpType};
use itertools::Itertools;

#[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash)]
pub(crate) struct SpIdentifierDisplay(pub(crate) SpIdentifier);

impl From<SpIdentifier> for SpIdentifierDisplay {
fn from(id: SpIdentifier) -> Self {
SpIdentifierDisplay(id)
}
}

impl<'a> From<&'a SpIdentifier> for SpIdentifierDisplay {
fn from(id: &'a SpIdentifier) -> Self {
SpIdentifierDisplay(*id)
}
}

impl fmt::Display for SpIdentifierDisplay {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self.0.type_ {
SpType::Sled => write!(f, "sled {}", self.0.slot),
SpType::Switch => write!(f, "switch {}", self.0.slot),
SpType::Power => write!(f, "PSC {}", self.0.slot),
}
}
}

pub(crate) fn sps_to_string<S: Into<SpIdentifierDisplay>>(
sps: impl IntoIterator<Item = S>,
) -> String {
sps.into_iter().map_into().join(", ")
}
Loading

0 comments on commit b03dd6b

Please sign in to comment.