From 27f5ff81009c620a00f511ce241fc5176c8dff48 Mon Sep 17 00:00:00 2001 From: "Andrew J. Stone" Date: Wed, 9 Nov 2022 20:27:24 -0500 Subject: [PATCH] Add wicketd-client (#1930) Add a progenitor generated-client using the wicketd openapi spec. The structure of the output for the `/inventory` endpoint was changed since tuples are unsupported. While doing this, I prepared for further changes that will include a `Vec` per SP. That code is currently awaiting some progenitor additions to allow deriving types like `Eq, Ord`, etc... on arbitrary types so that wicket can reuse MGS types directly. While waiting for those changes, I'll begin integrating wicket and wicketd. --- Cargo.lock | 19 ++ Cargo.toml | 1 + openapi/wicketd.json | 349 ++++++++++++++++++++ wicketd-client/Cargo.toml | 28 ++ wicketd-client/src/lib.rs | 9 + wicketd/Cargo.toml | 11 +- wicketd/src/http_entrypoints.rs | 2 +- wicketd/src/inventory.rs | 62 +++- wicketd/src/lib.rs | 2 +- wicketd/src/mgs.rs | 67 +++- wicketd/tests/integration_tests/commands.rs | 43 +++ wicketd/tests/integration_tests/mod.rs | 5 + wicketd/tests/mod.rs | 17 + 13 files changed, 597 insertions(+), 18 deletions(-) create mode 100644 openapi/wicketd.json create mode 100644 wicketd-client/Cargo.toml create mode 100644 wicketd-client/src/lib.rs create mode 100644 wicketd/tests/integration_tests/commands.rs create mode 100644 wicketd/tests/integration_tests/mod.rs create mode 100644 wicketd/tests/mod.rs diff --git a/Cargo.lock b/Cargo.lock index ddee917a9c..b4d9873d73 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7116,23 +7116,42 @@ version = "0.1.0" dependencies = [ "clap 4.0.18", "dropshot", + "expectorate", "futures", "gateway-client", "hex", "http", "hyper", "omicron-common", + "omicron-test-utils", + "openapi-lint", + "openapiv3", "reqwest", "schemars", "serde", + "serde_json", "slog", "slog-dtrace", "snafu", + "subprocess", "tokio", "toml", "uuid", ] +[[package]] +name = "wicketd-client" +version = "0.1.0" +dependencies = [ + "chrono", + "omicron-common", + "progenitor 0.2.1-dev (git+https://github.com/oxidecomputer/progenitor)", + "reqwest", + "serde", + "slog", + "uuid", +] + [[package]] name = "widestring" version = "0.5.1" diff --git a/Cargo.toml b/Cargo.toml index 3b6bd77245..b0e6f78b35 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,6 +36,7 @@ members = [ "test-utils", "wicket", "wicketd", + "wicketd-client" ] default-members = [ diff --git a/openapi/wicketd.json b/openapi/wicketd.json new file mode 100644 index 0000000000..033b819a5d --- /dev/null +++ b/openapi/wicketd.json @@ -0,0 +1,349 @@ +{ + "openapi": "3.0.3", + "info": { + "title": "Oxide Technician Port Control Service", + "description": "API for use by the technician port TUI: wicket", + "contact": { + "url": "https://oxide.computer", + "email": "api@oxide.computer" + }, + "version": "0.0.1" + }, + "paths": { + "/inventory": { + "get": { + "summary": "A status endpoint used to report high level information known to wicketd.", + "description": "This endpoint can be polled to see if there have been state changes in the system that are useful to report to wicket.\nWicket, and possibly other callers, will retrieve the changed information, with follow up calls.", + "operationId": "get_inventory", + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RackV1Inventory" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + } + }, + "components": { + "responses": { + "Error": { + "description": "Error", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + } + }, + "schemas": { + "Error": { + "description": "Error information from a response.", + "type": "object", + "properties": { + "error_code": { + "type": "string" + }, + "message": { + "type": "string" + }, + "request_id": { + "type": "string" + } + }, + "required": [ + "message", + "request_id" + ] + }, + "RackV1Inventory": { + "description": "The current state of the v1 Rack as known to wicketd", + "type": "object", + "properties": { + "sps": { + "type": "array", + "items": { + "$ref": "#/components/schemas/SpInventory" + } + } + }, + "required": [ + "sps" + ] + }, + "SpComponentInfo": { + "description": "Overview of a single SP component.", + "type": "object", + "properties": { + "capabilities": { + "description": "`capabilities` is a bitmask; interpret it via [`gateway_messages::DeviceCapabilities`].", + "type": "integer", + "format": "uint32", + "minimum": 0 + }, + "component": { + "description": "The unique identifier for this component.", + "type": "string" + }, + "description": { + "description": "A human-readable description of the component.", + "type": "string" + }, + "device": { + "description": "The name of the physical device.", + "type": "string" + }, + "presence": { + "description": "Whether or not the component is present, to the best of the SP's ability to judge.", + "allOf": [ + { + "$ref": "#/components/schemas/SpComponentPresence" + } + ] + }, + "serial_number": { + "nullable": true, + "description": "The component's serial number, if it has one.", + "type": "string" + } + }, + "required": [ + "capabilities", + "component", + "description", + "device", + "presence" + ] + }, + "SpComponentPresence": { + "description": "Description of the presence or absence of a component.\n\nThe presence of some components may vary based on the power state of the sled (e.g., components that time out or appear unavailable if the sled is in A2 may become present when the sled moves to A0).", + "oneOf": [ + { + "description": "The component is present.", + "type": "string", + "enum": [ + "present" + ] + }, + { + "description": "The component is not present.", + "type": "string", + "enum": [ + "not_present" + ] + }, + { + "description": "The component is present but in a failed or faulty state.", + "type": "string", + "enum": [ + "failed" + ] + }, + { + "description": "The SP is unable to determine the presence of the component.", + "type": "string", + "enum": [ + "unavailable" + ] + }, + { + "description": "The SP's attempt to determine the presence of the component timed out.", + "type": "string", + "enum": [ + "timeout" + ] + }, + { + "description": "The SP's attempt to determine the presence of the component failed.", + "type": "string", + "enum": [ + "error" + ] + } + ] + }, + "SpId": { + "description": "A local type we can use as a key to a BTreeMap.\n\nThis maps directly to `gateway_client::types::SpIdentifer`, but we create a separate type because the progenitor generated type doesn't derive Ord/PartialOrd.", + "type": "object", + "properties": { + "slot": { + "type": "integer", + "format": "uint32", + "minimum": 0 + }, + "type": { + "$ref": "#/components/schemas/SpType" + } + }, + "required": [ + "slot", + "type" + ] + }, + "SpIgnition": { + "oneOf": [ + { + "type": "object", + "properties": { + "present": { + "type": "string", + "enum": [ + "no" + ] + } + }, + "required": [ + "present" + ] + }, + { + "type": "object", + "properties": { + "ctrl_detect_0": { + "type": "boolean" + }, + "ctrl_detect_1": { + "type": "boolean" + }, + "flt_a2": { + "type": "boolean" + }, + "flt_a3": { + "type": "boolean" + }, + "flt_rot": { + "type": "boolean" + }, + "flt_sp": { + "type": "boolean" + }, + "id": { + "type": "integer", + "format": "uint16", + "minimum": 0 + }, + "power": { + "type": "boolean" + }, + "present": { + "type": "string", + "enum": [ + "yes" + ] + } + }, + "required": [ + "ctrl_detect_0", + "ctrl_detect_1", + "flt_a2", + "flt_a3", + "flt_rot", + "flt_sp", + "id", + "power", + "present" + ] + } + ] + }, + "SpInventory": { + "description": "SP related data", + "type": "object", + "properties": { + "components": { + "type": "array", + "items": { + "$ref": "#/components/schemas/SpComponentInfo" + } + }, + "id": { + "$ref": "#/components/schemas/SpId" + }, + "ignition": { + "$ref": "#/components/schemas/SpIgnition" + }, + "state": { + "$ref": "#/components/schemas/SpState" + } + }, + "required": [ + "components", + "id", + "ignition", + "state" + ] + }, + "SpState": { + "oneOf": [ + { + "type": "object", + "properties": { + "state": { + "type": "string", + "enum": [ + "disabled" + ] + } + }, + "required": [ + "state" + ] + }, + { + "type": "object", + "properties": { + "state": { + "type": "string", + "enum": [ + "unresponsive" + ] + } + }, + "required": [ + "state" + ] + }, + { + "type": "object", + "properties": { + "serial_number": { + "type": "string" + }, + "state": { + "type": "string", + "enum": [ + "enabled" + ] + } + }, + "required": [ + "serial_number", + "state" + ] + } + ] + }, + "SpType": { + "type": "string", + "enum": [ + "sled", + "power", + "switch" + ] + } + } + } +} \ No newline at end of file diff --git a/wicketd-client/Cargo.toml b/wicketd-client/Cargo.toml new file mode 100644 index 0000000000..9fbc95409f --- /dev/null +++ b/wicketd-client/Cargo.toml @@ -0,0 +1,28 @@ +[package] +name = "wicketd-client" +version = "0.1.0" +edition = "2021" +license = "MPL-2.0" + +[dependencies] +progenitor = { git = "https://github.com/oxidecomputer/progenitor" } +reqwest = { version = "0.11.12", default-features = false, features = ["rustls-tls", "stream"] } + +[dependencies.chrono] +version = "0.4" +features = [ "serde" ] + +[dependencies.omicron-common] +path = "../common" + +[dependencies.serde] +version = "1.0" +features = [ "derive" ] + +[dependencies.slog] +version = "2.5" +features = [ "max_level_trace", "release_max_level_debug" ] + +[dependencies.uuid] +version = "1.2.1" +features = [ "serde", "v4" ] diff --git a/wicketd-client/src/lib.rs b/wicketd-client/src/lib.rs new file mode 100644 index 0000000000..dd7342e089 --- /dev/null +++ b/wicketd-client/src/lib.rs @@ -0,0 +1,9 @@ +// 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/. + +//! Interface for making API requests to wicketd + +use omicron_common::generate_logging_api; + +generate_logging_api!("../openapi/wicketd.json"); diff --git a/wicketd/Cargo.toml b/wicketd/Cargo.toml index 5b3b5d3636..1f306ccaed 100644 --- a/wicketd/Cargo.toml +++ b/wicketd/Cargo.toml @@ -32,4 +32,13 @@ features = [ "full" ] [[bin]] name = "wicketd" -doc = false \ No newline at end of file +doc = false + +[dev-dependencies] +expectorate = "1.0.5" +http = "0.2.7" +omicron-test-utils = { path = "../test-utils" } +openapi-lint = { git = "https://github.com/oxidecomputer/openapi-lint", branch = "main" } +openapiv3 = "1.0" +serde_json = "1.0" +subprocess = "0.2.9" diff --git a/wicketd/src/http_entrypoints.rs b/wicketd/src/http_entrypoints.rs index 5b1c8454c6..b30b646e9d 100644 --- a/wicketd/src/http_entrypoints.rs +++ b/wicketd/src/http_entrypoints.rs @@ -45,7 +45,7 @@ pub fn api() -> WicketdApiDescription { }] async fn get_inventory( rqctx: Arc>, -) -> Result>, HttpError> { +) -> Result, HttpError> { match rqctx.context().mgs_handle.get_inventory().await { Ok(inventory) => Ok(HttpResponseOk(inventory)), Err(_) => { diff --git a/wicketd/src/inventory.rs b/wicketd/src/inventory.rs index a4b42d9453..396fb2eef4 100644 --- a/wicketd/src/inventory.rs +++ b/wicketd/src/inventory.rs @@ -4,12 +4,68 @@ //! Rack inventory for display by wicket -use gateway_client::types::{SpComponentInfo, SpInfo}; +use gateway_client::types::{ + SpComponentInfo, SpIdentifier, SpIgnition, SpState, SpType, +}; use schemars::JsonSchema; use serde::Serialize; +use std::fmt::Display; + +/// SP related data +#[derive(Debug, Clone, Serialize, JsonSchema)] +#[serde(tag = "sp_inventory", rename_all = "snake_case")] +pub struct SpInventory { + pub id: SpId, + pub ignition: SpIgnition, + pub state: SpState, + pub components: Vec, +} + +impl SpInventory { + /// The ignition info and state of the SP are retrieved initiailly + /// + /// The components are filled in via a separate call + pub fn new(id: SpId, ignition: SpIgnition, state: SpState) -> SpInventory { + SpInventory { id, ignition, state, components: vec![] } + } +} /// The current state of the v1 Rack as known to wicketd -#[derive(Default, Debug, Serialize, JsonSchema)] +#[derive(Default, Clone, Debug, Serialize, JsonSchema)] +#[serde(tag = "inventory", rename_all = "snake_case")] pub struct RackV1Inventory { - pub sps: Vec<(SpInfo, Vec)>, + pub sps: Vec, +} + +/// A local type we can use as a key to a BTreeMap. +/// +/// This maps directly to `gateway_client::types::SpIdentifer`, +/// but we create a separate type because the progenitor generated type +/// doesn't derive Ord/PartialOrd. +#[derive( + Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, JsonSchema, +)] +#[serde(tag = "sp_id", rename_all = "snake_case")] +pub struct SpId { + #[serde(rename = "type")] + pub typ: SpType, + pub slot: u32, +} + +impl From for SpId { + fn from(id: SpIdentifier) -> Self { + SpId { typ: id.type_, slot: id.slot } + } +} + +impl From for SpIdentifier { + fn from(id: SpId) -> Self { + SpIdentifier { type_: id.typ, slot: id.slot } + } +} + +impl Display for SpId { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{:?} {}", self.typ, self.slot) + } } diff --git a/wicketd/src/lib.rs b/wicketd/src/lib.rs index 087b23e4fe..9bd13208cd 100644 --- a/wicketd/src/lib.rs +++ b/wicketd/src/lib.rs @@ -10,7 +10,7 @@ mod mgs; pub use config::Config; pub(crate) use context::ServerContext; -pub use inventory::RackV1Inventory; +pub use inventory::{RackV1Inventory, SpId, SpInventory}; pub(crate) use mgs::{MgsHandle, MgsManager}; use dropshot::ConfigDropshot; diff --git a/wicketd/src/mgs.rs b/wicketd/src/mgs.rs index 40b4a3e8bf..909f0f6b63 100644 --- a/wicketd/src/mgs.rs +++ b/wicketd/src/mgs.rs @@ -5,11 +5,11 @@ //! The collection of tasks used for interacting with MGS and maintaining //! runtime state. -use crate::RackV1Inventory; +use crate::{RackV1Inventory, SpId, SpInventory}; use gateway_client::types::SpInfo; use slog::{debug, info, o, warn, Logger}; +use std::collections::{BTreeMap, BTreeSet}; use std::net::SocketAddrV6; -use std::sync::Arc; use tokio::sync::{mpsc, oneshot}; use tokio::time::{interval, Duration, MissedTickBehavior}; @@ -31,7 +31,7 @@ pub struct ShutdownInProgress; pub enum MgsRequest { GetInventory { etag: Option, - reply_tx: oneshot::Sender>, + reply_tx: oneshot::Sender, }, } @@ -43,7 +43,7 @@ pub struct MgsHandle { impl MgsHandle { pub async fn get_inventory( &self, - ) -> Result, ShutdownInProgress> { + ) -> Result { let (reply_tx, reply_rx) = oneshot::channel(); let etag = None; self.tx @@ -73,8 +73,7 @@ pub struct MgsManager { tx: mpsc::Sender, rx: mpsc::Receiver, mgs_client: gateway_client::Client, - // Remove the need to copy a potentially sizeable amount of data - inventory: Arc, + inventory: RackV1Inventory, } impl MgsManager { @@ -96,7 +95,7 @@ impl MgsManager { client, log.clone(), ); - let inventory = Arc::new(RackV1Inventory::default()); + let inventory = RackV1Inventory::default(); MgsManager { log, tx, rx, mgs_client, inventory } } @@ -106,15 +105,13 @@ impl MgsManager { pub async fn run(mut self) { let mgs_client = self.mgs_client; - let mut inventory_rx = poll_inventory(&self.log, mgs_client).await; + let mut inventory_rx = poll_sps(&self.log, mgs_client).await; loop { tokio::select! { // Poll MGS inventory Some(sps) = inventory_rx.recv() => { - self.inventory = Arc::new(RackV1Inventory { - sps: sps.into_iter().map(|sp| (sp, vec![])).collect() - }); + update_inventory(&mut self.inventory, sps); } // Handle requests from clients @@ -131,7 +128,53 @@ impl MgsManager { } } -pub async fn poll_inventory( +// For the latest set of sps returned from MGS: +// 1. Update their state if it has changed +// 2. Remove any SPs in our current inventory that aren't in the new state +fn update_inventory(inventory: &mut RackV1Inventory, sps: Vec) { + let new_keys: BTreeSet = + sps.iter().map(|sp| sp.info.id.clone().into()).collect(); + + // Remove all keys that are not in the latest update + let mut new_inventory: BTreeMap = inventory + .sps + .iter() + .filter_map(|sp| { + if new_keys.contains(&sp.id) { + Some((sp.id, sp.clone())) + } else { + None + } + }) + .collect(); + + // Update any existing SPs that have changed state + // or add any new ones. + for sp in sps.into_iter() { + let state = sp.details; + let id: SpId = sp.info.id.into(); + let ignition = sp.info.details; + + new_inventory + .entry(id) + .and_modify(|curr| { + // TODO: // Reset the components only if the state changes. + // This is blocked waiting on some small progenitor changes + // that will allow us to derive Eq/PartialEq, etc... + //if curr.state != state { + // Clear the components, so we can refetch them. We don't know + // if the actual component has changed or if it has been upgraded, etc.. + // curr.components = vec![]; + //} + curr.state = state.clone(); + curr.ignition = ignition.clone(); + }) + .or_insert(SpInventory::new(id, ignition, state)); + } + inventory.sps = new_inventory.into_values().collect(); +} + +async fn poll_sps( log: &Logger, client: gateway_client::Client, ) -> mpsc::Receiver> { diff --git a/wicketd/tests/integration_tests/commands.rs b/wicketd/tests/integration_tests/commands.rs new file mode 100644 index 0000000000..f14d608879 --- /dev/null +++ b/wicketd/tests/integration_tests/commands.rs @@ -0,0 +1,43 @@ +// 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/. + +//! Tests for the executable commands in this repo. Most functionality is tested +//! elsewhere, so this really just sanity checks argument parsing, bad args, and +//! the --openapi mode. + +use std::path::PathBuf; + +use expectorate::assert_contents; +use omicron_test_utils::dev::test_cmds::{ + assert_exit_code, path_to_executable, run_command, EXIT_SUCCESS, +}; +use openapiv3::OpenAPI; +use subprocess::Exec; + +// name of wicketd executable +const CMD_WICKETD: &str = env!("CARGO_BIN_EXE_wicketd"); + +fn path_to_wicketd() -> PathBuf { + path_to_executable(CMD_WICKETD) +} + +#[test] +fn test_wicketd_openapi() { + let exec = Exec::cmd(path_to_wicketd()).arg("openapi"); + let (exit_status, stdout_text, stderr_text) = run_command(exec); + assert_exit_code(exit_status, EXIT_SUCCESS, &stderr_text); + assert_contents("tests/output/cmd-wicketd-openapi-stderr", &stderr_text); + + let spec: OpenAPI = serde_json::from_str(&stdout_text) + .expect("stdout was not valid OpenAPI"); + + // Check for lint errors. + let errors = openapi_lint::validate(&spec); + assert!(errors.is_empty(), "{}", errors.join("\n\n")); + + // Confirm that the output hasn't changed. It's expected that we'll change + // this file as the API evolves, but pay attention to the diffs to ensure + // that the changes match your expectations. + assert_contents("../openapi/wicketd.json", &stdout_text); +} diff --git a/wicketd/tests/integration_tests/mod.rs b/wicketd/tests/integration_tests/mod.rs new file mode 100644 index 0000000000..1bf43dc00c --- /dev/null +++ b/wicketd/tests/integration_tests/mod.rs @@ -0,0 +1,5 @@ +// 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/. + +mod commands; diff --git a/wicketd/tests/mod.rs b/wicketd/tests/mod.rs new file mode 100644 index 0000000000..325a59bfe2 --- /dev/null +++ b/wicketd/tests/mod.rs @@ -0,0 +1,17 @@ +// 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/. + +//! Integration tests for the Sled Agent. +//! +//! Why use this weird layer of indirection, you might ask? Cargo chooses to +//! compile *each file* within the "tests/" subdirectory as a separate crate. +//! This means that doing "file-granularity" conditional compilation is +//! difficult, since a file like "test_for_illumos_only.rs" would get compiled +//! and tested regardless of the contents of "mod.rs". +//! +//! However, by lumping all tests into a submodule, all integration tests are +//! joined into a single crate, which itself can filter individual files +//! by (for example) choice of target OS. + +mod integration_tests;