Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

System Update API #2100

Merged
merged 50 commits into from
Jan 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
b7b60c0
stub system update status, list, and detail
david-crespo Dec 29, 2022
9a5ae8c
wrap semver::Version and impl JsonSchema for it
david-crespo Dec 29, 2022
468a263
/components endpoints for update detail and system version, tweak names
david-crespo Dec 29, 2022
1ce8ae6
add update start and stop without deciding what their responses are
david-crespo Dec 29, 2022
9d86f52
move AssetIdentityMetadata next to Asset, add identity() method
david-crespo Dec 30, 2022
24ab40c
add system updates table
david-crespo Dec 30, 2022
cde4429
stub integration tests for stubbed endpoints
david-crespo Jan 2, 2023
859605c
so damn close with the component join table but it doesn't compile
david-crespo Jan 2, 2023
e3e3123
thanks @smklein
david-crespo Jan 2, 2023
4eed1d7
update openapi spec, fix nexus method name
david-crespo Jan 2, 2023
afe32c1
Merge branch 'main' into stub-update-api
david-crespo Jan 2, 2023
186994e
paginate list of updates
david-crespo Jan 3, 2023
ea4bbde
implement to/from sql for semver version
david-crespo Jan 4, 2023
38e3444
Merge branch 'main' into stub-update-api
david-crespo Jan 4, 2023
dea5ada
fix VersionStatus enum in openapi spec with serde tag
david-crespo Jan 4, 2023
a7d898c
use list of component types from RFD 300
david-crespo Jan 4, 2023
54e1f45
updateable components table (TODO: status and reason)
david-crespo Jan 4, 2023
34ebe77
nexus method to create a system update + test for it
david-crespo Jan 5, 2023
4c59334
create_component_update() and working test
david-crespo Jan 5, 2023
7aa207c
other system update should not be associated with any component updates
david-crespo Jan 5, 2023
b9ac8b6
create updateable component, test for it
david-crespo Jan 5, 2023
c143f8a
make the unauthorized things pass, mediocrely
david-crespo Jan 5, 2023
c6f9dff
update iam roles policy test and nexus_tags.txt
david-crespo Jan 5, 2023
78e3fb7
make existing system update refresh endpoint match the rest
david-crespo Jan 7, 2023
8ffa99a
Merge branch 'main' into stub-update-api
david-crespo Jan 12, 2023
1eeaf4e
make version the PK of the system update, fetch by version instead of id
david-crespo Jan 12, 2023
c5b6264
remove id from system_update. horrible because it goes against the grain
david-crespo Jan 12, 2023
ffea521
Revert "remove id from system_update. horrible because it goes agains…
david-crespo Jan 12, 2023
190af7c
change comment: we're not getting rid of the ID. update openapi spec
david-crespo Jan 12, 2023
fde6e98
clean up around TODO comments, add SystemUpdateDeployment view, fix t…
david-crespo Jan 13, 2023
3ef6f38
can't give the params the same name as the view
david-crespo Jan 13, 2023
f8898a8
Merge branch 'main' into stub-update-api
david-crespo Jan 19, 2023
c145d60
Merge branch 'main' into stub-update-api
david-crespo Jan 19, 2023
2660a52
add update deployments table and list/view endpoints
david-crespo Jan 23, 2023
b3df99c
Merge main into stub-update-api
david-crespo Jan 23, 2023
9ec0773
fix clippy and tests
david-crespo Jan 23, 2023
e992960
Merge branch 'main' into stub-update-api
david-crespo Jan 25, 2023
3dc2bb0
plumb through UpdateStatus, SystemUpdateDeployment -> UpdateDeployment
david-crespo Jan 25, 2023
0746adf
version_sort column that lets us sort by version
david-crespo Jan 25, 2023
5a2b99f
validate that semver version has low enough numbers for our sort hack
david-crespo Jan 26, 2023
a28b371
add version_sort to updateable_component so we can get low/high for s…
david-crespo Jan 26, 2023
0d22dd0
better test to prove we're not doing normal string sort on versions
david-crespo Jan 26, 2023
53d7d03
component tree and component update tree are no longer trees
david-crespo Jan 26, 2023
07dea02
use a transaction, working integration test for system version endpoint
david-crespo Jan 26, 2023
05635bd
put verb first in nexus function names
david-crespo Jan 26, 2023
9575587
create_update_deployment (doesn't check if we're already updating)
david-crespo Jan 27, 2023
6cdfe40
fix ON CONFLICT by not doing that. add tests for version conflicts
david-crespo Jan 27, 2023
b7a0b93
change system update pk back to ID, exempt /system/version from auth …
david-crespo Jan 27, 2023
a1772af
updateable component should have both own version and system version
david-crespo Jan 28, 2023
f109cfe
don't blame buildomat. look inward
david-crespo Jan 30, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ rustfmt-wrapper = "0.2"
rustls = "0.20.7"
samael = { git = "https://github.com/njaremko/samael", features = ["xmlsec"], branch = "master" }
schemars = "0.8.10"
semver = { version = "1.0.16", features = ["std", "serde"] }
serde = { version = "1.0", default-features = false, features = [ "derive" ] }
serde_derive = "1.0"
serde_json = "1.0.91"
Expand Down
1 change: 1 addition & 0 deletions common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ rand.workspace = true
reqwest = { workspace = true, features = ["rustls-tls", "stream"] }
ring.workspace = true
schemars = { workspace = true, features = [ "chrono", "uuid1" ] }
semver.workspace = true
serde.workspace = true
serde_derive.workspace = true
serde_json.workspace = true
Expand Down
38 changes: 38 additions & 0 deletions common/src/api/external/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use futures::stream::StreamExt;
use parse_display::Display;
use parse_display::FromStr;
use schemars::JsonSchema;
use semver;
use serde::Deserialize;
use serde::Serialize;
use serde_with::{DeserializeFromStr, SerializeDisplay};
Expand Down Expand Up @@ -373,6 +374,38 @@ impl JsonSchema for NameOrId {
}
}

