Skip to content

Commit e53c3e3

Browse files
hawkwjgallagher
andauthored
[gateway] update MGS Git dependency (#7924)
It turns out that our Git dependency on the oxidecomputer/management-gateway-service repo hasn't been updated in... a while. We're currently on a commit from September of last year, oxidecomputer/management-gateway-service@9bbac47. This branch updates it to the current HEAD commit, oxidecomputer/management-gateway-service@f9566e6. The only changes in MGS that required code changes in Omicron are: - oxidecomputer/management-gateway-service#291, where I added a new `MeasurementKind` for AMD CPU T<sub>ctl</sub> values (which are not temperatures in degrees Celcius, but a secret third thing). - oxidecomputer/management-gateway-service#316 by @mkeeter, adding the interface to read SP task dumps over the network. Since this adds methods to the `sp_impl::SpHandler` trait, the SP simulator implementations need to be updated, or else they will no longer compile. For now, I've just made these `unimplemented!()`, as we're not currently actually _using_ them. In my PR #7903 implementing ereport ingestion from SPs, I had to make these changes as part of changing the MGS dependency to pull in the new `gateway-sp-comms` code for ereports. Since this isn't actually related, and is just necessary to update the Git dep, I figured I'd pull that commit (49973ae) into its own PR. --------- Co-authored-by: John Gallagher <[email protected]>
1 parent 62c3dd6 commit e53c3e3

File tree

10 files changed

+179
-79
lines changed

10 files changed

+179
-79
lines changed

Cargo.lock

Lines changed: 62 additions & 56 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -415,8 +415,8 @@ gateway-client = { path = "clients/gateway-client" }
415415
# is "fine", because SP/MGS communication maintains forwards and backwards
416416
# compatibility, but will mean that faux-mgs might be missing new
417417
# functionality.)
418-
gateway-messages = { git = "https://github.com/oxidecomputer/management-gateway-service", rev = "9bbac475dcaac88286c07a20b6bd3e94fc81d7f0", default-features = false, features = ["std"] }
419-
gateway-sp-comms = { git = "https://github.com/oxidecomputer/management-gateway-service", rev = "9bbac475dcaac88286c07a20b6bd3e94fc81d7f0" }
418+
gateway-messages = { git = "https://github.com/oxidecomputer/management-gateway-service", rev = "f9566e68e0a0ccb7c3eeea081ae1cea279c11b2a", default-features = false, features = ["std"] }
419+
gateway-sp-comms = { git = "https://github.com/oxidecomputer/management-gateway-service", rev = "f9566e68e0a0ccb7c3eeea081ae1cea279c11b2a" }
420420
gateway-test-utils = { path = "gateway-test-utils" }
421421
gateway-types = { path = "gateway-types" }
422422
gethostname = "0.5.0"

dev-tools/omdb/src/bin/omdb/mgs/sensors.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,13 @@ pub(crate) struct Sensor {
119119
impl Sensor {
120120
fn units(&self) -> &str {
121121
match self.kind {
122+
MeasurementKind::CpuTctl => {
123+
// Part of me kiiiind of wants to make this "%", as Tctl is
124+
// always in the range of 0-100 --- it's essentially a sort of
125+
// abstract "thermal shutdown Badness Percentage". But, nobody
126+
// else seems to format it as a percentage AFAIK...
127+
""
128+
}
122129
MeasurementKind::Temperature => "°C",
123130
MeasurementKind::Current | MeasurementKind::InputCurrent => "A",
124131
MeasurementKind::Voltage | MeasurementKind::InputVoltage => "V",
@@ -149,6 +156,7 @@ impl Sensor {
149156

150157
fn to_kind_string(&self) -> &str {
151158
match self.kind {
159+
MeasurementKind::CpuTctl => "cpu-tctl",
152160
MeasurementKind::Temperature => "temp",
153161
MeasurementKind::Power => "power",
154162
MeasurementKind::Current => "current",
@@ -161,6 +169,7 @@ impl Sensor {
161169

162170
fn from_string(name: &str, kind: &str) -> Option<Self> {
163171
let k = match kind {
172+
"cpu-tctl" => Some(MeasurementKind::CpuTctl),
164173
"temp" | "temperature" => Some(MeasurementKind::Temperature),
165174
"power" => Some(MeasurementKind::Power),
166175
"current" => Some(MeasurementKind::Current),

gateway-types/src/component_details.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,7 @@ pub enum MeasurementKind {
374374
InputCurrent,
375375
InputVoltage,
376376
Speed,
377+
CpuTctl,
377378
}
378379

379380
impl From<gateway_messages::measurement::MeasurementKind> for MeasurementKind {
@@ -387,6 +388,7 @@ impl From<gateway_messages::measurement::MeasurementKind> for MeasurementKind {
387388
MeasurementKind::InputCurrent => Self::InputCurrent,
388389
MeasurementKind::InputVoltage => Self::InputVoltage,
389390
MeasurementKind::Speed => Self::Speed,
391+
MeasurementKind::CpuTctl => Self::CpuTctl,
390392
}
391393
}
392394
}

gateway/src/metrics.rs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -791,11 +791,13 @@ impl SpPoller {
791791
// We do this first so that we only have to clone the
792792
// sensor's name if there's an error, rather than always
793793
// cloning it in *case* there's an error.
794+
const TCTL_NAME: &str = "amd_cpu_tctl";
794795
if let Err(error) = m.value {
795796
let kind = match m.kind {
796797
MeasurementKind::Temperature if is_tctl => {
797-
"amd_cpu_tctl"
798+
TCTL_NAME
798799
}
800+
MeasurementKind::CpuTctl => TCTL_NAME,
799801
MeasurementKind::Temperature => "temperature",
800802
MeasurementKind::Current => "current",
801803
MeasurementKind::Voltage => "voltage",
@@ -878,6 +880,17 @@ impl SpPoller {
878880
&metric::Temperature { sensor, datum: 0.0 },
879881
)
880882
}
883+
884+
(Ok(datum), MeasurementKind::CpuTctl) => Sample::new(
885+
target,
886+
&metric::AmdCpuTctl { sensor, datum },
887+
),
888+
(Err(_), MeasurementKind::CpuTctl) => {
889+
Sample::new_missing(
890+
target,
891+
&metric::AmdCpuTctl { sensor, datum: 0.0 },
892+
)
893+
}
881894
(Ok(datum), MeasurementKind::Current) => Sample::new(
882895
target,
883896
&metric::Current { sensor, datum },
@@ -1204,5 +1217,8 @@ fn comms_error_str(error: CommunicationError) -> &'static str {
12041217
CommunicationError::BadTrailingDataSize { .. } => {
12051218
"bad_trailing_data_size"
12061219
}
1220+
CommunicationError::BadDecompressionSize { .. } => {
1221+
"bad_decompression_size"
1222+
}
12071223
}
12081224
}

nexus/tests/integration_tests/metrics.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -712,6 +712,7 @@ async fn test_mgs_metrics(
712712
for sensor in &component.sensors {
713713
use gateway_messages::measurement::MeasurementKind as Kind;
714714
match sensor.def.kind {
715+
Kind::CpuTctl => cpu_tctl += 1,
715716
Kind::Temperature => {
716717
// Currently, Tctl measurements are reported as a
717718
// "temperature" measurement, but are tracked by a

openapi/gateway.json

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1953,6 +1953,20 @@
19531953
"required": [
19541954
"kind"
19551955
]
1956+
},
1957+
{
1958+
"type": "object",
1959+
"properties": {
1960+
"kind": {
1961+
"type": "string",
1962+
"enum": [
1963+
"cpu_tctl"
1964+
]
1965+
}
1966+
},
1967+
"required": [
1968+
"kind"
1969+
]
19561970
}
19571971
]
19581972
},

sp-sim/src/gimlet.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ use futures::future;
2222
use gateway_messages::CfpaPage;
2323
use gateway_messages::ComponentAction;
2424
use gateway_messages::ComponentActionResponse;
25+
use gateway_messages::DumpSegment;
26+
use gateway_messages::DumpTask;
2527
use gateway_messages::Header;
2628
use gateway_messages::MgsRequest;
2729
use gateway_messages::MgsResponse;
@@ -1502,6 +1504,30 @@ impl SpHandler for Handler {
15021504
}
15031505
}
15041506
}
1507+
1508+
fn get_task_dump_count(&mut self) -> Result<u32, SpError> {
1509+
debug!(&self.log, "received get_task_dump_count");
1510+
Err(SpError::RequestUnsupportedForSp)
1511+
}
1512+
1513+
fn task_dump_read_start(
1514+
&mut self,
1515+
index: u32,
1516+
_key: [u8; 16],
1517+
) -> Result<DumpTask, SpError> {
1518+
debug!(&self.log, "received task_dump_read_start"; "index" => index);
1519+
Err(SpError::RequestUnsupportedForSp)
1520+
}
1521+
1522+
fn task_dump_read_continue(
1523+
&mut self,
1524+
_key: [u8; 16],
1525+
seq: u32,
1526+
_buf: &mut [u8],
1527+
) -> Result<Option<DumpSegment>, SpError> {
1528+
debug!(&self.log, "received task_dump_read_continue"; "seq" => seq);
1529+
Err(SpError::RequestUnsupportedForSp)
1530+
}
15051531
}
15061532

15071533
impl SimSpHandler for Handler {

sp-sim/src/sidecar.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ use gateway_messages::ComponentAction;
2727
use gateway_messages::ComponentActionResponse;
2828
use gateway_messages::ComponentDetails;
2929
use gateway_messages::DiscoverResponse;
30+
use gateway_messages::DumpSegment;
31+
use gateway_messages::DumpTask;
3032
use gateway_messages::IgnitionCommand;
3133
use gateway_messages::IgnitionState;
3234
use gateway_messages::MgsError;
@@ -1202,6 +1204,30 @@ impl SpHandler for Handler {
12021204
}
12031205
}
12041206
}
1207+
1208+
fn get_task_dump_count(&mut self) -> Result<u32, SpError> {
1209+
debug!(&self.log, "received get_task_dump_count");
1210+
Err(SpError::RequestUnsupportedForSp)
1211+
}
1212+
1213+
fn task_dump_read_start(
1214+
&mut self,
1215+
index: u32,
1216+
_key: [u8; 16],
1217+
) -> Result<DumpTask, SpError> {
1218+
debug!(&self.log, "received task_dump_read_start"; "index" => index);
1219+
Err(SpError::RequestUnsupportedForSp)
1220+
}
1221+
1222+
fn task_dump_read_continue(
1223+
&mut self,
1224+
_key: [u8; 16],
1225+
seq: u32,
1226+
_buf: &mut [u8],
1227+
) -> Result<Option<DumpSegment>, SpError> {
1228+
debug!(&self.log, "received task_dump_read_continue"; "seq" => seq);
1229+
Err(SpError::RequestUnsupportedForSp)
1230+
}
12051231
}
12061232

12071233
impl SimSpHandler for Handler {

0 commit comments

Comments
 (0)