Skip to content

Commit

Permalink
Merge branch 'main' into ajs/vmm-reservoir-bg-thread
Browse files Browse the repository at this point in the history
  • Loading branch information
andrewjstone committed Mar 7, 2024
2 parents c611a81 + 81447ec commit 9027e5b
Show file tree
Hide file tree
Showing 17 changed files with 264 additions and 120 deletions.
8 changes: 4 additions & 4 deletions Cargo.lock

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

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ rand = "0.8.5"
ratatui = "0.26.1"
rayon = "1.9"
rcgen = "0.12.1"
reedline = "0.29.0"
reedline = "0.30.0"
ref-cast = "1.0"
regex = "1.10.3"
regress = "0.8.0"
Expand Down
20 changes: 20 additions & 0 deletions nexus/src/app/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1183,6 +1183,25 @@ impl super::Nexus {
let ssh_keys: Vec<String> =
ssh_keys.map(|ssh_key| ssh_key.public_key).collect();

// Construct instance metadata used to track its statistics.
//
// This requires another fetch on the silo and project, to extract their
// IDs.
let (.., db_project) = self
.project_lookup(
opctx,
params::ProjectSelector {
project: NameOrId::Id(db_instance.project_id),
},
)?
.fetch()
.await?;
let (_, db_silo) = self.current_silo_lookup(opctx)?.fetch().await?;
let metadata = sled_agent_client::types::InstanceMetadata {
silo_id: db_silo.id(),
project_id: db_project.id(),
};

// Ask the sled agent to begin the state change. Then update the
// database to reflect the new intermediate state. If this update is
// not the newest one, that's fine. That might just mean the sled agent
Expand Down Expand Up @@ -1226,6 +1245,7 @@ impl super::Nexus {
initial_vmm.propolis_port.into(),
)
.to_string(),
metadata,
},
)
.await
Expand Down
27 changes: 27 additions & 0 deletions openapi/sled-agent.json
Original file line number Diff line number Diff line change
Expand Up @@ -4708,6 +4708,14 @@
}
]
},
"metadata": {
"description": "Metadata used to track instance statistics.",
"allOf": [
{
"$ref": "#/components/schemas/InstanceMetadata"
}
]
},
"propolis_addr": {
"description": "The address at which this VMM should serve a Propolis server API.",
"type": "string"
Expand All @@ -4729,6 +4737,7 @@
"required": [
"hardware",
"instance_runtime",
"metadata",
"propolis_addr",
"propolis_id",
"vmm_runtime"
Expand Down Expand Up @@ -4860,6 +4869,24 @@
"snapshot_id"
]
},
"InstanceMetadata": {
"description": "Metadata used to track statistics about an instance.",
"type": "object",
"properties": {
"project_id": {
"type": "string",
"format": "uuid"
},
"silo_id": {
"type": "string",
"format": "uuid"
}
},
"required": [
"project_id",
"silo_id"
]
},
"InstanceMigrationSourceParams": {
"description": "Instance runtime state to update for a migration.",
"type": "object",
Expand Down
43 changes: 31 additions & 12 deletions oximeter/instruments/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,46 @@ edition = "2021"
license = "MPL-2.0"

[dependencies]
cfg-if.workspace = true
chrono.workspace = true
dropshot.workspace = true
futures.workspace = true
cfg-if = { workspace = true, optional = true }
chrono = { workspace = true, optional = true }
dropshot = { workspace = true, optional = true }
futures = { workspace = true, optional = true }
http = { workspace = true, optional = true }
oximeter.workspace = true
slog.workspace = true
tokio.workspace = true
thiserror.workspace = true
uuid.workspace = true
oximeter = { workspace = true, optional = true }
slog = { workspace = true, optional = true }
tokio = { workspace = true, optional = true }
thiserror = { workspace = true, optional = true }
uuid = { workspace = true, optional = true }
omicron-workspace-hack.workspace = true

[features]
default = ["http-instruments", "kstat"]
http-instruments = ["http"]
kstat = ["kstat-rs"]
default = ["http-instruments", "datalink"]
http-instruments = [
"dep:chrono",
"dep:dropshot",
"dep:futures",
"dep:http",
"dep:oximeter",
"dep:uuid"
]
kstat = [
"dep:cfg-if",
"dep:chrono",
"dep:futures",
"dep:kstat-rs",
"dep:oximeter",
"dep:slog",
"dep:tokio",
"dep:thiserror",
"dep:uuid"
]
datalink = ["kstat"]

[dev-dependencies]
rand.workspace = true
slog-async.workspace = true
slog-term.workspace = true
oximeter.workspace = true

[target.'cfg(target_os = "illumos")'.dependencies]
kstat-rs = { workspace = true, optional = true }
9 changes: 5 additions & 4 deletions oximeter/instruments/src/kstat/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// 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/.

// Copyright 2023 Oxide Computer Company
// Copyright 2024 Oxide Computer Company

//! Types for publishing kernel statistics via oximeter.
//!
Expand Down Expand Up @@ -87,6 +87,7 @@ use std::cmp::Ordering;
use std::collections::BTreeMap;
use std::time::Duration;

#[cfg(any(feature = "datalink", test))]
pub mod link;
mod sampler;

Expand Down Expand Up @@ -206,9 +207,9 @@ pub fn hrtime_to_utc(hrtime: i64) -> Result<DateTime<Utc>, Error> {
}
}