// TODO: remove wrapper for semver::Version once this PR goes through
// https://github.com/GREsau/schemars/pull/195
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq, Display)]
#[display("{0}")]
pub struct SemverVersion(pub semver::Version);

impl SemverVersion {
pub fn new(major: u64, minor: u64, patch: u64) -> Self {
Self(semver::Version::new(major, minor, patch))
}
}

impl JsonSchema for SemverVersion {
fn schema_name() -> String {
"SemverVersion".to_string()
}

fn json_schema(
_: &mut schemars::gen::SchemaGenerator,
) -> schemars::schema::Schema {
schemars::schema::SchemaObject {
instance_type: Some(schemars::schema::InstanceType::String.into()),
string: Some(Box::new(schemars::schema::StringValidation {
pattern: Some(r"^\d+\.\d+\.\d+([\-\+].+)?$".to_owned()),
..Default::default()
})),
..Default::default()
}
.into()
}
}

/// Name for a built-in role
#[derive(
Clone,
Expand Down Expand Up @@ -667,6 +700,11 @@ pub enum ResourceType {
MetricProducer,
RoleBuiltin,
UpdateAvailableArtifact,
SystemUpdate,
ComponentUpdate,
SystemUpdateComponentUpdate,
UpdateDeployment,
UpdateableComponent,
UserBuiltin,
Zpool,
}
Expand Down
146 changes: 146 additions & 0 deletions common/src/sql/dbinit.sql
Original file line number Diff line number Diff line change
Expand Up @@ -1512,6 +1512,152 @@ CREATE INDEX ON omicron.public.update_available_artifact (
targets_role_version
);

