Skip to content

Commit

Permalink
Make oximeter producer kind required
Browse files Browse the repository at this point in the history
- Pulls in updated Dendrite, Propolis, and Crucible deps, which include
  the new producer kind enum in metric registration requests. From their
  perspective, this is still an optional parameter, but it is supplied.
- Make the kind a required field in API requests.
- Make the kind a required column in the database, and remove any rows
  with a NULL value.
- Update OpenAPI documents and internal consumers to reflect the
  required parameter.
  • Loading branch information
bnaecker committed Nov 28, 2023
1 parent 91b0261 commit 4fb8f08
Show file tree
Hide file tree
Showing 20 changed files with 59 additions and 43 deletions.
2 changes: 1 addition & 1 deletion clients/nexus-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ impl From<&omicron_common::api::internal::nexus::ProducerEndpoint>
address: s.address.to_string(),
base_route: s.base_route.clone(),
id: s.id,
kind: s.kind.map(Into::into),
kind: s.kind.into(),
interval: s.interval.into(),
}
}
Expand Down
2 changes: 1 addition & 1 deletion clients/oximeter-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ impl From<&omicron_common::api::internal::nexus::ProducerEndpoint>
address: s.address.to_string(),
base_route: s.base_route.clone(),
id: s.id,
kind: s.kind.map(Into::into),
kind: s.kind.into(),
interval: s.interval.into(),
}
}
Expand Down
2 changes: 1 addition & 1 deletion common/src/api/internal/nexus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ pub struct ProducerEndpoint {
/// A unique ID for this producer.
pub id: Uuid,
/// The kind of producer.
pub kind: Option<ProducerKind>,
pub kind: ProducerKind,
/// The IP address and port at which `oximeter` can collect metrics from the
/// producer.
pub address: SocketAddr,
Expand Down
4 changes: 2 additions & 2 deletions nexus/db-model/src/producer_endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pub struct ProducerEndpoint {
#[diesel(embed)]
identity: ProducerEndpointIdentity,

pub kind: Option<ProducerKind>,
pub kind: ProducerKind,
pub ip: ipnetwork::IpNetwork,
pub port: SqlU16,
pub interval: f64,
Expand All @@ -69,7 +69,7 @@ impl ProducerEndpoint {
) -> Self {
Self {
identity: ProducerEndpointIdentity::new(endpoint.id),
kind: endpoint.kind.map(Into::into),
kind: endpoint.kind.into(),
ip: endpoint.address.ip().into(),
port: endpoint.address.port().into(),
base_route: endpoint.base_route.clone(),
Expand Down
4 changes: 2 additions & 2 deletions nexus/db-model/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ table! {
id -> Uuid,
time_created -> Timestamptz,
time_modified -> Timestamptz,
kind -> Nullable<crate::ProducerKindEnum>,
kind -> crate::ProducerKindEnum,
ip -> Inet,
port -> Int4,
interval -> Float8,
Expand Down Expand Up @@ -1299,7 +1299,7 @@ table! {
///
/// This should be updated whenever the schema is changed. For more details,
/// refer to: schema/crdb/README.adoc
pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(14, 0, 0);
pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(15, 0, 0);

allow_tables_to_appear_in_same_query!(
system_update,
Expand Down
6 changes: 2 additions & 4 deletions nexus/src/app/oximeter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,7 @@ impl super::Nexus {
for producer in producers.into_iter() {
let producer_info = oximeter_client::types::ProducerEndpoint {
id: producer.id(),
kind: producer
.kind
.map(|kind| nexus::ProducerKind::from(kind).into()),
kind: nexus::ProducerKind::from(producer.kind).into(),
address: SocketAddr::new(
producer.ip.ip(),
producer.port.try_into().unwrap(),
Expand All @@ -152,7 +150,7 @@ impl super::Nexus {
pub(crate) async fn register_as_producer(&self, address: SocketAddr) {
let producer_endpoint = nexus::ProducerEndpoint {
id: self.id,
kind: Some(nexus::ProducerKind::Service),
kind: nexus::ProducerKind::Service,
address,
base_route: String::from("/metrics/collect"),
interval: Duration::from_secs(10),
Expand Down
2 changes: 1 addition & 1 deletion nexus/test-utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1093,7 +1093,7 @@ pub async fn start_producer_server(
let producer_address = SocketAddr::new(Ipv6Addr::LOCALHOST.into(), 0);
let server_info = ProducerEndpoint {
id,
kind: Some(ProducerKind::Service),
kind: ProducerKind::Service,
address: producer_address,
base_route: "/collect".to_string(),
interval: Duration::from_secs(1),
Expand Down
2 changes: 1 addition & 1 deletion nexus/tests/integration_tests/oximeter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ async fn test_oximeter_collector_reregistration_gets_all_assignments() {
ids.insert(id);
let info = ProducerEndpoint {
id,
kind: Some(ProducerKind::Service),
kind: ProducerKind::Service,
address: SocketAddr::new(Ipv6Addr::LOCALHOST.into(), 12345),
base_route: String::from("/collect"),
interval: Duration::from_secs(1),
Expand Down
4 changes: 2 additions & 2 deletions openapi/nexus-internal.json
Original file line number Diff line number Diff line change
Expand Up @@ -4343,7 +4343,6 @@
]
},
"kind": {
"nullable": true,
"description": "The kind of producer.",
"allOf": [
{
Expand All @@ -4356,7 +4355,8 @@
"address",
"base_route",
"id",
"interval"
"interval",
"kind"
]
},
"ProducerKind": {
Expand Down
4 changes: 2 additions & 2 deletions openapi/oximeter.json
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,6 @@
]
},
"kind": {
"nullable": true,
"description": "The kind of producer.",
"allOf": [
{
Expand All @@ -225,7 +224,8 @@
"address",
"base_route",
"id",
"interval"
"interval",
"kind"
]
},
"ProducerEndpointResultsPage": {
Expand Down
6 changes: 3 additions & 3 deletions oximeter/collector/src/agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,7 @@ mod tests {
let interval = Duration::from_secs(1);
let endpoint = ProducerEndpoint {
id: Uuid::new_v4(),
kind: Some(ProducerKind::Service),
kind: ProducerKind::Service,
address,
base_route: String::from("/"),
interval,
Expand Down Expand Up @@ -754,7 +754,7 @@ mod tests {
let interval = Duration::from_secs(1);
let endpoint = ProducerEndpoint {
id: Uuid::new_v4(),
kind: Some(ProducerKind::Service),
kind: ProducerKind::Service,
address: SocketAddr::V6(SocketAddrV6::new(
Ipv6Addr::LOCALHOST,
0,
Expand Down Expand Up @@ -843,7 +843,7 @@ mod tests {
let interval = Duration::from_secs(1);
let endpoint = ProducerEndpoint {
id: Uuid::new_v4(),
kind: Some(ProducerKind::Service),
kind: ProducerKind::Service,
address,
base_route: String::from("/"),
interval,
Expand Down
2 changes: 1 addition & 1 deletion oximeter/producer/examples/producer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ async fn main() -> anyhow::Result<()> {
registry.register_producer(producer).unwrap();
let server_info = ProducerEndpoint {
id: registry.producer_id(),
kind: Some(ProducerKind::Service),
kind: ProducerKind::Service,
address: args.address,
base_route: "/collect".to_string(),
interval: Duration::from_secs(10),
Expand Down
28 changes: 14 additions & 14 deletions package-manifest.toml
Original file line number Diff line number Diff line change
Expand Up @@ -384,21 +384,21 @@ only_for_targets.image = "standard"
# 3. Use source.type = "manual" instead of "prebuilt"
source.type = "prebuilt"
source.repo = "crucible"
source.commit = "51a3121c8318fc7ac97d74f917ce1d37962e785f"
source.commit = "945f040d259ca8013d3fb26f510453da7cd7b1a6"
# The SHA256 digest is automatically posted to:
# https://buildomat.eng.oxide.computer/public/file/oxidecomputer/crucible/image/<commit>/crucible.sha256.txt
source.sha256 = "897d0fd6c0b82db42256a63a13c228152e1117434afa2681f649b291e3c6f46d"
# https://buildomat.eng.oxide.computer/public/file/oxidecomputer/crucible/rbuild/<commit>/crucible.sha256.txt
source.sha256 = "f8c23cbf89fd0bbd928d8e3db1357bbea6e6b50560e221f873da5b56ed9d7527"
output.type = "zone"

[package.crucible-pantry]
service_name = "crucible_pantry"
only_for_targets.image = "standard"
source.type = "prebuilt"
source.repo = "crucible"
source.commit = "51a3121c8318fc7ac97d74f917ce1d37962e785f"
source.commit = "945f040d259ca8013d3fb26f510453da7cd7b1a6"
# The SHA256 digest is automatically posted to:
# https://buildomat.eng.oxide.computer/public/file/oxidecomputer/crucible/image/<commit>/crucible-pantry.sha256.txt
source.sha256 = "fe545de7ac4f15454d7827927149c5f0fc68ce9545b4f1ef96aac9ac8039805a"
# https://buildomat.eng.oxide.computer/public/file/oxidecomputer/crucible/rbuild/<commit>/crucible-pantry.sha256.txt
source.sha256 = "a25b31c81798eb65564dbe259858fdd9715784d212d3508791b1ef0cf6d17da6"
output.type = "zone"

# Refer to
Expand All @@ -409,10 +409,10 @@ service_name = "propolis-server"
only_for_targets.image = "standard"
source.type = "prebuilt"
source.repo = "propolis"
source.commit = "54398875a2125227d13827d4236dce943c019b1c"
source.commit = "3e1d129151c3621d28ead5c6e5760693ba6e7fec"
# The SHA256 digest is automatically posted to:
# https://buildomat.eng.oxide.computer/public/file/oxidecomputer/propolis/image/<commit>/propolis-server.sha256.txt
source.sha256 = "01b8563db6626f90ee3fb6d97e7921b0a680373d843c1bea7ebf46fcea4f7b28"
source.sha256 = "cd341409eb2ffc3d8bec89fd20cad61d170f89d3adf926f6104eb01f4f4da881"
output.type = "zone"

[package.mg-ddm-gz]
Expand Down Expand Up @@ -476,8 +476,8 @@ only_for_targets.image = "standard"
# 2. Copy dendrite.tar.gz from dendrite/out to omicron/out
source.type = "prebuilt"
source.repo = "dendrite"
source.commit = "8ff834e7d0a6adb263240edd40537f2c0768f1a4"
source.sha256 = "c00e79f55e0bdf048069b2d18a4d009ddfef46e7e5d846887cf96e843a8884bd"
source.commit = "2af6adea85c62ac37e451148b84e5eb0ef005f36"
source.sha256 = "dc93b671cce54e83ed55faaa267f81ba9e65abcd6714aa559d68a8783d73b1c1"
output.type = "zone"
output.intermediate_only = true

Expand All @@ -501,8 +501,8 @@ only_for_targets.image = "standard"
# 2. Copy the output zone image from dendrite/out to omicron/out
source.type = "prebuilt"
source.repo = "dendrite"
source.commit = "8ff834e7d0a6adb263240edd40537f2c0768f1a4"
source.sha256 = "428cce1e9aa399b1b49c04e7fd0bc1cb0e3f3fae6fda96055892a42e010c9d6f"
source.commit = "2af6adea85c62ac37e451148b84e5eb0ef005f36"
source.sha256 = "c34b10d47fa3eb9f9f6b3655ea4ed8a726f93399ea177efea79f5c89f2ab5a1e"
output.type = "zone"
output.intermediate_only = true

Expand All @@ -519,8 +519,8 @@ only_for_targets.image = "standard"
# 2. Copy dendrite.tar.gz from dendrite/out to omicron/out/dendrite-softnpu.tar.gz
source.type = "prebuilt"
source.repo = "dendrite"
source.commit = "8ff834e7d0a6adb263240edd40537f2c0768f1a4"
source.sha256 = "5dd3534bec5eb4f857d0bf3994b26650288f650d409eec6aaa29860a2f481c37"
source.commit = "2af6adea85c62ac37e451148b84e5eb0ef005f36"
source.sha256 = "ce7065227c092ee82704f39a966b7441e3ae82d75eedb6eb281bd8b3e5873e32"
output.type = "zone"
output.intermediate_only = true

Expand Down
14 changes: 14 additions & 0 deletions schema/crdb/15.0.0/up01.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/*
* Previous commits added the optional kind of a producer. In this version,
* we're making the value required and not nullable. We'll first delete all
* records with a NULL kind -- there should not be any, since all producers both
* in an out of tree have been updated. Nonetheless, this is safe because
* currently we're updating offline, and all producers should re-register when
* they are restarted.
*
* NOTE: Full table scans are disallowed, however we don't have an index on
* producer kind (and don't currently need one). Allow full table scans for the
* context of this one statement.
*/
SET LOCAL disallow_full_table_scans = off;
DELETE FROM omicron.public.metric_producer WHERE kind IS NULL;
4 changes: 4 additions & 0 deletions schema/crdb/15.0.0/up02.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/*
* Next, we make the field itself required in the database.
*/
ALTER TABLE IF EXISTS omicron.public.metric_producer ALTER COLUMN kind SET NOT NULL;
4 changes: 2 additions & 2 deletions schema/crdb/dbinit.sql
Original file line number Diff line number Diff line change
Expand Up @@ -1172,7 +1172,7 @@ CREATE TABLE IF NOT EXISTS omicron.public.metric_producer (
id UUID PRIMARY KEY,
time_created TIMESTAMPTZ NOT NULL,
time_modified TIMESTAMPTZ NOT NULL,
kind omicron.public.producer_kind,
kind omicron.public.producer_kind NOT NULL,
ip INET NOT NULL,
port INT4 CHECK (port BETWEEN 0 AND 65535) NOT NULL,
interval FLOAT NOT NULL,
Expand Down Expand Up @@ -2997,7 +2997,7 @@ INSERT INTO omicron.public.db_metadata (
version,
target_version
) VALUES
( TRUE, NOW(), NOW(), '14.0.0', NULL)
( TRUE, NOW(), NOW(), '15.0.0', NULL)
ON CONFLICT DO NOTHING;

COMMIT;
2 changes: 1 addition & 1 deletion sled-agent/src/sim/disk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ impl SimDisk {
let producer_address = SocketAddr::new(Ipv6Addr::LOCALHOST.into(), 0);
let server_info = ProducerEndpoint {
id,
kind: Some(ProducerKind::SledAgent),
kind: ProducerKind::SledAgent,
address: producer_address,
base_route: "/collect".to_string(),
interval: Duration::from_millis(200),
Expand Down
2 changes: 1 addition & 1 deletion sled-agent/src/sled_agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ impl SledAgent {
// Nexus. This should not block progress here.
let endpoint = ProducerEndpoint {
id: request.body.id,
kind: Some(ProducerKind::SledAgent),
kind: ProducerKind::SledAgent,
address: sled_address.into(),
base_route: String::from("/metrics/collect"),
interval: crate::metrics::METRIC_COLLECTION_INTERVAL,
Expand Down
2 changes: 1 addition & 1 deletion tools/dendrite_openapi_version
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
COMMIT="8ff834e7d0a6adb263240edd40537f2c0768f1a4"
COMMIT="2af6adea85c62ac37e451148b84e5eb0ef005f36"
SHA2="07d115bfa8498a8015ca2a8447efeeac32e24aeb25baf3d5e2313216e11293c0"
6 changes: 3 additions & 3 deletions tools/dendrite_stub_checksums
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
CIDL_SHA256_ILLUMOS="c00e79f55e0bdf048069b2d18a4d009ddfef46e7e5d846887cf96e843a8884bd"
CIDL_SHA256_LINUX_DPD="b5d829b4628759ac374106f3c56c29074b29577fd0ff72f61c3b8289fea430fe"
CIDL_SHA256_LINUX_SWADM="afc68828f54dc57b32dc1556fc588baeab12341c30e96cc0fadb49f401b4b48f"
CIDL_SHA256_ILLUMOS="dc93b671cce54e83ed55faaa267f81ba9e65abcd6714aa559d68a8783d73b1c1"
CIDL_SHA256_LINUX_DPD="b13b391a085ba6bf16fdd99774f64c9d53cd7220ad518d5839c8558fb925c40c"
CIDL_SHA256_LINUX_SWADM="6bfa4e367eb2b0be89f1588ac458026a186314597a4feb9fee6cea60101c7ebe"

0 comments on commit 4fb8f08

Please sign in to comment.