// Helper trait for converting a `NamedData` item into a specific contained data
// type, if possible.
pub(crate) trait ConvertNamedData {
/// Helper trait for converting a `NamedData` item into a specific contained data
/// type, if possible.
pub trait ConvertNamedData {
fn as_i32(&self) -> Result<i32, Error>;
fn as_u32(&self) -> Result<u32, Error>;
fn as_i64(&self) -> Result<i64, Error>;
Expand Down
4 changes: 2 additions & 2 deletions oximeter/instruments/src/kstat/sampler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,7 @@ impl KstatSamplerWorker {
let n_current_samples = current_samples.len();
let n_total_samples = n_new_samples + n_current_samples;
let n_overflow_samples =
n_total_samples.checked_sub(self.sample_limit).unwrap_or(0);
n_total_samples.saturating_sub(self.sample_limit);
if n_overflow_samples > 0 {
warn!(
self.log,
Expand Down Expand Up @@ -788,7 +788,7 @@ impl KstatSamplerWorker {
// or the time we added the kstat if not.
let start = sampled_kstat
.time_of_last_collection
.unwrap_or_else(|| sampled_kstat.time_added);
.unwrap_or(sampled_kstat.time_added);
let expire_at = start + duration;
cfg_if::cfg_if! {
if #[cfg(test)] {
Expand Down
2 changes: 1 addition & 1 deletion oximeter/instruments/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

//! General-purpose types for instrumenting code to producer oximeter metrics.

// Copyright 2023 Oxide Computer Company
// Copyright 2024 Oxide Computer Company

#[cfg(feature = "http-instruments")]
pub mod http;
Expand Down
22 changes: 15 additions & 7 deletions oximeter/producer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ pub enum Error {
#[error("Error running producer HTTP server: {0}")]
Server(String),

#[error("Error registering as metric producer: {0}")]
RegistrationError(String),
#[error("Error registering as metric producer: {msg}")]
RegistrationError { retryable: bool, msg: String },

#[error("Producer registry and config UUIDs do not match")]
UuidMismatch,
Expand Down Expand Up @@ -251,11 +251,19 @@ pub async fn register(
) -> Result<(), Error> {
let client =
nexus_client::Client::new(&format!("http://{}", address), log.clone());
client
.cpapi_producers_post(&server_info.into())
.await
.map(|_| ())
.map_err(|msg| Error::RegistrationError(msg.to_string()))
client.cpapi_producers_post(&server_info.into()).await.map(|_| ()).map_err(
|err| {
let retryable = match &err {
nexus_client::Error::CommunicationError(..) => true,
nexus_client::Error::ErrorResponse(resp) => {
resp.status().is_server_error()
}
_ => false,
};
let msg = err.to_string();
Error::RegistrationError { retryable, msg }
},
)
}

/// Handle a request to pull available metric data from a [`ProducerRegistry`].
Expand Down
1 change: 1 addition & 0 deletions sled-agent/src/http_entrypoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,7 @@ async fn instance_register(
body_args.instance_runtime,
body_args.vmm_runtime,
body_args.propolis_addr,
body_args.metadata,
)
.await?,
))
Expand Down
11 changes: 10 additions & 1 deletion sled-agent/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use crate::nexus::NexusClientWithResolver;
use crate::params::ZoneBundleMetadata;
use crate::params::{InstanceExternalIpBody, ZoneBundleCause};
use crate::params::{
InstanceHardware, InstanceMigrationSourceParams,
InstanceHardware, InstanceMetadata, InstanceMigrationSourceParams,
InstanceMigrationTargetParams, InstancePutStateResponse,
InstanceStateRequested, InstanceUnregisterResponse, VpcFirewallRule,
};
Expand Down Expand Up @@ -311,6 +311,11 @@ struct InstanceRunner {
// Properties visible to Propolis
properties: propolis_client::types::InstanceProperties,

// This is currently unused, but will be sent to Propolis as part of the
// work tracked in https://github.com/oxidecomputer/omicron/issues/4851. It
// will be included in the InstanceProperties above, most likely.
_metadata: InstanceMetadata,

// The ID of the Propolis server (and zone) running this instance
propolis_id: Uuid,

Expand Down Expand Up @@ -928,6 +933,7 @@ impl Instance {
ticket: InstanceTicket,
state: InstanceInitialState,
services: InstanceManagerServices,
metadata: InstanceMetadata,
) -> Result<Self, Error> {
info!(log, "initializing new Instance";
"instance_id" => %id,
Expand Down Expand Up @@ -1005,6 +1011,9 @@ impl Instance {
// InstanceCpuCount here, to avoid any casting...
vcpus: hardware.properties.ncpus.0 as u8,
},
// This will be used in a follow up, tracked under
// https://github.com/oxidecomputer/omicron/issues/4851.
_metadata: metadata,
propolis_id,
propolis_addr,
vnic_allocator,
Expand Down
Loading

0 comments on commit 9027e5b

Please sign in to comment.