/*
* System updates
*/
CREATE TABLE omicron.public.system_update (
/* Identity metadata (asset) */
id UUID PRIMARY KEY,
time_created TIMESTAMPTZ NOT NULL,
time_modified TIMESTAMPTZ NOT NULL,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does it mean for an update to be modified? The version updated?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, is there a need for an index to be created for this or other tables here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really know why these would be modified. The only reason I have it on here is because it's convenient to make these Assets, which are like Resource but without name and description. I don't think it's too bad to have the field there and never use it — the alternative (not being an Asset) is probably worse, though I haven't thought about it too hard.

Yes to indexes, will put that on my to-do list.


-- Because the version is unique, it could be the PK, but that would make
-- this resource different from every other resource for little benefit.

-- Unique semver version
version STRING(64) NOT NULL, -- TODO: length
-- version string with maj/min/patch 0-padded to be string sortable
version_sort STRING(64) NOT NULL -- TODO: length
);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the version as the PK while still keeping the ID around is one of the stranger parts of this PR, but I landed on it after trying less weird-sounding alternatives that turned out to be uglier.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, minor nitpick, that doesn't really matter much but might help with consistency:

  1. Can we make the ID the PRIMARY KEY if we're gonna have it?
  2. Can we then create a unique index on the version?

I think this should be functionally very similar, but it matches the pattern of other objects with "both IDs and unique names".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the problem here was with the lookup macros. I could not figure out how to get a nice lookup-by-version with lookup_resource! without making version the primary key, because the way the macro is designed, it sounds like it has to be either by primary key or by name, and a non-PK version string is neither. I would love to find out that I understood this wrong, or that it's not that hard to change the macro (I looked but got spooked).

https://github.com/oxidecomputer/omicron/pull/2100/files#diff-6e518912406e7c5a535066ef28084be22fdbdae61895bab565eb656aa4682e01R698-R705


CREATE UNIQUE INDEX ON omicron.public.system_update (
version
);

CREATE UNIQUE INDEX ON omicron.public.system_update (
version_sort
);

CREATE TYPE omicron.public.updateable_component_type AS ENUM (
'bootloader_for_rot',
'bootloader_for_sp',
'bootloader_for_host_proc',
'hubris_for_psc_rot',
'hubris_for_psc_sp',
'hubris_for_sidecar_rot',
'hubris_for_sidecar_sp',
'hubris_for_gimlet_rot',
'hubris_for_gimlet_sp',
'helios_host_phase_1',
'helios_host_phase_2',
'host_omicron'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're probably gonna add zones-updateable-separately-from-the-host-OS like CRDB + Clickhouse to this list, right? (Not in this PR, just kinda thinking out loud)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hadn't thought about it, but yeah, sounds right. I just copied these from the example tree in RFD 334.

);

/*
* Component updates. Associated with at least one system_update through
* system_update_component_update.
*/
CREATE TABLE omicron.public.component_update (
/* Identity metadata (asset) */
id UUID PRIMARY KEY,
time_created TIMESTAMPTZ NOT NULL,
time_modified TIMESTAMPTZ NOT NULL,

-- On component updates there's no device ID because the update can apply to
-- multiple instances of a given device kind

-- The *system* update version associated with this version (this is confusing, will rename)
version STRING(64) NOT NULL, -- TODO: length
-- TODO: add component update version to component_update

component_type omicron.public.updateable_component_type NOT NULL
);

-- version is unique per component type
CREATE UNIQUE INDEX ON omicron.public.component_update (
component_type, version
);

/*
* Associate system updates with component updates. Not done with a
* system_update_id field on component_update because the same component update
* may be part of more than one system update.
*/
CREATE TABLE omicron.public.system_update_component_update (
system_update_id UUID NOT NULL,
component_update_id UUID NOT NULL,

PRIMARY KEY (system_update_id, component_update_id)
);

-- For now, the plan is to treat stopped, failed, completed as sub-cases of
-- "steady" described by a "reason". But reason is not implemented yet.
-- Obviously this could be a boolean, but boolean status fields never stay
-- boolean for long.
CREATE TYPE omicron.public.update_status AS ENUM (
'updating',
'steady'
);

/*
* Updateable components and their update status
*/
CREATE TABLE omicron.public.updateable_component (
/* Identity metadata (asset) */
id UUID PRIMARY KEY,
time_created TIMESTAMPTZ NOT NULL,
time_modified TIMESTAMPTZ NOT NULL,

-- Free-form string that comes from the device
device_id STRING(40) NOT NULL,

component_type omicron.public.updateable_component_type NOT NULL,

-- The semver version of this component's own software
version STRING(64) NOT NULL, -- TODO: length

-- The version of the system update this component's software came from.
-- This may need to be nullable if we are registering components before we
-- know about system versions at all
system_version STRING(64) NOT NULL, -- TODO: length
-- version string with maj/min/patch 0-padded to be string sortable
system_version_sort STRING(64) NOT NULL, -- TODO: length

status omicron.public.update_status NOT NULL
-- TODO: status reason for updateable_component
);

