From 4fb8f084457a71c58f824393e06196bd6a3ed3e3 Mon Sep 17 00:00:00 2001 From: Benjamin Naecker Date: Tue, 28 Nov 2023 22:14:29 +0000 Subject: [PATCH 1/2] Make oximeter producer kind required - 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. --- clients/nexus-client/src/lib.rs | 2 +- clients/oximeter-client/src/lib.rs | 2 +- common/src/api/internal/nexus.rs | 2 +- nexus/db-model/src/producer_endpoint.rs | 4 ++-- nexus/db-model/src/schema.rs | 4 ++-- nexus/src/app/oximeter.rs | 6 ++--- nexus/test-utils/src/lib.rs | 2 +- nexus/tests/integration_tests/oximeter.rs | 2 +- openapi/nexus-internal.json | 4 ++-- openapi/oximeter.json | 4 ++-- oximeter/collector/src/agent.rs | 6 ++--- oximeter/producer/examples/producer.rs | 2 +- package-manifest.toml | 28 +++++++++++------------ schema/crdb/15.0.0/up01.sql | 14 ++++++++++++ schema/crdb/15.0.0/up02.sql | 4 ++++ schema/crdb/dbinit.sql | 4 ++-- sled-agent/src/sim/disk.rs | 2 +- sled-agent/src/sled_agent.rs | 2 +- tools/dendrite_openapi_version | 2 +- tools/dendrite_stub_checksums | 6 ++--- 20 files changed, 59 insertions(+), 43 deletions(-) create mode 100644 schema/crdb/15.0.0/up01.sql create mode 100644 schema/crdb/15.0.0/up02.sql diff --git a/clients/nexus-client/src/lib.rs b/clients/nexus-client/src/lib.rs index 6667f759e45..3ecba7e7100 100644 --- a/clients/nexus-client/src/lib.rs +++ b/clients/nexus-client/src/lib.rs @@ -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(), } } diff --git a/clients/oximeter-client/src/lib.rs b/clients/oximeter-client/src/lib.rs index 8a03304e060..11aa1452f82 100644 --- a/clients/oximeter-client/src/lib.rs +++ b/clients/oximeter-client/src/lib.rs @@ -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(), } } diff --git a/common/src/api/internal/nexus.rs b/common/src/api/internal/nexus.rs index 1daa85dbe75..780e60b1a2e 100644 --- a/common/src/api/internal/nexus.rs +++ b/common/src/api/internal/nexus.rs @@ -103,7 +103,7 @@ pub struct ProducerEndpoint { /// A unique ID for this producer. pub id: Uuid, /// The kind of producer. - pub kind: Option, + pub kind: ProducerKind, /// The IP address and port at which `oximeter` can collect metrics from the /// producer. pub address: SocketAddr, diff --git a/nexus/db-model/src/producer_endpoint.rs b/nexus/db-model/src/producer_endpoint.rs index 52a69e05085..f282f6f08f9 100644 --- a/nexus/db-model/src/producer_endpoint.rs +++ b/nexus/db-model/src/producer_endpoint.rs @@ -52,7 +52,7 @@ pub struct ProducerEndpoint { #[diesel(embed)] identity: ProducerEndpointIdentity, - pub kind: Option, + pub kind: ProducerKind, pub ip: ipnetwork::IpNetwork, pub port: SqlU16, pub interval: f64, @@ -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(), diff --git a/nexus/db-model/src/schema.rs b/nexus/db-model/src/schema.rs index afeac5e6cd9..9d68ce97ab9 100644 --- a/nexus/db-model/src/schema.rs +++ b/nexus/db-model/src/schema.rs @@ -399,7 +399,7 @@ table! { id -> Uuid, time_created -> Timestamptz, time_modified -> Timestamptz, - kind -> Nullable, + kind -> crate::ProducerKindEnum, ip -> Inet, port -> Int4, interval -> Float8, @@ -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, diff --git a/nexus/src/app/oximeter.rs b/nexus/src/app/oximeter.rs index 66f39a32b61..a168b352937 100644 --- a/nexus/src/app/oximeter.rs +++ b/nexus/src/app/oximeter.rs @@ -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(), @@ -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), diff --git a/nexus/test-utils/src/lib.rs b/nexus/test-utils/src/lib.rs index 1e7de6132b5..52ff8910f91 100644 --- a/nexus/test-utils/src/lib.rs +++ b/nexus/test-utils/src/lib.rs @@ -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), diff --git a/nexus/tests/integration_tests/oximeter.rs b/nexus/tests/integration_tests/oximeter.rs index e97f36daf4a..7dc453d7135 100644 --- a/nexus/tests/integration_tests/oximeter.rs +++ b/nexus/tests/integration_tests/oximeter.rs @@ -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), diff --git a/openapi/nexus-internal.json b/openapi/nexus-internal.json index c358b4109b5..e0580e7c13b 100644 --- a/openapi/nexus-internal.json +++ b/openapi/nexus-internal.json @@ -4343,7 +4343,6 @@ ] }, "kind": { - "nullable": true, "description": "The kind of producer.", "allOf": [ { @@ -4356,7 +4355,8 @@ "address", "base_route", "id", - "interval" + "interval", + "kind" ] }, "ProducerKind": { diff --git a/openapi/oximeter.json b/openapi/oximeter.json index f7e534c95da..f5c78d53cd8 100644 --- a/openapi/oximeter.json +++ b/openapi/oximeter.json @@ -212,7 +212,6 @@ ] }, "kind": { - "nullable": true, "description": "The kind of producer.", "allOf": [ { @@ -225,7 +224,8 @@ "address", "base_route", "id", - "interval" + "interval", + "kind" ] }, "ProducerEndpointResultsPage": { diff --git a/oximeter/collector/src/agent.rs b/oximeter/collector/src/agent.rs index f6da1729094..5866375fa10 100644 --- a/oximeter/collector/src/agent.rs +++ b/oximeter/collector/src/agent.rs @@ -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, @@ -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, @@ -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, diff --git a/oximeter/producer/examples/producer.rs b/oximeter/producer/examples/producer.rs index baa4f57bf70..8dbe0b6ad97 100644 --- a/oximeter/producer/examples/producer.rs +++ b/oximeter/producer/examples/producer.rs @@ -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), diff --git a/package-manifest.toml b/package-manifest.toml index 26c45f0ff74..a0f4de85cc4 100644 --- a/package-manifest.toml +++ b/package-manifest.toml @@ -384,10 +384,10 @@ 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//crucible.sha256.txt -source.sha256 = "897d0fd6c0b82db42256a63a13c228152e1117434afa2681f649b291e3c6f46d" +# https://buildomat.eng.oxide.computer/public/file/oxidecomputer/crucible/rbuild//crucible.sha256.txt +source.sha256 = "f8c23cbf89fd0bbd928d8e3db1357bbea6e6b50560e221f873da5b56ed9d7527" output.type = "zone" [package.crucible-pantry] @@ -395,10 +395,10 @@ 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//crucible-pantry.sha256.txt -source.sha256 = "fe545de7ac4f15454d7827927149c5f0fc68ce9545b4f1ef96aac9ac8039805a" +# https://buildomat.eng.oxide.computer/public/file/oxidecomputer/crucible/rbuild//crucible-pantry.sha256.txt +source.sha256 = "a25b31c81798eb65564dbe259858fdd9715784d212d3508791b1ef0cf6d17da6" output.type = "zone" # Refer to @@ -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//propolis-server.sha256.txt -source.sha256 = "01b8563db6626f90ee3fb6d97e7921b0a680373d843c1bea7ebf46fcea4f7b28" +source.sha256 = "cd341409eb2ffc3d8bec89fd20cad61d170f89d3adf926f6104eb01f4f4da881" output.type = "zone" [package.mg-ddm-gz] @@ -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 @@ -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 @@ -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 diff --git a/schema/crdb/15.0.0/up01.sql b/schema/crdb/15.0.0/up01.sql new file mode 100644 index 00000000000..f9806c59174 --- /dev/null +++ b/schema/crdb/15.0.0/up01.sql @@ -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; diff --git a/schema/crdb/15.0.0/up02.sql b/schema/crdb/15.0.0/up02.sql new file mode 100644 index 00000000000..9c1ad2ea472 --- /dev/null +++ b/schema/crdb/15.0.0/up02.sql @@ -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; diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 728b084982a..12983fea899 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -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, @@ -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; diff --git a/sled-agent/src/sim/disk.rs b/sled-agent/src/sim/disk.rs index f131fd2bff9..fc388f6ce25 100644 --- a/sled-agent/src/sim/disk.rs +++ b/sled-agent/src/sim/disk.rs @@ -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), diff --git a/sled-agent/src/sled_agent.rs b/sled-agent/src/sled_agent.rs index f5b71106cdf..9f8d31b3c5e 100644 --- a/sled-agent/src/sled_agent.rs +++ b/sled-agent/src/sled_agent.rs @@ -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, diff --git a/tools/dendrite_openapi_version b/tools/dendrite_openapi_version index ba4b5a57220..c2dda4dbd07 100644 --- a/tools/dendrite_openapi_version +++ b/tools/dendrite_openapi_version @@ -1,2 +1,2 @@ -COMMIT="8ff834e7d0a6adb263240edd40537f2c0768f1a4" +COMMIT="2af6adea85c62ac37e451148b84e5eb0ef005f36" SHA2="07d115bfa8498a8015ca2a8447efeeac32e24aeb25baf3d5e2313216e11293c0" diff --git a/tools/dendrite_stub_checksums b/tools/dendrite_stub_checksums index 619a6bf287e..77ee198fc56 100644 --- a/tools/dendrite_stub_checksums +++ b/tools/dendrite_stub_checksums @@ -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" From 2a886c597c79007f72befd434b5db0c15904c2fb Mon Sep 17 00:00:00 2001 From: Benjamin Naecker Date: Wed, 29 Nov 2023 17:04:15 +0000 Subject: [PATCH 2/2] Fix Buildomat URL --- package-manifest.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package-manifest.toml b/package-manifest.toml index a0f4de85cc4..3bce4aafee3 100644 --- a/package-manifest.toml +++ b/package-manifest.toml @@ -386,7 +386,7 @@ source.type = "prebuilt" source.repo = "crucible" source.commit = "945f040d259ca8013d3fb26f510453da7cd7b1a6" # The SHA256 digest is automatically posted to: -# https://buildomat.eng.oxide.computer/public/file/oxidecomputer/crucible/rbuild//crucible.sha256.txt +# https://buildomat.eng.oxide.computer/public/file/oxidecomputer/crucible/image//crucible.sha256.txt source.sha256 = "f8c23cbf89fd0bbd928d8e3db1357bbea6e6b50560e221f873da5b56ed9d7527" output.type = "zone" @@ -397,7 +397,7 @@ source.type = "prebuilt" source.repo = "crucible" source.commit = "945f040d259ca8013d3fb26f510453da7cd7b1a6" # The SHA256 digest is automatically posted to: -# https://buildomat.eng.oxide.computer/public/file/oxidecomputer/crucible/rbuild//crucible-pantry.sha256.txt +# https://buildomat.eng.oxide.computer/public/file/oxidecomputer/crucible/image//crucible-pantry.sha256.txt source.sha256 = "a25b31c81798eb65564dbe259858fdd9715784d212d3508791b1ef0cf6d17da6" output.type = "zone"