-
Notifications
You must be signed in to change notification settings - Fork 42
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
System Update API #2100
Changes from 24 commits
b7b60c0
9a5ae8c
468a263
1ce8ae6
9d86f52
24ab40c
cde4429
859605c
e3e3123
4eed1d7
afe32c1
186994e
ea4bbde
38e3444
dea5ada
a7d898c
54e1f45
34ebe77
4c59334
7aa207c
b9ac8b6
c143f8a
c6f9dff
78e3fb7
8ffa99a
1eeaf4e
c5b6264
ffea521
190af7c
fde6e98
3ef6f38
f8898a8
c145d60
2660a52
b3df99c
9ec0773
e992960
3dc2bb0
0746adf
5a2b99f
a28b371
0d22dd0
53d7d03
07dea02
05635bd
9575587
6cdfe40
b7a0b93
a1772af
f109cfe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1419,6 +1419,83 @@ 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, | ||
|
||
/* Unique semver version */ | ||
/* TODO: If the version is really supposed to be unique, we could make it the PK? */ | ||
version STRING(40) NOT NULL | ||
); | ||
|
||
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' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
||
-- TODO: figure out the actual length version strings should have | ||
version STRING(40) NOT NULL, | ||
component_type omicron.public.updateable_component_type NOT NULL, | ||
parent_id UUID | ||
); | ||
|
||
/* | ||
* 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) | ||
); | ||
|
||
/* | ||
* 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, | ||
version STRING(40) NOT NULL, | ||
This comment was marked as resolved.
Sorry, something went wrong. |
||
parent_id UUID | ||
-- TODO: status and reason | ||
); | ||
|
||
/*******************************************************************/ | ||
|
||
/* | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
// This Source Code Form is subject to the terms of the Mozilla Public | ||
// 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/. | ||
|
||
use diesel::backend::{Backend, RawValue}; | ||
use diesel::deserialize::{self, FromSql}; | ||
use diesel::query_builder::bind_collector::RawBytesBindCollector; | ||
use diesel::serialize::{self, ToSql}; | ||
use diesel::sql_types; | ||
use omicron_common::api::external; | ||
use parse_display::Display; | ||
use serde::{Deserialize, Serialize}; | ||
|
||
// We wrap semver::Version in external to impl JsonSchema, and we wrap it again | ||
// here to impl ToSql/FromSql | ||
|
||
#[derive( | ||
Clone, | ||
Debug, | ||
AsExpression, | ||
FromSqlRow, | ||
Serialize, | ||
Deserialize, | ||
PartialEq, | ||
Display, | ||
)] | ||
#[diesel(sql_type = sql_types::Text)] | ||
#[display("{0}")] | ||
pub struct SemverVersion(pub external::SemverVersion); | ||
|
||
NewtypeFrom! { () pub struct SemverVersion(external::SemverVersion); } | ||
NewtypeDeref! { () pub struct SemverVersion(external::SemverVersion); } | ||
|
||
impl SemverVersion { | ||
pub fn new(major: u64, minor: u64, patch: u64) -> Self { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will we ever make it possible to have either "prerelease" or "build metadata" as a string here? (IMO if we say "no for this PR", I'm totally fine with that, just wondering about future expansion) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I think so. You could always use the struct constructor directly, though in our case that's pretty annoying, as we have two different wrappers. So we'd probably just add more constructor functions. We may not even need them. We might find ourselves parsing the versions from strings in the artifact metadata. |
||
Self(external::SemverVersion(semver::Version::new(major, minor, patch))) | ||
} | ||
} | ||
|
||
impl<DB> ToSql<sql_types::Text, DB> for SemverVersion | ||
where | ||
DB: Backend<BindCollector = RawBytesBindCollector<DB>>, | ||
String: ToSql<sql_types::Text, DB>, | ||
{ | ||
fn to_sql<'a>( | ||
&'a self, | ||
out: &mut serialize::Output<'a, '_, DB>, | ||
) -> serialize::Result { | ||
(self.0).0.to_string().to_sql(&mut out.reborrow()) | ||
} | ||
} | ||
|
||
impl<DB> FromSql<sql_types::Text, DB> for SemverVersion | ||
where | ||
DB: Backend, | ||
String: FromSql<sql_types::Text, DB>, | ||
{ | ||
fn from_sql(raw: RawValue<DB>) -> deserialize::Result<Self> { | ||
String::from_sql(raw)? | ||
.parse() | ||
.map(|s| SemverVersion(external::SemverVersion(s))) | ||
.map_err(|e| e.into()) | ||
} | ||
} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
Asset
s, which are likeResource
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 anAsset
) is probably worse, though I haven't thought about it too hard.Yes to indexes, will put that on my to-do list.