-- can't have two components of the same type with the same device ID
CREATE UNIQUE INDEX ON omicron.public.updateable_component (
component_type, device_id
);

CREATE INDEX ON omicron.public.updateable_component (
system_version_sort
);

/*
* System updates
*/
CREATE TABLE omicron.public.update_deployment (
/* Identity metadata (asset) */
id UUID PRIMARY KEY,
time_created TIMESTAMPTZ NOT NULL,
time_modified TIMESTAMPTZ NOT NULL,

-- semver version of corresponding system update
-- TODO: this makes sense while version is the PK of system_update, but
-- if/when I change that back to ID, this needs to be the ID too
version STRING(64) NOT NULL,

status omicron.public.update_status NOT NULL
-- TODO: status reason for update_deployment
);

CREATE INDEX on omicron.public.update_deployment (
time_created
);

/*******************************************************************/

/*
Expand Down
1 change: 1 addition & 0 deletions nexus/db-model/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ rand.workspace = true
ref-cast.workspace = true
thiserror.workspace = true
schemars = { workspace = true, features = ["chrono", "uuid1"] }
semver.workspace = true
serde.workspace = true
serde_json.workspace = true
steno.workspace = true
Expand Down
4 changes: 4 additions & 0 deletions nexus/db-model/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ mod organization;
mod oximeter_info;
mod producer_endpoint;
mod project;
mod semver_version;
mod system_update;
// These actually represent subqueries, not real table.
// However, they must be defined in the same crate as our tables
// for join-based marker trait generation.
Expand Down Expand Up @@ -119,6 +121,7 @@ pub use region::*;
pub use region_snapshot::*;
pub use role_assignment::*;
pub use role_builtin::*;
pub use semver_version::*;
pub use service::*;
pub use service_kind::*;
pub use silo::*;
Expand All @@ -128,6 +131,7 @@ pub use silo_user_password_hash::*;
pub use sled::*;
pub use snapshot::*;
pub use ssh_key::*;
pub use system_update::*;
pub use update_artifact::*;
pub use user_builtin::*;
pub use virtual_provisioning_collection::*;
Expand Down
64 changes: 64 additions & 0 deletions nexus/db-model/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,70 @@ table! {
}
}

table! {
system_update (id) {
id -> Uuid,
time_created -> Timestamptz,
time_modified -> Timestamptz,

version -> Text,
version_sort -> Text,
}
}

table! {
update_deployment (id) {
id -> Uuid,
time_created -> Timestamptz,
time_modified -> Timestamptz,

version -> Text,
status -> crate::UpdateStatusEnum,
// TODO: status reason for updateable_component
}
}

table! {
component_update (id) {
id -> Uuid,
time_created -> Timestamptz,
time_modified -> Timestamptz,

version -> Text,
component_type -> crate::UpdateableComponentTypeEnum,
}
}

table! {
updateable_component (id) {
id -> Uuid,
time_created -> Timestamptz,
time_modified -> Timestamptz,

device_id -> Text,
version -> Text,
system_version -> Text,
system_version_sort -> Text,
component_type -> crate::UpdateableComponentTypeEnum,
status -> crate::UpdateStatusEnum,
// TODO: status reason for updateable_component
}
}

table! {
system_update_component_update (system_update_id, component_update_id) {
system_update_id -> Uuid,
component_update_id -> Uuid,
}
}

allow_tables_to_appear_in_same_query!(
system_update,
component_update,
system_update_component_update,
);
joinable!(system_update_component_update -> component_update (component_update_id));

allow_tables_to_appear_in_same_query!(ip_pool_range, ip_pool);
joinable!(ip_pool_range -> ip_pool (ip_pool_id));

Expand Down
Loading